| Message ID | 20220107093134.136441-2-yang.zhong@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | AMX support in Qemu | expand |
> From: Zhong, Yang <yang.zhong@intel.com> > Sent: Friday, January 7, 2022 5:31 PM > > From: Jing Liu <jing2.liu@intel.com> > > The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1] > are all zero, while spec actually introduces that bit 01 > should indicate if the extended state component locates > on the next 64-byte boundary following the preceding state > component when the compacted format of an XSAVE area is > used. Above would read clearer if you revise to: "The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1] indicate whether the extended state component locates on the next 64-byte boundary following the preceding state component when the compacted format of an XSAVE area is used. But ECX[1] is always cleared in current implementation." > > Fix the subleaves value according to the host supported > cpuid. The upcoming AMX feature would be the first one > using it. > > Signed-off-by: Jing Liu <jing2.liu@intel.com> > Signed-off-by: Yang Zhong <yang.zhong@intel.com> > --- > target/i386/cpu.h | 1 + > target/i386/cpu.c | 1 + > target/i386/kvm/kvm-cpu.c | 3 +++ > 3 files changed, 5 insertions(+) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 04f2b790c9..7f9700544f 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1354,6 +1354,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8); > typedef struct ExtSaveArea { > uint32_t feature, bits; > uint32_t offset, size; > + uint32_t need_align; > } ExtSaveArea; > > #define XSAVE_STATE_AREA_COUNT (XSTATE_PKRU_BIT + 1) > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index aa9e636800..47bc4d5c1a 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -5487,6 +5487,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; > } > } > break; > diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c > index d95028018e..6c4c1c6f9d 100644 > --- a/target/i386/kvm/kvm-cpu.c > +++ b/target/i386/kvm/kvm-cpu.c > @@ -105,6 +105,9 @@ static void kvm_cpu_xsave_init(void) > assert(esa->size == sz); > esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX); > } > + > + uint32_t ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX); > + esa->need_align = ecx & (1u << 1) ? 1 : 0; > } > } > }
On Mon, Jan 10, 2022 at 04:20:41PM +0800, Tian, Kevin wrote: > > From: Zhong, Yang <yang.zhong@intel.com> > > Sent: Friday, January 7, 2022 5:31 PM > > > > From: Jing Liu <jing2.liu@intel.com> > > > > The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1] > > are all zero, while spec actually introduces that bit 01 > > should indicate if the extended state component locates > > on the next 64-byte boundary following the preceding state > > component when the compacted format of an XSAVE area is > > used. > > Above would read clearer if you revise to: > > "The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1] > indicate whether the extended state component locates > on the next 64-byte boundary following the preceding state > component when the compacted format of an XSAVE area is > used. > > But ECX[1] is always cleared in current implementation." Thanks Kevin, I will update this in next version. Yang
On 1/11/22 03:22, Yang Zhong wrote:
> Thanks Kevin, I will update this in next version.
Also:
The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1]
indicate whether the extended state component locates
on the next 64-byte boundary following the preceding state
component when the compacted format of an XSAVE area is
used.
Right now, they are all zero because no supported component
needed the bit to be set, but the upcoming AMX feature will
use it. Fix the subleaves value according to KVM's supported
cpuid.
Paolo
On Tue, Jan 18, 2022 at 01:37:20PM +0100, Paolo Bonzini wrote: > On 1/11/22 03:22, Yang Zhong wrote: > > Thanks Kevin, I will update this in next version. > > Also: > > The extended state subleaves (EAX=0Dh, ECX=n, n>1).ECX[1] > indicate whether the extended state component locates > on the next 64-byte boundary following the preceding state > component when the compacted format of an XSAVE area is > used. > > Right now, they are all zero because no supported component > needed the bit to be set, but the upcoming AMX feature will > use it. Fix the subleaves value according to KVM's supported > cpuid. > Thanks Paolo, I will update this in new version. Yang > Paolo
diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 04f2b790c9..7f9700544f 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1354,6 +1354,7 @@ QEMU_BUILD_BUG_ON(sizeof(XSavePKRU) != 0x8); typedef struct ExtSaveArea { uint32_t feature, bits; uint32_t offset, size; + uint32_t need_align; } ExtSaveArea; #define XSAVE_STATE_AREA_COUNT (XSTATE_PKRU_BIT + 1) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index aa9e636800..47bc4d5c1a 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5487,6 +5487,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; } } break; diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c index d95028018e..6c4c1c6f9d 100644 --- a/target/i386/kvm/kvm-cpu.c +++ b/target/i386/kvm/kvm-cpu.c @@ -105,6 +105,9 @@ static void kvm_cpu_xsave_init(void) assert(esa->size == sz); esa->offset = kvm_arch_get_supported_cpuid(s, 0xd, i, R_EBX); } + + uint32_t ecx = kvm_arch_get_supported_cpuid(s, 0xd, i, R_ECX); + esa->need_align = ecx & (1u << 1) ? 1 : 0; } } }