Re: [PATCH v3 1/5] x86/pmstat: Check size of PMSTAT_get_pxstat buffers

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2025 17:26, Ross Lagerwall wrote:
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -215,11 +215,22 @@ typedef struct pm_px_val pm_px_val_t;
>  DEFINE_XEN_GUEST_HANDLE(pm_px_val_t);
>  
>  struct pm_px_stat {
> -    uint8_t total;        /* total Px states */
> +    /*
> +     * IN: Number of elements in pt, number of rows/columns in trans_pt
> +     *     (PMSTAT_get_pxstat)
> +     * OUT: total Px states (PMSTAT_get_max_px, PMSTAT_get_pxstat)
> +     */
> +    uint8_t total;
>      uint8_t usable;       /* usable Px states */
>      uint8_t last;         /* last Px state */
>      uint8_t cur;          /* current Px state */
> -    XEN_GUEST_HANDLE_64(uint64) trans_pt;   /* Px transition table */
> +    /*
> +     * OUT: Px transition table. This should have total * total elements.
> +     *      As it is a 2-D array, this will not be copied if it is smaller 
> than
> +     *      the hypervisor's Px transition table. (PMSTAT_get_pxstat)
> +     */
> +    XEN_GUEST_HANDLE_64(uint64) trans_pt;
> +    /* OUT: This should have total elements (PMSTAT_get_pxstat) */
>      XEN_GUEST_HANDLE_64(pm_px_val_t) pt;
>  };

Commentary here is still confusing imo: Since "total" now has two meanings,
saying "This should have .." in OUT: descriptions is ambiguous. Imo for
trans_pt you want to say something like "will not be copied if input total is
less than output total", and for pt "the number of elements copied is the
smaller of input and output total".

If that's okay with you, I can edit things along these lines while committing,
at which point
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan