Re: [PATCH] KVM: x86: Fix C++ user API for structures with variable length arrays

[Sean Christopherson]

On Thu, Feb 26, 2026, David Woodhouse wrote:
> From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> 
> Commit 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with
> flexible-array members") broke the userspace API for C++. Not just in
> the sense of 'userspace needs to be updated, but UAPI is supposed to be
> stable", but broken in the sense that I can't actually see *how* the
> structures can be used from C++ in the same way that they were usable
> before.
> 
> These structures ending in VLAs are typically a *header*, which can be
> followed by an arbitrary number of entries. Userspace typically creates
> a larger structure with some non-zero number of entries, for example in
> QEMU's kvm_arch_get_supported_msr_feature():
> 
>     struct {
>         struct kvm_msrs info;
>         struct kvm_msr_entry entries[1];
>     } msr_data = {};
> 
> While that works in C, it fails in C++ with an error like:
>  flexible array member ‘kvm_msrs::entries’ not at end of ‘struct msr_data’
> 
> Fix this by using __DECLARE_FLEX_ARRAY() for the VLA, which is a helper
> provided by <linux/stddef.h> that already uses [0] for C++ compilation.
> 
> Also put the header fields into a struct_group() to provide (in C) a
> separate struct (e.g 'struct kvm_msrs_hdr') without the trailing VLA.

Unless I'm missing something, this is an entirely optional change that needs to
be done separately, especialy since I want to tag this for:

  Cc: stable@xxxxxxxxxxxxxxx

I definitely don't hate the __struct_group definitions, but I don't know that I
love them either as they make the code a bit harder to read, and more importantly
there's a non-zero chance that defining the new structurs could break userspace
builds and force an update, e.g. if userspace already concocts its own header
overlay, which would be very unpleasant for a stable@ patch.

If we do define headers, I think I'd want a wrapper around __struct_group() to
prettify the common case and force consistent naming, e.g.

#define kvm_struct_header(NAME, MEMBERS...)				\
	__struct_group(NAME ##_header, h, /* no attrs */, MEMBERS)

struct kvm_msrs {
	kvm_struct_header(kvm_msrs,
		__u32 nmsrs; /* number of msrs in entries */
		__u32 pad;
	);

	__DECLARE_FLEX_ARRAY(struct kvm_msr_entry, entries);
};

But that's likely going to lead to some amount of bikeshedding, e.g. arguably
kvm_header() would be sufficient and easier on the eyes.  Which is all the more
reason to handle it separately.

> Fixes: 94dfc73e7cf4 ("treewide: uapi: Replace zero-length arrays with flexible-array members")
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> ---
>  arch/x86/include/uapi/asm/kvm.h | 29 ++++++++++++++++++-----------
>  include/uapi/linux/kvm.h        |  9 ++++++---
>  /* for KVM_GET_PIT and KVM_SET_PIT */
> @@ -397,8 +402,10 @@ struct kvm_xsave {
>  	 * The offsets of the state save areas in struct kvm_xsave follow
>  	 * the contents of CPUID leaf 0xD on the host.
>  	 */
> -	__u32 region[1024];
> -	__u32 extra[];
> +	__struct_group(kvm_xsave_hdr, hdr, /* no attrs */,
> +		__u32 region[1024];
> +	);

This is *very* misleading, as XSTATE itself has a header, but this is something
else entirely (just the always-allocated region).

> +	__DECLARE_FLEX_ARRAY(__u32, extra);
>  };

There are several structs that got missed:

  kvm_pmu_event_filter
  kvm_reg_list
  kvm_signal_mask
  kvm_coalesced_mmio_ring
  kvm_cpuid
  kvm_stats_desc