| Message ID | 20220107093134.136441-5-yang.zhong@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AMX support in Qemu | expand |
> 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? 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?). > } 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; > } > } > }
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; > > } > > } > > }
On 1/7/22 10:31, Yang Zhong wrote: > - uint32_t need_align; > + uint32_t need_align, support_xfd; These can be replaced by a single field "uint32_t ecx". You can add also macros like #define ESA_FEATURE_ALIGN64_BIT (1) #define ESA_FEATURE_XFD_BIT (2) to simplify access. Paolo
On Tue, Jan 18, 2022 at 01:52:51PM +0100, Paolo Bonzini wrote: > On 1/7/22 10:31, Yang Zhong wrote: > >- uint32_t need_align; > >+ uint32_t need_align, support_xfd; > > These can be replaced by a single field "uint32_t ecx". > > You can add also macros like > > #define ESA_FEATURE_ALIGN64_BIT (1) > #define ESA_FEATURE_XFD_BIT (2) > > to simplify access. Thanks Paolo, this is a more simplified solution, thanks! Yang > Paolo
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; } 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; } } }