Re: [PATCH v3A 1/6] target/i386: add support for FRED in CPUID enumeration

[Zhao Liu <zhao1.liu@intel.com>]

On Fri, Dec 22, 2023 at 03:34:02PM +0800, Zhao Liu wrote:
> Date: Fri, 22 Dec 2023 15:34:02 +0800
> From: Zhao Liu <zhao1.liu@intel.com>
> Subject: Re: [PATCH v3A 1/6] target/i386: add support for FRED in CPUID
>  enumeration
> 
> On Thu, Dec 21, 2023 at 07:03:36PM -0800, Xin Li wrote:
> > Date: Thu, 21 Dec 2023 19:03:36 -0800
> > From: Xin Li <xin3.li@intel.com>
> > Subject: [PATCH v3A 1/6] target/i386: add support for FRED in CPUID
> >  enumeration
> > X-Mailer: git-send-email 2.43.0
> > 
> > FRED, i.e., the Intel flexible return and event delivery architecture,
> > defines simple new transitions that change privilege level (ring
> > transitions).
> > 
> > The new transitions defined by the FRED architecture are FRED event
> > delivery and, for returning from events, two FRED return instructions.
> > FRED event delivery can effect a transition from ring 3 to ring 0, but
> > it is used also to deliver events incident to ring 0.  One FRED
> > instruction (ERETU) effects a return from ring 0 to ring 3, while the
> > other (ERETS) returns while remaining in ring 0.  Collectively, FRED
> > event delivery and the FRED return instructions are FRED transitions.
> > 
> > In addition to these transitions, the FRED architecture defines a new
> > instruction (LKGS) for managing the state of the GS segment register.
> > The LKGS instruction can be used by 64-bit operating systems that do
> > not use the new FRED transitions.
> > 
> > WRMSRNS is an instruction that behaves exactly like WRMSR, with the
> > only difference being that it is not a serializing instruction by
> > default.  Under certain conditions, WRMSRNS may replace WRMSR to improve
> > performance.  FRED uses it to switch RSP0 in a faster manner.
> > 
> > Search for the latest FRED spec in most search engines with this search
> > pattern:
> > 
> >   site:intel.com FRED (flexible return and event delivery) specification
> > 
> > The CPUID feature flag CPUID.(EAX=7,ECX=1):EAX[17] enumerates FRED, and
> > the CPUID feature flag CPUID.(EAX=7,ECX=1):EAX[18] enumerates LKGS, and
> > the CPUID feature flag CPUID.(EAX=7,ECX=1):EAX[19] enumerates WRMSRNS.
> > 
> > Add CPUID definitions for FRED/LKGS/WRMSRNS, and expose them to KVM guests.
> > 
> > Because FRED relies on LKGS and WRMSRNS, add that to feature dependency
> > map.
> > 
> > Tested-by: Shan Kang <shan.kang@intel.com>
> > Signed-off-by: Xin Li <xin3.li@intel.com>
> > ---
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> 
> > 
> > Changelog
> > v3A:
> > - Fix reversed dependency (Wu Dan1).
> > ---
> >  target/i386/cpu.c | 10 +++++++++-
> >  target/i386/cpu.h |  6 ++++++
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 358d9c0a65..66551c7eae 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -965,7 +965,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >              "avx-vnni", "avx512-bf16", NULL, "cmpccxadd",
> >              NULL, NULL, "fzrm", "fsrs",
> >              "fsrc", NULL, NULL, NULL,
> > -            NULL, NULL, NULL, NULL,
> > +            NULL, "fred", "lkgs", "wrmsrns",
> >              NULL, "amx-fp16", NULL, "avx-ifma",
> >              NULL, NULL, NULL, NULL,
> >              NULL, NULL, NULL, NULL,
> > @@ -1552,6 +1552,14 @@ static FeatureDep feature_dependencies[] = {
> >          .from = { FEAT_VMX_SECONDARY_CTLS,  VMX_SECONDARY_EXEC_ENABLE_USER_WAIT_PAUSE },
> >          .to = { FEAT_7_0_ECX,               CPUID_7_0_ECX_WAITPKG },
> >      },
> > +    {
> > +        .from = { FEAT_7_1_EAX,             CPUID_7_1_EAX_LKGS },
> > +        .to = { FEAT_7_1_EAX,               CPUID_7_1_EAX_FRED },
> > +    },
> > +    {
> > +        .from = { FEAT_7_1_EAX,             CPUID_7_1_EAX_WRMSRNS },
> > +        .to = { FEAT_7_1_EAX,               CPUID_7_1_EAX_FRED },
> > +    },

Oh, sorry, one thing that comes to mind, is this dependency required?
Since the FRED spec (v3.0) is all about WRMSR as the example, without
mentioning WRMSRNS, could there be other implementations that depend on
WRMSR instead of WRMSRNS?

The dependencies of LKGS are clearly defined in spec.

-Zhao

> >  };
> >  
> >  typedef struct X86RegisterInfo32 {
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index cd2e295bd6..5faf00551d 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -934,6 +934,12 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
> >  #define CPUID_7_1_EDX_AMX_COMPLEX       (1U << 8)
> >  /* PREFETCHIT0/1 Instructions */
> >  #define CPUID_7_1_EDX_PREFETCHITI       (1U << 14)
> > +/* Flexible return and event delivery (FRED) */
> > +#define CPUID_7_1_EAX_FRED              (1U << 17)
> > +/* Load into IA32_KERNEL_GS_BASE (LKGS) */
> > +#define CPUID_7_1_EAX_LKGS              (1U << 18)
> > +/* Non-Serializing Write to Model Specific Register (WRMSRNS) */
> > +#define CPUID_7_1_EAX_WRMSRNS           (1U << 19)
> >  
> >  /* Do not exhibit MXCSR Configuration Dependent Timing (MCDT) behavior */
> >  #define CPUID_7_2_EDX_MCDT_NO           (1U << 5)
> > -- 
> > 2.43.0
> > 
> > 
>