On Mon, Nov 08, 2021 at 11:02:26PM -0800, Vikram Garhwal wrote:
> Signed-off-by: Vikram Garhwal <fnu.vik...@xilinx.com>
> ---
> tools/include/libxl.h | 5 ++++
> tools/libs/light/Makefile | 3 ++
> tools/libs/light/libxl_overlay.c | 65
> ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 73 insertions(+)
> create mode 100644 tools/libs/light/libxl_overlay.c
>
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 2e8679d..3dcb3e7 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -2406,6 +2406,11 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx
> *ctx, uint32_t domid,
> int *num);
> void libxl_device_pci_list_free(libxl_device_pci* list, int num);
>
> +#if defined (CONFIG_OVERLAY_DTB)
> +int libxl_dt_overlay(libxl_ctx *ctx, void *overlay,
> + int overlay_size, uint8_t op);
> +#endif
> +
This won't work. "libxl.h" is a public header to be installed, so
CONFIG_OVERLAY_DTB won't mean anything to other application making use
of libxl.
Instead, can you always provide libxl_dt_overlay() but which would
return ENOSYS when libxl is built without CONFIG_OVERLAY_DTB?
> /*
> * Turns the current process into a backend device service daemon
> * for a driver domain.
> diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
> index 194bc5f..0fffa93 100644
> --- a/tools/libs/light/Makefile
> +++ b/tools/libs/light/Makefile
> @@ -117,6 +117,9 @@ SRCS-y += libxl_genid.c
> SRCS-y += _libxl_types.c
> SRCS-y += libxl_flask.c
> SRCS-y += _libxl_types_internal.c
> +ifeq ($(CONFIG_OVERLAY_DTB),y)
> +SRCS-y += libxl_overlay.o
FYI, the "-y" is so that you can do
SRCS-$(CONFIG_OVERLAY_DTB) += libxl_overlay.o
;-)
> diff --git a/tools/libs/light/libxl_overlay.c
> b/tools/libs/light/libxl_overlay.c
> new file mode 100644
> index 0000000..d965aee
> --- /dev/null
> +++ b/tools/libs/light/libxl_overlay.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2021 Xilinx Inc.
> + * Author Vikram Garhwal <fnu.vik...@xilinx.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include "libxl_internal.h"
> +#include <libfdt.h>
> +#include <xenguest.h>
> +#include <xenctrl.h>
> +
> +static int check_overlay_fdt(libxl__gc *gc, void *fdt, size_t size)
Does "fdt" can have a better type than a plain pointer?
There is a "size" provided, is it the size of the blob pointed by "fdt"?
If so, can the size been the first thing to be check? Surely there is a
minimum size for *fdt.
> +{
> + int r;
> +
> + if (fdt_magic(fdt) != FDT_MAGIC) {
> + LOG(ERROR, "Overlay FDT is not a valid Flat Device Tree");
> + return ERROR_FAIL;
> + }
> +
> + r = fdt_check_header(fdt);
> + if (r) {
> + LOG(ERROR, "Failed to check the overlay FDT (%d)", r);
> + return ERROR_FAIL;
> + }
> +
> + if (fdt_totalsize(fdt) > size) {
> + LOG(ERROR, "Overlay FDT totalsize is too big");
> + return ERROR_FAIL;
> + }
> +
> + return 0;
> +}
> +
> +int libxl_dt_overlay(libxl_ctx *ctx, void *overlay_dt, int overlay_dt_size,
> + uint8_t op)
What is "op"? What the function is supposed to do beside calling libxc?
> +{
> + int rc = 0;
> + GC_INIT(ctx);
The function should have "GC_FREE;" before returning.
> +
> + if (check_overlay_fdt(gc, overlay_dt, overlay_dt_size)) {
> + LOG(ERROR, "Overlay DTB check failed\n");
> + return ERROR_FAIL;
> + } else
> + LOG(DEBUG, "Overlay DTB check passed\n");
FYI, there is no need for "\n" when using LOG().
> +
> + /* We don't need to do xc_interface_open here. */
> + rc = xc_dt_overlay(ctx->xch, overlay_dt, overlay_dt_size, op);
"rc" is reserved for libxl error code, xc_* return value should be
stored in "r" instead.
> +
> + if (rc)
> + LOG(ERROR, "%s: Adding/Removing overlay dtb failed.\n", __func__);
> +
> + return rc;
> +}
> +
Thanks,
--
Anthony PERARD
