Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Nov 2012 16:25:50 +0100
From:      Jilles Tjoelker <jilles@stack.nl>
To:        Peter Grehan <grehan@FreeBSD.org>
Cc:        svn-src-projects@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r243667 - in projects/bhyve/sys/amd64/vmm: . intel
Message-ID:  <20121129152550.GB29338@stack.nl>
In-Reply-To: <201211290626.qAT6QhOr007958@svn.freebsd.org>
References:  <201211290626.qAT6QhOr007958@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Nov 29, 2012 at 06:26:43AM +0000, Peter Grehan wrote:
> Author: grehan
> Date: Thu Nov 29 06:26:42 2012
> New Revision: 243667
> URL: http://svnweb.freebsd.org/changeset/base/243667

> Log:
>   Add support for the 0x81 AND instruction, now generated
>   by clang in the local APIC code.

>   0x81 is a read-modify-write instruction - the EPT check
>   that only allowed read or write and not both has been
>   relaxed to allow read and write.

>   Reviewed by:	neel
>   Obtained from:	NetApp

I don't see a check on the ModRM reg field, which indicates whether the
0x81 opcode is an ADD, OR, ADC, SBB, AND, SUB, XOR or CMP. So if one of
the other seven happens, it will silently execute as an AND. I don't
think it is useful to implement the other seven but they should be
rejected explicitly (if the ModRM reg field is not 4).

A hardware AND of any kind will update %rflags and compilers may take
advantage of this to branch based on the result without the need for a
separate instruction to test it. I suppose this is unlikely here (the
AND serving only to clear one or more bits).

> Modified:
>   projects/bhyve/sys/amd64/vmm/intel/vmx.c
>   projects/bhyve/sys/amd64/vmm/vmm_instruction_emul.c
> 
> Modified: projects/bhyve/sys/amd64/vmm/intel/vmx.c
> ==============================================================================
> --- projects/bhyve/sys/amd64/vmm/intel/vmx.c	Thu Nov 29 05:46:46 2012	(r243666)
> +++ projects/bhyve/sys/amd64/vmm/intel/vmx.c	Thu Nov 29 06:26:42 2012	(r243667)
> @@ -1159,15 +1159,16 @@ vmx_ept_fault(struct vm *vm, int cpu,
>  	if (ept_qual & EPT_VIOLATION_INST_FETCH)
>  		return (UNHANDLED);
>  
> -	/* EPT violation must be a read fault or a write fault but not both */
> +	/* EPT violation must be a read fault or a write fault */
>  	read = ept_qual & EPT_VIOLATION_DATA_READ ? 1 : 0;
>  	write = ept_qual & EPT_VIOLATION_DATA_WRITE ? 1 : 0;
> -	if ((read ^ write) == 0)
> +	if ((read | write) == 0)
>  		return (UNHANDLED);
>  
>  	/*
> -	 * The EPT violation must have been caused by accessing a guest-physical
> -	 * address that is a translation of a guest-linear address.
> +	 * The EPT violation must have been caused by accessing a
> +	 * guest-physical address that is a translation of a guest-linear
> +	 * address.
>  	 */
>  	if ((ept_qual & EPT_VIOLATION_GLA_VALID) == 0 ||
>  	    (ept_qual & EPT_VIOLATION_XLAT_VALID) == 0) {
> 
> Modified: projects/bhyve/sys/amd64/vmm/vmm_instruction_emul.c
> ==============================================================================
> --- projects/bhyve/sys/amd64/vmm/vmm_instruction_emul.c	Thu Nov 29 05:46:46 2012	(r243666)
> +++ projects/bhyve/sys/amd64/vmm/vmm_instruction_emul.c	Thu Nov 29 06:26:42 2012	(r243667)
> @@ -81,6 +81,11 @@ static const struct vie_op one_byte_opco
>  	[0x23] = {
>  		.op_byte = 0x23,
>  		.op_type = VIE_OP_TYPE_AND,
> +	},
> +	[0x81] = {
> +		.op_byte = 0x81,
> +		.op_type = VIE_OP_TYPE_AND,
> +		.op_flags = VIE_OP_F_IMM,
>  	}
>  };
>  
> @@ -299,6 +304,30 @@ emulate_and(void *vm, int vcpuid, uint64
>  		val1 &= val2;
>  		error = vie_update_register(vm, vcpuid, reg, val1, size);
>  		break;
> +	case 0x81:
> +		printf("0x81 AND\n");
> +		/*
> +		 * AND reg (ModRM:reg) with immediate and store the
> +		 * result in reg
> +		 *
> +		 * 81/          and r/m32, imm32
> +		 * REX.W + 81/  and r/m64, imm32 sign-extended to 64
> +		 */


+		/*
+		 * OP mem (ModRM:r/m) with immediate and store the
+		 * result in mem
+		 *
+		 * 81/          op r/m32, imm32
+		 * REX.W + 81/  op r/m64, imm32 sign-extended to 64
+		 *
+		 * Currently, only the AND operation is implemented
+		 * (ModRM:reg = 4).
+		 */

> +		if (vie->rex_w)
> +			size = 8;
> +		
> +		/* get the first operand */
> +                error = memread(vm, vcpuid, gpa, &val1, size, arg);
> +                if (error)
> +			break;
> +
> +                /*
> +		 * perform the operation with the pre-fetched immediate
> +		 * operand and write the result
> +		 */
> +                val1 &= vie->immediate;
> +                error = memwrite(vm, vcpuid, gpa, val1, size, arg);
> +		break;
>  	default:
>  		break;
>  	}

-- 
Jilles Tjoelker



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121129152550.GB29338>