diff mbox series

[XEN,RFC,v4,15/16] tools/libs/light: Implement new libxl functions for device tree overlay ops

Message ID 20221207061815.7404-9-vikram.garhwal@amd.com (mailing list archive)
State New, archived
Headers show
Series dynamic node programming using overlay dtbo | expand

Commit Message

Vikram Garhwal Dec. 7, 2022, 6:18 a.m. UTC 
Signed-off-by: Vikram Garhwal <vikram.garhwal@amd.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
---
 tools/include/libxl.h               |  8 ++++
 tools/libs/light/Makefile           |  3 ++
 tools/libs/light/libxl_dt_overlay.c | 72 +++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 tools/libs/light/libxl_dt_overlay.c

Comments

Anthony PERARD Dec. 9, 2022, 4:59 p.m. UTC | #1 
On Tue, Dec 06, 2022 at 10:18:14PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 2321a648a5..82fc7ce6b9 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -245,6 +245,11 @@
>   */
>  #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
>  
> +/**
> + * This means Device Tree Overlay is supported.
> + */
> +#define LIBXL_HAVE_DT_OVERLAY 1
> +
>  /*
>   * libxl_domain_build_info has device_model_user to specify the user to
>   * run the device model with. See docs/misc/qemu-deprivilege.txt.
> @@ -2448,6 +2453,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,
> +                     uint32_t overlay_size, uint8_t overlay_op);
> +

Could you guard both the LIBXL_HAVE_* macro and this prototype with "#if
arm"? Since the dt_overlay operation to libxl built on arm.

>  /*
>   * 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 374be1cfab..2fde58246e 100644
> --- a/tools/libs/light/Makefile
> +++ b/tools/libs/light/Makefile
> @@ -111,6 +111,9 @@ OBJS-y += _libxl_types.o
>  OBJS-y += libxl_flask.o
>  OBJS-y += _libxl_types_internal.o
>  
> +# Device tree overlay is enabled only for ARM architecture.
> +OBJS-$(CONFIG_ARM) += libxl_dt_overlay.o
> +
>  ifeq ($(CONFIG_LIBNL),y)
>  CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
>  endif
> diff --git a/tools/libs/light/libxl_dt_overlay.c b/tools/libs/light/libxl_dt_overlay.c
> new file mode 100644
> index 0000000000..38cab880a0
> --- /dev/null
> +++ b/tools/libs/light/libxl_dt_overlay.c
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include "libxl_internal.h"
> +#include <libfdt.h>
> +#include <xenguest.h>
> +#include <xenctrl.h>

Don't you need just xenctrl.h and not xenguest.h? (They both already are
libxl_internal.h so I'm not sure if xenguest.h is needed., but
xc_dt_overlay() is in xenctrl.h)


Thanks,
Vikram Garhwal Feb. 1, 2023, 5:22 p.m. UTC | #2 
Hi Anthony,

On 12/9/22 8:59 AM, Anthony PERARD wrote:
> On Tue, Dec 06, 2022 at 10:18:14PM -0800, Vikram Garhwal wrote:
>> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
>> index 2321a648a5..82fc7ce6b9 100644
>> --- a/tools/include/libxl.h
>> +++ b/tools/include/libxl.h
>> @@ -245,6 +245,11 @@
>>    */
>>   #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
>>   
>> +/**
>> + * This means Device Tree Overlay is supported.
>> + */
>> +#define LIBXL_HAVE_DT_OVERLAY 1
>> +
>>   /*
>>    * libxl_domain_build_info has device_model_user to specify the user to
>>    * run the device model with. See docs/misc/qemu-deprivilege.txt.
>> @@ -2448,6 +2453,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,
>> +                     uint32_t overlay_size, uint8_t overlay_op);
>> +
> Could you guard both the LIBXL_HAVE_* macro and this prototype with "#if
> arm"? Since the dt_overlay operation to libxl built on arm.
Will do this in v5
>
>>   /*
>>    * 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 374be1cfab..2fde58246e 100644
>> --- a/tools/libs/light/Makefile
>> +++ b/tools/libs/light/Makefile
>> @@ -111,6 +111,9 @@ OBJS-y += _libxl_types.o
>>   OBJS-y += libxl_flask.o
>>   OBJS-y += _libxl_types_internal.o
>>   
>> +# Device tree overlay is enabled only for ARM architecture.
>> +OBJS-$(CONFIG_ARM) += libxl_dt_overlay.o
>> +
>>   ifeq ($(CONFIG_LIBNL),y)
>>   CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
>>   endif
>> diff --git a/tools/libs/light/libxl_dt_overlay.c b/tools/libs/light/libxl_dt_overlay.c
>> new file mode 100644
>> index 0000000000..38cab880a0
>> --- /dev/null
>> +++ b/tools/libs/light/libxl_dt_overlay.c
>> +#include "libxl_osdeps.h" /* must come before any other headers */
>> +#include "libxl_internal.h"
>> +#include <libfdt.h>
>> +#include <xenguest.h>
>> +#include <xenctrl.h>
> Don't you need just xenctrl.h and not xenguest.h? (They both already are
> libxl_internal.h so I'm not sure if xenguest.h is needed., but
> xc_dt_overlay() is in xenctrl.h)
Thanks for spotting this. I will remove xenguest.h
>
> Thanks,
>
diff mbox series

Patch

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 2321a648a5..82fc7ce6b9 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -245,6 +245,11 @@ 
  */
 #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
 
+/**
+ * This means Device Tree Overlay is supported.
+ */
+#define LIBXL_HAVE_DT_OVERLAY 1
+
 /*
  * libxl_domain_build_info has device_model_user to specify the user to
  * run the device model with. See docs/misc/qemu-deprivilege.txt.
@@ -2448,6 +2453,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,
+                     uint32_t 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 374be1cfab..2fde58246e 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -111,6 +111,9 @@  OBJS-y += _libxl_types.o
 OBJS-y += libxl_flask.o
 OBJS-y += _libxl_types_internal.o
 
+# Device tree overlay is enabled only for ARM architecture.
+OBJS-$(CONFIG_ARM) += libxl_dt_overlay.o
+
 ifeq ($(CONFIG_LIBNL),y)
 CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
 endif
diff --git a/tools/libs/light/libxl_dt_overlay.c b/tools/libs/light/libxl_dt_overlay.c
new file mode 100644
index 0000000000..38cab880a0
--- /dev/null
+++ b/tools/libs/light/libxl_dt_overlay.c
@@ -0,0 +1,72 @@ 
+/*
+ * 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, uint32_t overlay_dt_size,
+                     uint8_t overlay_op)
+{
+    int rc;
+    int r;
+    GC_INIT(ctx);
+
+    if (check_overlay_fdt(gc, overlay_dt, overlay_dt_size)) {
+        LOG(ERROR, "Overlay DTB check failed");
+        rc = ERROR_FAIL;
+        goto out;
+    } else {
+        LOG(DEBUG, "Overlay DTB check passed");
+        rc = 0;
+    }
+
+    r = xc_dt_overlay(ctx->xch, overlay_dt, overlay_dt_size, overlay_op);
+
+    if (r) {
+        LOG(ERROR, "%s: Adding/Removing overlay dtb failed.", __func__);
+        rc = ERROR_FAIL;
+    }
+
+out:
+    GC_FREE;
+    return rc;
+}
+