Re: [PATCH v2 4/8] x86: Add XFD faulting bit for state components

[Yang Zhong]

On Mon, Feb 21, 2022 at 01:00:41PM +0000, David Edmondson wrote:
> On Wednesday, 2022-02-16 at 22:04:30 -08, Yang Zhong wrote:
> 
> > From: Jing Liu <jing2.liu@intel.com>
> >
> > Intel introduces XFD faulting mechanism for extended
> > XSAVE features to dynamically enable the features in
> > runtime. If CPUID (EAX=0Dh, ECX=n, n>1).ECX[2] is set
> > as 1, it indicates support for XFD faulting of this
> > state component.
> >
> > Signed-off-by: Jing Liu <jing2.liu@intel.com>
> > Signed-off-by: Yang Zhong <yang.zhong@intel.com>
> 
> Small comment below...
> 
> Reviewed-by: David Edmondson <david.edmondson@oracle.com>
> 
> > ---
> >  target/i386/cpu.h | 2 ++
> >  target/i386/cpu.c | 3 ++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index d4ad0f56bd..f7fc2e97a6 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -558,8 +558,10 @@ typedef enum X86Seg {
> >  #define ARCH_REQ_XCOMP_GUEST_PERM       0x1025
> >
> >  #define ESA_FEATURE_ALIGN64_BIT         1
> > +#define ESA_FEATURE_XFD_BIT             2
> >
> >  #define ESA_FEATURE_ALIGN64_MASK        (1U << ESA_FEATURE_ALIGN64_BIT)
> > +#define ESA_FEATURE_XFD_MASK            (1U << ESA_FEATURE_XFD_BIT)
> >
> >  /* CPUID feature words */
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 377d993438..5a7ee8c7e1 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5497,7 +5497,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> > uint32_t count,
> >                  const ExtSaveArea *esa = &x86_ext_save_areas[count];
> >                  *eax = esa->size;
> >                  *ebx = esa->offset;
> > -                *ecx = esa->ecx & ESA_FEATURE_ALIGN64_MASK;
> > +                *ecx = (esa->ecx & ESA_FEATURE_ALIGN64_MASK) |
> > +                       (esa->ecx & ESA_FEATURE_XFD_MASK);
> 
> Is:
> 
>                 *ecx = esa->ecx &
>                        (ESA_FEATURE_ALIGN64_MASK | ESA_FEATURE_XFD_MASK);
> 
> not more usual?


  Thanks David, I will update this in next version.

  Yang

> 
> >              }
> >          }
> >          break;
> 
> dme.
> -- 
> All of us, we're going out tonight. We're gonna walk all over your cars.