Re: [PATCH fsgsbase v2 4/4] x86/fsgsbase: Fix Xen PV support

[Andrew Cooper]

On 29/06/2020 06:17, JÃrgen Groà wrote:
> On 26.06.20 19:24, Andy Lutomirski wrote:
>> On Xen PV, SWAPGS doesn't work. Teach __rdfsbase_inactive() and
>> __wrgsbase_inactive() to use rdmsrl()/wrmsrl() on Xen PV. The Xen
>> pvop code will understand this and issue the correct hypercalls.
>>
>> Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> Cc: Juergen Gross <jgross@xxxxxxxx>
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>> Â arch/x86/kernel/process_64.c | 20 ++++++++++++++------
>> Â 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
>> index cb8e37d3acaa..457d02aa10d8 100644
>> --- a/arch/x86/kernel/process_64.c
>> +++ b/arch/x86/kernel/process_64.c
>> @@ -163,9 +163,13 @@ static noinstr unsigned long
>> __rdgsbase_inactive(void)
>> Â ÂÂÂÂÂ lockdep_assert_irqs_disabled();
>> Â -ÂÂÂ native_swapgs();
>> -ÂÂÂ gsbase = rdgsbase();
>> -ÂÂÂ native_swapgs();
>> +ÂÂÂ if (!static_cpu_has(X86_FEATURE_XENPV)) {
>> +ÂÂÂÂÂÂÂ native_swapgs();
>> +ÂÂÂÂÂÂÂ gsbase = rdgsbase();
>> +ÂÂÂÂÂÂÂ native_swapgs();
>> +ÂÂÂ } else {
>> +ÂÂÂÂÂÂÂ rdmsrl(MSR_KERNEL_GS_BASE, gsbase);
>> +ÂÂÂ }
>> Â ÂÂÂÂÂ return gsbase;
>> Â }
>> @@ -182,9 +186,13 @@ static noinstr void __wrgsbase_inactive(unsigned
>> long gsbase)
>> Â {
>> ÂÂÂÂÂ lockdep_assert_irqs_disabled();
>> Â -ÂÂÂ native_swapgs();
>> -ÂÂÂ wrgsbase(gsbase);
>> -ÂÂÂ native_swapgs();
>> +ÂÂÂ if (!static_cpu_has(X86_FEATURE_XENPV)) {
>> +ÂÂÂÂÂÂÂ native_swapgs();
>> +ÂÂÂÂÂÂÂ wrgsbase(gsbase);
>> +ÂÂÂÂÂÂÂ native_swapgs();
>> +ÂÂÂ } else {
>> +ÂÂÂÂÂÂÂ wrmsrl(MSR_KERNEL_GS_BASE, gsbase);
>> +ÂÂÂ }
>> Â }
>> Â Â /*
>>
>
> Another possibility would be to just do (I'm fine either way):
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index acc49fa6a097..b78dd373adbf 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -318,6 +318,8 @@ static void __init xen_init_capabilities(void)
> ÂÂÂÂÂÂÂÂ setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> ÂÂÂÂÂÂÂÂ setup_clear_cpu_cap(X86_FEATURE_OSXSAVE);
> ÂÂÂÂ }
> +
> +ÂÂÂ setup_clear_cpu_cap(X86_FEATURE_FSGSBASE);

That will stop both userspace and Xen (side effect of the guest kernel's
CR4 choice) from using the instructions.

Even when the kernel is using the paravirt fastpath, its still Xen
actually taking the hit. MSR_{FS,GS}_BASE/SHADOW are thousands of
cycles to access, whereas {RD,WR}{FS,GS}BASE are a handful.

~Andrew