Re: [PATCH v2 4/5] libxc/PM: Ensure pxstat buffers are correctly sized

[Anthony PERARD <anthony@xxxxxxxxxxxxxx>]

On Mon, May 12, 2025 at 03:46:55PM +0100, Ross Lagerwall wrote:
> diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
> index ff7b5ada053f..cffbd1b8a955 100644
> --- a/tools/libs/ctrl/xc_pm.c
> +++ b/tools/libs/ctrl/xc_pm.c
> @@ -46,35 +46,33 @@ int xc_pm_get_pxstat(xc_interface *xch, int cpuid, struct 
> xc_px_stat *pxpt)
>  {
>      struct xen_sysctl sysctl = {};
>      /* Sizes unknown until xc_pm_get_max_px */

This comment can be removed now.

> -    DECLARE_NAMED_HYPERCALL_BOUNCE(trans, pxpt->trans_pt, 0, 
> XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> -    DECLARE_NAMED_HYPERCALL_BOUNCE(pt, pxpt->pt, 0, 
> XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> +    DECLARE_NAMED_HYPERCALL_BOUNCE(trans, pxpt->trans_pt,
> +                                   pxpt->total * pxpt->total,
> +                                   XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> +    DECLARE_NAMED_HYPERCALL_BOUNCE(pt, pxpt->pt, pxpt->total,
> +                                   XC_HYPERCALL_BUFFER_BOUNCE_BOTH);

I don't think the macro takes the sizeof(*pt) or sizeof(*trans_pt) into
account when using the size provided. So it doesn't looks like you can
use `pxpt->total` alone, and you still need to multiple it by sizeof(*)
like it was done in the HYPERCALL_BOUNCE_SET_SIZE() call.

[...]
> -    HYPERCALL_BOUNCE_SET_SIZE(trans, max_px * max_px * sizeof(uint64_t));
> -    HYPERCALL_BOUNCE_SET_SIZE(pt, max_px * sizeof(struct xc_px_val));


Thanks,

-- 
Anthony PERARD