On Tue, Sep 08, 2026 at 01:26:28PM +0200, Magnus Kulke wrote:
> On Mon, Sep 07, 2026 at 05:23:54PM +0300, Doru Blânzeanu wrote:
> > 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 <[email protected]>
> > ---
> > accel/mshv/mshv-all.c | 21 +++++++++++++++++++++
> > accel/mshv/trace-events | 2 ++
> > include/system/mshv_int.h | 3 +++
> > target/i386/mshv/mshv-cpu.c | 34 +++++++++++++++++++++++++---------
> > 4 files changed, 51 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..9d0957a65b 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 xsave_data_size;
>
> maybe xsavec_data_size to indicate that it's compacted?
>
> > };
> >
> > 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..7ce05bfb17 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->xsave_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->xsave_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,24 @@ void mshv_arch_init_vcpu(CPUState *cpu)
> > int ret;
> > X86XSaveHeader *header;
> >
> > + /* get the xsave data size */
> > + if (!mshv_state->xsave_data_size) {
>
> I assume we are doing that here to not do it for every vcpu? if yes,
> maybe we can use the `cpu_is_bsp(cpu);` to make it more explicit?
>
Yes, this is meant to be done only once for the first vCPU, and the retrieved
xsave size value to be used later in get/set xsave.
> > + ret = mshv_get_max_xsave_size(mshv_state->vm,
> > + &mshv_state->xsave_data_size);
> > + if (ret < 0) {
> > + warn_report("failed to get the max xsave area size: %s",
> > + strerror(errno));
> > + /*
> > + * Use the maximum size for xsave because the partition is
> > + * provisioned with every XSAVE component supported
> > + */
>
> do we expect the call to fail in some circumstance? I'm not sure a
> graceful fallback is a good idea, the max size will vary depending on the
> host CPU and failing hv calls should be a strong signal that something
> is broken and abort the guest launch.
>
This should not fail, but I agree that in case an unexpected value is returned,
failing is a signal that something is broken.
> > + mshv_state->xsave_data_size =
> > + ROUND_UP(mshv_get_supported_cpuid(0xD, 0, R_ECX), page);
> > + }
> > + /* never allow a zero-sized xsave area */
> > + mshv_state->xsave_data_size = MAX(mshv_state->xsave_data_size,
> > page);
> > + }
> > +
> > /* sanity check, to make sure we don't overflow the page */
> > QEMU_BUILD_BUG_ON((MAX_REGISTER_COUNT
> > * sizeof(hv_register_assoc)
> > @@ -2149,9 +2167,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->xsave_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
