Re: [PATCH v3 5/7] xen/x86: disable global pages for domains with XPTI active

[Juergen Gross <jgross@suse.com>]

On 22/03/18 17:30, Jan Beulich wrote:
>>>> On 21.03.18 at 13:51, <jgross@suse.com> wrote:
>> Instead of flushing the TLB from global pages when switching address
>> spaces with XPTI being active just disable global pages via %cr4
>> completely when a domain subject to XPTI is active. This avoids the
>> need for extra TLB flushes as loading %cr3 will remove all TLB
>> entries.
> 
> I continue to be not entirely convinced of this move. I had an
> alternative in mind: Since retaining global pages is particularly
> relevant for switches between guest user and guest kernel
> modes, what if we made a shortcut from e.g. lstar_enter through
> switch_to_kernel to restore_all_guest without ever switching to
> the full page Xen tables?

With patch 7 of this series in mind I'm not convinced the extra effort
is really making sense. Today most processors do have PCID support so
for that old hardware I don't think we need to make the handling even
more complex.

> 
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -508,18 +508,23 @@ void make_cr3(struct vcpu *v, mfn_t mfn)
>>  void write_ptbase(struct vcpu *v)
>>  {
>>      struct cpu_info *cpu_info = get_cpu_info();
>> +    unsigned long new_cr4;
>> +
>> +    new_cr4 = (is_pv_vcpu(v) && !is_idle_vcpu(v))
>> +              ? pv_guest_cr4_to_real_cr4(v) : mmu_cr4_features;
> 
> I'm not overly happy to see any new uses of mmu_cr4_features.
> This should really only be used for priming certain values imo,
> which isn't the case here (otoh pv_guest_cr4_to_real_cr4() does
> so too, and perhaps better wouldn't). Hence I wonder whether
> this shouldn't be read_cr4() | X86_CR4_PGE, not the least
> because we've just got rid of the blanket reversion to
> mmu_cr4_features in VMX code.

I do understand that using mmu_cr4_features isn't the best way to set
cr4. But I think it is a good idea to have a default value which should
normally be used instead of only switching various bits on and off.

In case cr4 is loaded with a strange value in some corner case that
value might be used from then on instead of being repaired by loading a
dedicated value at certain points in time, e.g. when doing a context
switch.

So maybe we should introduce cr4_default which is derived from
mmu_cr4_features? mmu_cr4_features would contain all bits which are
allowed on the current processor with the current command line options,
while cr4_default would be a subset of mmu_cr4_features.


Juergen

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