[PATCH v4 8/9] tools: Introduce the "xl dt-overlay {attach,detach}" commands

Henry Wang posted 9 patches 6 months ago
There is a newer version of this series
[PATCH v4 8/9] tools: Introduce the "xl dt-overlay {attach,detach}" commands
Posted by Henry Wang 6 months ago
With the XEN_DOMCTL_dt_overlay DOMCTL added, users should be able to
attach/detach devices from the provided DT overlay to domains.
Support this by introducing a new set of "xl dt-overlay" commands and
related documentation, i.e. "xl dt-overlay {attach,detach}". Slightly
rework the command option parsing logic.

Signed-off-by: Henry Wang <xin.wang2@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
---
v4:
- Add Jason's Reviewed-by tag.
v3:
- Introduce new API libxl_dt_overlay_domain() and co., instead of
  reusing existing API libxl_dt_overlay().
- Add in-code comments for the LIBXL_DT_OVERLAY_* macros.
- Use find_domain() to avoid getting domain_id from strtol().
v2:
- New patch.
---
 tools/include/libxl.h               | 10 +++++++
 tools/include/xenctrl.h             |  3 +++
 tools/libs/ctrl/xc_dt_overlay.c     | 31 +++++++++++++++++++++
 tools/libs/light/libxl_dt_overlay.c | 28 +++++++++++++++++++
 tools/xl/xl_cmdtable.c              |  4 +--
 tools/xl/xl_vmcontrol.c             | 42 ++++++++++++++++++++---------
 6 files changed, 104 insertions(+), 14 deletions(-)

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..6cc6d6bf6a 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -2549,8 +2549,18 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, uint32_t domid,
 void libxl_device_pci_list_free(libxl_device_pci* list, int num);
 
 #if defined(__arm__) || defined(__aarch64__)
+/* Values should keep consistent with the op from XEN_SYSCTL_dt_overlay */
+#define LIBXL_DT_OVERLAY_ADD                   1
+#define LIBXL_DT_OVERLAY_REMOVE                2
 int libxl_dt_overlay(libxl_ctx *ctx, void *overlay,
                      uint32_t overlay_size, uint8_t overlay_op);
+
+/* Values should keep consistent with the op from XEN_DOMCTL_dt_overlay */
+#define LIBXL_DT_OVERLAY_DOMAIN_ATTACH         1
+#define LIBXL_DT_OVERLAY_DOMAIN_DETACH         2
+int libxl_dt_overlay_domain(libxl_ctx *ctx, uint32_t domain_id,
+                            void *overlay_dt, uint32_t overlay_dt_size,
+                            uint8_t overlay_op);
 #endif
 
 /*
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 4996855944..9ceca0cffc 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2657,6 +2657,9 @@ int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
 #if defined(__arm__) || defined(__aarch64__)
 int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
                   uint32_t overlay_fdt_size, uint8_t overlay_op);
+int xc_dt_overlay_domain(xc_interface *xch, void *overlay_fdt,
+                         uint32_t overlay_fdt_size, uint8_t overlay_op,
+                         uint32_t domain_id);
 #endif
 
 /* Compat shims */
diff --git a/tools/libs/ctrl/xc_dt_overlay.c b/tools/libs/ctrl/xc_dt_overlay.c
index c2224c4d15..ea1da522d1 100644
--- a/tools/libs/ctrl/xc_dt_overlay.c
+++ b/tools/libs/ctrl/xc_dt_overlay.c
@@ -48,3 +48,34 @@ err:
 
     return err;
 }
+
+int xc_dt_overlay_domain(xc_interface *xch, void *overlay_fdt,
+                         uint32_t overlay_fdt_size, uint8_t overlay_op,
+                         uint32_t domain_id)
+{
+    int err;
+    struct xen_domctl domctl = {
+        .cmd = XEN_DOMCTL_dt_overlay,
+        .domain = domain_id,
+        .u.dt_overlay = {
+            .overlay_op = overlay_op,
+            .overlay_fdt_size = overlay_fdt_size,
+        }
+    };
+
+    DECLARE_HYPERCALL_BOUNCE(overlay_fdt, overlay_fdt_size,
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( (err = xc_hypercall_bounce_pre(xch, overlay_fdt)) )
+        goto err;
+
+    set_xen_guest_handle(domctl.u.dt_overlay.overlay_fdt, overlay_fdt);
+
+    if ( (err = do_domctl(xch, &domctl)) != 0 )
+        PERROR("%s failed", __func__);
+
+err:
+    xc_hypercall_bounce_post(xch, overlay_fdt);
+
+    return err;
+}
diff --git a/tools/libs/light/libxl_dt_overlay.c b/tools/libs/light/libxl_dt_overlay.c
index a6c709a6dc..00503b76bd 100644
--- a/tools/libs/light/libxl_dt_overlay.c
+++ b/tools/libs/light/libxl_dt_overlay.c
@@ -69,3 +69,31 @@ out:
     return rc;
 }
 
+int libxl_dt_overlay_domain(libxl_ctx *ctx, uint32_t domain_id,
+                            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_domain(ctx->xch, overlay_dt, overlay_dt_size, overlay_op,
+                             domain_id);
+    if (r) {
+        LOG(ERROR, "%s: Attaching/Detaching overlay dtb failed.", __func__);
+        rc = ERROR_FAIL;
+    }
+
+out:
+    GC_FREE;
+    return rc;
+}
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 1f3c6b5897..37770b20e3 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -634,8 +634,8 @@ const struct cmd_spec cmd_table[] = {
 #ifdef LIBXL_HAVE_DT_OVERLAY
     { "dt-overlay",
       &main_dt_overlay, 0, 1,
-      "Add/Remove a device tree overlay",
-      "add/remove <.dtbo>",
+      "Add/Remove a device tree overlay to Xen device tree, attach/detach the device to a domain",
+      "<operation=add|remove> <.dtbo> OR <operation=attach|detach> <.dtbo> <Domain>",
       "-h print this help\n"
     },
 #endif
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 02575d5d36..ccfeec39f1 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -1268,32 +1268,46 @@ int main_create(int argc, char **argv)
 #ifdef LIBXL_HAVE_DT_OVERLAY
 int main_dt_overlay(int argc, char **argv)
 {
-    const char *overlay_ops = NULL;
     const char *overlay_config_file = NULL;
     void *overlay_dtb = NULL;
     int rc;
     uint8_t op;
     int overlay_dtb_size = 0;
-    const int overlay_add_op = 1;
-    const int overlay_remove_op = 2;
+    uint32_t domain_id = 0;
+    bool domain_op = false;
 
     if (argc < 2) {
         help("dt-overlay");
         return EXIT_FAILURE;
     }
 
-    overlay_ops = argv[1];
-    overlay_config_file = argv[2];
-
-    if (strcmp(overlay_ops, "add") == 0)
-        op = overlay_add_op;
-    else if (strcmp(overlay_ops, "remove") == 0)
-        op = overlay_remove_op;
-    else {
+    if (strcmp(argv[optind], "add") == 0)
+        op = LIBXL_DT_OVERLAY_ADD;
+    else if (strcmp(argv[optind], "remove") == 0)
+        op = LIBXL_DT_OVERLAY_REMOVE;
+    else if (strcmp(argv[optind], "attach") == 0) {
+        op = LIBXL_DT_OVERLAY_DOMAIN_ATTACH;
+        domain_op = true;
+    } else if (strcmp(argv[optind], "detach") == 0) {
+        op = LIBXL_DT_OVERLAY_DOMAIN_DETACH;
+        domain_op = true;
+    } else {
         fprintf(stderr, "Invalid dt overlay operation\n");
         return EXIT_FAILURE;
     }
 
+    overlay_config_file = argv[optind+1];
+
+    if (domain_op) {
+        if (argc <= optind + 2) {
+            fprintf(stderr, "Missing domain ID\n");
+            help("dt-overlay");
+            return EXIT_FAILURE;
+        } else {
+            domain_id = find_domain(argv[optind+2]);
+        }
+    }
+
     if (overlay_config_file) {
         rc = libxl_read_file_contents(ctx, overlay_config_file,
                                       &overlay_dtb, &overlay_dtb_size);
@@ -1309,7 +1323,11 @@ int main_dt_overlay(int argc, char **argv)
         return EXIT_FAILURE;
     }
 
-    rc = libxl_dt_overlay(ctx, overlay_dtb, overlay_dtb_size, op);
+    if (!domain_op)
+        rc = libxl_dt_overlay(ctx, overlay_dtb, overlay_dtb_size, op);
+    else
+        rc = libxl_dt_overlay_domain(ctx, domain_id, overlay_dtb,
+                                     overlay_dtb_size, op);
 
     free(overlay_dtb);
 
-- 
2.34.1
Re: [PATCH v4 8/9] tools: Introduce the "xl dt-overlay {attach,detach}" commands
Posted by Julien Grall 6 months ago
Hi Henry,

On 23/05/2024 08:40, Henry Wang wrote:
> With the XEN_DOMCTL_dt_overlay DOMCTL added, users should be able to
> attach/detach devices from the provided DT overlay to domains.
> Support this by introducing a new set of "xl dt-overlay" commands and
> related documentation, i.e. "xl dt-overlay {attach,detach}". Slightly
> rework the command option parsing logic.
> 
> Signed-off-by: Henry Wang <xin.wang2@amd.com>
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> v4:
> - Add Jason's Reviewed-by tag.
> v3:
> - Introduce new API libxl_dt_overlay_domain() and co., instead of
>    reusing existing API libxl_dt_overlay().
> - Add in-code comments for the LIBXL_DT_OVERLAY_* macros.
> - Use find_domain() to avoid getting domain_id from strtol().
> v2:
> - New patch.
> ---
>   tools/include/libxl.h               | 10 +++++++
>   tools/include/xenctrl.h             |  3 +++
>   tools/libs/ctrl/xc_dt_overlay.c     | 31 +++++++++++++++++++++
>   tools/libs/light/libxl_dt_overlay.c | 28 +++++++++++++++++++
>   tools/xl/xl_cmdtable.c              |  4 +--
>   tools/xl/xl_vmcontrol.c             | 42 ++++++++++++++++++++---------
>   6 files changed, 104 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 62cb07dea6..6cc6d6bf6a 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h

I think you also need to introduce LIBXL_HAVE_...

Cheers,

-- 
Julien Grall
Re: [PATCH v4 8/9] tools: Introduce the "xl dt-overlay {attach,detach}" commands
Posted by Stefano Stabellini 6 months ago
On Fri, 24 May 2024, Julien Grall wrote:
> Hi Henry,
> 
> On 23/05/2024 08:40, Henry Wang wrote:
> > With the XEN_DOMCTL_dt_overlay DOMCTL added, users should be able to
> > attach/detach devices from the provided DT overlay to domains.
> > Support this by introducing a new set of "xl dt-overlay" commands and
> > related documentation, i.e. "xl dt-overlay {attach,detach}". Slightly
> > rework the command option parsing logic.
> > 
> > Signed-off-by: Henry Wang <xin.wang2@amd.com>
> > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> > ---
> > v4:
> > - Add Jason's Reviewed-by tag.
> > v3:
> > - Introduce new API libxl_dt_overlay_domain() and co., instead of
> >    reusing existing API libxl_dt_overlay().
> > - Add in-code comments for the LIBXL_DT_OVERLAY_* macros.
> > - Use find_domain() to avoid getting domain_id from strtol().
> > v2:
> > - New patch.
> > ---
> >   tools/include/libxl.h               | 10 +++++++
> >   tools/include/xenctrl.h             |  3 +++
> >   tools/libs/ctrl/xc_dt_overlay.c     | 31 +++++++++++++++++++++
> >   tools/libs/light/libxl_dt_overlay.c | 28 +++++++++++++++++++
> >   tools/xl/xl_cmdtable.c              |  4 +--
> >   tools/xl/xl_vmcontrol.c             | 42 ++++++++++++++++++++---------
> >   6 files changed, 104 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> > index 62cb07dea6..6cc6d6bf6a 100644
> > --- a/tools/include/libxl.h
> > +++ b/tools/include/libxl.h
> 
> I think you also need to introduce LIBXL_HAVE_...

Added

I have removed the LIBXL_DT_OVERLAY_DOMAIN_DETACH and the relate
mentions. I kept Jasons' ack.