Re: [RFC PATCH 4/7] x86: Add XFD faulting bit for state components

[Yang Zhong]

On Mon, Jan 10, 2022 at 04:38:18PM +0800, Tian, Kevin wrote:
> > From: Zhong, Yang <yang.zhong@intel.com>
> > Sent: Friday, January 7, 2022 5:32 PM
> >
> > 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>
> > ---
> >  target/i386/cpu.h         | 2 +-
> >  target/i386/cpu.c         | 2 +-
> >  target/i386/kvm/kvm-cpu.c | 1 +
> >  3 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > index 79023fe723..22f7ff40a6 100644
> > --- a/target/i386/cpu.h
> > +++ b/target/i386/cpu.h
> > @@ -1375,7 +1375,7 @@
> > QEMU_BUILD_BUG_ON(sizeof(XSaveXTILE_DATA) != 0x2000);
> >  typedef struct ExtSaveArea {
> >      uint32_t feature, bits;
> >      uint32_t offset, size;
> > -    uint32_t need_align;
> > +    uint32_t need_align, support_xfd;
> 
> why each flag be a 32-bit field?
>
  
  Using the uint32_t to define those flags for below ecx value 
  *ecx = (esa->need_align << 1) | (esa->support_xfd << 2);

 
> also it's more natural to have them in separate lines, though I'm not
> sure why existing fields are put this way (possibly due to short names?).
> 

  Yes, support_xfd flag will be in another line to define, thanks!

  Yang


> >  } ExtSaveArea;
> >
> >  #define XSAVE_STATE_AREA_COUNT (XSTATE_XTILE_DATA_BIT + 1)
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index dd2c919c33..1adc3f0f99 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -5495,7 +5495,7 @@ 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->need_align << 1;
> > +                *ecx = (esa->need_align << 1) | (esa->support_xfd << 2);
> >              }
> >          }
> >          break;
> > diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
> > index 6c4c1c6f9d..3b3c203f11 100644
> > --- a/target/i386/kvm/kvm-cpu.c
> > +++ b/target/i386/kvm/kvm-cpu.c
> > @@ -108,6 +108,7 @@ static void kvm_cpu_xsave_init(void)
> >
> >              uint32_t ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX);
> >              esa->need_align = ecx & (1u << 1) ? 1 : 0;
> > +            esa->support_xfd = ecx & (1u << 2) ? 1 : 0;
> >          }
> >      }
> >  }