Re: [Xen-devel] [PATCH v8 9/9] xen/x86: use PCID feature

[Juergen Gross <jgross@xxxxxxxx>]

On 18/04/18 11:13, Jan Beulich wrote:
>>>> On 18.04.18 at 10:30, <jgross@xxxxxxxx> wrote:
>> Avoid flushing the complete TLB when switching %cr3 for mitigation of
>> Meltdown by using the PCID feature if available.
>>
>> We are using 4 PCID values for a 64 bit pv domain subject to XPTI and
>> 2 values for the non-XPTI case:
>>
>> - guest active and in kernel mode
>> - guest active and in user mode
>> - hypervisor active and guest in user mode (XPTI only)
>> - hypervisor active and guest in kernel mode (XPTI only)
>>
>> We use PCID only if PCID _and_ INVPCID are supported. With PCID in use
>> we disable global pages in cr4. A command line parameter controls in
>> which cases PCID is being used.
>>
>> As the non-XPTI case has shown not to perform better with PCID at least
>> on some machines the default is to use PCID only for domains subject to
>> XPTI.
>>
>> With PCID enabled we always disable global pages. This avoids having to
>> either flush the complete TLB or do a cycle through all PCID values
>> when invalidating a single global page.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> V6.1:
>> - address some minor comments (Jan Beulich)
> 
> No v7 changes? Afaict ...
> 
>> @@ -717,7 +718,7 @@ int __init dom0_construct_pv(struct domain *d,
>>          update_cr3(v);
>>  
>>      /* We run on dom0's page tables for the final part of the build 
>> process. */
>> -    switch_cr3_cr4(v->arch.cr3, read_cr4());
>> +    switch_cr3_cr4(cr3_pa(v->arch.cr3), read_cr4());
> 
> ... at least this was added. And to be honest I'm not convinced cr3_pa() is
> the right construct to use here: It's neither clear why the other bits don't
> matter at this point, nor why any of the bits that you mask out this way
> need masking out in the first place (e.g. why, in the crash case that Andrew
> had observed, the noflush bit was wrongly set).

At this point in time we only want to use another cr3 value. So PCIDs
should _not_ be used as this would require adapting cr4, resulting in
writing the cr3_pa() bits only. It would have been possible to use
write_cr3() here, but only together with open coding a TLB flush in
order to avoid any global pages remaining in the TLB. So I decided to
use switch_cr3_cr4().

The noflush bit was set as dom0 is subject to XPTI and PCID usage and
update_cr3() is updating v->arch.cr3 accordingly.

I know you are not fond of setting noflush in v->arch.cr3. The only
sensible alternative to that would be adding something like
v->arch.cr3_pcid_bits being or-ed to the cr3 value in restore_all_guest.
This would let us get rid of having to use cr3_pa() in some places
while adding a (very little) performance penalty.

> I hope there aren't any other such hidden changes in this version of the
> series.

Oh sorry, I missed to update the history.

I'm not aware of other changes in v8 (apart from patch 1 and the
resulting needed change in patch 3).


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel