| Message ID | 20220217060434.52460-7-yang.zhong@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | AMX support in Qemu | expand |
On Wednesday, 2022-02-16 at 22:04:32 -08, Yang Zhong wrote: > From: Jing Liu <jing2.liu@intel.com> > > When dynamic xfeatures (e.g. AMX) are used by the guest, the xsave > area would be larger than 4KB. KVM_GET_XSAVE2 and KVM_SET_XSAVE > under KVM_CAP_XSAVE2 works with a xsave buffer larger than 4KB. > Always use the new ioctls under KVM_CAP_XSAVE2 when KVM supports it. > > Signed-off-by: Jing Liu <jing2.liu@intel.com> > Signed-off-by: Zeng Guang <guang.zeng@intel.com> > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > Signed-off-by: Yang Zhong <yang.zhong@intel.com> > --- > target/i386/cpu.h | 4 ++++ > target/i386/kvm/kvm.c | 42 ++++++++++++++++++++++++-------------- > target/i386/xsave_helper.c | 33 ++++++++++++++++++++++++++++++ > 3 files changed, 64 insertions(+), 15 deletions(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index f7fc2e97a6..de9da38e42 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1528,6 +1528,10 @@ typedef struct CPUX86State { > uint64_t opmask_regs[NB_OPMASK_REGS]; > YMMReg zmmh_regs[CPU_NB_REGS]; > ZMMReg hi16_zmm_regs[CPU_NB_REGS]; > +#ifdef TARGET_X86_64 > + uint8_t xtilecfg[64]; > + uint8_t xtiledata[8192]; > +#endif Can we have defined constants for these sizes? They also appear in patch 2. > > /* sysenter registers */ > uint32_t sysenter_cs; > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 8562d3d138..ff064e3d8f 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -122,6 +122,7 @@ static uint32_t num_architectural_pmu_gp_counters; > static uint32_t num_architectural_pmu_fixed_counters; > > static int has_xsave; > +static int has_xsave2; > static int has_xcrs; > static int has_pit_state2; > static int has_sregs2; > @@ -1585,6 +1586,26 @@ static Error *invtsc_mig_blocker; > > #define KVM_MAX_CPUID_ENTRIES 100 > > +static void kvm_init_xsave(CPUX86State *env) > +{ > + if (has_xsave2) { > + env->xsave_buf_len = QEMU_ALIGN_UP(has_xsave2, 4096); Idle curiosity - why do we round this up? > + } else if (has_xsave) { > + env->xsave_buf_len = sizeof(struct kvm_xsave); > + } else { > + return; > + } > + > + env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len); > + memset(env->xsave_buf, 0, env->xsave_buf_len); > + /* > + * The allocated storage must be large enough for all of the > + * possible XSAVE state components. > + */ > + assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX) <= > + env->xsave_buf_len); > +} > + > int kvm_arch_init_vcpu(CPUState *cs) > { > struct { > @@ -1614,6 +1635,8 @@ int kvm_arch_init_vcpu(CPUState *cs) > > cpuid_i = 0; > > + has_xsave2 = kvm_check_extension(cs->kvm_state, KVM_CAP_XSAVE2); > + > r = kvm_arch_set_tsc_khz(cs); > if (r < 0) { > return r; > @@ -2003,19 +2026,7 @@ int kvm_arch_init_vcpu(CPUState *cs) > if (r) { > goto fail; > } > - > - if (has_xsave) { > - env->xsave_buf_len = sizeof(struct kvm_xsave); > - env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len); > - memset(env->xsave_buf, 0, env->xsave_buf_len); > - > - /* > - * The allocated storage must be large enough for all of the > - * possible XSAVE state components. > - */ > - assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX) > - <= env->xsave_buf_len); > - } > + kvm_init_xsave(env); > > max_nested_state_len = kvm_max_nested_state_length(); > if (max_nested_state_len > 0) { > @@ -3319,13 +3330,14 @@ static int kvm_get_xsave(X86CPU *cpu) > { > CPUX86State *env = &cpu->env; > void *xsave = env->xsave_buf; > - int ret; > + int type, ret; > > if (!has_xsave) { > return kvm_get_fpu(cpu); > } > > - ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_XSAVE, xsave); > + type = has_xsave2 ? KVM_GET_XSAVE2 : KVM_GET_XSAVE; > + ret = kvm_vcpu_ioctl(CPU(cpu), type, xsave); > if (ret < 0) { > return ret; > } > diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c > index ac61a96344..b6a004505f 100644 > --- a/target/i386/xsave_helper.c > +++ b/target/i386/xsave_helper.c > @@ -5,6 +5,7 @@ > #include "qemu/osdep.h" > > #include "cpu.h" > +#include <asm/kvm.h> > > void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen) > { > @@ -126,6 +127,22 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen) > > memcpy(pkru, &env->pkru, sizeof(env->pkru)); > } > + > + e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT]; > + if (e->size && e->offset) { > + XSaveXTILECFG *tilecfg = buf + e->offset; > + > + memcpy(tilecfg, &env->xtilecfg, sizeof(env->xtilecfg)); > + } > + > + if (buflen > sizeof(struct kvm_xsave)) { > + e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT]; > + if (e->size && e->offset && buflen >= e->size + e->offset) { > + XSaveXTILEDATA *tiledata = buf + e->offset; > + > + memcpy(tiledata, &env->xtiledata, sizeof(env->xtiledata)); > + } > + } > #endif > } > > @@ -247,5 +264,21 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const void *buf, uint32_t buflen) > pkru = buf + e->offset; > memcpy(&env->pkru, pkru, sizeof(env->pkru)); > } > + > + e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT]; > + if (e->size && e->offset) { > + const XSaveXTILECFG *tilecfg = buf + e->offset; > + > + memcpy(&env->xtilecfg, tilecfg, sizeof(env->xtilecfg)); > + } > + > + if (buflen > sizeof(struct kvm_xsave)) { > + e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT]; > + if (e->size && e->offset && buflen >= e->size + e->offset) { > + const XSaveXTILEDATA *tiledata = buf + e->offset; > + > + memcpy(&env->xtiledata, tiledata, sizeof(env->xtiledata)); > + } > + } > #endif > } dme.
On Mon, Feb 21, 2022 at 01:25:53PM +0000, David Edmondson wrote: > On Wednesday, 2022-02-16 at 22:04:32 -08, Yang Zhong wrote: > > > From: Jing Liu <jing2.liu@intel.com> > > > > When dynamic xfeatures (e.g. AMX) are used by the guest, the xsave > > area would be larger than 4KB. KVM_GET_XSAVE2 and KVM_SET_XSAVE > > under KVM_CAP_XSAVE2 works with a xsave buffer larger than 4KB. > > Always use the new ioctls under KVM_CAP_XSAVE2 when KVM supports it. > > > > Signed-off-by: Jing Liu <jing2.liu@intel.com> > > Signed-off-by: Zeng Guang <guang.zeng@intel.com> > > Signed-off-by: Wei Wang <wei.w.wang@intel.com> > > Signed-off-by: Yang Zhong <yang.zhong@intel.com> > > --- > > target/i386/cpu.h | 4 ++++ > > target/i386/kvm/kvm.c | 42 ++++++++++++++++++++++++-------------- > > target/i386/xsave_helper.c | 33 ++++++++++++++++++++++++++++++ > > 3 files changed, 64 insertions(+), 15 deletions(-) > > > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > > index f7fc2e97a6..de9da38e42 100644 > > --- a/target/i386/cpu.h > > +++ b/target/i386/cpu.h > > @@ -1528,6 +1528,10 @@ typedef struct CPUX86State { > > uint64_t opmask_regs[NB_OPMASK_REGS]; > > YMMReg zmmh_regs[CPU_NB_REGS]; > > ZMMReg hi16_zmm_regs[CPU_NB_REGS]; > > +#ifdef TARGET_X86_64 > > + uint8_t xtilecfg[64]; > > + uint8_t xtiledata[8192]; > > +#endif > > Can we have defined constants for these sizes? They also appear in patch > 2. David, the constants we used here are mainly consistent with other members in this struct and file. thanks! Yang
diff --git a/target/i386/cpu.h b/target/i386/cpu.h index f7fc2e97a6..de9da38e42 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1528,6 +1528,10 @@ typedef struct CPUX86State { uint64_t opmask_regs[NB_OPMASK_REGS]; YMMReg zmmh_regs[CPU_NB_REGS]; ZMMReg hi16_zmm_regs[CPU_NB_REGS]; +#ifdef TARGET_X86_64 + uint8_t xtilecfg[64]; + uint8_t xtiledata[8192]; +#endif /* sysenter registers */ uint32_t sysenter_cs; diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 8562d3d138..ff064e3d8f 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -122,6 +122,7 @@ static uint32_t num_architectural_pmu_gp_counters; static uint32_t num_architectural_pmu_fixed_counters; static int has_xsave; +static int has_xsave2; static int has_xcrs; static int has_pit_state2; static int has_sregs2; @@ -1585,6 +1586,26 @@ static Error *invtsc_mig_blocker; #define KVM_MAX_CPUID_ENTRIES 100 +static void kvm_init_xsave(CPUX86State *env) +{ + if (has_xsave2) { + env->xsave_buf_len = QEMU_ALIGN_UP(has_xsave2, 4096); + } else if (has_xsave) { + env->xsave_buf_len = sizeof(struct kvm_xsave); + } else { + return; + } + + env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len); + memset(env->xsave_buf, 0, env->xsave_buf_len); + /* + * The allocated storage must be large enough for all of the + * possible XSAVE state components. + */ + assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX) <= + env->xsave_buf_len); +} + int kvm_arch_init_vcpu(CPUState *cs) { struct { @@ -1614,6 +1635,8 @@ int kvm_arch_init_vcpu(CPUState *cs) cpuid_i = 0; + has_xsave2 = kvm_check_extension(cs->kvm_state, KVM_CAP_XSAVE2); + r = kvm_arch_set_tsc_khz(cs); if (r < 0) { return r; @@ -2003,19 +2026,7 @@ int kvm_arch_init_vcpu(CPUState *cs) if (r) { goto fail; } - - if (has_xsave) { - env->xsave_buf_len = sizeof(struct kvm_xsave); - env->xsave_buf = qemu_memalign(4096, env->xsave_buf_len); - memset(env->xsave_buf, 0, env->xsave_buf_len); - - /* - * The allocated storage must be large enough for all of the - * possible XSAVE state components. - */ - assert(kvm_arch_get_supported_cpuid(kvm_state, 0xd, 0, R_ECX) - <= env->xsave_buf_len); - } + kvm_init_xsave(env); max_nested_state_len = kvm_max_nested_state_length(); if (max_nested_state_len > 0) { @@ -3319,13 +3330,14 @@ static int kvm_get_xsave(X86CPU *cpu) { CPUX86State *env = &cpu->env; void *xsave = env->xsave_buf; - int ret; + int type, ret; if (!has_xsave) { return kvm_get_fpu(cpu); } - ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_XSAVE, xsave); + type = has_xsave2 ? KVM_GET_XSAVE2 : KVM_GET_XSAVE; + ret = kvm_vcpu_ioctl(CPU(cpu), type, xsave); if (ret < 0) { return ret; } diff --git a/target/i386/xsave_helper.c b/target/i386/xsave_helper.c index ac61a96344..b6a004505f 100644 --- a/target/i386/xsave_helper.c +++ b/target/i386/xsave_helper.c @@ -5,6 +5,7 @@ #include "qemu/osdep.h" #include "cpu.h" +#include <asm/kvm.h> void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen) { @@ -126,6 +127,22 @@ void x86_cpu_xsave_all_areas(X86CPU *cpu, void *buf, uint32_t buflen) memcpy(pkru, &env->pkru, sizeof(env->pkru)); } + + e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT]; + if (e->size && e->offset) { + XSaveXTILECFG *tilecfg = buf + e->offset; + + memcpy(tilecfg, &env->xtilecfg, sizeof(env->xtilecfg)); + } + + if (buflen > sizeof(struct kvm_xsave)) { + e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT]; + if (e->size && e->offset && buflen >= e->size + e->offset) { + XSaveXTILEDATA *tiledata = buf + e->offset; + + memcpy(tiledata, &env->xtiledata, sizeof(env->xtiledata)); + } + } #endif } @@ -247,5 +264,21 @@ void x86_cpu_xrstor_all_areas(X86CPU *cpu, const void *buf, uint32_t buflen) pkru = buf + e->offset; memcpy(&env->pkru, pkru, sizeof(env->pkru)); } + + e = &x86_ext_save_areas[XSTATE_XTILE_CFG_BIT]; + if (e->size && e->offset) { + const XSaveXTILECFG *tilecfg = buf + e->offset; + + memcpy(&env->xtilecfg, tilecfg, sizeof(env->xtilecfg)); + } + + if (buflen > sizeof(struct kvm_xsave)) { + e = &x86_ext_save_areas[XSTATE_XTILE_DATA_BIT]; + if (e->size && e->offset && buflen >= e->size + e->offset) { + const XSaveXTILEDATA *tiledata = buf + e->offset; + + memcpy(&env->xtiledata, tiledata, sizeof(env->xtiledata)); + } + } #endif }