Re: [XEN][RFC PATCH v4 14/16] tools/libs/ctrl: Implement new xc interfaces for dt overlay

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Dec 06, 2022 at 10:18:13PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 0c8b4c3aa7..a71bc0bb1d 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -2633,6 +2633,9 @@ int xc_livepatch_replace(xc_interface *xch, char *name, 
> uint32_t timeout, uint32
>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
>  
> +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
> +                  uint32_t overlay_fdt_size, uint8_t overlay_op);
> +

Since xc_dt_overlay.c is only built on CONFIG_ARM=y, could you enclose
that prototype in "#if defined(__arm__) || defined(__aarch64__)"?

>  /* Compat shims */
>  #include "xenctrl_compat.h"
>  
> diff --git a/tools/libs/ctrl/Makefile.common b/tools/libs/ctrl/Makefile.common
> index 0a09c28fd3..247afbe5f9 100644
> --- a/tools/libs/ctrl/Makefile.common
> +++ b/tools/libs/ctrl/Makefile.common
> @@ -24,6 +24,7 @@ OBJS-y       += xc_hcall_buf.o
>  OBJS-y       += xc_foreign_memory.o
>  OBJS-y       += xc_kexec.o
>  OBJS-y       += xc_resource.o
> +OBJS-$(CONFIG_ARM)  += xc_dt_overlay.o
>  OBJS-$(CONFIG_X86) += xc_psr.o
>  OBJS-$(CONFIG_X86) += xc_pagetab.o
>  OBJS-$(CONFIG_Linux) += xc_linux.o
> diff --git a/tools/libs/ctrl/xc_dt_overlay.c b/tools/libs/ctrl/xc_dt_overlay.c
> new file mode 100644
> index 0000000000..325c9ef4c0
> --- /dev/null
> +++ b/tools/libs/ctrl/xc_dt_overlay.c
> +#include "xc_bitops.h"
> +#include "xc_private.h"
> +#include <xen/hvm/hvm_op.h>
> +#include <libfdt.h>

Why all these headers? "xc_private.h" seems sufficient.
There isn't any bitmap operation, so xc_bitops.h seems unused.
The device tree isn't manipulated so libfdt.h seems unused.
hvm_op.h seems to be for HVMOP, not SYSCTL hypercall, xc_private.h might
already include the right header to do SYSCTL.

Thanks,

-- 
Anthony PERARD