Re: [PATCH 08/10] nEPT: Nested INVEPT

2011-12-11 Thread Nadav Har'El 
On Thu, Nov 10, 2011, Avi Kivity wrote about Re: [PATCH 08/10] nEPT: Nested 
INVEPT:
 On 11/10/2011 12:01 PM, Nadav Har'El wrote:
  If we let L1 use EPT, we should probably also support the INVEPT 
  instruction.
..
  +   if (vmcs12  nested_cpu_has_ept(vmcs12) 
  +   (vmcs12-ept_pointer == operand.eptp) 
  +   vmx-nested.last_eptp02)
  +   ept_sync_context(vmx-nested.last_eptp02);
  +   else
  +   ept_sync_global();
 
 Are either of these needed?  Won't a write to a shadowed EPT table cause
 them anyway?

This is very good point... You're right that as it stands, any changes
to the guest EPT table (EPT12) will cause changes to the shadow EPT
table (EPT02), and these already cause KVM to do an INVEPT, so no point
to do this again when the guest asks.  So basically, I can have INVEPT
emulated by doing absolutely nothing (after checking all the checks), right?

I wonder if I am missing any reason why a hypervisor might want to do
INVEPT without changing the EPT12 table first.

-- 
Nadav Har'El|Sunday, Dec 11 2011, 
n...@math.technion.ac.il |-
Phone +972-523-790466, ICQ 13349191 |Why do programmers mix up Christmas and
http://nadav.harel.org.il   |Halloween? Because DEC 25 = OCT 31
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] nEPT: Nested INVEPT

2011-12-11 Thread Avi Kivity 
On 12/11/2011 04:24 PM, Nadav Har'El wrote:
 On Thu, Nov 10, 2011, Avi Kivity wrote about Re: [PATCH 08/10] nEPT: Nested 
 INVEPT:
  On 11/10/2011 12:01 PM, Nadav Har'El wrote:
   If we let L1 use EPT, we should probably also support the INVEPT 
   instruction.
 ..
   + if (vmcs12  nested_cpu_has_ept(vmcs12) 
   + (vmcs12-ept_pointer == operand.eptp) 
   + vmx-nested.last_eptp02)
   + ept_sync_context(vmx-nested.last_eptp02);
   + else
   + ept_sync_global();
  
  Are either of these needed?  Won't a write to a shadowed EPT table cause
  them anyway?

 This is very good point... You're right that as it stands, any changes
 to the guest EPT table (EPT12) will cause changes to the shadow EPT
 table (EPT02), and these already cause KVM to do an INVEPT, so no point
 to do this again when the guest asks.  So basically, I can have INVEPT
 emulated by doing absolutely nothing (after checking all the checks), right?

Right.  This was the case for INVLPG before we added out-of-sync pages;
we didn't even intercept the instruction.

 I wonder if I am missing any reason why a hypervisor might want to do
 INVEPT without changing the EPT12 table first.

Shouldn't happen, but why do you care?  If EPT12 has not changed, any
access through EPT02 or its TLB entry is valid.

-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] nEPT: Nested INVEPT

2011-11-10 Thread Avi Kivity 
On 11/10/2011 12:01 PM, Nadav Har'El wrote:
 If we let L1 use EPT, we should probably also support the INVEPT instruction.

 + case VMX_EPT_EXTENT_CONTEXT:
 + if (!(nested_vmx_ept_caps  VMX_EPT_EXTENT_CONTEXT_BIT))
 + nested_vmx_failValid(vcpu,
 + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 + else {
 + /*
 +  * We efficiently handle the common case, of L1
 +  * invalidating the last eptp it used to run L2.
 +  * TODO: Instead of saving one last_eptp02, look up
 +  * operand.eptp in the shadow EPT table cache, to
 +  * find its shadow. Then last_eptp02 won't be needed.
 +  */
 + struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
 + struct vcpu_vmx *vmx = to_vmx(vcpu);
 + if (vmcs12  nested_cpu_has_ept(vmcs12) 
 + (vmcs12-ept_pointer == operand.eptp) 
 + vmx-nested.last_eptp02)
 + ept_sync_context(vmx-nested.last_eptp02);
 + else
 + ept_sync_global();

Are either of these needed?  Won't a write to a shadowed EPT table cause
them anyway?

 + nested_vmx_succeed(vcpu);
 + }
 + break;
 + case VMX_EPT_EXTENT_INDIVIDUAL_ADDR:
 + if (!(nested_vmx_ept_caps  VMX_EPT_EXTENT_INDIVIDUAL_BIT))
 + nested_vmx_failValid(vcpu,
 + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 + else {
 + struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
 + struct vcpu_vmx *vmx = to_vmx(vcpu);
 + if (vmcs12  nested_cpu_has_ept(vmcs12) 
 + (vmcs12-ept_pointer == operand.eptp) 
 + vmx-nested.last_eptp02)
 + ept_sync_individual_addr(
 + vmx-nested.last_eptp02, operand.gpa);

Same here.

 + else
 + ept_sync_global();
 + nested_vmx_succeed(vcpu);
 + }
 + break;
 + default:
 + nested_vmx_failValid(vcpu,
 + VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 + }
 +
 + skip_emulated_instruction(vcpu);
 + return 1;
 +}
 +


-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html