On Wednesday, 2022-02-16 at 22:04:32 -08, Yang Zhong wrote:
> From: Jing Liu <jing2....@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....@intel.com>
> Signed-off-by: Zeng Guang <guang.z...@intel.com>
> Signed-off-by: Wei Wang <wei.w.w...@intel.com>
> Signed-off-by: Yang Zhong <yang.zh...@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.
--
Why does it have to be like this? I can never tell.
