| Message ID | 20230914044805.301390-9-xin3.li@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: enable FRED for x86-64 | expand |
On 14.09.23 06:47, Xin Li wrote: > From: "H. Peter Anvin (Intel)" <hpa@zytor.com> > > 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) <hpa@zytor.com> > Tested-by: Shan Kang <shan.kang@intel.com> > Signed-off-by: Xin Li <xin3.li@intel.com> 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 */
On 14.09.2023 08:03, Juergen Gross wrote: > On 14.09.23 06:47, Xin Li wrote: >> From: "H. Peter Anvin (Intel)" <hpa@zytor.com> >> >> 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) <hpa@zytor.com> >> Tested-by: Shan Kang <shan.kang@intel.com> >> Signed-off-by: Xin Li <xin3.li@intel.com> > > 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
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)" <hpa@zytor.com> >>> >>> 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) <hpa@zytor.com> >>> Tested-by: Shan Kang <shan.kang@intel.com> >>> Signed-off-by: Xin Li <xin3.li@intel.com> >> 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
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
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
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 */