Re: [PATCH v2 4/6] xen/x86: disable global pages for domains with XPTI active

[Juergen Gross <jgross@suse.com>]

On 08/03/18 15:33, Jan Beulich wrote:
>>>> On 08.03.18 at 15:05, <jgross@suse.com> wrote:
>> On 08/03/18 14:38, Jan Beulich wrote:
>>>>>> On 02.03.18 at 09:14, <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.
>>>
>>> Hmm, it's far from obvious that this is an improvement overall.
>>> Besides Xen's global pages, we also prevent guest user pages to
>>> be evicted from the TLB across user <-> kernel mode changes.
>>> And the effects of this are likely quite work load dependent.
>>
>> With XPTI active we flush the TLB, including all global entries, when
>> switching between page tables when returning to the guest. So there are
>> no entries which could survive.
> 
> Well, right, but that's the "light" in "XPTI light". From a functionality
> POV we don't need the guest user mappings to be flushed. Did you
> happen to look at what the effect would be of running Xen with
> PTE.G uniformly clear (and with the CR4 writes dropped from the
> exit paths)?

Not yet. I wanted to do that when adding PCID support for XPTI=false to
have an idea whether it makes sense to keep the global pages in case a
processor doesn't support PCID or if there are cases where global pages
are better than using PCID.

> 
>>>> @@ -412,18 +414,22 @@ static void prepare_set(void)
>>>>  	write_cr0(read_cr0() | X86_CR0_CD);
>>>>  	wbinvd();
>>>>  
>>>> -	/*  TLB flushing here relies on Xen always using CR4.PGE. */
>>>> -	BUILD_BUG_ON(!(XEN_MINIMAL_CR4 & X86_CR4_PGE));
>>>> -	write_cr4(read_cr4() & ~X86_CR4_PGE);
>>>> +	cr4 = read_cr4();
>>>> +	if (cr4 & X86_CR4_PGE)
>>>> +		write_cr4(cr4 & ~X86_CR4_PGE);
>>>> +	else
>>>> +		asm volatile( "mov %0, %%cr3" : : "r" (read_cr3()) : "memory" );
>>>>  
>>>>  	/*  Save MTRR state */
>>>>  	rdmsrl(MSR_MTRRdefType, deftype);
>>>>  
>>>>  	/*  Disable MTRRs, and set the default type to uncached  */
>>>>  	mtrr_wrmsr(MSR_MTRRdefType, deftype & ~0xcff);
>>>> +
>>>> +	return !!(cr4 & X86_CR4_PGE);
>>>
>>> Unnecessary !!.
>>
>> Return type is bool. Isn't !! better in this case?
> 
> That was strictly a requirement when we were still using bool_t.
> But with bool the compiler is required to do the conversion even
> without the !! (and you'll observe that step by step we're getting
> rid of those !! now).

Okay, I'll drop the !!.


Juergen

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