| Message ID | 20220308194704.14061-14-fnu.vikram@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dynamic node programming using overlay dtbo | expand |
> On 8 Mar 2022, at 19:47, Vikram Garhwal <fnu.vikram@xilinx.com> wrote: > > Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com> I don’t know if an empty commit message is ok here, will leave to the maintainer the choice, for me the code looks ok Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
On Tue, Mar 08, 2022 at 11:47:03AM -0800, Vikram Garhwal wrote: > Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com> > --- > tools/include/libxl.h | 3 ++ > tools/libs/light/Makefile | 1 + > tools/libs/light/libxl_overlay.c | 67 ++++++++++++++++++++++++++++++++ > 3 files changed, 71 insertions(+) > create mode 100644 tools/libs/light/libxl_overlay.c > > diff --git a/tools/include/libxl.h b/tools/include/libxl.h > index 51a9b6cfac..b31e17c2ce 100644 > --- a/tools/include/libxl.h > +++ b/tools/include/libxl.h > @@ -2419,6 +2419,9 @@ 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); > > +int libxl_dt_overlay(libxl_ctx *ctx, void *overlay, > + int overlay_size, uint8_t overlay_op); > + As you are making changes to libxl's API, you are going to need to add a "#define LIBXL_HAVE_*" in "include/libxl.h" about the availability of the new function. > /* > * 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 453bea0067..405115c13c 100644 > --- a/tools/libs/light/Makefile > +++ b/tools/libs/light/Makefile > @@ -116,6 +116,7 @@ SRCS-y += libxl_genid.c > SRCS-y += _libxl_types.c > SRCS-y += libxl_flask.c > SRCS-y += _libxl_types_internal.c > +SRCS-y += libxl_overlay.o Building this new file unconditionally is an issue at the moment. "./configure" doesn't check if libfdt is on the system unless we happen to build for Arm. And libfdt is mandatory when building for Arm. Could you build this new source file on Arm only? With a comment why. "SRCS-$(CONFIG_ARM) +=" Alternatively, you could try to rework configure to always check for libfdt, but I doubt that device tree overlay are going to be useful on x86 at the moment. Then, libxl_dt_overlay() will need a stub that just return an error when building on system that don't have libfdt. > > ifeq ($(CONFIG_LIBNL),y) > CFLAGS_LIBXL += $(LIBNL3_CFLAGS) > diff --git a/tools/libs/light/libxl_overlay.c b/tools/libs/light/libxl_overlay.c > new file mode 100644 > index 0000000000..e370e8cac8 > --- /dev/null > +++ b/tools/libs/light/libxl_overlay.c Could you rename this new file "libxl_dt_overlay.c"? There could be other kind of "overlay". > @@ -0,0 +1,67 @@ > +/* > + * Copyright (C) 2021 Xilinx Inc. > + * Author Vikram Garhwal <fnu.vikram@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) > +{ > + 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, I wonder whether the type of "overlay_dt_size" should be something else. check_overlay_fdt() takes a "size_t", but the hypercall takes "uint32_t". > + uint8_t overlay_op) > +{ > + int rc = 0; By CODING_STYLE, "rc" shouldn't be set when declared. > + GC_INIT(ctx); > + > + if (check_overlay_fdt(gc, overlay_dt, overlay_dt_size)) { > + LOG(ERROR, "Overlay DTB check failed\n"); No need for \n, it's already added by LOG(). > + GC_FREE; > + return ERROR_FAIL; Instead of writing GC_FREE more than once, could you set "rc" then "goto out;"? > + } else > + LOG(DEBUG, "Overlay DTB check passed\n"); This needs to be in a {} block because the true side of the "if" uses braces. > + > + /* We don't need to do xc_interface_open here. */ That comment isn't very useful, as it is a fact of using "ctx". > + rc = xc_dt_overlay(ctx->xch, overlay_dt, overlay_dt_size, overlay_op); Instead of "rc", could you store the result of xc_dt_overlay() in "r"? "rc" is reserved for libxl error code. Also, the return value of libxl_dt_overlay() can't be the return value of xc_dt_overlay(). There's some better explanation in the CODING_STYLE file, if needed. > + if (rc) > + LOG(ERROR, "%s: Adding/Removing overlay dtb failed.\n", __func__); > + > + GC_FREE; > + return rc; > +} Thanks,
diff --git a/tools/include/libxl.h b/tools/include/libxl.h index 51a9b6cfac..b31e17c2ce 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -2419,6 +2419,9 @@ 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); +int libxl_dt_overlay(libxl_ctx *ctx, void *overlay, + int overlay_size, uint8_t overlay_op); + /* * 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 453bea0067..405115c13c 100644 --- a/tools/libs/light/Makefile +++ b/tools/libs/light/Makefile @@ -116,6 +116,7 @@ SRCS-y += libxl_genid.c SRCS-y += _libxl_types.c SRCS-y += libxl_flask.c SRCS-y += _libxl_types_internal.c +SRCS-y += libxl_overlay.o ifeq ($(CONFIG_LIBNL),y) CFLAGS_LIBXL += $(LIBNL3_CFLAGS) diff --git a/tools/libs/light/libxl_overlay.c b/tools/libs/light/libxl_overlay.c new file mode 100644 index 0000000000..e370e8cac8 --- /dev/null +++ b/tools/libs/light/libxl_overlay.c @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2021 Xilinx Inc. + * Author Vikram Garhwal <fnu.vikram@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) +{ + 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 overlay_op) +{ + int rc = 0; + GC_INIT(ctx); + + if (check_overlay_fdt(gc, overlay_dt, overlay_dt_size)) { + LOG(ERROR, "Overlay DTB check failed\n"); + GC_FREE; + return ERROR_FAIL; + } else + LOG(DEBUG, "Overlay DTB check passed\n"); + + /* We don't need to do xc_interface_open here. */ + rc = xc_dt_overlay(ctx->xch, overlay_dt, overlay_dt_size, overlay_op); + + if (rc) + LOG(ERROR, "%s: Adding/Removing overlay dtb failed.\n", __func__); + + GC_FREE; + return rc; +} +
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com> --- tools/include/libxl.h | 3 ++ tools/libs/light/Makefile | 1 + tools/libs/light/libxl_overlay.c | 67 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 tools/libs/light/libxl_overlay.c