Re: [PATCH v3] KVM: x86: Implement PCID/INVPCID for guests with EPT

[Avi Kivity <avi@redhat.com>]

On 05/25/2012 03:59 AM, Marcelo Tosatti wrote:
>> > 
>> > The problem with enabling PCID for the L2 guest is that it can share same PCID
>> > values with the L1 hypervisor.
>> > 
>> > However, if the L1 hypervisor enables and configures VPID (given that
>> > the L0 hypervisor emulates and exposes it), there is no problem in enabling
>> > PCID for both L1 and L2 guests because TLB entries will be differentiated by
>> > their VPID values, even if their PCID values are the same.
>> > 
>> 
>> This may not be a problem because:
>> 
>> 1. If both L1 and L2 use VPID, there's no problem as you have mentioned.
>> 2. If neither L1 and L2 use VPID, the TLB entries are all tagged with VPID 0 and any VM entries or exits will invalidate them.
>> 3. If one of L1 and L2 uses VPID but the other don't, the TLB entries still have different VPID and won't affect each other.
>> 
>> I haven't thought over exposing PCID to L2 guests before but it seems that no problem exists in exposing PCID to L2 guests. Is it looks ok to you if PCID is always exposed, no matter for L1 or L2 guests?
> 
> Yes, it appears to be OK, because of the TLB flush on vm-entry/vm-exit
> without VPID (2 above).
> 
> Avi ?

I agree.

> 
>> > So i think that checking whether EPT _and_ VPID are supported should be a
>> > precondition to enable PCID support for guests.
>> > 
>> 
>> Should this check be carried out on L0 too? If so, this will add one more dependency of PCID on VPID, which doesn't exist in the manual.
> 
> Actually it is not necessary because L0 cannot access data which is
> EPT-tagged (and you already require EPT). 
> 
> So it is fine to remove the cpu_has_hypervisor check along with
> vmx_pcid_supported().
> 

We can hide INVPCID from the nested guest (by not exposing
IA32_EPT_VPID_CAP) but we can't avoid exposing PCID.  So we have to be
sure that if a nested guest sets CR4.PCID, it should work fine.

btw, it looks like the lines

	vmx_set_cr0(vcpu, vmcs12->guest_cr0);
	vmcs_writel(CR0_READ_SHADOW, nested_read_cr0(vmcs12));

	vmx_set_cr4(vcpu, vmcs12->guest_cr4);
	vmcs_writel(CR4_READ_SHADOW, nested_read_cr4(vmcs12));


in prepare_vmcs02() are too dangerous.  They only let the hardware check
cr4, but not software, so a future cr4 bit which is not handled
correctly by kvm can be set in a nested guest. L1 appears to be
protected, we use kvm_set_cr4().

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