Re: [XEN][PATCH v7 17/19] tools/libs/ctrl: Implement new xc interfaces for dt overlay

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, Jun 01, 2023 at 05:48:22PM -0700, Vikram Garhwal wrote:
> --- /dev/null
> +++ b/tools/libs/ctrl/xc_dt_overlay.c
> +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
> +                  uint32_t overlay_fdt_size, uint8_t overlay_op)
> +{
> +    int err;
> +    DECLARE_SYSCTL;
> +
> +    DECLARE_HYPERCALL_BOUNCE(overlay_fdt, overlay_fdt_size,
> +                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +
> +    if ( (err = xc_hypercall_bounce_pre(xch, overlay_fdt)) )
> +        goto err;
> +
> +    sysctl.cmd = XEN_SYSCTL_dt_overlay;
> +    sysctl.u.dt_overlay.overlay_op = overlay_op;
> +    sysctl.u.dt_overlay.overlay_fdt_size = overlay_fdt_size;
> +    sysctl.u.dt_overlay.pad[0]= 0;
> +    sysctl.u.dt_overlay.pad[1]= 0;
> +    sysctl.u.dt_overlay.pad[2]= 0;

This is kind of silly to have to write explicitly 0 to each pads,
there's probably a better way.

I haven't find any example for "sysctl" but they are plenty for
"domctl", and I think this is maybe something that's wanted, so could
you replace "DECLARE_SYSCTL" by:

    struct xen_sysctl sysctl = {
        .cmd = XEN_SYSCTL_dt_overlay,
        .u.dt_overlay = {
            .overlay_op = overlay_op,
            .overlay_fdt_size = overlay_fdt_size,
        }
    };

With that, I don't think there would be need to set a value for "pad",
it would default to 0. And it maybe looks cleaner?

With that: Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks,

-- 
Anthony PERARD