2023-09-14 12:06:44

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v10 08/38] x86/cpufeatures: Add the cpu feature bit for FRED

On 14.09.23 06:47, Xin Li wrote:
> From: "H. Peter Anvin (Intel)" <[email protected]>
>
> Any FRED CPU will always have the following features as its baseline:
> 1) LKGS, load attributes of the GS segment but the base address into
> the IA32_KERNEL_GS_BASE MSR instead of the GS segment’s descriptor
> cache.
> 2) WRMSRNS, non-serializing WRMSR for faster MSR writes.
>
> Signed-off-by: H. Peter Anvin (Intel) <[email protected]>
> Tested-by: Shan Kang <[email protected]>
> Signed-off-by: Xin Li <[email protected]>

In order to avoid having to add paravirt support for FRED I think
xen_init_capabilities() should gain:

+ setup_clear_cpu_cap(X86_FEATURE_FRED);


Juergen

> ---
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/kernel/cpu/cpuid-deps.c | 2 ++
> tools/arch/x86/include/asm/cpufeatures.h | 1 +
> 3 files changed, 4 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 330876d34b68..57ae93dc1e52 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -321,6 +321,7 @@
> #define X86_FEATURE_FZRM (12*32+10) /* "" Fast zero-length REP MOVSB */
> #define X86_FEATURE_FSRS (12*32+11) /* "" Fast short REP STOSB */
> #define X86_FEATURE_FSRC (12*32+12) /* "" Fast short REP {CMPSB,SCASB} */
> +#define X86_FEATURE_FRED (12*32+17) /* Flexible Return and Event Delivery */
> #define X86_FEATURE_LKGS (12*32+18) /* "" Load "kernel" (userspace) GS */
> #define X86_FEATURE_WRMSRNS (12*32+19) /* "" Non-Serializing Write to Model Specific Register instruction */
> #define X86_FEATURE_AMX_FP16 (12*32+21) /* "" AMX fp16 Support */
> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index e462c1d3800a..b7174209d855 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -82,6 +82,8 @@ static const struct cpuid_dep cpuid_deps[] = {
> { X86_FEATURE_XFD, X86_FEATURE_XGETBV1 },
> { X86_FEATURE_AMX_TILE, X86_FEATURE_XFD },
> { X86_FEATURE_SHSTK, X86_FEATURE_XSAVES },
> + { X86_FEATURE_FRED, X86_FEATURE_LKGS },
> + { X86_FEATURE_FRED, X86_FEATURE_WRMSRNS },
> {}
> };
>
> diff --git a/tools/arch/x86/include/asm/cpufeatures.h b/tools/arch/x86/include/asm/cpufeatures.h
> index 1b9d86ba5bc2..18bab7987d7f 100644
> --- a/tools/arch/x86/include/asm/cpufeatures.h
> +++ b/tools/arch/x86/include/asm/cpufeatures.h
> @@ -317,6 +317,7 @@
> #define X86_FEATURE_FZRM (12*32+10) /* "" Fast zero-length REP MOVSB */
> #define X86_FEATURE_FSRS (12*32+11) /* "" Fast short REP STOSB */
> #define X86_FEATURE_FSRC (12*32+12) /* "" Fast short REP {CMPSB,SCASB} */
> +#define X86_FEATURE_FRED (12*32+17) /* Flexible Return and Event Delivery */
> #define X86_FEATURE_LKGS (12*32+18) /* "" Load "kernel" (userspace) GS */
> #define X86_FEATURE_WRMSRNS (12*32+19) /* "" Non-Serializing Write to Model Specific Register instruction */
> #define X86_FEATURE_AMX_FP16 (12*32+21) /* "" AMX fp16 Support */


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature.asc (505.00 B)
OpenPGP digital signature
Download all attachments

2023-09-14 15:03:24

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH v10 08/38] x86/cpufeatures: Add the cpu feature bit for FRED

On 14.09.2023 08:03, Juergen Gross wrote:
> On 14.09.23 06:47, Xin Li wrote:
>> From: "H. Peter Anvin (Intel)" <[email protected]>
>>
>> Any FRED CPU will always have the following features as its baseline:
>> 1) LKGS, load attributes of the GS segment but the base address into
>> the IA32_KERNEL_GS_BASE MSR instead of the GS segment’s descriptor
>> cache.
>> 2) WRMSRNS, non-serializing WRMSR for faster MSR writes.
>>
>> Signed-off-by: H. Peter Anvin (Intel) <[email protected]>
>> Tested-by: Shan Kang <[email protected]>
>> Signed-off-by: Xin Li <[email protected]>
>
> In order to avoid having to add paravirt support for FRED I think
> xen_init_capabilities() should gain:
>
> + setup_clear_cpu_cap(X86_FEATURE_FRED);

I don't view it as very likely that Xen would expose FRED to PV guests
(Andrew?), at which point such a precaution may not be necessary.

Jan

2023-09-15 02:11:50

by Andrew Cooper

[permalink] [raw]
Subject: Re: [PATCH v10 08/38] x86/cpufeatures: Add the cpu feature bit for FRED

On 14/09/2023 7:09 am, Jan Beulich wrote:
> On 14.09.2023 08:03, Juergen Gross wrote:
>> On 14.09.23 06:47, Xin Li wrote:
>>> From: "H. Peter Anvin (Intel)" <[email protected]>
>>>
>>> Any FRED CPU will always have the following features as its baseline:
>>> 1) LKGS, load attributes of the GS segment but the base address into
>>> the IA32_KERNEL_GS_BASE MSR instead of the GS segment’s descriptor
>>> cache.
>>> 2) WRMSRNS, non-serializing WRMSR for faster MSR writes.
>>>
>>> Signed-off-by: H. Peter Anvin (Intel) <[email protected]>
>>> Tested-by: Shan Kang <[email protected]>
>>> Signed-off-by: Xin Li <[email protected]>
>> In order to avoid having to add paravirt support for FRED I think
>> xen_init_capabilities() should gain:
>>
>> + setup_clear_cpu_cap(X86_FEATURE_FRED);
> I don't view it as very likely that Xen would expose FRED to PV guests
> (Andrew?), at which point such a precaution may not be necessary.

PV guests are never going to see FRED (or LKGS for that matter) because
it advertises too much stuff which simply traps because the kernel is in
CPL3.

That said, the 64bit PV ABI is a whole lot closer to FRED than it is to
IDT delivery.  (Almost as if we decided 15 years ago that giving the PV
guest kernel a good stack and GSbase was the right thing to do...)

In some copious free time, I think we ought to provide a
minorly-paravirt FRED to PV guests because there are still some
improvements available as low hanging fruit.

My plan was to have a PV hypervisor leaf advertising paravirt versions
of hardware features, so a guest could see "I don't have architectural
FRED, but I do have paravirt-FRED which is as similar as we can
reasonably make it".  The same goes for a whole bunch of other features.

~Andrew

2023-09-15 04:41:33

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v10 08/38] x86/cpufeatures: Add the cpu feature bit for FRED

On Thu, Sep 14 2023 at 14:15, andrew wrote:
> PV guests are never going to see FRED (or LKGS for that matter) because
> it advertises too much stuff which simply traps because the kernel is in
> CPL3.
>
> That said, the 64bit PV ABI is a whole lot closer to FRED than it is to
> IDT delivery.  (Almost as if we decided 15 years ago that giving the PV
> guest kernel a good stack and GSbase was the right thing to do...)

No argument about that.

> In some copious free time, I think we ought to provide a
> minorly-paravirt FRED to PV guests because there are still some
> improvements available as low hanging fruit.
>
> My plan was to have a PV hypervisor leaf advertising paravirt versions
> of hardware features, so a guest could see "I don't have architectural
> FRED, but I do have paravirt-FRED which is as similar as we can
> reasonably make it".  The same goes for a whole bunch of other features.

*GROAN*

I told you before that we want less paravirt nonsense and not more. I'm
serious about that. XENPV CPL3 virtualization is a dead horse from a
technical POV. No point in wasting brain cycles to enhance the zombie
unless you can get rid of the existing PV nonsense, which you can't for
obvious reasons.

That said, we can debate this once the more fundamental issues of
XEN[PV] have been addressed. I expect that to happen quite some time
after I retired :)

Thanks,

tglx

2023-09-15 12:02:06

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH v10 08/38] x86/cpufeatures: Add the cpu feature bit for FRED

On 15.09.23 03:07, Thomas Gleixner wrote:
> On Thu, Sep 14 2023 at 14:15, andrew wrote:
>> PV guests are never going to see FRED (or LKGS for that matter) because
>> it advertises too much stuff which simply traps because the kernel is in
>> CPL3.
>>
>> That said, the 64bit PV ABI is a whole lot closer to FRED than it is to
>> IDT delivery.  (Almost as if we decided 15 years ago that giving the PV
>> guest kernel a good stack and GSbase was the right thing to do...)
>
> No argument about that.
>
>> In some copious free time, I think we ought to provide a
>> minorly-paravirt FRED to PV guests because there are still some
>> improvements available as low hanging fruit.
>>
>> My plan was to have a PV hypervisor leaf advertising paravirt versions
>> of hardware features, so a guest could see "I don't have architectural
>> FRED, but I do have paravirt-FRED which is as similar as we can
>> reasonably make it".  The same goes for a whole bunch of other features.
>
> *GROAN*
>
> I told you before that we want less paravirt nonsense and not more.

I agree.

We will still have to support the PV stuff for non-FRED hypervisors even with
pv-FRED being available on new Xen. So adding pv-FRED would just add more PV
interfaces without the ability to remove old stuff.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature.asc (505.00 B)
OpenPGP digital signature
Download all attachments