diff mbox series

[XEN,RFC,v2,10/12] tools/libs/ctrl: Implement new xc interfaces for dt overlay

Message ID 1636441347-133850-11-git-send-email-fnu.vikram@xilinx.com (mailing list archive)
State New, archived
Headers show
Series dynamic node programming using overlay dtbo | expand

Commit Message

Vikram Garhwal Nov. 9, 2021, 7:02 a.m. UTC
xc_dt_overlay() sends the device tree binary overlay, size of .dtbo and overlay
operation type i.e. add or remove to xen.

Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
 tools/include/xenctrl.h      |  5 +++++
 tools/libs/ctrl/Makefile     |  1 +
 tools/libs/ctrl/xc_overlay.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 tools/libs/ctrl/xc_overlay.c

Comments

Anthony PERARD Nov. 11, 2021, 4:54 p.m. UTC | #1
On Mon, Nov 08, 2021 at 11:02:25PM -0800, Vikram Garhwal wrote:
> xc_dt_overlay() sends the device tree binary overlay, size of .dtbo and overlay
> operation type i.e. add or remove to xen.
> 
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
>  tools/include/xenctrl.h      |  5 +++++
>  tools/libs/ctrl/Makefile     |  1 +
>  tools/libs/ctrl/xc_overlay.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
>  create mode 100644 tools/libs/ctrl/xc_overlay.c
> 
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 07b96e6..cfd7c5c 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -2684,6 +2684,11 @@ 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);
>  
> +#if defined (CONFIG_OVERLAY_DTB)
> +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, int overlay_fdt_size,
> +                  uint8_t overlayop);
> +#endif
> +
>  /* Compat shims */
>  #include "xenctrl_compat.h"
>  
> diff --git a/tools/libs/ctrl/Makefile b/tools/libs/ctrl/Makefile
> index 519246b..a21a949 100644
> --- a/tools/libs/ctrl/Makefile
> +++ b/tools/libs/ctrl/Makefile
> @@ -3,6 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk
>  
>  SRCS-y       += xc_altp2m.c
>  SRCS-y       += xc_cpupool.c
> +SRCS-$(CONFIG_OVERLAY_DTB) += xc_overlay.c

So, this patch seems to introduce the use of CONFIG_OVERLAY_DTB, is
there a reason why the new functionality can't be always builtin?

Thanks,
Vikram Garhwal Nov. 11, 2021, 7:46 p.m. UTC | #2
On Thu, Nov 11, 2021 at 04:54:25PM +0000, Anthony PERARD wrote:
Hi Anthony,
> On Mon, Nov 08, 2021 at 11:02:25PM -0800, Vikram Garhwal wrote:
> > xc_dt_overlay() sends the device tree binary overlay, size of .dtbo and overlay
> > operation type i.e. add or remove to xen.
> > 
> > Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> > ---
> >  tools/include/xenctrl.h      |  5 +++++
> >  tools/libs/ctrl/Makefile     |  1 +
> >  tools/libs/ctrl/xc_overlay.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 57 insertions(+)
> >  create mode 100644 tools/libs/ctrl/xc_overlay.c
> > 
> > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> > index 07b96e6..cfd7c5c 100644
> > --- a/tools/include/xenctrl.h
> > +++ b/tools/include/xenctrl.h
> > @@ -2684,6 +2684,11 @@ 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);
> >  
> > +#if defined (CONFIG_OVERLAY_DTB)
> > +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, int overlay_fdt_size,
> > +                  uint8_t overlayop);
> > +#endif
> > +
> >  /* Compat shims */
> >  #include "xenctrl_compat.h"
> >  
> > diff --git a/tools/libs/ctrl/Makefile b/tools/libs/ctrl/Makefile
> > index 519246b..a21a949 100644
> > --- a/tools/libs/ctrl/Makefile
> > +++ b/tools/libs/ctrl/Makefile
> > @@ -3,6 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk
> >  
> >  SRCS-y       += xc_altp2m.c
> >  SRCS-y       += xc_cpupool.c
> > +SRCS-$(CONFIG_OVERLAY_DTB) += xc_overlay.c
> 
> So, this patch seems to introduce the use of CONFIG_OVERLAY_DTB, is
> there a reason why the new functionality can't be always builtin?
> 
Above, if you meant removing "CONFIG_OEVRLAY_DTB" then here is my answer:
This feature is supported by ARM based FPGA devices only so there were a few
comments on v1 series to keep the code inside a config only. Now, for the tool
side also I kept the CONFIG_OVERLAY_DTB to align the xen-tools with Xen.

Although, now i saw your comments on patch 10 regarding  "always provide
libxl_dt_overlay() but which would return ENOSYS when libxl is built without
CONFIG_OVERLAY_DTB". That seems better approach here for all three xen-tool
patches.

Initially, i was not sure what to do here that's why i wrote a question in the
cover letter about this.

Also, do you know how to enable this config via menuconfig when building the Xen
tools? I know how to enable for Xen but not sure about tools.

Thanks for reviewing this.

Regards,
Vikram Garhwal

> Thanks,
> 
> -- 
> Anthony PERARD
Anthony PERARD Nov. 12, 2021, 2:21 p.m. UTC | #3
On Thu, Nov 11, 2021 at 11:46:36AM -0800, Vikram Garhwal wrote:
> On Thu, Nov 11, 2021 at 04:54:25PM +0000, Anthony PERARD wrote:
> Hi Anthony,
> > On Mon, Nov 08, 2021 at 11:02:25PM -0800, Vikram Garhwal wrote:
> > > +SRCS-$(CONFIG_OVERLAY_DTB) += xc_overlay.c
> > 
> > So, this patch seems to introduce the use of CONFIG_OVERLAY_DTB, is
> > there a reason why the new functionality can't be always builtin?
> > 
> Above, if you meant removing "CONFIG_OEVRLAY_DTB" then here is my answer:
> This feature is supported by ARM based FPGA devices only so there were a few
> comments on v1 series to keep the code inside a config only. Now, for the tool
> side also I kept the CONFIG_OVERLAY_DTB to align the xen-tools with Xen.
> 
> Although, now i saw your comments on patch 10 regarding  "always provide
> libxl_dt_overlay() but which would return ENOSYS when libxl is built without
> CONFIG_OVERLAY_DTB". That seems better approach here for all three xen-tool
> patches.
> 
> Initially, i was not sure what to do here that's why i wrote a question in the
> cover letter about this.
> 
> Also, do you know how to enable this config via menuconfig when building the Xen
> tools? I know how to enable for Xen but not sure about tools.

It isn't possible to use the configuration of the hypervisor to build
the tool. We use autoconf (configure.ac, ...) to configure the tools but
I don't think in this case that having CONFIG_OVERLAY_DTB for the tools
is the right thing to do. In the tools, we don't really have a way to
select functionalities available in the different libraries, or it is
mostly based on the architecture or operating system or available
system libraries.

Contrary to the hypervisor side, the added code in the libraries is
mostly glue which calls the hypervisor, so their isn't really a need to
avoid building it. If the functionality isn't available in the
hypervisor, it should return an error and the library can deal with that
error.

You might want to limit the build to Arm, but I don't know if that
"overlay dtb" thing is really Arm specific (even though device trees are
mostly use on Arm I guess).
diff mbox series

Patch

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 07b96e6..cfd7c5c 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2684,6 +2684,11 @@  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);
 
+#if defined (CONFIG_OVERLAY_DTB)
+int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, int overlay_fdt_size,
+                  uint8_t overlayop);
+#endif
+
 /* Compat shims */
 #include "xenctrl_compat.h"
 
diff --git a/tools/libs/ctrl/Makefile b/tools/libs/ctrl/Makefile
index 519246b..a21a949 100644
--- a/tools/libs/ctrl/Makefile
+++ b/tools/libs/ctrl/Makefile
@@ -3,6 +3,7 @@  include $(XEN_ROOT)/tools/Rules.mk
 
 SRCS-y       += xc_altp2m.c
 SRCS-y       += xc_cpupool.c
+SRCS-$(CONFIG_OVERLAY_DTB) += xc_overlay.c
 SRCS-y       += xc_domain.c
 SRCS-y       += xc_evtchn.c
 SRCS-y       += xc_gnttab.c
diff --git a/tools/libs/ctrl/xc_overlay.c b/tools/libs/ctrl/xc_overlay.c
new file mode 100644
index 0000000..77f9edc
--- /dev/null
+++ b/tools/libs/ctrl/xc_overlay.c
@@ -0,0 +1,51 @@ 
+/*
+ *
+ * Overlay control functions.
+ * Copyright (C) 2021 Xilinx Inc.
+ * Author Vikram Garhwal <fnu.vikram@xilinx.com>
+ *
+ * This library 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 of the License.
+ *
+ * This library 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.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "xc_bitops.h"
+#include "xc_private.h"
+#include <xen/hvm/hvm_op.h>
+#include <libfdt.h>
+
+int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, int overlay_fdt_size,
+                  uint8_t op)
+{
+    int err;
+    DECLARE_SYSCTL;
+
+    DECLARE_HYPERCALL_BOUNCE(overlay_fdt, overlay_fdt_size,
+                        XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    if ( (err = xc_hypercall_bounce_pre(xch, overlay_fdt)) )
+        goto err;
+
+    sysctl.cmd = XEN_SYSCTL_overlay;
+    sysctl.u.overlay_dt.overlay_op= op;
+    sysctl.u.overlay_dt.overlay_fdt_size = overlay_fdt_size;
+
+    set_xen_guest_handle(sysctl.u.overlay_dt.overlay_fdt, overlay_fdt);
+
+    if ( (err = do_sysctl(xch, &sysctl)) != 0 )
+        PERROR("%s failed\n", __func__);
+
+err:
+    xc_hypercall_bounce_post(xch, overlay_fdt);
+
+    return err;
+}