Re: [RFC PATCH v1 01/10] pmu.h: add a BUILD_BUG_ON to ensure it fits within one page

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.07.2025 17:06, Edwin Török wrote:
> Linux already has a similar BUILD_BUG_ON.
> Currently this struct is ~224 bytes on x86-64.
> 
> No functional change.
> 
> Signed-off-by: Edwin Török <edwin.torok@xxxxxxxxx>
> ---
>  xen/arch/x86/cpu/vpmu.c  | 1 +
>  xen/include/public/pmu.h | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index c28192ea26..7be79c2d00 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -401,6 +401,7 @@ static int vpmu_arch_initialise(struct vcpu *v)
>      uint8_t vendor = current_cpu_data.x86_vendor;
>      int ret;
>  
> +    BUILD_BUG_ON(sizeof(struct xen_pmu_data) > PAGE_SIZE);
>      BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
>      BUILD_BUG_ON(sizeof(struct xen_pmu_amd_ctxt) > XENPMU_CTXT_PAD_SZ);
>      BUILD_BUG_ON(sizeof(struct xen_pmu_regs) > XENPMU_REGS_PAD_SZ);

I'm fine with this change, and in isolation it can have my ack.

> --- a/xen/include/public/pmu.h
> +++ b/xen/include/public/pmu.h
> @@ -93,6 +93,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_pmu_params_t);
>   * Architecture-independent fields of xen_pmu_data are WO for the hypervisor
>   * and RO for the guest but some fields in xen_pmu_arch can be writable
>   * by both the hypervisor and the guest (see arch-$arch/pmu.h).
> + *
> + * PAGE_SIZE bytes of memory are allocated.
> + * This struct cannot be larger than PAGE_SIZE.
>   */
>  struct xen_pmu_data {
>      /* Interrupted VCPU */

I'm not happy about this change: PAGE_SIZE is a Xen-internal entity, which
has no specific meaning in the public interface. (The fact that under io/
there are a number of similar references doesn't justify the use here.)

Jan