[PATCH v2 0/2] acce/mshv: size XSAVE buffers from the hypervisor

[PATCH v2 0/2] acce/mshv: size XSAVE buffers from the hypervisor

[Doru Blânzeanu]

This patch series allows Qemu to dynamically size the xsave area from the
hypervisor by reading the property HV_PARTITION_PROPERTY_MAX_XSAVE_DATA_SIZE.

Previously, the xsave area size was set to 4kB and as a consequence AMX Tiles
feature couldn't be enabled because it needs more than the 4kB allocated.
With this change, Qemu enables all the xsave features in the partition, and it
queries the hypervisor to retrieve the maximum size of xsave.
The size is cached in `MshvState` at vCPU arch initialization time to be used
later for MSHV_{GET/SET}_VP_XSAVE.

To correctly advertise AMX support, we remove the logic disabling the AMX at
CPUID and partition level.

Changes from v1:
- change max xsave area size retrieval to be done only the first vcpu by
  checking against the `first_cpu` global
- change error handling for `mshv_get_max_xsave_size` and instead add assert for
  the return value to signal something broken in case of failure

Doru Blânzeanu (2):
  accel/mshv: size XSAVE buffers from the hypervisor
  accel/mshv: enable amx tiles support for guests

 accel/mshv/mshv-all.c       | 39 ++++++++++++++++++++++++------------
 accel/mshv/trace-events     |  2 ++
 include/system/mshv_int.h   |  3 +++
 target/i386/mshv/mshv-cpu.c | 40 ++++++++++++++-----------------------
 4 files changed, 46 insertions(+), 38 deletions(-)

-- 
2.53.0

[PATCH v2 1/2] accel/mshv: size XSAVE buffers from the hypervisor

[Doru Blânzeanu]

Query MAX_XSAVE_DATA_SIZE at vcpu creation and cache it in MshvState.
Introduce a new trace event for the xsave area size reported by the
hypervisor.

Also fixes a leak of xsavec_buf on the get error path and switches
qemu_memalign() buffers to qemu_vfree().
No functional change on hosts reporting under 4 KiB.

Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
 accel/mshv/mshv-all.c       | 21 +++++++++++++++++++++
 accel/mshv/trace-events     |  2 ++
 include/system/mshv_int.h   |  3 +++
 target/i386/mshv/mshv-cpu.c | 24 +++++++++++++++---------
 4 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/accel/mshv/mshv-all.c b/accel/mshv/mshv-all.c
index 5921ce693e..725ccd0511 100644
--- a/accel/mshv/mshv-all.c
+++ b/accel/mshv/mshv-all.c
@@ -162,6 +162,27 @@ static int get_proc_features(int vm_fd,
     return 0;
 }
 
+int mshv_get_max_xsave_size(int vm_fd, uint32_t *size)
+{
+    uint64_t value = 0;
+    int ret;
+
+    ret = get_partition_property(vm_fd,
+                                 HV_PARTITION_PROPERTY_MAX_XSAVE_DATA_SIZE,
+                                 &value);
+    if (ret < 0) {
+        error_report("Failed to get partition property MAX_XSAVE_DATA_SIZE");
+        return -1;
+    }
+
+    /* round up to page size */
+    *size = ROUND_UP(value, HV_HYP_PAGE_SIZE);
+
+    trace_mshv_xsave_data_size(value, *size);
+
+    return 0;
+}
+
 static int create_partition(int mshv_fd, int *vm_fd)
 {
     int ret;
diff --git a/accel/mshv/trace-events b/accel/mshv/trace-events
index 859e8bfb0f..9473530140 100644
--- a/accel/mshv/trace-events
+++ b/accel/mshv/trace-events
@@ -12,6 +12,8 @@ mshv_mem_ioeventfd_del(uint64_t addr, uint32_t size, uint32_t data) "addr=0x%" P
 
 mshv_hvcall_args(const char* hvcall, uint16_t code, uint16_t in_sz) "built args for '%s' code: %d in_sz: %d"
 
+mshv_xsave_data_size(uint64_t required, uint32_t allocated) "required=%" PRIu64 " allocated=%u"
+
 mshv_handle_interrupt(uint32_t cpu, int mask) "cpu_index=%d mask=0x%x"
 mshv_set_msi_routing(uint32_t gsi, uint64_t addr, uint32_t data) "gsi=%d addr=0x%" PRIx64 " data=0x%x"
 mshv_remove_msi_routing(uint32_t gsi) "gsi=%d"
diff --git a/include/system/mshv_int.h b/include/system/mshv_int.h
index 3dffe3c5fb..38b7525ef3 100644
--- a/include/system/mshv_int.h
+++ b/include/system/mshv_int.h
@@ -70,6 +70,8 @@ struct MshvState {
     unsigned long *used_gsi_bitmap;
     unsigned int gsi_count;
     union hv_partition_processor_features processor_features;
+    /* compacted xsave area size rounded up to the page size */
+    uint32_t xsavec_data_size;
 };
 
 typedef struct MshvMsiControl {
@@ -107,6 +109,7 @@ void mshv_arch_amend_proc_features(
 void mshv_arch_disable_partition_proc_features(
      union hv_partition_processor_features *disabled_features);
 int mshv_arch_post_init_vm(int vm_fd);
+int mshv_get_max_xsave_size(int vm_fd, uint32_t *size);
 int mshv_get_vp_state(int cpu_fd, struct mshv_get_set_vp_state *state);
 int mshv_set_vp_state(int cpu_fd, const struct mshv_get_set_vp_state *state);
 typedef struct mshv_root_hvcall mshv_root_hvcall;
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index f528dd2b9a..5b845b3e4e 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -157,9 +157,8 @@ static int get_xsave_state(CPUState *cpu)
     int ret;
     void *xsavec_buf;
     const size_t page = HV_HYP_PAGE_SIZE;
-    size_t xsavec_buf_len = page;
+    size_t xsavec_buf_len = mshv_state->xsavec_data_size;
 
-    /* TODO: should properly determine xsavec size based on CPUID */
     xsavec_buf = qemu_memalign(page, xsavec_buf_len);
     memset(xsavec_buf, 0, xsavec_buf_len);
 
@@ -172,11 +171,12 @@ static int get_xsave_state(CPUState *cpu)
     ret = ioctl(cpu_fd, MSHV_GET_VP_STATE, &args);
     if (ret < 0) {
         error_report("failed to get xsave state: %s", strerror(errno));
+        qemu_vfree(xsavec_buf);
         return -errno;
     }
 
     ret = decompact_xsave_area(xsavec_buf, xsavec_buf_len, env);
-    g_free(xsavec_buf);
+    qemu_vfree(xsavec_buf);
     if (ret < 0) {
         error_report("failed to decompact xsave area");
         return ret;
@@ -196,8 +196,8 @@ static int set_xsave_state(const CPUState *cpu)
     size_t page = HV_HYP_PAGE_SIZE, xsavec_buf_len;
 
     /* allocate and populate compacted buffer */
-    xsavec_buf = qemu_memalign(page, page);
-    xsavec_buf_len = page;
+    xsavec_buf_len = mshv_state->xsavec_data_size;
+    xsavec_buf = qemu_memalign(page, xsavec_buf_len);
 
     /* save registers to standard format buffer */
     x86_cpu_xsave_all_areas(x86cpu, env->xsave_buf, env->xsave_buf_len);
@@ -212,7 +212,7 @@ static int set_xsave_state(const CPUState *cpu)
     };
 
     ret = ioctl(cpu_fd, MSHV_SET_VP_STATE, &args);
-    g_free(xsavec_buf);
+    qemu_vfree(xsavec_buf);
     if (ret < 0) {
         error_report("failed to set xsave state: %s", strerror(errno));
         return -errno;
@@ -2125,6 +2125,14 @@ void mshv_arch_init_vcpu(CPUState *cpu)
     int ret;
     X86XSaveHeader *header;
 
+    /* get the xsave data size */
+    if (cpu == first_cpu) {
+        ret = mshv_get_max_xsave_size(mshv_state->vm,
+                                      &mshv_state->xsavec_data_size);
+        /* If this fails, something is broken */
+        assert(ret == 0);
+    }
+
     /* sanity check, to make sure we don't overflow the page */
     QEMU_BUILD_BUG_ON((MAX_REGISTER_COUNT
                       * sizeof(hv_register_assoc)
@@ -2149,9 +2157,7 @@ void mshv_arch_init_vcpu(CPUState *cpu)
 
     env->emu_mmio_buf = g_new(char, 4096);
 
-    /* Initialize XSAVE buffer page-aligned */
-    /* TODO: pick proper size based on CPUID */
-    xsave_len = page;
+    xsave_len = mshv_state->xsavec_data_size;
     env->xsave_buf = qemu_memalign(page, xsave_len);
     env->xsave_buf_len = xsave_len;
     memset(env->xsave_buf, 0, env->xsave_buf_len);
-- 
2.53.0

[PATCH v2 2/2] accel/mshv: enable amx tiles support for guests

[Doru Blânzeanu]

The AMX tiles feature uses a larger than 4kB area in xsave.
This fix removes the code that disables AMX tiles in the partition and it
adjusts the supported CPUID to advertise AMX tiles.

Signed-off-by: Doru Blânzeanu <dblanzeanu@linux.microsoft.com>
---
 accel/mshv/mshv-all.c       | 18 +++++-------------
 target/i386/mshv/mshv-cpu.c | 16 ----------------
 2 files changed, 5 insertions(+), 29 deletions(-)

diff --git a/accel/mshv/mshv-all.c b/accel/mshv/mshv-all.c
index 725ccd0511..99b2b3362a 100644
--- a/accel/mshv/mshv-all.c
+++ b/accel/mshv/mshv-all.c
@@ -187,7 +187,6 @@ static int create_partition(int mshv_fd, int *vm_fd)
 {
     int ret;
     uint64_t pt_flags, host_proc_features;
-    union hv_partition_processor_xsave_features disabled_xsave_features;
     union hv_partition_processor_features disabled_partition_features = {0};
 
     struct mshv_create_partition_v2 args = {0};
@@ -200,17 +199,6 @@ static int create_partition(int mshv_fd, int *vm_fd)
                (1ULL << MSHV_PT_BIT_GPA_SUPER_PAGES) |
                (1ULL << MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES);
 
-    /* enable all */
-    disabled_xsave_features.as_uint64 = 0;
-    /*
-     * AMX TILE XSAVE state (XTILE_DATA) is 8KB, which exceeds the
-     * current fixed 4KB XSAVE buffer size.
-     */
-    disabled_xsave_features.amx_tile_support = 1;
-    disabled_xsave_features.amx_bf16_support = 1;
-    disabled_xsave_features.amx_int8_support = 1;
-    disabled_xsave_features.amx_fp16_support = 1;
-
     /*
      * query host for supported processor features and disable unsupported
      * features: (0 means supported, 1 means disabled, hence the negation)
@@ -238,10 +226,14 @@ static int create_partition(int mshv_fd, int *vm_fd)
     args.pt_cpu_fbanks[0] |= disabled_partition_features.as_uint64[0];
     args.pt_cpu_fbanks[1] |= disabled_partition_features.as_uint64[1];
 
+    /*
+     * for now, all the xsave features are enabled, the filtering is only done
+     * at the CPUID level
+     * TODO: filter the xsave features based on the CPUID derived from Cpu model
+     */
     /* populate args structure */
     args.pt_flags = pt_flags;
     args.pt_isolation = MSHV_PT_ISOLATION_NONE;
-    args.pt_disabled_xsave = disabled_xsave_features.as_uint64;
     args.pt_num_cpu_fbanks = MSHV_NUM_CPU_FEATURES_BANKS;
 
     ret = ioctl(mshv_fd, MSHV_CREATE_PARTITION, &args);
diff --git a/target/i386/mshv/mshv-cpu.c b/target/i386/mshv/mshv-cpu.c
index 5b845b3e4e..21a3534578 100644
--- a/target/i386/mshv/mshv-cpu.c
+++ b/target/i386/mshv/mshv-cpu.c
@@ -2234,22 +2234,6 @@ uint32_t mshv_get_supported_cpuid(uint32_t func, uint32_t idx, int reg)
          */
         ret &= ~CPUID_7_0_ECX_LA57;
     }
-    if (func == 0x07 && idx == 0 && reg == R_EDX) {
-        /*
-         * AMX TILE XSAVE state (XTILE_DATA) is 8KB, which exceeds the
-         * current fixed 4KB XSAVE buffer size. Filter until buffer
-         * sizing is computed dynamically from CPUID.
-         */
-        ret &= ~CPUID_7_0_EDX_AMX_TILE;
-        ret &= ~CPUID_7_0_EDX_AMX_BF16;
-        ret &= ~CPUID_7_0_EDX_AMX_INT8;
-    }
-    if (func == 0x07 && idx == 1 && reg == R_EAX) {
-        ret &= ~CPUID_7_1_EAX_AMX_FP16;
-    }
-    if (func == 0x07 && idx == 1 && reg == R_EDX) {
-        ret &= ~CPUID_7_1_EDX_AMX_COMPLEX;
-    }
 
     return ret;
 }
-- 
2.53.0