* [XEN][RFC PATCH 01/13] device tree: Remove __init from function type
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-20 17:55 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 02/13] libfdt: Keep fdt functions after init Vikram Garhwal
` (11 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal, Volodymyr Babchuk
Change function type of following function to access during runtime:
1. handle_device_interrupt()
2. map_irq_to_domain()
3. map_range_to_domain()
4. unflatten_dt_node()
5. unflatten_device_tree()
Remove .init from domain_build.o and move map_range_data declaration to
domain_build.h.
These changes are done to support the dynamic programming of a nodes where an
overlay node will be added to fdt and unflattened node will be added to dt_host.
Furthermore, irq and mmio mapping will be done for the added node.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/domain_build.c | 15 ++++-----------
xen/common/device_tree.c | 18 +++++++++---------
xen/include/asm-arm/domain_build.h | 10 ++++++++++
xen/include/xen/device_tree.h | 5 +++++
5 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 3d3b97b..bef4517 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -15,7 +15,7 @@ obj-y += decode.o
obj-y += device.o
obj-$(CONFIG_IOREQ_SERVER) += dm.o
obj-y += domain.o
-obj-y += domain_build.init.o
+obj-y += domain_build.o
obj-y += domctl.o
obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
obj-y += gic.o
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 206038d..3a457d3 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -47,12 +47,6 @@ static int __init parse_dom0_mem(const char *s)
}
custom_param("dom0_mem", parse_dom0_mem);
-struct map_range_data
-{
- struct domain *d;
- p2m_type_t p2mt;
-};
-
/* Override macros from asm/page.h to make them work with mfn_t */
#undef virt_to_mfn
#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
@@ -1144,7 +1138,7 @@ int __init make_chosen_node(const struct kernel_info *kinfo)
return res;
}
-int __init map_irq_to_domain(struct domain *d, unsigned int irq,
+int map_irq_to_domain(struct domain *d, unsigned int irq,
bool need_mapping, const char *devname)
{
int res;
@@ -1210,7 +1204,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
return 0;
}
-static int __init map_range_to_domain(const struct dt_device_node *dev,
+int map_range_to_domain(const struct dt_device_node *dev,
u64 addr, u64 len,
void *data)
{
@@ -1300,9 +1294,8 @@ static int __init map_device_children(struct domain *d,
* < 0 error
* 0 success
*/
-static int __init handle_device_interrupts(struct domain *d,
- struct dt_device_node *dev,
- bool need_mapping)
+int handle_device_interrupts(struct domain *d, struct dt_device_node *dev,
+ bool need_mapping)
{
unsigned int i, nirq;
int res;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 03d25a8..cda21be 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -1750,12 +1750,12 @@ int dt_count_phandle_with_args(const struct dt_device_node *np,
* @allnextpp: pointer to ->allnext from last allocated device_node
* @fpsize: Size of the node path up at the current depth.
*/
-static unsigned long __init unflatten_dt_node(const void *fdt,
- unsigned long mem,
- unsigned long *p,
- struct dt_device_node *dad,
- struct dt_device_node ***allnextpp,
- unsigned long fpsize)
+static unsigned long unflatten_dt_node(const void *fdt,
+ unsigned long mem,
+ unsigned long *p,
+ struct dt_device_node *dad,
+ struct dt_device_node ***allnextpp,
+ unsigned long fpsize)
{
struct dt_device_node *np;
struct dt_property *pp, **prev_pp = NULL;
@@ -1986,7 +1986,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
}
/**
- * __unflatten_device_tree - create tree of device_nodes from flat blob
+ * unflatten_device_tree - create tree of device_nodes from flat blob
*
* unflattens a device-tree, creating the
* tree of struct device_node. It also fills the "name" and "type"
@@ -1995,7 +1995,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
* @fdt: The fdt to expand
* @mynodes: The device_node tree created by the call
*/
-static void __init __unflatten_device_tree(const void *fdt,
+void unflatten_device_tree(const void *fdt,
struct dt_device_node **mynodes)
{
unsigned long start, mem, size;
@@ -2118,7 +2118,7 @@ dt_find_interrupt_controller(const struct dt_device_match *matches)
void __init dt_unflatten_host_device_tree(void)
{
- __unflatten_device_tree(device_tree_flattened, &dt_host);
+ unflatten_device_tree(device_tree_flattened, &dt_host);
dt_alias_scan();
}
diff --git a/xen/include/asm-arm/domain_build.h b/xen/include/asm-arm/domain_build.h
index 34ceddc..17449b1 100644
--- a/xen/include/asm-arm/domain_build.h
+++ b/xen/include/asm-arm/domain_build.h
@@ -4,10 +4,20 @@
#include <xen/sched.h>
#include <asm/kernel.h>
+struct map_range_data
+{
+ struct domain *d;
+ p2m_type_t p2mt;
+};
+
int map_irq_to_domain(struct domain *d, unsigned int irq,
bool need_mapping, const char *devname);
int make_chosen_node(const struct kernel_info *kinfo);
void evtchn_allocate(struct domain *d);
+int handle_device_interrupts(struct domain *d, struct dt_device_node *dev,
+ bool need_mapping);
+int map_range_to_domain(const struct dt_device_node *dev, u64 addr, u64 len,
+ void *data);
#ifndef CONFIG_ACPI
static inline int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index b02696b..a4e98a7 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -177,6 +177,11 @@ int device_tree_for_each_node(const void *fdt, int node,
*/
void dt_unflatten_host_device_tree(void);
+/*
+ * unflatten any device tree.
+ */
+void unflatten_device_tree(const void *fdt, struct dt_device_node **mynodes);
+
/**
* IRQ translation callback
* TODO: For the moment we assume that we only have ONE
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 01/13] device tree: Remove __init from function type
2021-09-02 6:05 ` [XEN][RFC PATCH 01/13] device tree: Remove __init from function type Vikram Garhwal
@ 2021-10-20 17:55 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-20 17:55 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel; +Cc: sstabellini, Volodymyr Babchuk
Hi Vikram,
Apologies for the late answer. I was away most of September and still
catching up with my e-mails.
On 02/09/2021 07:05, Vikram Garhwal wrote:
> Change function type of following function to access during runtime:
> 1. handle_device_interrupt()
> 2. map_irq_to_domain()
> 3. map_range_to_domain()
> 4. unflatten_dt_node()
> 5. unflatten_device_tree()
I would prefer if the __init is removed as it get used.
>
> Remove .init from domain_build.o and move map_range_data declaration to
> domain_build.h.
Skimming through the rest of the series, I think the function you are
trying to use will be misplaced because you are not looking to build a
domain. Instead, you are looking to add more devices.
So I think it would be better to move the functions outside of
domain_build.c. One possibility would be device.c.
Cheers,
>
> These changes are done to support the dynamic programming of a nodes where an
> overlay node will be added to fdt and unflattened node will be added to dt_host.
> Furthermore, irq and mmio mapping will be done for the added node.
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/domain_build.c | 15 ++++-----------
> xen/common/device_tree.c | 18 +++++++++---------
> xen/include/asm-arm/domain_build.h | 10 ++++++++++
> xen/include/xen/device_tree.h | 5 +++++
> 5 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 3d3b97b..bef4517 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -15,7 +15,7 @@ obj-y += decode.o
> obj-y += device.o
> obj-$(CONFIG_IOREQ_SERVER) += dm.o
> obj-y += domain.o
> -obj-y += domain_build.init.o
> +obj-y += domain_build.o
> obj-y += domctl.o
> obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> obj-y += gic.o
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 206038d..3a457d3 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -47,12 +47,6 @@ static int __init parse_dom0_mem(const char *s)
> }
> custom_param("dom0_mem", parse_dom0_mem);
>
> -struct map_range_data
> -{
> - struct domain *d;
> - p2m_type_t p2mt;
> -};
> -
> /* Override macros from asm/page.h to make them work with mfn_t */
> #undef virt_to_mfn
> #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> @@ -1144,7 +1138,7 @@ int __init make_chosen_node(const struct kernel_info *kinfo)
> return res;
> }
>
> -int __init map_irq_to_domain(struct domain *d, unsigned int irq,
> +int map_irq_to_domain(struct domain *d, unsigned int irq,
> bool need_mapping, const char *devname)
> {
> int res;
> @@ -1210,7 +1204,7 @@ static int __init map_dt_irq_to_domain(const struct dt_device_node *dev,
> return 0;
> }
>
> -static int __init map_range_to_domain(const struct dt_device_node *dev,
> +int map_range_to_domain(const struct dt_device_node *dev,
> u64 addr, u64 len,
> void *data)
> {
> @@ -1300,9 +1294,8 @@ static int __init map_device_children(struct domain *d,
> * < 0 error
> * 0 success
> */
> -static int __init handle_device_interrupts(struct domain *d,
> - struct dt_device_node *dev,
> - bool need_mapping)
> +int handle_device_interrupts(struct domain *d, struct dt_device_node *dev,
> + bool need_mapping)
> {
> unsigned int i, nirq;
> int res;
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 03d25a8..cda21be 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1750,12 +1750,12 @@ int dt_count_phandle_with_args(const struct dt_device_node *np,
> * @allnextpp: pointer to ->allnext from last allocated device_node
> * @fpsize: Size of the node path up at the current depth.
> */
> -static unsigned long __init unflatten_dt_node(const void *fdt,
> - unsigned long mem,
> - unsigned long *p,
> - struct dt_device_node *dad,
> - struct dt_device_node ***allnextpp,
> - unsigned long fpsize)
> +static unsigned long unflatten_dt_node(const void *fdt,
> + unsigned long mem,
> + unsigned long *p,
> + struct dt_device_node *dad,
> + struct dt_device_node ***allnextpp,
> + unsigned long fpsize)
> {
> struct dt_device_node *np;
> struct dt_property *pp, **prev_pp = NULL;
> @@ -1986,7 +1986,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
> }
>
> /**
> - * __unflatten_device_tree - create tree of device_nodes from flat blob
> + * unflatten_device_tree - create tree of device_nodes from flat blob
> *
> * unflattens a device-tree, creating the
> * tree of struct device_node. It also fills the "name" and "type"
> @@ -1995,7 +1995,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt,
> * @fdt: The fdt to expand
> * @mynodes: The device_node tree created by the call
> */
> -static void __init __unflatten_device_tree(const void *fdt,
> +void unflatten_device_tree(const void *fdt,
> struct dt_device_node **mynodes)
> {
> unsigned long start, mem, size;
> @@ -2118,7 +2118,7 @@ dt_find_interrupt_controller(const struct dt_device_match *matches)
>
> void __init dt_unflatten_host_device_tree(void)
> {
> - __unflatten_device_tree(device_tree_flattened, &dt_host);
> + unflatten_device_tree(device_tree_flattened, &dt_host);
> dt_alias_scan();
> }
>
> diff --git a/xen/include/asm-arm/domain_build.h b/xen/include/asm-arm/domain_build.h
> index 34ceddc..17449b1 100644
> --- a/xen/include/asm-arm/domain_build.h
> +++ b/xen/include/asm-arm/domain_build.h
> @@ -4,10 +4,20 @@
> #include <xen/sched.h>
> #include <asm/kernel.h>
>
> +struct map_range_data
> +{
> + struct domain *d;
> + p2m_type_t p2mt;
> +};
> +
> int map_irq_to_domain(struct domain *d, unsigned int irq,
> bool need_mapping, const char *devname);
> int make_chosen_node(const struct kernel_info *kinfo);
> void evtchn_allocate(struct domain *d);
> +int handle_device_interrupts(struct domain *d, struct dt_device_node *dev,
> + bool need_mapping);
> +int map_range_to_domain(const struct dt_device_node *dev, u64 addr, u64 len,
> + void *data);
>
> #ifndef CONFIG_ACPI
> static inline int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index b02696b..a4e98a7 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -177,6 +177,11 @@ int device_tree_for_each_node(const void *fdt, int node,
> */
> void dt_unflatten_host_device_tree(void);
>
> +/*
> + * unflatten any device tree.
> + */
> +void unflatten_device_tree(const void *fdt, struct dt_device_node **mynodes);
> +
> /**
> * IRQ translation callback
> * TODO: For the moment we assume that we only have ONE
>
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 02/13] libfdt: Keep fdt functions after init.
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
2021-09-02 6:05 ` [XEN][RFC PATCH 01/13] device tree: Remove __init from function type Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-20 18:00 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 03/13] libfdt: import fdt_overlay from Linux Vikram Garhwal
` (10 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
Keep libfdt library functionalities after boot of hw_domain. This is done to
access fdt library function which are required for adding device tree overlay
nodes for dynamic programming of nodes.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/libfdt/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
index 6bd207c..8002f8c 100644
--- a/xen/common/libfdt/Makefile
+++ b/xen/common/libfdt/Makefile
@@ -1,7 +1,6 @@
include Makefile.libfdt
SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
-OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
obj-y += libfdt.o
nocov-y += libfdt.o
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 02/13] libfdt: Keep fdt functions after init.
2021-09-02 6:05 ` [XEN][RFC PATCH 02/13] libfdt: Keep fdt functions after init Vikram Garhwal
@ 2021-10-20 18:00 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-20 18:00 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel; +Cc: sstabellini
Hi Vikram,
On 02/09/2021 07:05, Vikram Garhwal wrote:
> Keep libfdt library functionalities after boot of hw_domain. This is done to
> access fdt library function which are required for adding device tree overlay
> nodes for dynamic programming of nodes.
AFAICT, the new feature will be mostly useful on HW with FPGA. I expect
that some users may not want this feature (for instance because the HW
doesn't have FPGA), so I think it would be best to introduce a new
Kconfig to enable the new feature.
This could then be used to decide whether libfdt should be part of .init
or not.
Cheers,
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/common/libfdt/Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/xen/common/libfdt/Makefile b/xen/common/libfdt/Makefile
> index 6bd207c..8002f8c 100644
> --- a/xen/common/libfdt/Makefile
> +++ b/xen/common/libfdt/Makefile
> @@ -1,7 +1,6 @@
> include Makefile.libfdt
>
> SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
> -OBJCOPYFLAGS := $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s))
>
> obj-y += libfdt.o
> nocov-y += libfdt.o
>
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 03/13] libfdt: import fdt_overlay from Linux
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
2021-09-02 6:05 ` [XEN][RFC PATCH 01/13] device tree: Remove __init from function type Vikram Garhwal
2021-09-02 6:05 ` [XEN][RFC PATCH 02/13] libfdt: Keep fdt functions after init Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-20 18:05 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 04/13] libfdt: Copy required libfdt functions " Vikram Garhwal
` (9 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
Xen is missing fdt overlay functionalities. FDT overlay is required for changing
the device tree nodes during run-time.
fdt_overlay.c file is copied from Linux tree's following patch:
commit: 6e9c9686d826564f44c93cdd6f111b1c0a9dc224
scripts/dtc: Update to upstream version v1.6.0-31-gcbca977ea121
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/libfdt/fdt_overlay.c | 882 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 882 insertions(+)
create mode 100644 xen/common/libfdt/fdt_overlay.c
diff --git a/xen/common/libfdt/fdt_overlay.c b/xen/common/libfdt/fdt_overlay.c
new file mode 100644
index 0000000..d217e79
--- /dev/null
+++ b/xen/common/libfdt/fdt_overlay.c
@@ -0,0 +1,882 @@
+// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+/*
+ * libfdt - Flat Device Tree manipulation
+ * Copyright (C) 2016 Free Electrons
+ * Copyright (C) 2016 NextThing Co.
+ */
+#include "libfdt_env.h"
+
+#include <fdt.h>
+#include <libfdt.h>
+
+#include "libfdt_internal.h"
+
+/**
+ * overlay_get_target_phandle - retrieves the target phandle of a fragment
+ * @fdto: pointer to the device tree overlay blob
+ * @fragment: node offset of the fragment in the overlay
+ *
+ * overlay_get_target_phandle() retrieves the target phandle of an
+ * overlay fragment when that fragment uses a phandle (target
+ * property) instead of a path (target-path property).
+ *
+ * returns:
+ * the phandle pointed by the target property
+ * 0, if the phandle was not found
+ * -1, if the phandle was malformed
+ */
+static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
+{
+ const fdt32_t *val;
+ int len;
+
+ val = fdt_getprop(fdto, fragment, "target", &len);
+ if (!val)
+ return 0;
+
+ if ((len != sizeof(*val)) || (fdt32_to_cpu(*val) == (uint32_t)-1))
+ return (uint32_t)-1;
+
+ return fdt32_to_cpu(*val);
+}
+
+/**
+ * overlay_get_target - retrieves the offset of a fragment's target
+ * @fdt: Base device tree blob
+ * @fdto: Device tree overlay blob
+ * @fragment: node offset of the fragment in the overlay
+ * @pathp: pointer which receives the path of the target (or NULL)
+ *
+ * overlay_get_target() retrieves the target offset in the base
+ * device tree of a fragment, no matter how the actual targeting is
+ * done (through a phandle or a path)
+ *
+ * returns:
+ * the targeted node offset in the base device tree
+ * Negative error code on error
+ */
+static int overlay_get_target(const void *fdt, const void *fdto,
+ int fragment, char const **pathp)
+{
+ uint32_t phandle;
+ const char *path = NULL;
+ int path_len = 0, ret;
+
+ /* Try first to do a phandle based lookup */
+ phandle = overlay_get_target_phandle(fdto, fragment);
+ if (phandle == (uint32_t)-1)
+ return -FDT_ERR_BADPHANDLE;
+
+ /* no phandle, try path */
+ if (!phandle) {
+ /* And then a path based lookup */
+ path = fdt_getprop(fdto, fragment, "target-path", &path_len);
+ if (path)
+ ret = fdt_path_offset(fdt, path);
+ else
+ ret = path_len;
+ } else
+ ret = fdt_node_offset_by_phandle(fdt, phandle);
+
+ /*
+ * If we haven't found either a target or a
+ * target-path property in a node that contains a
+ * __overlay__ subnode (we wouldn't be called
+ * otherwise), consider it a improperly written
+ * overlay
+ */
+ if (ret < 0 && path_len == -FDT_ERR_NOTFOUND)
+ ret = -FDT_ERR_BADOVERLAY;
+
+ /* return on error */
+ if (ret < 0)
+ return ret;
+
+ /* return pointer to path (if available) */
+ if (pathp)
+ *pathp = path ? path : NULL;
+
+ return ret;
+}
+
+/**
+ * overlay_phandle_add_offset - Increases a phandle by an offset
+ * @fdt: Base device tree blob
+ * @node: Device tree overlay blob
+ * @name: Name of the property to modify (phandle or linux,phandle)
+ * @delta: offset to apply
+ *
+ * overlay_phandle_add_offset() increments a node phandle by a given
+ * offset.
+ *
+ * returns:
+ * 0 on success.
+ * Negative error code on error
+ */
+static int overlay_phandle_add_offset(void *fdt, int node,
+ const char *name, uint32_t delta)
+{
+ const fdt32_t *val;
+ uint32_t adj_val;
+ int len;
+
+ val = fdt_getprop(fdt, node, name, &len);
+ if (!val)
+ return len;
+
+ if (len != sizeof(*val))
+ return -FDT_ERR_BADPHANDLE;
+
+ adj_val = fdt32_to_cpu(*val);
+ if ((adj_val + delta) < adj_val)
+ return -FDT_ERR_NOPHANDLES;
+
+ adj_val += delta;
+ if (adj_val == (uint32_t)-1)
+ return -FDT_ERR_NOPHANDLES;
+
+ return fdt_setprop_inplace_u32(fdt, node, name, adj_val);
+}
+
+/**
+ * overlay_adjust_node_phandles - Offsets the phandles of a node
+ * @fdto: Device tree overlay blob
+ * @node: Offset of the node we want to adjust
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_adjust_node_phandles() adds a constant to all the phandles
+ * of a given node. This is mainly use as part of the overlay
+ * application process, when we want to update all the overlay
+ * phandles to not conflict with the overlays of the base device tree.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_adjust_node_phandles(void *fdto, int node,
+ uint32_t delta)
+{
+ int child;
+ int ret;
+
+ ret = overlay_phandle_add_offset(fdto, node, "phandle", delta);
+ if (ret && ret != -FDT_ERR_NOTFOUND)
+ return ret;
+
+ ret = overlay_phandle_add_offset(fdto, node, "linux,phandle", delta);
+ if (ret && ret != -FDT_ERR_NOTFOUND)
+ return ret;
+
+ fdt_for_each_subnode(child, fdto, node) {
+ ret = overlay_adjust_node_phandles(fdto, child, delta);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * overlay_adjust_local_phandles - Adjust the phandles of a whole overlay
+ * @fdto: Device tree overlay blob
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_adjust_local_phandles() adds a constant to all the
+ * phandles of an overlay. This is mainly use as part of the overlay
+ * application process, when we want to update all the overlay
+ * phandles to not conflict with the overlays of the base device tree.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_adjust_local_phandles(void *fdto, uint32_t delta)
+{
+ /*
+ * Start adjusting the phandles from the overlay root
+ */
+ return overlay_adjust_node_phandles(fdto, 0, delta);
+}
+
+/**
+ * overlay_update_local_node_references - Adjust the overlay references
+ * @fdto: Device tree overlay blob
+ * @tree_node: Node offset of the node to operate on
+ * @fixup_node: Node offset of the matching local fixups node
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_update_local_nodes_references() update the phandles
+ * pointing to a node within the device tree overlay by adding a
+ * constant delta.
+ *
+ * This is mainly used as part of a device tree application process,
+ * where you want the device tree overlays phandles to not conflict
+ * with the ones from the base device tree before merging them.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_update_local_node_references(void *fdto,
+ int tree_node,
+ int fixup_node,
+ uint32_t delta)
+{
+ int fixup_prop;
+ int fixup_child;
+ int ret;
+
+ fdt_for_each_property_offset(fixup_prop, fdto, fixup_node) {
+ const fdt32_t *fixup_val;
+ const char *tree_val;
+ const char *name;
+ int fixup_len;
+ int tree_len;
+ int i;
+
+ fixup_val = fdt_getprop_by_offset(fdto, fixup_prop,
+ &name, &fixup_len);
+ if (!fixup_val)
+ return fixup_len;
+
+ if (fixup_len % sizeof(uint32_t))
+ return -FDT_ERR_BADOVERLAY;
+ fixup_len /= sizeof(uint32_t);
+
+ tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
+ if (!tree_val) {
+ if (tree_len == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_BADOVERLAY;
+
+ return tree_len;
+ }
+
+ for (i = 0; i < fixup_len; i++) {
+ fdt32_t adj_val;
+ uint32_t poffset;
+
+ poffset = fdt32_to_cpu(fixup_val[i]);
+
+ /*
+ * phandles to fixup can be unaligned.
+ *
+ * Use a memcpy for the architectures that do
+ * not support unaligned accesses.
+ */
+ memcpy(&adj_val, tree_val + poffset, sizeof(adj_val));
+
+ adj_val = cpu_to_fdt32(fdt32_to_cpu(adj_val) + delta);
+
+ ret = fdt_setprop_inplace_namelen_partial(fdto,
+ tree_node,
+ name,
+ strlen(name),
+ poffset,
+ &adj_val,
+ sizeof(adj_val));
+ if (ret == -FDT_ERR_NOSPACE)
+ return -FDT_ERR_BADOVERLAY;
+
+ if (ret)
+ return ret;
+ }
+ }
+
+ fdt_for_each_subnode(fixup_child, fdto, fixup_node) {
+ const char *fixup_child_name = fdt_get_name(fdto, fixup_child,
+ NULL);
+ int tree_child;
+
+ tree_child = fdt_subnode_offset(fdto, tree_node,
+ fixup_child_name);
+ if (tree_child == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_BADOVERLAY;
+ if (tree_child < 0)
+ return tree_child;
+
+ ret = overlay_update_local_node_references(fdto,
+ tree_child,
+ fixup_child,
+ delta);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * overlay_update_local_references - Adjust the overlay references
+ * @fdto: Device tree overlay blob
+ * @delta: Offset to shift the phandles of
+ *
+ * overlay_update_local_references() update all the phandles pointing
+ * to a node within the device tree overlay by adding a constant
+ * delta to not conflict with the base overlay.
+ *
+ * This is mainly used as part of a device tree application process,
+ * where you want the device tree overlays phandles to not conflict
+ * with the ones from the base device tree before merging them.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_update_local_references(void *fdto, uint32_t delta)
+{
+ int fixups;
+
+ fixups = fdt_path_offset(fdto, "/__local_fixups__");
+ if (fixups < 0) {
+ /* There's no local phandles to adjust, bail out */
+ if (fixups == -FDT_ERR_NOTFOUND)
+ return 0;
+
+ return fixups;
+ }
+
+ /*
+ * Update our local references from the root of the tree
+ */
+ return overlay_update_local_node_references(fdto, 0, fixups,
+ delta);
+}
+
+/**
+ * overlay_fixup_one_phandle - Set an overlay phandle to the base one
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ * @symbols_off: Node offset of the symbols node in the base device tree
+ * @path: Path to a node holding a phandle in the overlay
+ * @path_len: number of path characters to consider
+ * @name: Name of the property holding the phandle reference in the overlay
+ * @name_len: number of name characters to consider
+ * @poffset: Offset within the overlay property where the phandle is stored
+ * @label: Label of the node referenced by the phandle
+ *
+ * overlay_fixup_one_phandle() resolves an overlay phandle pointing to
+ * a node in the base device tree.
+ *
+ * This is part of the device tree overlay application process, when
+ * you want all the phandles in the overlay to point to the actual
+ * base dt nodes.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_fixup_one_phandle(void *fdt, void *fdto,
+ int symbols_off,
+ const char *path, uint32_t path_len,
+ const char *name, uint32_t name_len,
+ int poffset, const char *label)
+{
+ const char *symbol_path;
+ uint32_t phandle;
+ fdt32_t phandle_prop;
+ int symbol_off, fixup_off;
+ int prop_len;
+
+ if (symbols_off < 0)
+ return symbols_off;
+
+ symbol_path = fdt_getprop(fdt, symbols_off, label,
+ &prop_len);
+ if (!symbol_path)
+ return prop_len;
+
+ symbol_off = fdt_path_offset(fdt, symbol_path);
+ if (symbol_off < 0)
+ return symbol_off;
+
+ phandle = fdt_get_phandle(fdt, symbol_off);
+ if (!phandle)
+ return -FDT_ERR_NOTFOUND;
+
+ fixup_off = fdt_path_offset_namelen(fdto, path, path_len);
+ if (fixup_off == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_BADOVERLAY;
+ if (fixup_off < 0)
+ return fixup_off;
+
+ phandle_prop = cpu_to_fdt32(phandle);
+ return fdt_setprop_inplace_namelen_partial(fdto, fixup_off,
+ name, name_len, poffset,
+ &phandle_prop,
+ sizeof(phandle_prop));
+};
+
+/**
+ * overlay_fixup_phandle - Set an overlay phandle to the base one
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ * @symbols_off: Node offset of the symbols node in the base device tree
+ * @property: Property offset in the overlay holding the list of fixups
+ *
+ * overlay_fixup_phandle() resolves all the overlay phandles pointed
+ * to in a __fixups__ property, and updates them to match the phandles
+ * in use in the base device tree.
+ *
+ * This is part of the device tree overlay application process, when
+ * you want all the phandles in the overlay to point to the actual
+ * base dt nodes.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
+ int property)
+{
+ const char *value;
+ const char *label;
+ int len;
+
+ value = fdt_getprop_by_offset(fdto, property,
+ &label, &len);
+ if (!value) {
+ if (len == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_INTERNAL;
+
+ return len;
+ }
+
+ do {
+ const char *path, *name, *fixup_end;
+ const char *fixup_str = value;
+ uint32_t path_len, name_len;
+ uint32_t fixup_len;
+ char *sep, *endptr;
+ int poffset, ret;
+
+ fixup_end = memchr(value, '\0', len);
+ if (!fixup_end)
+ return -FDT_ERR_BADOVERLAY;
+ fixup_len = fixup_end - fixup_str;
+
+ len -= fixup_len + 1;
+ value += fixup_len + 1;
+
+ path = fixup_str;
+ sep = memchr(fixup_str, ':', fixup_len);
+ if (!sep || *sep != ':')
+ return -FDT_ERR_BADOVERLAY;
+
+ path_len = sep - path;
+ if (path_len == (fixup_len - 1))
+ return -FDT_ERR_BADOVERLAY;
+
+ fixup_len -= path_len + 1;
+ name = sep + 1;
+ sep = memchr(name, ':', fixup_len);
+ if (!sep || *sep != ':')
+ return -FDT_ERR_BADOVERLAY;
+
+ name_len = sep - name;
+ if (!name_len)
+ return -FDT_ERR_BADOVERLAY;
+
+ poffset = strtoul(sep + 1, &endptr, 10);
+ if ((*endptr != '\0') || (endptr <= (sep + 1)))
+ return -FDT_ERR_BADOVERLAY;
+
+ ret = overlay_fixup_one_phandle(fdt, fdto, symbols_off,
+ path, path_len, name, name_len,
+ poffset, label);
+ if (ret)
+ return ret;
+ } while (len > 0);
+
+ return 0;
+}
+
+/**
+ * overlay_fixup_phandles - Resolve the overlay phandles to the base
+ * device tree
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_fixup_phandles() resolves all the overlay phandles pointing
+ * to nodes in the base device tree.
+ *
+ * This is one of the steps of the device tree overlay application
+ * process, when you want all the phandles in the overlay to point to
+ * the actual base dt nodes.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_fixup_phandles(void *fdt, void *fdto)
+{
+ int fixups_off, symbols_off;
+ int property;
+
+ /* We can have overlays without any fixups */
+ fixups_off = fdt_path_offset(fdto, "/__fixups__");
+ if (fixups_off == -FDT_ERR_NOTFOUND)
+ return 0; /* nothing to do */
+ if (fixups_off < 0)
+ return fixups_off;
+
+ /* And base DTs without symbols */
+ symbols_off = fdt_path_offset(fdt, "/__symbols__");
+ if ((symbols_off < 0 && (symbols_off != -FDT_ERR_NOTFOUND)))
+ return symbols_off;
+
+ fdt_for_each_property_offset(property, fdto, fixups_off) {
+ int ret;
+
+ ret = overlay_fixup_phandle(fdt, fdto, symbols_off, property);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * overlay_apply_node - Merges a node into the base device tree
+ * @fdt: Base Device Tree blob
+ * @target: Node offset in the base device tree to apply the fragment to
+ * @fdto: Device tree overlay blob
+ * @node: Node offset in the overlay holding the changes to merge
+ *
+ * overlay_apply_node() merges a node into a target base device tree
+ * node pointed.
+ *
+ * This is part of the final step in the device tree overlay
+ * application process, when all the phandles have been adjusted and
+ * resolved and you just have to merge overlay into the base device
+ * tree.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_apply_node(void *fdt, int target,
+ void *fdto, int node)
+{
+ int property;
+ int subnode;
+
+ fdt_for_each_property_offset(property, fdto, node) {
+ const char *name;
+ const void *prop;
+ int prop_len;
+ int ret;
+
+ prop = fdt_getprop_by_offset(fdto, property, &name,
+ &prop_len);
+ if (prop_len == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_INTERNAL;
+ if (prop_len < 0)
+ return prop_len;
+
+ ret = fdt_setprop(fdt, target, name, prop, prop_len);
+ if (ret)
+ return ret;
+ }
+
+ fdt_for_each_subnode(subnode, fdto, node) {
+ const char *name = fdt_get_name(fdto, subnode, NULL);
+ int nnode;
+ int ret;
+
+ nnode = fdt_add_subnode(fdt, target, name);
+ if (nnode == -FDT_ERR_EXISTS) {
+ nnode = fdt_subnode_offset(fdt, target, name);
+ if (nnode == -FDT_ERR_NOTFOUND)
+ return -FDT_ERR_INTERNAL;
+ }
+
+ if (nnode < 0)
+ return nnode;
+
+ ret = overlay_apply_node(fdt, nnode, fdto, subnode);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * overlay_merge - Merge an overlay into its base device tree
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_merge() merges an overlay into its base device tree.
+ *
+ * This is the next to last step in the device tree overlay application
+ * process, when all the phandles have been adjusted and resolved and
+ * you just have to merge overlay into the base device tree.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_merge(void *fdt, void *fdto)
+{
+ int fragment;
+
+ fdt_for_each_subnode(fragment, fdto, 0) {
+ int overlay;
+ int target;
+ int ret;
+
+ /*
+ * Each fragments will have an __overlay__ node. If
+ * they don't, it's not supposed to be merged
+ */
+ overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
+ if (overlay == -FDT_ERR_NOTFOUND)
+ continue;
+
+ if (overlay < 0)
+ return overlay;
+
+ target = overlay_get_target(fdt, fdto, fragment, NULL);
+ if (target < 0)
+ return target;
+
+ ret = overlay_apply_node(fdt, target, fdto, overlay);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int get_path_len(const void *fdt, int nodeoffset)
+{
+ int len = 0, namelen;
+ const char *name;
+
+ FDT_RO_PROBE(fdt);
+
+ for (;;) {
+ name = fdt_get_name(fdt, nodeoffset, &namelen);
+ if (!name)
+ return namelen;
+
+ /* root? we're done */
+ if (namelen == 0)
+ break;
+
+ nodeoffset = fdt_parent_offset(fdt, nodeoffset);
+ if (nodeoffset < 0)
+ return nodeoffset;
+ len += namelen + 1;
+ }
+
+ /* in case of root pretend it's "/" */
+ if (len == 0)
+ len++;
+ return len;
+}
+
+/**
+ * overlay_symbol_update - Update the symbols of base tree after a merge
+ * @fdt: Base Device Tree blob
+ * @fdto: Device tree overlay blob
+ *
+ * overlay_symbol_update() updates the symbols of the base tree with the
+ * symbols of the applied overlay
+ *
+ * This is the last step in the device tree overlay application
+ * process, allowing the reference of overlay symbols by subsequent
+ * overlay operations.
+ *
+ * returns:
+ * 0 on success
+ * Negative error code on failure
+ */
+static int overlay_symbol_update(void *fdt, void *fdto)
+{
+ int root_sym, ov_sym, prop, path_len, fragment, target;
+ int len, frag_name_len, ret, rel_path_len;
+ const char *s, *e;
+ const char *path;
+ const char *name;
+ const char *frag_name;
+ const char *rel_path;
+ const char *target_path;
+ char *buf;
+ void *p;
+
+ ov_sym = fdt_subnode_offset(fdto, 0, "__symbols__");
+
+ /* if no overlay symbols exist no problem */
+ if (ov_sym < 0)
+ return 0;
+
+ root_sym = fdt_subnode_offset(fdt, 0, "__symbols__");
+
+ /* it no root symbols exist we should create them */
+ if (root_sym == -FDT_ERR_NOTFOUND)
+ root_sym = fdt_add_subnode(fdt, 0, "__symbols__");
+
+ /* any error is fatal now */
+ if (root_sym < 0)
+ return root_sym;
+
+ /* iterate over each overlay symbol */
+ fdt_for_each_property_offset(prop, fdto, ov_sym) {
+ path = fdt_getprop_by_offset(fdto, prop, &name, &path_len);
+ if (!path)
+ return path_len;
+
+ /* verify it's a string property (terminated by a single \0) */
+ if (path_len < 1 || memchr(path, '\0', path_len) != &path[path_len - 1])
+ return -FDT_ERR_BADVALUE;
+
+ /* keep end marker to avoid strlen() */
+ e = path + path_len;
+
+ if (*path != '/')
+ return -FDT_ERR_BADVALUE;
+
+ /* get fragment name first */
+ s = strchr(path + 1, '/');
+ if (!s) {
+ /* Symbol refers to something that won't end
+ * up in the target tree */
+ continue;
+ }
+
+ frag_name = path + 1;
+ frag_name_len = s - path - 1;
+
+ /* verify format; safe since "s" lies in \0 terminated prop */
+ len = sizeof("/__overlay__/") - 1;
+ if ((e - s) > len && (memcmp(s, "/__overlay__/", len) == 0)) {
+ /* /<fragment-name>/__overlay__/<relative-subnode-path> */
+ rel_path = s + len;
+ rel_path_len = e - rel_path - 1;
+ } else if ((e - s) == len
+ && (memcmp(s, "/__overlay__", len - 1) == 0)) {
+ /* /<fragment-name>/__overlay__ */
+ rel_path = "";
+ rel_path_len = 0;
+ } else {
+ /* Symbol refers to something that won't end
+ * up in the target tree */
+ continue;
+ }
+
+ /* find the fragment index in which the symbol lies */
+ ret = fdt_subnode_offset_namelen(fdto, 0, frag_name,
+ frag_name_len);
+ /* not found? */
+ if (ret < 0)
+ return -FDT_ERR_BADOVERLAY;
+ fragment = ret;
+
+ /* an __overlay__ subnode must exist */
+ ret = fdt_subnode_offset(fdto, fragment, "__overlay__");
+ if (ret < 0)
+ return -FDT_ERR_BADOVERLAY;
+
+ /* get the target of the fragment */
+ ret = overlay_get_target(fdt, fdto, fragment, &target_path);
+ if (ret < 0)
+ return ret;
+ target = ret;
+
+ /* if we have a target path use */
+ if (!target_path) {
+ ret = get_path_len(fdt, target);
+ if (ret < 0)
+ return ret;
+ len = ret;
+ } else {
+ len = strlen(target_path);
+ }
+
+ ret = fdt_setprop_placeholder(fdt, root_sym, name,
+ len + (len > 1) + rel_path_len + 1, &p);
+ if (ret < 0)
+ return ret;
+
+ if (!target_path) {
+ /* again in case setprop_placeholder changed it */
+ ret = overlay_get_target(fdt, fdto, fragment, &target_path);
+ if (ret < 0)
+ return ret;
+ target = ret;
+ }
+
+ buf = p;
+ if (len > 1) { /* target is not root */
+ if (!target_path) {
+ ret = fdt_get_path(fdt, target, buf, len + 1);
+ if (ret < 0)
+ return ret;
+ } else
+ memcpy(buf, target_path, len + 1);
+
+ } else
+ len--;
+
+ buf[len] = '/';
+ memcpy(buf + len + 1, rel_path, rel_path_len);
+ buf[len + 1 + rel_path_len] = '\0';
+ }
+
+ return 0;
+}
+
+int fdt_overlay_apply(void *fdt, void *fdto)
+{
+ uint32_t delta;
+ int ret;
+
+ FDT_RO_PROBE(fdt);
+ FDT_RO_PROBE(fdto);
+
+ ret = fdt_find_max_phandle(fdt, &delta);
+ if (ret)
+ goto err;
+
+ ret = overlay_adjust_local_phandles(fdto, delta);
+ if (ret)
+ goto err;
+
+ ret = overlay_update_local_references(fdto, delta);
+ if (ret)
+ goto err;
+
+ ret = overlay_fixup_phandles(fdt, fdto);
+ if (ret)
+ goto err;
+
+ ret = overlay_merge(fdt, fdto);
+ if (ret)
+ goto err;
+
+ ret = overlay_symbol_update(fdt, fdto);
+ if (ret)
+ goto err;
+
+ /*
+ * The overlay has been damaged, erase its magic.
+ */
+ fdt_set_magic(fdto, ~0);
+
+ return 0;
+
+err:
+ /*
+ * The overlay might have been damaged, erase its magic.
+ */
+ fdt_set_magic(fdto, ~0);
+
+ /*
+ * The base device tree might have been damaged, erase its
+ * magic.
+ */
+ fdt_set_magic(fdt, ~0);
+
+ return ret;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 03/13] libfdt: import fdt_overlay from Linux
2021-09-02 6:05 ` [XEN][RFC PATCH 03/13] libfdt: import fdt_overlay from Linux Vikram Garhwal
@ 2021-10-20 18:05 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-20 18:05 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel; +Cc: sstabellini
Hi Vikram,
On 02/09/2021 07:05, Vikram Garhwal wrote:
> Xen is missing fdt overlay functionalities. FDT overlay is required for changing
> the device tree nodes during run-time.
>
> fdt_overlay.c file is copied from Linux tree's following patch:
> commit: 6e9c9686d826564f44c93cdd6f111b1c0a9dc224
> scripts/dtc: Update to upstream version v1.6.0-31-gcbca977ea121
This patch and the next one add quite a bit of code in libfdt. The
directory xen/common/libfdt is pretty much a snapshot of v1.4.0 (we
tweaked the Makefile and import 2 bug fixes).
So I would prefer if we re-sync our directory with the one in
https://github.com/dgibson/dtc.git. v1.6.1 seems a good candidate.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 04/13] libfdt: Copy required libfdt functions from Linux
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (2 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 03/13] libfdt: import fdt_overlay from Linux Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-09-02 6:05 ` [XEN][RFC PATCH 05/13] libfdt: Change overlay_get_target() type Vikram Garhwal
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
fdt_overlay.c uses a number of functions. Below is the list of functions copied
from Linux tree(commid: 6e9c9686d826564f44c93cdd6f111b1c0a9dc224) to compile
the fdt_overlay.c():
libfdt_internal.h: FDT_RO_PROBE() and can_assume().
libfdt.h: fdt_for_each_subnode(), fdt_get_max_phandle(),
fdt_for_each_property_offset() and fdt_getprop_namelen_w().
fdt.c: fdt_ro_probe_().
fdt_ro.c: fdt_find_max_phandle(), fdt_path_offset_namelen() and
fdt_path_offset().
fdt_rw.c: fdt_rw_probe_(), FDT_RW_PROBE(), fdt_del_last_string_(),
fdt_setprop_placeholder() and fdt_setprop().
fdt_wip.c: fdt_setprop_inplace_namelen_partial().
Updated fdt_rw.c: _fdt_find_add_string() with required changes from Linux
fdt_rw.c:fdt_find_add_string_().
Updated fdt_rw.c: _fdt_add_property() with required changes from Linux
fdt_rw.c:fdt_find_add_string_().
Replaced strtoul() with simple_strtoull().
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/libfdt/Makefile.libfdt | 2 +-
xen/common/libfdt/fdt.c | 35 ++++++
xen/common/libfdt/fdt_overlay.c | 6 +-
xen/common/libfdt/fdt_ro.c | 52 ++++++--
xen/common/libfdt/fdt_rw.c | 81 +++++++++++--
xen/common/libfdt/fdt_wip.c | 20 ++++
xen/common/libfdt/libfdt_internal.h | 130 ++++++++++++++++++++
xen/include/xen/libfdt/libfdt.h | 230 +++++++++++++++++++++++++++++++++++-
8 files changed, 536 insertions(+), 20 deletions(-)
diff --git a/xen/common/libfdt/Makefile.libfdt b/xen/common/libfdt/Makefile.libfdt
index 91126c0..aea9d9b 100644
--- a/xen/common/libfdt/Makefile.libfdt
+++ b/xen/common/libfdt/Makefile.libfdt
@@ -6,5 +6,5 @@
LIBFDT_soname = libfdt.$(SHAREDLIB_EXT).1
LIBFDT_INCLUDES = fdt.h libfdt.h libfdt_env.h
LIBFDT_VERSION = version.lds
-LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c
+LIBFDT_SRCS = fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c fdt_empty_tree.c fdt_overlay.c
LIBFDT_OBJS = $(LIBFDT_SRCS:%.c=%.o)
diff --git a/xen/common/libfdt/fdt.c b/xen/common/libfdt/fdt.c
index bbc7717..55a9de6 100644
--- a/xen/common/libfdt/fdt.c
+++ b/xen/common/libfdt/fdt.c
@@ -53,6 +53,41 @@
#include "libfdt_internal.h"
+/*
+ * Minimal sanity check for a read-only tree. fdt_ro_probe_() checks
+ * that the given buffer contains what appears to be a flattened
+ * device tree with sane information in its header.
+ */
+int32_t fdt_ro_probe_(const void *fdt)
+{
+ uint32_t totalsize = fdt_totalsize(fdt);
+
+ if (can_assume(VALID_DTB))
+ return totalsize;
+
+ if (fdt_magic(fdt) == FDT_MAGIC) {
+ /* Complete tree */
+ if (!can_assume(LATEST)) {
+ if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
+ return -FDT_ERR_BADVERSION;
+ if (fdt_last_comp_version(fdt) >
+ FDT_LAST_SUPPORTED_VERSION)
+ return -FDT_ERR_BADVERSION;
+ }
+ } else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
+ /* Unfinished sequential-write blob */
+ if (!can_assume(VALID_INPUT) && fdt_size_dt_struct(fdt) == 0)
+ return -FDT_ERR_BADSTATE;
+ } else {
+ return -FDT_ERR_BADMAGIC;
+ }
+
+ if (totalsize < INT32_MAX)
+ return totalsize;
+ else
+ return -FDT_ERR_TRUNCATED;
+}
+
int fdt_check_header(const void *fdt)
{
if (fdt_magic(fdt) == FDT_MAGIC) {
diff --git a/xen/common/libfdt/fdt_overlay.c b/xen/common/libfdt/fdt_overlay.c
index d217e79..15a8cdb 100644
--- a/xen/common/libfdt/fdt_overlay.c
+++ b/xen/common/libfdt/fdt_overlay.c
@@ -9,6 +9,7 @@
#include <fdt.h>
#include <libfdt.h>
+#include <xen/lib.h>
#include "libfdt_internal.h"
/**
@@ -446,7 +447,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
const char *fixup_str = value;
uint32_t path_len, name_len;
uint32_t fixup_len;
- char *sep, *endptr;
+ char *sep;
+ const char *endptr;
int poffset, ret;
fixup_end = memchr(value, '\0', len);
@@ -476,7 +478,7 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
if (!name_len)
return -FDT_ERR_BADOVERLAY;
- poffset = strtoul(sep + 1, &endptr, 10);
+ poffset = simple_strtoull(sep + 1, &endptr, 10);
if ((*endptr != '\0') || (endptr <= (sep + 1)))
return -FDT_ERR_BADOVERLAY;
diff --git a/xen/common/libfdt/fdt_ro.c b/xen/common/libfdt/fdt_ro.c
index 36f9b48..383791d 100644
--- a/xen/common/libfdt/fdt_ro.c
+++ b/xen/common/libfdt/fdt_ro.c
@@ -86,6 +86,34 @@ static int _fdt_string_eq(const void *fdt, int stroffset,
return (strlen(p) == len) && (memcmp(p, s, len) == 0);
}
+int fdt_find_max_phandle(const void *fdt, uint32_t *phandle)
+{
+ uint32_t max = 0;
+ int offset = -1;
+
+ while (true) {
+ uint32_t value;
+
+ offset = fdt_next_node(fdt, offset, NULL);
+ if (offset < 0) {
+ if (offset == -FDT_ERR_NOTFOUND)
+ break;
+
+ return offset;
+ }
+
+ value = fdt_get_phandle(fdt, offset);
+
+ if (value > max)
+ max = value;
+ }
+
+ if (phandle)
+ *phandle = max;
+
+ return 0;
+}
+
int fdt_get_mem_rsv(const void *fdt, int n, uint64_t *address, uint64_t *size)
{
FDT_CHECK_HEADER(fdt);
@@ -152,17 +180,17 @@ int fdt_subnode_offset(const void *fdt, int parentoffset,
return fdt_subnode_offset_namelen(fdt, parentoffset, name, strlen(name));
}
-int fdt_path_offset(const void *fdt, const char *path)
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen)
{
- const char *end = path + strlen(path);
+ const char *end = path + namelen;
const char *p = path;
int offset = 0;
- FDT_CHECK_HEADER(fdt);
+ FDT_RO_PROBE(fdt);
/* see if we have an alias */
if (*path != '/') {
- const char *q = strchr(path, '/');
+ const char *q = memchr(path, '/', end - p);
if (!q)
q = end;
@@ -175,14 +203,15 @@ int fdt_path_offset(const void *fdt, const char *path)
p = q;
}
- while (*p) {
+ while (p < end) {
const char *q;
- while (*p == '/')
+ while (*p == '/') {
p++;
- if (! *p)
- return offset;
- q = strchr(p, '/');
+ if (p == end)
+ return offset;
+ }
+ q = memchr(p, '/', end - p);
if (! q)
q = end;
@@ -196,6 +225,11 @@ int fdt_path_offset(const void *fdt, const char *path)
return offset;
}
+int fdt_path_offset(const void *fdt, const char *path)
+{
+ return fdt_path_offset_namelen(fdt, path, strlen(path));
+}
+
const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
{
const struct fdt_node_header *nh = _fdt_offset_ptr(fdt, nodeoffset);
diff --git a/xen/common/libfdt/fdt_rw.c b/xen/common/libfdt/fdt_rw.c
index 8b8cd25..fc53644 100644
--- a/xen/common/libfdt/fdt_rw.c
+++ b/xen/common/libfdt/fdt_rw.c
@@ -65,6 +65,30 @@ static int _fdt_blocks_misordered(const void *fdt,
(fdt_off_dt_strings(fdt) + fdt_size_dt_strings(fdt)));
}
+static int fdt_rw_probe_(void *fdt)
+{
+ if (can_assume(VALID_DTB))
+ return 0;
+ FDT_RO_PROBE(fdt);
+
+ if (!can_assume(LATEST) && fdt_version(fdt) < 17)
+ return -FDT_ERR_BADVERSION;
+ if (_fdt_blocks_misordered(fdt, sizeof(struct fdt_reserve_entry),
+ fdt_size_dt_struct(fdt)))
+ return -FDT_ERR_BADLAYOUT;
+ if (!can_assume(LATEST) && fdt_version(fdt) > 17)
+ fdt_set_version(fdt, 17);
+
+ return 0;
+}
+
+#define FDT_RW_PROBE(fdt) \
+ { \
+ int err_; \
+ if ((err_ = fdt_rw_probe_(fdt)) != 0) \
+ return err_; \
+ }
+
static int _fdt_rw_check_header(void *fdt)
{
FDT_CHECK_HEADER(fdt);
@@ -133,6 +157,14 @@ static int _fdt_splice_struct(void *fdt, void *p,
return 0;
}
+/* Must only be used to roll back in case of error */
+static void fdt_del_last_string_(void *fdt, const char *s)
+{
+ int newlen = strlen(s) + 1;
+
+ fdt_set_size_dt_strings(fdt, fdt_size_dt_strings(fdt) - newlen);
+}
+
static int _fdt_splice_string(void *fdt, int newlen)
{
void *p = (char *)fdt
@@ -146,7 +178,16 @@ static int _fdt_splice_string(void *fdt, int newlen)
return 0;
}
-static int _fdt_find_add_string(void *fdt, const char *s)
+/**
+ * New _fdt_find_add_string() - Find or allocate a string
+ *
+ * @fdt: pointer to the device tree to check/adjust
+ * @s: string to find/add
+ * @allocated: Set to 0 if the string was found, 1 if not found and so
+ * allocated. Ignored if can_assume(NO_ROLLBACK)
+ * @return offset of string in the string table (whether found or added)
+ */
+static int _fdt_find_add_string(void *fdt, const char *s, int *allocated)
{
char *strtab = (char *)fdt + fdt_off_dt_strings(fdt);
const char *p;
@@ -154,6 +195,9 @@ static int _fdt_find_add_string(void *fdt, const char *s)
int len = strlen(s) + 1;
int err;
+ if (!can_assume(NO_ROLLBACK))
+ *allocated = 0;
+
p = _fdt_find_string(strtab, fdt_size_dt_strings(fdt), s);
if (p)
/* found it */
@@ -164,6 +208,9 @@ static int _fdt_find_add_string(void *fdt, const char *s)
if (err)
return err;
+ if (!can_assume(NO_ROLLBACK))
+ *allocated = 1;
+
memcpy(new, s, len);
return (new - strtab);
}
@@ -226,11 +273,12 @@ static int _fdt_add_property(void *fdt, int nodeoffset, const char *name,
int nextoffset;
int namestroff;
int err;
+ int allocated;
if ((nextoffset = _fdt_check_node_offset(fdt, nodeoffset)) < 0)
return nextoffset;
- namestroff = _fdt_find_add_string(fdt, name);
+ namestroff = _fdt_find_add_string(fdt, name, &allocated);
if (namestroff < 0)
return namestroff;
@@ -238,8 +286,12 @@ static int _fdt_add_property(void *fdt, int nodeoffset, const char *name,
proplen = sizeof(**prop) + FDT_TAGALIGN(len);
err = _fdt_splice_struct(fdt, *prop, 0, proplen);
- if (err)
+ if (err) {
+ /* Delete the string if we failed to add it */
+ if (!can_assume(NO_ROLLBACK) && allocated)
+ fdt_del_last_string_(fdt, name);
return err;
+ }
(*prop)->tag = cpu_to_fdt32(FDT_PROP);
(*prop)->nameoff = cpu_to_fdt32(namestroff);
@@ -270,13 +322,13 @@ int fdt_set_name(void *fdt, int nodeoffset, const char *name)
return 0;
}
-int fdt_setprop(void *fdt, int nodeoffset, const char *name,
- const void *val, int len)
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+ int len, void **prop_data)
{
struct fdt_property *prop;
int err;
- FDT_RW_CHECK_HEADER(fdt);
+ FDT_RW_PROBE(fdt);
err = _fdt_resize_property(fdt, nodeoffset, name, len, &prop);
if (err == -FDT_ERR_NOTFOUND)
@@ -284,7 +336,22 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
if (err)
return err;
- memcpy(prop->data, val, len);
+ *prop_data = prop->data;
+ return 0;
+}
+
+int fdt_setprop(void *fdt, int nodeoffset, const char *name,
+ const void *val, int len)
+{
+ void *prop_data;
+ int err;
+
+ err = fdt_setprop_placeholder(fdt, nodeoffset, name, len, &prop_data);
+ if (err)
+ return err;
+
+ if (len)
+ memcpy(prop_data, val, len);
return 0;
}
diff --git a/xen/common/libfdt/fdt_wip.c b/xen/common/libfdt/fdt_wip.c
index 2d1cac0..3f61085 100644
--- a/xen/common/libfdt/fdt_wip.c
+++ b/xen/common/libfdt/fdt_wip.c
@@ -53,6 +53,26 @@
#include "libfdt_internal.h"
+int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
+ const char *name, int namelen,
+ uint32_t idx, const void *val,
+ int len)
+{
+ void *propval;
+ int proplen;
+
+ propval = fdt_getprop_namelen_w(fdt, nodeoffset, name, namelen,
+ &proplen);
+ if (!propval)
+ return proplen;
+
+ if ((unsigned)proplen < (len + idx))
+ return -FDT_ERR_NOSPACE;
+
+ memcpy((char *)propval + idx, val, len);
+ return 0;
+}
+
int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
const void *val, int len)
{
diff --git a/xen/common/libfdt/libfdt_internal.h b/xen/common/libfdt/libfdt_internal.h
index d50c4e1..523bcee 100644
--- a/xen/common/libfdt/libfdt_internal.h
+++ b/xen/common/libfdt/libfdt_internal.h
@@ -60,6 +60,14 @@
return err; \
}
+int32_t fdt_ro_probe_(const void *fdt);
+#define FDT_RO_PROBE(fdt) \
+ { \
+ int32_t totalsize_; \
+ if ((totalsize_ = fdt_ro_probe_(fdt)) < 0) \
+ return totalsize_; \
+ }
+
int _fdt_check_node_offset(const void *fdt, int offset);
int _fdt_check_prop_offset(const void *fdt, int offset);
const char *_fdt_find_string(const char *strtab, int tabsize, const char *s);
@@ -90,4 +98,126 @@ static inline struct fdt_reserve_entry *_fdt_mem_rsv_w(void *fdt, int n)
#define FDT_SW_MAGIC (~FDT_MAGIC)
+/**********************************************************************/
+/* Checking controls */
+/**********************************************************************/
+
+#ifndef FDT_ASSUME_MASK
+#define FDT_ASSUME_MASK 0
+#endif
+
+/*
+ * Defines assumptions which can be enabled. Each of these can be enabled
+ * individually. For maximum safety, don't enable any assumptions!
+ *
+ * For minimal code size and no safety, use ASSUME_PERFECT at your own risk.
+ * You should have another method of validating the device tree, such as a
+ * signature or hash check before using libfdt.
+ *
+ * For situations where security is not a concern it may be safe to enable
+ * ASSUME_SANE.
+ */
+enum {
+ /*
+ * This does essentially no checks. Only the latest device-tree
+ * version is correctly handled. Inconsistencies or errors in the device
+ * tree may cause undefined behaviour or crashes. Invalid parameters
+ * passed to libfdt may do the same.
+ *
+ * If an error occurs when modifying the tree it may leave the tree in
+ * an intermediate (but valid) state. As an example, adding a property
+ * where there is insufficient space may result in the property name
+ * being added to the string table even though the property itself is
+ * not added to the struct section.
+ *
+ * Only use this if you have a fully validated device tree with
+ * the latest supported version and wish to minimise code size.
+ */
+ ASSUME_PERFECT = 0xff,
+
+ /*
+ * This assumes that the device tree is sane. i.e. header metadata
+ * and basic hierarchy are correct.
+ *
+ * With this assumption enabled, normal device trees produced by libfdt
+ * and the compiler should be handled safely. Malicious device trees and
+ * complete garbage may cause libfdt to behave badly or crash. Truncated
+ * device trees (e.g. those only partially loaded) can also cause
+ * problems.
+ *
+ * Note: Only checks that relate exclusively to the device tree itself
+ * (not the parameters passed to libfdt) are disabled by this
+ * assumption. This includes checking headers, tags and the like.
+ */
+ ASSUME_VALID_DTB = 1 << 0,
+
+ /*
+ * This builds on ASSUME_VALID_DTB and further assumes that libfdt
+ * functions are called with valid parameters, i.e. not trigger
+ * FDT_ERR_BADOFFSET or offsets that are out of bounds. It disables any
+ * extensive checking of parameters and the device tree, making various
+ * assumptions about correctness.
+ *
+ * It doesn't make sense to enable this assumption unless
+ * ASSUME_VALID_DTB is also enabled.
+ */
+ ASSUME_VALID_INPUT = 1 << 1,
+
+ /*
+ * This disables checks for device-tree version and removes all code
+ * which handles older versions.
+ *
+ * Only enable this if you know you have a device tree with the latest
+ * version.
+ */
+ ASSUME_LATEST = 1 << 2,
+
+ /*
+ * This assumes that it is OK for a failed addition to the device tree,
+ * due to lack of space or some other problem, to skip any rollback
+ * steps (such as dropping the property name from the string table).
+ * This is safe to enable in most circumstances, even though it may
+ * leave the tree in a sub-optimal state.
+ */
+ ASSUME_NO_ROLLBACK = 1 << 3,
+
+ /*
+ * This assumes that the device tree components appear in a 'convenient'
+ * order, i.e. the memory reservation block first, then the structure
+ * block and finally the string block.
+ *
+ * This order is not specified by the device-tree specification,
+ * but is expected by libfdt. The device-tree compiler always created
+ * device trees with this order.
+ *
+ * This assumption disables a check in fdt_open_into() and removes the
+ * ability to fix the problem there. This is safe if you know that the
+ * device tree is correctly ordered. See fdt_blocks_misordered_().
+ */
+ ASSUME_LIBFDT_ORDER = 1 << 4,
+
+ /*
+ * This assumes that libfdt itself does not have any internal bugs. It
+ * drops certain checks that should never be needed unless libfdt has an
+ * undiscovered bug.
+ *
+ * This can generally be considered safe to enable.
+ */
+ ASSUME_LIBFDT_FLAWLESS = 1 << 5,
+};
+
+/**
+ * can_assume_() - check if a particular assumption is enabled
+ *
+ * @mask: Mask to check (ASSUME_...)
+ * @return true if that assumption is enabled, else false
+ */
+static inline bool can_assume_(int mask)
+{
+ return FDT_ASSUME_MASK & mask;
+}
+
+/** helper macros for checking assumptions */
+#define can_assume(_assume) can_assume_(ASSUME_ ## _assume)
+
#endif /* _LIBFDT_INTERNAL_H */
diff --git a/xen/include/xen/libfdt/libfdt.h b/xen/include/xen/libfdt/libfdt.h
index 7c75688..b6c8b67 100644
--- a/xen/include/xen/libfdt/libfdt.h
+++ b/xen/include/xen/libfdt/libfdt.h
@@ -114,7 +114,30 @@
* Should never be returned, if it is, it indicates a bug in
* libfdt itself. */
-#define FDT_ERR_MAX 13
+/* Errors in device tree content */
+#define FDT_ERR_BADNCELLS 14
+ /* FDT_ERR_BADNCELLS: Device tree has a #address-cells, #size-cells
+ * or similar property with a bad format or value */
+
+#define FDT_ERR_BADVALUE 15
+ /* FDT_ERR_BADVALUE: Device tree has a property with an unexpected
+ * value. For example: a property expected to contain a string list
+ * is not NUL-terminated within the length of its value. */
+
+#define FDT_ERR_BADOVERLAY 16
+ /* FDT_ERR_BADOVERLAY: The device tree overlay, while
+ * correctly structured, cannot be applied due to some
+ * unexpected or missing value, property or node. */
+
+#define FDT_ERR_NOPHANDLES 17
+ /* FDT_ERR_NOPHANDLES: The device tree doesn't have any
+ * phandle available anymore without causing an overflow */
+
+#define FDT_ERR_BADFLAGS 18
+ /* FDT_ERR_BADFLAGS: The function was passed a flags field that
+ * contains invalid flags or an invalid combination of flags. */
+
+#define FDT_ERR_MAX 18
/**********************************************************************/
/* Low-level functions (you probably don't need these) */
@@ -156,6 +179,33 @@ int fdt_first_subnode(const void *fdt, int offset);
*/
int fdt_next_subnode(const void *fdt, int offset);
+/**
+ * fdt_for_each_subnode - iterate over all subnodes of a parent
+ *
+ * @node: child node (int, lvalue)
+ * @fdt: FDT blob (const void *)
+ * @parent: parent node (int)
+ *
+ * This is actually a wrapper around a for loop and would be used like so:
+ *
+ * fdt_for_each_subnode(node, fdt, parent) {
+ * Use node
+ * ...
+ * }
+ *
+ * if ((node < 0) && (node != -FDT_ERR_NOTFOUND)) {
+ * Error handling
+ * }
+ *
+ * Note that this is implemented as a macro and @node is used as
+ * iterator in the loop. The parent variable be constant or even a
+ * literal.
+ */
+#define fdt_for_each_subnode(node, fdt, parent) \
+ for (node = fdt_first_subnode(fdt, parent); \
+ node >= 0; \
+ node = fdt_next_subnode(fdt, node))
+
/**********************************************************************/
/* General functions */
/**********************************************************************/
@@ -247,6 +297,47 @@ int fdt_move(const void *fdt, void *buf, int bufsize);
const char *fdt_string(const void *fdt, int stroffset);
/**
+ * fdt_find_max_phandle - find and return the highest phandle in a tree
+ * @fdt: pointer to the device tree blob
+ * @phandle: return location for the highest phandle value found in the tree
+ *
+ * fdt_find_max_phandle() finds the highest phandle value in the given device
+ * tree. The value returned in @phandle is only valid if the function returns
+ * success.
+ *
+ * returns:
+ * 0 on success or a negative error code on failure
+ */
+int fdt_find_max_phandle(const void *fdt, uint32_t *phandle);
+
+/**
+ * fdt_get_max_phandle - retrieves the highest phandle in a tree
+ * @fdt: pointer to the device tree blob
+ *
+ * fdt_get_max_phandle retrieves the highest phandle in the given
+ * device tree. This will ignore badly formatted phandles, or phandles
+ * with a value of 0 or -1.
+ *
+ * This function is deprecated in favour of fdt_find_max_phandle().
+ *
+ * returns:
+ * the highest phandle on success
+ * 0, if no phandle was found in the device tree
+ * -1, if an error occurred
+ */
+static inline uint32_t fdt_get_max_phandle(const void *fdt)
+{
+ uint32_t phandle;
+ int err;
+
+ err = fdt_find_max_phandle(fdt, &phandle);
+ if (err < 0)
+ return (uint32_t)-1;
+
+ return phandle;
+}
+
+/**
* fdt_num_mem_rsv - retrieve the number of memory reserve map entries
* @fdt: pointer to the device tree blob
*
@@ -316,6 +407,21 @@ int fdt_subnode_offset_namelen(const void *fdt, int parentoffset,
int fdt_subnode_offset(const void *fdt, int parentoffset, const char *name);
/**
+ * fdt_path_offset_namelen - find a tree node by its full path
+ * @fdt: pointer to the device tree blob
+ * @path: full path of the node to locate
+ * @namelen: number of characters of path to consider
+ *
+ * Identical to fdt_path_offset(), but only consider the first namelen
+ * characters of path as the path name.
+ *
+ * Return: offset of the node or negative libfdt error value otherwise
+ */
+#ifndef SWIG /* Not available in Python */
+int fdt_path_offset_namelen(const void *fdt, const char *path, int namelen);
+#endif
+
+/**
* fdt_path_offset - find a tree node by its full path
* @fdt: pointer to the device tree blob
* @path: full path of the node to locate
@@ -404,6 +510,33 @@ int fdt_first_property_offset(const void *fdt, int nodeoffset);
int fdt_next_property_offset(const void *fdt, int offset);
/**
+ * fdt_for_each_property_offset - iterate over all properties of a node
+ *
+ * @property: property offset (int, lvalue)
+ * @fdt: FDT blob (const void *)
+ * @node: node offset (int)
+ *
+ * This is actually a wrapper around a for loop and would be used like so:
+ *
+ * fdt_for_each_property_offset(property, fdt, node) {
+ * Use property
+ * ...
+ * }
+ *
+ * if ((property < 0) && (property != -FDT_ERR_NOTFOUND)) {
+ * Error handling
+ * }
+ *
+ * Note that this is implemented as a macro and property is used as
+ * iterator in the loop. The node variable can be constant or even a
+ * literal.
+ */
+#define fdt_for_each_property_offset(property, fdt, node) \
+ for (property = fdt_first_property_offset(fdt, node); \
+ property >= 0; \
+ property = fdt_next_property_offset(fdt, property))
+
+/**
* fdt_get_property_by_offset - retrieve the property at a given offset
* @fdt: pointer to the device tree blob
* @offset: offset of the property to retrieve
@@ -532,6 +665,14 @@ const void *fdt_getprop_by_offset(const void *fdt, int offset,
const void *fdt_getprop_namelen(const void *fdt, int nodeoffset,
const char *name, int namelen, int *lenp);
+static inline void *fdt_getprop_namelen_w(void *fdt, int nodeoffset,
+ const char *name, int namelen,
+ int *lenp)
+{
+ return (void *)(uintptr_t)fdt_getprop_namelen(fdt, nodeoffset, name,
+ namelen, lenp);
+}
+
/**
* fdt_getprop - retrieve the value of a given property
* @fdt: pointer to the device tree blob
@@ -993,6 +1134,31 @@ static inline int fdt_setprop_inplace_cell(void *fdt, int nodeoffset,
int fdt_nop_property(void *fdt, int nodeoffset, const char *name);
/**
+ * fdt_setprop_inplace_namelen_partial - change a property's value,
+ * but not its size
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @namelen: number of characters of name to consider
+ * @idx: index of the property to change in the array
+ * @val: pointer to data to replace the property value with
+ * @len: length of the property value
+ *
+ * Identical to fdt_setprop_inplace(), but modifies the given property
+ * starting from the given index, and using only the first characters
+ * of the name. It is useful when you want to manipulate only one value of
+ * an array and you have a string that doesn't end with \0.
+ *
+ * Return: 0 on success, negative libfdt error value otherwise
+ */
+#ifndef SWIG /* Not available in Python */
+int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
+ const char *name, int namelen,
+ uint32_t idx, const void *val,
+ int len);
+#endif
+
+/**
* fdt_nop_node - replace a node (subtree) with nop tags
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node to nop
@@ -1158,6 +1324,37 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
const void *val, int len);
/**
+ * fdt_setprop_placeholder - allocate space for a property
+ * @fdt: pointer to the device tree blob
+ * @nodeoffset: offset of the node whose property to change
+ * @name: name of the property to change
+ * @len: length of the property value
+ * @prop_data: return pointer to property data
+ *
+ * fdt_setprop_placeholer() allocates the named property in the given node.
+ * If the property exists it is resized. In either case a pointer to the
+ * property data is returned.
+ *
+ * This function may insert or delete data from the blob, and will
+ * therefore change the offsets of some existing nodes.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there is insufficient free space in the blob to
+ * contain the new property value
+ * -FDT_ERR_BADOFFSET, nodeoffset did not point to FDT_BEGIN_NODE tag
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_setprop_placeholder(void *fdt, int nodeoffset, const char *name,
+ int len, void **prop_data);
+
+/**
* fdt_setprop_u32 - set a property to a 32-bit integer
* @fdt: pointer to the device tree blob
* @nodeoffset: offset of the node whose property to change
@@ -1503,6 +1700,37 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name);
*/
int fdt_del_node(void *fdt, int nodeoffset);
+/**
+ * fdt_overlay_apply - Applies a DT overlay on a base DT
+ * @fdt: pointer to the base device tree blob
+ * @fdto: pointer to the device tree overlay blob
+ *
+ * fdt_overlay_apply() will apply the given device tree overlay on the
+ * given base device tree.
+ *
+ * Expect the base device tree to be modified, even if the function
+ * returns an error.
+ *
+ * returns:
+ * 0, on success
+ * -FDT_ERR_NOSPACE, there's not enough space in the base device tree
+ * -FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
+ * properties in the base DT
+ * -FDT_ERR_BADPHANDLE,
+ * -FDT_ERR_BADOVERLAY,
+ * -FDT_ERR_NOPHANDLES,
+ * -FDT_ERR_INTERNAL,
+ * -FDT_ERR_BADLAYOUT,
+ * -FDT_ERR_BADMAGIC,
+ * -FDT_ERR_BADOFFSET,
+ * -FDT_ERR_BADPATH,
+ * -FDT_ERR_BADVERSION,
+ * -FDT_ERR_BADSTRUCTURE,
+ * -FDT_ERR_BADSTATE,
+ * -FDT_ERR_TRUNCATED, standard meanings
+ */
+int fdt_overlay_apply(void *fdt, void *fdto);
+
/**********************************************************************/
/* Debugging / informational functions */
/**********************************************************************/
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [XEN][RFC PATCH 05/13] libfdt: Change overlay_get_target() type
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (3 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 04/13] libfdt: Copy required libfdt functions " Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-20 18:09 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 06/13] device tree: Add dt_print_node_names() Vikram Garhwal
` (7 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
Remove static function type from overlay_get_target().
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/libfdt/fdt_overlay.c | 2 +-
xen/include/xen/libfdt/libfdt.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/common/libfdt/fdt_overlay.c b/xen/common/libfdt/fdt_overlay.c
index 15a8cdb..1ffb669 100644
--- a/xen/common/libfdt/fdt_overlay.c
+++ b/xen/common/libfdt/fdt_overlay.c
@@ -56,7 +56,7 @@ static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
* the targeted node offset in the base device tree
* Negative error code on error
*/
-static int overlay_get_target(const void *fdt, const void *fdto,
+int overlay_get_target(const void *fdt, const void *fdto,
int fragment, char const **pathp)
{
uint32_t phandle;
diff --git a/xen/include/xen/libfdt/libfdt.h b/xen/include/xen/libfdt/libfdt.h
index b6c8b67..e1cc6f2 100644
--- a/xen/include/xen/libfdt/libfdt.h
+++ b/xen/include/xen/libfdt/libfdt.h
@@ -1737,4 +1737,6 @@ int fdt_overlay_apply(void *fdt, void *fdto);
const char *fdt_strerror(int errval);
+int overlay_get_target(const void *fdt, const void *fdto, int fragment,
+ char const **pathp);
#endif /* _LIBFDT_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 05/13] libfdt: Change overlay_get_target() type
2021-09-02 6:05 ` [XEN][RFC PATCH 05/13] libfdt: Change overlay_get_target() type Vikram Garhwal
@ 2021-10-20 18:09 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-20 18:09 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel; +Cc: sstabellini
Hi Vikram
On 02/09/2021 07:05, Vikram Garhwal wrote:
> Remove static function type from overlay_get_target().
Please explain why this is necessary. But if we really need then...
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/common/libfdt/fdt_overlay.c | 2 +-
> xen/include/xen/libfdt/libfdt.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/libfdt/fdt_overlay.c b/xen/common/libfdt/fdt_overlay.c
> index 15a8cdb..1ffb669 100644
> --- a/xen/common/libfdt/fdt_overlay.c
> +++ b/xen/common/libfdt/fdt_overlay.c
> @@ -56,7 +56,7 @@ static uint32_t overlay_get_target_phandle(const void *fdto, int fragment)
> * the targeted node offset in the base device tree
> * Negative error code on error
> */
> -static int overlay_get_target(const void *fdt, const void *fdto,
> +int overlay_get_target(const void *fdt, const void *fdto,
> int fragment, char const **pathp)
... the function should be prefixed to fdt_*.
I would also like to avoid diverging from what the original source. So
can this please be sent to the libfdt community first?
> {
> uint32_t phandle;
> diff --git a/xen/include/xen/libfdt/libfdt.h b/xen/include/xen/libfdt/libfdt.h
> index b6c8b67..e1cc6f2 100644
> --- a/xen/include/xen/libfdt/libfdt.h
> +++ b/xen/include/xen/libfdt/libfdt.h
> @@ -1737,4 +1737,6 @@ int fdt_overlay_apply(void *fdt, void *fdto);
>
> const char *fdt_strerror(int errval);
>
> +int overlay_get_target(const void *fdt, const void *fdto, int fragment,
> + char const **pathp);
> #endif /* _LIBFDT_H */
>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 06/13] device tree: Add dt_print_node_names()
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (4 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 05/13] libfdt: Change overlay_get_target() type Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-21 9:04 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 07/13] device tree: Add _dt_find_node_by_path() to find nodes in device tree Vikram Garhwal
` (6 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
Add dt_print_node_names() to print all nodes under a dt_device_node.
dt_print_node_names() takes a dt_device_node type input and prints the node name
of all the subsequent nodes. This is added for debugging purpose for device tree
overlays.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/device_tree.c | 10 ++++++++++
xen/include/xen/device_tree.h | 5 +++++
2 files changed, 15 insertions(+)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index cda21be..bfe3191 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -308,6 +308,16 @@ struct dt_device_node *dt_find_node_by_path(const char *path)
return np;
}
+void dt_print_node_names(struct dt_device_node *dt)
+{
+ struct dt_device_node *np;
+
+ dt_for_each_device_node(dt, np)
+ dt_dprintk("Node name: %s Full name %s\n", np->name, np->full_name);
+
+ return;
+}
+
int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
struct dt_device_node **node)
{
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index a4e98a7..dcd96b4 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -483,6 +483,11 @@ struct dt_device_node *dt_find_node_by_path(const char *path);
int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
struct dt_device_node **node);
+/*
+ * Prints all node names.
+ */
+void dt_print_node_names(struct dt_device_node *dt);
+
/**
* dt_get_parent - Get a node's parent if any
* @node: Node to get parent
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 06/13] device tree: Add dt_print_node_names()
2021-09-02 6:05 ` [XEN][RFC PATCH 06/13] device tree: Add dt_print_node_names() Vikram Garhwal
@ 2021-10-21 9:04 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-21 9:04 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel; +Cc: sstabellini
Hi Vikram,
On 02/09/2021 07:05, Vikram Garhwal wrote:
> Add dt_print_node_names() to print all nodes under a dt_device_node.
> dt_print_node_names() takes a dt_device_node type input and prints the node name
> of all the subsequent nodes. This is added for debugging purpose for device tree
> overlays.
I can't find any use of this function in the rest of series. While I
understand it was helpful for you, I am not entirely convinced that we
should add it in Xen with no-use. The code is simple enough for anyone
to re-implement it if needed.
Cheers,
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/common/device_tree.c | 10 ++++++++++
> xen/include/xen/device_tree.h | 5 +++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index cda21be..bfe3191 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -308,6 +308,16 @@ struct dt_device_node *dt_find_node_by_path(const char *path)
> return np;
> }
>
> +void dt_print_node_names(struct dt_device_node *dt)
> +{
> + struct dt_device_node *np;
> +
> + dt_for_each_device_node(dt, np)
> + dt_dprintk("Node name: %s Full name %s\n", np->name, np->full_name);
> +
> + return;
> +}
> +
> int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
> struct dt_device_node **node)
> {
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index a4e98a7..dcd96b4 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -483,6 +483,11 @@ struct dt_device_node *dt_find_node_by_path(const char *path);
> int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
> struct dt_device_node **node);
>
> +/*
> + * Prints all node names.
> + */
> +void dt_print_node_names(struct dt_device_node *dt);
> +
> /**
> * dt_get_parent - Get a node's parent if any
> * @node: Node to get parent
>
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 07/13] device tree: Add _dt_find_node_by_path() to find nodes in device tree
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (5 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 06/13] device tree: Add dt_print_node_names() Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-09-02 6:05 ` [XEN][RFC PATCH 08/13] xen/iommu: Introduce iommu_remove_dt_devices function Vikram Garhwal
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, julien, Vikram Garhwal
Add _dt_find_by_path() to find a matching node with path for a dt_device_node.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/common/device_tree.c | 10 ++++++++--
xen/include/xen/device_tree.h | 9 +++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index bfe3191..4946e83 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -297,17 +297,23 @@ struct dt_device_node *dt_find_node_by_type(struct dt_device_node *from,
return np;
}
-struct dt_device_node *dt_find_node_by_path(const char *path)
+struct dt_device_node *_dt_find_node_by_path(struct dt_device_node *dt,
+ const char *path)
{
struct dt_device_node *np;
- dt_for_each_device_node(dt_host, np)
+ dt_for_each_device_node(dt, np)
if ( np->full_name && (dt_node_cmp(np->full_name, path) == 0) )
break;
return np;
}
+struct dt_device_node *dt_find_node_by_path(const char *path)
+{
+ return _dt_find_node_by_path(dt_host, path);
+}
+
void dt_print_node_names(struct dt_device_node *dt)
{
struct dt_device_node *np;
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index dcd96b4..7cc6093 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -469,6 +469,15 @@ struct dt_device_node *dt_find_node_by_alias(const char *alias);
*/
struct dt_device_node *dt_find_node_by_path(const char *path);
+/**
+ * _dt_find_node_by_path - Find a node matching a full DT path
+ * @dt_node: The device tree to search
+ * @path: The full path to match
+ *
+ * Returns a node pointer.
+ */
+struct dt_device_node *_dt_find_node_by_path(struct dt_device_node *dt,
+ const char *path);
/**
* dt_find_node_by_gpath - Same as dt_find_node_by_path but retrieve the
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [XEN][RFC PATCH 08/13] xen/iommu: Introduce iommu_remove_dt_devices function
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (6 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 07/13] device tree: Add _dt_find_node_by_path() to find nodes in device tree Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-10-21 9:34 ` Julien Grall
2021-09-02 6:05 ` [XEN][RFC PATCH 09/13] xen/arm: Implement device tree node removal functionalities Vikram Garhwal
` (4 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Volodymyr Babchuk,
Jan Beulich, Paul Durrant
iommu_remove_dt_device function is introduced for supporting dynamic programming
i.e. adding and removing a node during runtime. When user removes the device
node, iommu_remove_dt_device() removes the device entry from smmu-masters too
using following steps:
1. Find if SMMU master exists for the device node.
2. Remove the SMMU master.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/drivers/passthrough/arm/smmu.c | 53 +++++++++++++++++++++++++++++++++++
xen/drivers/passthrough/device_tree.c | 30 ++++++++++++++++++++
xen/include/xen/iommu.h | 2 ++
3 files changed, 85 insertions(+)
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index c9dfc4c..7b615bc 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -816,6 +816,17 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
return 0;
}
+static int remove_smmu_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master *master)
+{
+ if (!(smmu->masters.rb_node))
+ return -ENOENT;
+
+ rb_erase(&master->node, &smmu->masters);
+
+ return 0;
+}
+
static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu,
struct device *dev,
struct iommu_fwspec *fwspec)
@@ -853,6 +864,31 @@ static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu,
return insert_smmu_master(smmu, master);
}
+static int arm_smmu_dt_remove_device_legacy(struct arm_smmu_device *smmu,
+ struct device *dev)
+{
+ struct arm_smmu_master *master;
+ struct device_node *dev_node = dev_get_dev_node(dev);
+ int ret;
+
+ master = find_smmu_master(smmu, dev_node);
+ if (master == NULL) {
+ dev_err(dev,
+ "No registrations found for master device %s\n",
+ dev_node->name);
+ return -EINVAL;
+ }
+
+ ret = remove_smmu_master(smmu, master);
+
+ if (ret)
+ return ret;
+
+ master->of_node = NULL;
+ kfree(master);
+ return 0;
+}
+
static int register_smmu_master(struct arm_smmu_device *smmu,
struct device *dev,
struct of_phandle_args *masterspec)
@@ -876,6 +912,22 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
fwspec);
}
+static int arm_smmu_dt_remove_device_generic(u8 devfn, struct device *dev)
+{
+ struct arm_smmu_device *smmu;
+ struct iommu_fwspec *fwspec;
+
+ fwspec = dev_iommu_fwspec_get(dev);
+ if (fwspec == NULL)
+ return -ENXIO;
+
+ smmu = find_smmu(fwspec->iommu_dev);
+ if (smmu == NULL)
+ return -ENXIO;
+
+ return arm_smmu_dt_remove_device_legacy(smmu, dev);
+}
+
static int arm_smmu_dt_add_device_generic(u8 devfn, struct device *dev)
{
struct arm_smmu_device *smmu;
@@ -2876,6 +2928,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
.init = arm_smmu_iommu_domain_init,
.hwdom_init = arm_smmu_iommu_hwdom_init,
.add_device = arm_smmu_dt_add_device_generic,
+ .remove_device = arm_smmu_dt_remove_device_generic,
.teardown = arm_smmu_iommu_domain_teardown,
.iotlb_flush = arm_smmu_iotlb_flush,
.iotlb_flush_all = arm_smmu_iotlb_flush_all,
diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
index 98f2aa0..37f4945 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -127,6 +127,36 @@ int iommu_release_dt_devices(struct domain *d)
return 0;
}
+int iommu_remove_dt_device(struct dt_device_node *np)
+{
+ const struct iommu_ops *ops = iommu_get_ops();
+ struct device *dev = dt_to_dev(np);
+ int rc = 1;
+
+ if ( !ops )
+ return -EINVAL;
+
+ if ( iommu_dt_device_is_assigned(np) )
+ return -EPERM;
+
+ /*
+ * The driver which supports generic IOMMU DT bindings must have
+ * these callback implemented.
+ */
+ if ( !ops->remove_device )
+ return -EINVAL;
+
+ /*
+ * Remove master device from the IOMMU if latter is present and available.
+ */
+ rc = ops->remove_device(0, dev);
+
+ if ( rc == 0 )
+ iommu_fwspec_free(dev);
+
+ return rc;
+}
+
int iommu_add_dt_device(struct dt_device_node *np)
{
const struct iommu_ops *ops = iommu_get_ops();
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 6b2cdff..c4d5d12 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -215,6 +215,8 @@ int iommu_release_dt_devices(struct domain *d);
*/
int iommu_add_dt_device(struct dt_device_node *np);
+int iommu_remove_dt_device(struct dt_device_node *np);
+
int iommu_do_dt_domctl(struct xen_domctl *, struct domain *,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 08/13] xen/iommu: Introduce iommu_remove_dt_devices function
2021-09-02 6:05 ` [XEN][RFC PATCH 08/13] xen/iommu: Introduce iommu_remove_dt_devices function Vikram Garhwal
@ 2021-10-21 9:34 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-21 9:34 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel
Cc: sstabellini, Volodymyr Babchuk, Jan Beulich, Paul Durrant
Hi Vikram,
On 02/09/2021 07:05, Vikram Garhwal wrote:
> iommu_remove_dt_device function is introduced for supporting dynamic programming
> i.e. adding and removing a node during runtime. When user removes the device
> node, iommu_remove_dt_device() removes the device entry from smmu-masters too
> using following steps:
> 1. Find if SMMU master exists for the device node.
> 2. Remove the SMMU master.
I would prefer if this patch is split in two:
* Part 1: Add the generic helper
* Part 2: Implement the callback for the SMMU
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/drivers/passthrough/arm/smmu.c | 53 +++++++++++++++++++++++++++++++++++
> xen/drivers/passthrough/device_tree.c | 30 ++++++++++++++++++++
> xen/include/xen/iommu.h | 2 ++
> 3 files changed, 85 insertions(+)
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
> index c9dfc4c..7b615bc 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c
> @@ -816,6 +816,17 @@ static int insert_smmu_master(struct arm_smmu_device *smmu,
> return 0;
> }
>
> +static int remove_smmu_master(struct arm_smmu_device *smmu,
> + struct arm_smmu_master *master)
> +{
> + if (!(smmu->masters.rb_node))
> + return -ENOENT;
> +
> + rb_erase(&master->node, &smmu->masters);
> +
> + return 0;
> +}
> +
> static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu,
> struct device *dev,
> struct iommu_fwspec *fwspec)
> @@ -853,6 +864,31 @@ static int arm_smmu_dt_add_device_legacy(struct arm_smmu_device *smmu,
> return insert_smmu_master(smmu, master);
> }
>
> +static int arm_smmu_dt_remove_device_legacy(struct arm_smmu_device *smmu,
> + struct device *dev)
> +{
> + struct arm_smmu_master *master;
> + struct device_node *dev_node = dev_get_dev_node(dev);
> + int ret;
> +
> + master = find_smmu_master(smmu, dev_node);
> + if (master == NULL) {
> + dev_err(dev,
> + "No registrations found for master device %s\n",
> + dev_node->name);
> + return -EINVAL;
> + }
> +
> + ret = remove_smmu_master(smmu, master);
Can you add a comment why you are not clearing the dev_node->is_protected?
> +
> + if (ret)
> + return ret;
> +
> + master->of_node = NULL;
NIT: This is a bit pointless to do given that you are freeing master
right after.
> + kfree(master);
> + return 0;
> +}
> +
> static int register_smmu_master(struct arm_smmu_device *smmu,
> struct device *dev,
> struct of_phandle_args *masterspec)
> @@ -876,6 +912,22 @@ static int register_smmu_master(struct arm_smmu_device *smmu,
> fwspec);
> }
>
> +static int arm_smmu_dt_remove_device_generic(u8 devfn, struct device *dev)
> +{
> + struct arm_smmu_device *smmu;
> + struct iommu_fwspec *fwspec;
> +
> + fwspec = dev_iommu_fwspec_get(dev);
> + if (fwspec == NULL)
> + return -ENXIO;
> +
> + smmu = find_smmu(fwspec->iommu_dev);
> + if (smmu == NULL)
> + return -ENXIO;
> +
> + return arm_smmu_dt_remove_device_legacy(smmu, dev);
> +}
> +
> static int arm_smmu_dt_add_device_generic(u8 devfn, struct device *dev)
> {
> struct arm_smmu_device *smmu;
> @@ -2876,6 +2928,7 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
> .init = arm_smmu_iommu_domain_init,
> .hwdom_init = arm_smmu_iommu_hwdom_init,
> .add_device = arm_smmu_dt_add_device_generic,
> + .remove_device = arm_smmu_dt_remove_device_generic,
> .teardown = arm_smmu_iommu_domain_teardown,
> .iotlb_flush = arm_smmu_iotlb_flush,
> .iotlb_flush_all = arm_smmu_iotlb_flush_all,
> diff --git a/xen/drivers/passthrough/device_tree.c b/xen/drivers/passthrough/device_tree.c
> index 98f2aa0..37f4945 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -127,6 +127,36 @@ int iommu_release_dt_devices(struct domain *d)
> return 0;
> }
>
> +int iommu_remove_dt_device(struct dt_device_node *np)
> +{
> + const struct iommu_ops *ops = iommu_get_ops();
> + struct device *dev = dt_to_dev(np);
> + int rc = 1;
You set rc to 1 but AFAICT the value is never used.
> +
> + if ( !ops )
> + return -EINVAL;
It would be better to return -EOPNOSUPP.
> +
> + if ( iommu_dt_device_is_assigned(np) )
> + return -EPERM;
> +
> + /*
> + * The driver which supports generic IOMMU DT bindings must have
> + * these callback implemented.
> + */
I think we should make ops->remove_device optional.
> + if ( !ops->remove_device )
> + return -EINVAL;
It would be better to return -EOPNOSUPP.
> +
> + /*
> + * Remove master device from the IOMMU if latter is present and available.
> + */
> + rc = ops->remove_device(0, dev);
This will need to be interlocked with other IOMMU request.
> +
> + if ( rc == 0 )
> + iommu_fwspec_free(dev);
> +
> + return rc;
> +}
> +
> int iommu_add_dt_device(struct dt_device_node *np)
> {
> const struct iommu_ops *ops = iommu_get_ops();
> diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> index 6b2cdff..c4d5d12 100644
> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -215,6 +215,8 @@ int iommu_release_dt_devices(struct domain *d);
> */
> int iommu_add_dt_device(struct dt_device_node *np);
>
> +int iommu_remove_dt_device(struct dt_device_node *np);
> +
> int iommu_do_dt_domctl(struct xen_domctl *, struct domain *,
> XEN_GUEST_HANDLE_PARAM(xen_domctl_t));
>
>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 09/13] xen/arm: Implement device tree node removal functionalities
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (7 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 08/13] xen/iommu: Introduce iommu_remove_dt_devices function Vikram Garhwal
@ 2021-09-02 6:05 ` Vikram Garhwal
2021-09-02 6:06 ` [XEN][RFC PATCH 10/13] xen/arm: Implement device tree node addition functionalities Vikram Garhwal
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:05 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Volodymyr Babchuk,
Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Wei Liu
Introduce domctl XEN_DOMCTL_delfpga to remove a device-tree node added through
device tree overlay. Currently, this supports removing one node at a time.
DT node removal works with the following steps:
1. finds a node with given path.
2. Check if the node is used by any of dom0 or domus. It removes the node
only when it's not used by any domain.
3. Removes IRQ permissions.
4. Remove MMIO access.
5. Find the node in dt_host and delete the device node entry from dt_host.
6. Free the overlay_tracker node.
Also, added overlay_track struct to keep the track of added node through device
tree overlay. overlay_track has dt_host_new which is unflattened form of updated
fdt and name of overlay node.
When a node is removed, we also free the memory used by overlay_track for the
particular overlay node.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/arch/arm/domctl.c | 183 ++++++++++++++++++++++++++++++++++++++++++
xen/common/device_tree.c | 51 ++++++++++++
xen/include/public/domctl.h | 9 +++
xen/include/xen/device_tree.h | 1 +
4 files changed, 244 insertions(+)
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index b7d27f3..5986934 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -9,11 +9,34 @@
#include <xen/hypercall.h>
#include <xen/iocap.h>
#include <xen/lib.h>
+#include <xen/list.h>
#include <xen/mm.h>
#include <xen/sched.h>
#include <xen/types.h>
#include <xsm/xsm.h>
#include <public/domctl.h>
+#include <xen/xmalloc.h>
+#include <xen/device_tree.h>
+#include <asm/domain_build.h>
+
+/*
+ * overlay_node_track describes information about added nodes through dtbo.
+ * @dt_host_new: Pointer to the updated dt_host_new unflattened 'updated fdt'.
+ * @node_fullname: Store the name of nodes.
+ * @entry: List pointer.
+ */
+struct overlay_track {
+ struct list_head entry;
+ struct dt_device_node *dt_host_new;
+ /*
+ * TODO: We keep max nodes to 10 in an overlay. But for now we will be
+ * adding one node only.
+ */
+ char *node_fullname;
+};
+
+static LIST_HEAD(overlay_tracker);
+static DEFINE_SPINLOCK(overlay_lock);
void arch_get_domain_info(const struct domain *d,
struct xen_domctl_getdomaininfo *info)
@@ -45,6 +68,132 @@ static int handle_vuart_init(struct domain *d,
return rc;
}
+/*
+ * First finds the device node to remove. Check if the device is being used by
+ * any dom and finally remove it from dt_host. IOMMU is already being taken care
+ * while destroying the domain.
+ */
+static long handle_del_fpga_nodes(char *full_dt_node_path)
+{
+ struct domain *d = hardware_domain;
+ int rc = 0;
+ uint32_t ret = 0;
+ struct dt_device_node *fpga_device;
+ struct overlay_track *entry, *temp;
+ unsigned int naddr;
+ unsigned int i, nirq;
+ struct dt_raw_irq rirq;
+ u64 addr, size;
+
+ fpga_device = dt_find_node_by_path(full_dt_node_path);
+
+ if ( fpga_device == NULL )
+ {
+ printk(XENLOG_G_ERR "Device %s is not present in the tree\n",
+ full_dt_node_path);
+ return -EINVAL;
+ }
+
+ ret = dt_device_used_by(fpga_device);
+
+ if ( ret != 0 && ret != DOMID_IO )
+ {
+ printk(XENLOG_G_ERR "Cannot remove the device as it is being used by"
+ "domain %d\n", ret);
+ return -EPERM;
+ }
+
+ spin_lock(&overlay_lock);
+
+ nirq = dt_number_of_irq(fpga_device);
+
+ /* Remove IRQ permission */
+ for ( i = 0; i < nirq; i++ )
+ {
+ rc = dt_device_get_raw_irq(fpga_device, i, &rirq);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Unable to retrieve irq %u for %s\n",
+ i, dt_node_full_name(fpga_device));
+ goto out;
+ }
+
+ rc = platform_get_irq(fpga_device, i);
+ if ( rc < 0 )
+ {
+ printk(XENLOG_ERR "Unable to get irq %u for %s\n",
+ i, dt_node_full_name(fpga_device));
+ goto out;
+ }
+
+ rc = irq_deny_access(d, rc);
+
+ if ( rc )
+ {
+ printk(XENLOG_ERR "unable to revoke access for irq %u for %s\n",
+ i, dt_node_full_name(fpga_device));
+ goto out;
+ }
+ }
+
+ rc = iommu_remove_dt_device(fpga_device);
+
+ if ( rc )
+ goto out;
+
+ naddr = dt_number_of_address(fpga_device);
+
+ /* Remove mmio access. */
+ for ( i = 0; i < naddr; i++ )
+ {
+ rc = dt_device_get_address(fpga_device, i, &addr, &size);
+
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
+ i, dt_node_full_name(fpga_device));
+ goto out;
+ }
+
+ rc = iomem_deny_access(d, paddr_to_pfn(addr),
+ paddr_to_pfn(PAGE_ALIGN(addr + size - 1)));
+
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Unable to remove dom%d access to"
+ " 0x%"PRIx64" - 0x%"PRIx64"\n",
+ d->domain_id,
+ addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
+ goto out;
+ }
+ }
+
+ rc = fpga_del_node(fpga_device);
+
+ if ( rc )
+ goto out;
+
+ list_for_each_entry_safe( entry, temp, &overlay_tracker, entry )
+ {
+ if ( (strcmp(full_dt_node_path, entry->node_fullname) == 0) )
+ {
+ list_del(&entry->entry);
+ xfree(entry->node_fullname);
+ xfree(entry->dt_host_new);
+ xfree(entry);
+ goto out;
+ }
+ }
+
+ printk(XENLOG_G_ERR "Cannot find the node in tracker. Memory will not"
+ "be freed\n");
+ rc = -ENOENT;
+
+out:
+ spin_unlock(&overlay_lock);
+ return rc;
+}
+
long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
@@ -173,6 +322,40 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
return rc;
}
+
+ case XEN_DOMCTL_delfpga:
+ {
+ char *full_dt_node_path;
+ int rc;
+
+ if ( domctl->u.fpga_del_dt.size > 0 )
+ full_dt_node_path = xmalloc_bytes(domctl->u.fpga_del_dt.size);
+ else
+ return -EINVAL;
+
+ if ( full_dt_node_path == NULL )
+ return -ENOMEM;
+
+ rc = copy_from_guest(full_dt_node_path,
+ domctl->u.fpga_del_dt.full_dt_node_path,
+ domctl->u.fpga_del_dt.size);
+ if ( rc )
+ {
+ gprintk(XENLOG_ERR, "copy from guest failed\n");
+ xfree(full_dt_node_path);
+
+ return -EFAULT;
+ }
+
+ full_dt_node_path[domctl->u.fpga_del_dt.size - 1] = '\0';
+
+ rc = handle_del_fpga_nodes(full_dt_node_path);
+
+ xfree(full_dt_node_path);
+
+ return rc;
+ }
+
default:
{
int rc;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 4946e83..04f2578 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -324,6 +324,57 @@ void dt_print_node_names(struct dt_device_node *dt)
return;
}
+int fpga_del_node(struct dt_device_node *device_node)
+{
+ struct dt_device_node *np;
+ struct dt_device_node *parent_node;
+ struct dt_device_node *current_node;
+
+ parent_node = device_node->parent;
+
+ current_node = parent_node;
+
+ if ( parent_node == NULL )
+ {
+ dt_dprintk("%s's parent node not found\n", device_node->name);
+ return -EFAULT;
+ }
+
+ np = parent_node->child;
+
+ if ( np == NULL )
+ {
+ dt_dprintk("parent node %s's not found\n", parent_node->name);
+ return -EFAULT;
+ }
+
+ /* If node to be removed is only child node or first child. */
+ if ( np->name == device_node->name )
+ {
+ current_node->allnext = np->next;
+ return 0;
+ }
+
+ for ( np = parent_node->child; np->sibling != NULL; np = np->sibling )
+ {
+ current_node = np;
+ if ( np->sibling->name == device_node->name )
+ {
+ /* Found the node. Now we remove it. */
+ current_node->allnext = np->allnext->allnext;
+
+ if ( np->sibling->sibling )
+ current_node->sibling = np->sibling->sibling;
+ else
+ current_node->sibling = NULL;
+
+ break;
+ }
+ }
+
+ return 0;
+}
+
int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
struct dt_device_node **node)
{
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 96696e3..b1b8efd 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1169,6 +1169,13 @@ struct xen_domctl_vmtrace_op {
typedef struct xen_domctl_vmtrace_op xen_domctl_vmtrace_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_domctl_vmtrace_op_t);
+/* XEN_DOMCTL_fpga_del. */
+struct xen_domctl_fpga_del_dt {
+ XEN_GUEST_HANDLE_64(char) full_dt_node_path;
+ uint32_t size;
+};
+
+
struct xen_domctl {
uint32_t cmd;
#define XEN_DOMCTL_createdomain 1
@@ -1254,6 +1261,7 @@ struct xen_domctl {
#define XEN_DOMCTL_get_cpu_policy 82
#define XEN_DOMCTL_set_cpu_policy 83
#define XEN_DOMCTL_vmtrace_op 84
+#define XEN_DOMCTL_delfpga 86
#define XEN_DOMCTL_gdbsx_guestmemio 1000
#define XEN_DOMCTL_gdbsx_pausevcpu 1001
#define XEN_DOMCTL_gdbsx_unpausevcpu 1002
@@ -1315,6 +1323,7 @@ struct xen_domctl {
struct xen_domctl_psr_alloc psr_alloc;
struct xen_domctl_vuart_op vuart_op;
struct xen_domctl_vmtrace_op vmtrace_op;
+ struct xen_domctl_fpga_del_dt fpga_del_dt;
uint8_t pad[128];
} u;
};
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 7cc6093..eb7f645 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -496,6 +496,7 @@ int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
* Prints all node names.
*/
void dt_print_node_names(struct dt_device_node *dt);
+int fpga_del_node(struct dt_device_node *device_node);
/**
* dt_get_parent - Get a node's parent if any
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [XEN][RFC PATCH 10/13] xen/arm: Implement device tree node addition functionalities
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (8 preceding siblings ...)
2021-09-02 6:05 ` [XEN][RFC PATCH 09/13] xen/arm: Implement device tree node removal functionalities Vikram Garhwal
@ 2021-09-02 6:06 ` Vikram Garhwal
2021-10-21 18:30 ` Julien Grall
2021-09-02 6:06 ` [XEN][RFC PATCH 11/13] tools/libs/ctrl: Implement new xc interfaces for fpga-add and fpga-del Vikram Garhwal
` (2 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:06 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Volodymyr Babchuk,
Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Wei Liu
Introduce domctl XEN_DOMCTL_addfpga to add a device-tree node through device
tree overlay. This works with a device tree overlay(.dtbo) as input.
Add check_pfdt() to do sanity check on the dtbo.
Also, added overlay_get_node_info() to get the node's full name with path. This
comes handy when checking node for duplication.
Each time a overlay node is added, a new fdt(memcpy of device_tree_flattened) is
created and updated with overlay node. This updated fdt is further unflattened
to a dt_host_new. Next, it checks if overlay node already exists in the dt_host.
If overlay node doesn't exist then find the overlay node in dt_host_new, find
the overlay node's parent in dt_host and add the node as child under parent in
the dt_host. The node is attached as the last node under target parent.
Finally, add IRQs, add device to IOMMUs, set permissions and map MMIO for the
overlay node.
When a node is added using overlay, a new entry is allocated in the
overlay_track to keep the track of memory allocation due to addition of overlay
node. This is helpful for freeing the memory allocated when a device tree node
is removed with domctl XEN_DOMCTL_delfpga domctl.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
xen/arch/arm/domctl.c | 262 ++++++++++++++++++++++++++++++++++++++++++
xen/common/device_tree.c | 54 +++++++++
xen/include/public/domctl.h | 7 ++
xen/include/xen/device_tree.h | 1 +
4 files changed, 324 insertions(+)
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
index 5986934..0ac635f 100644
--- a/xen/arch/arm/domctl.c
+++ b/xen/arch/arm/domctl.c
@@ -15,6 +15,8 @@
#include <xen/types.h>
#include <xsm/xsm.h>
#include <public/domctl.h>
+/* Included for FPGA dt add. */
+#include <xen/libfdt/libfdt.h>
#include <xen/xmalloc.h>
#include <xen/device_tree.h>
#include <asm/domain_build.h>
@@ -68,6 +70,61 @@ static int handle_vuart_init(struct domain *d,
return rc;
}
+static int check_pfdt(void *pfdt, uint32_t pfdt_size)
+{
+ if ( fdt_totalsize(pfdt) != pfdt_size )
+ {
+ printk(XENLOG_ERR "Partial FDT is not a valid Flat Device Tree\n");
+ return -EFAULT;
+ }
+
+ if ( fdt_check_header(pfdt) )
+ {
+ printk(XENLOG_ERR "Partial FDT is not a valid Flat Device Tree\n");
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static void overlay_get_node_info(void *fdto, char *node_full_path)
+{
+ int fragment;
+
+ /*
+ * Handle overlay nodes. But for now we are just handling one node.
+ */
+ fdt_for_each_subnode(fragment, fdto, 0)
+ {
+ int target;
+ int overlay;
+ int subnode;
+ const char *target_path;
+
+ target = overlay_get_target(device_tree_flattened, fdto, fragment,
+ &target_path);
+ overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
+
+ fdt_for_each_subnode(subnode, fdto, overlay)
+ {
+ const char *node_name = fdt_get_name(fdto, subnode, NULL);
+ int node_name_len = strlen(node_name);
+ int target_path_len = strlen(target_path);
+
+ memcpy(node_full_path, target_path, target_path_len);
+
+ node_full_path[target_path_len] = '/';
+
+ memcpy(node_full_path + target_path_len + 1, node_name,
+ node_name_len);
+
+ node_full_path[target_path_len + 1 + node_name_len] = '\0';
+
+ return;
+ }
+ }
+}
+
/*
* First finds the device node to remove. Check if the device is being used by
* any dom and finally remove it from dt_host. IOMMU is already being taken care
@@ -194,6 +251,181 @@ out:
return rc;
}
+/*
+ * Adds only one device node at a time under target node.
+ * We use dt_host_new to unflatten the updated device_tree_flattened. This is
+ * done to avoid the removal of device_tree generation, iomem regions mapping to
+ * DOM0 done by handle_node().
+ */
+static long handle_add_fpga_overlay(void *pfdt, uint32_t pfdt_size)
+{
+ int rc = 0;
+ struct dt_device_node *fpga_node;
+ char node_full_path[128];
+ void *fdt = xmalloc_bytes(fdt_totalsize(device_tree_flattened));
+ struct dt_device_node *dt_host_new;
+ struct domain *d = hardware_domain;
+ struct overlay_track *tr = NULL;
+ int node_full_path_namelen;
+ unsigned int naddr;
+ unsigned int i;
+ u64 addr, size;
+
+ if ( fdt == NULL )
+ return ENOMEM;
+
+ spin_lock(&overlay_lock);
+
+ memcpy(fdt, device_tree_flattened, fdt_totalsize(device_tree_flattened));
+
+ rc = check_pfdt(pfdt, pfdt_size);
+
+ if ( rc )
+ goto err;
+
+ overlay_get_node_info(pfdt, node_full_path);
+
+ rc = fdt_overlay_apply(fdt, pfdt);
+
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Adding overlay node %s failed with error %d\n",
+ node_full_path, rc);
+ goto err;
+ }
+
+ /* Check if node already exists in dt_host. */
+ fpga_node = dt_find_node_by_path(node_full_path);
+
+ if ( fpga_node != NULL )
+ {
+ printk(XENLOG_ERR "node %s exists in device tree\n", node_full_path);
+ rc = -EINVAL;
+ goto err;
+ }
+
+ /* Unflatten the fdt into a new dt_host. */
+ unflatten_device_tree(fdt, &dt_host_new);
+
+ /* Find the newly added node in dt_host_new by it's full path. */
+ fpga_node = _dt_find_node_by_path(dt_host_new, node_full_path);
+
+ if ( fpga_node == NULL )
+ {
+ dt_dprintk("%s node not found\n", node_full_path);
+ rc = -EFAULT;
+ xfree(dt_host_new);
+ goto err;
+ }
+
+ /* Just keep the node we intend to add. Remove every other node in list. */
+ fpga_node->allnext = NULL;
+ fpga_node->sibling = NULL;
+
+ /* Add the node to dt_host. */
+ rc = fpga_add_node(fpga_node, fpga_node->parent->full_name);
+
+ if ( rc )
+ {
+ /* Node not added in dt_host. Safe to free dt_host_new. */
+ xfree(dt_host_new);
+ goto err;
+ }
+
+ /* Get the node from dt_host and add interrupt and IOMMUs. */
+ fpga_node = dt_find_node_by_path(fpga_node->full_name);
+
+ if ( fpga_node == NULL )
+ {
+ /* Sanity check. But code will never come in this loop. */
+ printk(XENLOG_ERR "Cannot find %s node under updated dt_host\n",
+ fpga_node->name);
+ goto remove_node;
+ }
+
+ /* First let's handle the interrupts. */
+ rc = handle_device_interrupts(d, fpga_node, false);
+
+ if ( rc )
+ {
+ printk(XENLOG_G_ERR "Interrupt failed\n");
+ goto remove_node;
+ }
+
+ /* Add device to IOMMUs */
+ rc = iommu_add_dt_device(fpga_node);
+
+ if ( rc < 0 )
+ {
+ printk(XENLOG_G_ERR "Failed to add %s to the IOMMU\n",
+ dt_node_full_name(fpga_node));
+ goto remove_node;
+ }
+
+ /* Set permissions. */
+ naddr = dt_number_of_address(fpga_node);
+
+ dt_dprintk("%s passthrough = %d naddr = %u\n",
+ dt_node_full_name(fpga_node), false, naddr);
+
+ /* Give permission and map MMIOs */
+ for ( i = 0; i < naddr; i++ )
+ {
+ struct map_range_data mr_data = { .d = d, .p2mt = p2m_mmio_direct_c };
+ rc = dt_device_get_address(fpga_node, i, &addr, &size);
+ if ( rc )
+ {
+ printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
+ i, dt_node_full_name(fpga_node));
+ goto remove_node;
+ }
+
+ rc = map_range_to_domain(fpga_node, addr, size, &mr_data);
+ if ( rc )
+ goto remove_node;
+ }
+
+ /* This will happen if everything above goes right. */
+ tr = xzalloc(struct overlay_track);
+ tr->dt_host_new = dt_host_new;
+ node_full_path_namelen = strlen(node_full_path);
+ tr->node_fullname = xmalloc_bytes(node_full_path_namelen + 1);
+
+ if ( tr->node_fullname == NULL )
+ {
+ rc = -ENOMEM;
+ goto remove_node;
+ }
+
+ memcpy(tr->node_fullname, node_full_path, node_full_path_namelen);
+ tr->node_fullname[node_full_path_namelen] = '\0';
+
+ INIT_LIST_HEAD(&tr->entry);
+ list_add_tail(&tr->entry, &overlay_tracker);
+
+err:
+ spin_unlock(&overlay_lock);
+ xfree(fdt);
+ return rc;
+
+/*
+ * Failure case. We need to remove the node, free tracker(if tr exists) and
+ * dt_host_new. As the tracker is not in list yet so it doesn't get freed in
+ * handle_del_fpga_nodes() and due to that dt_host_new will not get freed so we
+ * we free tracker and dt_host_new here.
+ */
+remove_node:
+ spin_unlock(&overlay_lock);
+ handle_del_fpga_nodes(node_full_path);
+ xfree(dt_host_new);
+
+ if ( tr )
+ xfree(tr);
+
+ xfree(fdt);
+ return rc;
+}
+
long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
@@ -323,6 +555,36 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
return rc;
}
+ case XEN_DOMCTL_addfpga:
+ {
+ void *pfdt;
+ int rc;
+
+ if ( domctl->u.fpga_add_dt.pfdt_size > 0 )
+ pfdt = xmalloc_bytes(domctl->u.fpga_add_dt.pfdt_size);
+ else
+ return -EINVAL;
+
+ if ( pfdt == NULL )
+ return -ENOMEM;
+
+ rc = copy_from_guest(pfdt, domctl->u.fpga_add_dt.pfdt,
+ domctl->u.fpga_add_dt.pfdt_size);
+ if ( rc )
+ {
+ gprintk(XENLOG_ERR, "copy from guest failed\n");
+ xfree(pfdt);
+
+ return -EFAULT;
+ }
+
+ rc = handle_add_fpga_overlay(pfdt, domctl->u.fpga_add_dt.pfdt_size);
+
+ xfree(pfdt);
+
+ return rc;
+ }
+
case XEN_DOMCTL_delfpga:
{
char *full_dt_node_path;
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index 04f2578..d062c17 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -324,6 +324,60 @@ void dt_print_node_names(struct dt_device_node *dt)
return;
}
+int fpga_add_node(struct dt_device_node *fpga_node,
+ const char *parent_node_path)
+{
+ struct dt_device_node *parent_node;
+ struct dt_device_node *np;
+ struct dt_device_node *next_node;
+ struct dt_device_node *new_node;
+
+ parent_node = dt_find_node_by_path(parent_node_path);
+
+ new_node = fpga_node;
+
+ if ( new_node == NULL )
+ return -EINVAL;
+
+ if ( parent_node == NULL )
+ {
+ dt_dprintk("Node not found. Partial dtb will not be added");
+ return -EINVAL;
+ }
+
+ /*
+ * If node is found. We can attach the fpga_node as a child of the
+ * parent node.
+ */
+
+ for ( np = parent_node->child; np->sibling != NULL; np = np->sibling )
+ {
+ }
+
+ /*
+ * Before attaching also check if the parent node of fpga_node is also
+ * same named as parent.
+ */
+ next_node = np->allnext;
+
+ new_node->parent = parent_node;
+ np->sibling = new_node;
+ np->allnext = new_node;
+
+ /*
+ * Reach at the end of fpga_node.
+ * TODO: Remove this loop as we are just adding one node for now.
+ */
+ for ( np = new_node; np->allnext != NULL; np = np->allnext )
+ {
+ }
+
+ /* Now plug next_node at the end of fpga_node. */
+ np->allnext = next_node;
+
+ return 0;
+}
+
int fpga_del_node(struct dt_device_node *device_node)
{
struct dt_device_node *np;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index b1b8efd..ce4667e 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1175,6 +1175,11 @@ struct xen_domctl_fpga_del_dt {
uint32_t size;
};
+/* XEN_DOMCTL_fpga_add. */
+struct xen_domctl_fpga_add_dt {
+ XEN_GUEST_HANDLE_64(void) pfdt;
+ uint32_t pfdt_size; /* Partial dtb size. */
+};
struct xen_domctl {
uint32_t cmd;
@@ -1261,6 +1266,7 @@ struct xen_domctl {
#define XEN_DOMCTL_get_cpu_policy 82
#define XEN_DOMCTL_set_cpu_policy 83
#define XEN_DOMCTL_vmtrace_op 84
+#define XEN_DOMCTL_addfpga 85
#define XEN_DOMCTL_delfpga 86
#define XEN_DOMCTL_gdbsx_guestmemio 1000
#define XEN_DOMCTL_gdbsx_pausevcpu 1001
@@ -1323,6 +1329,7 @@ struct xen_domctl {
struct xen_domctl_psr_alloc psr_alloc;
struct xen_domctl_vuart_op vuart_op;
struct xen_domctl_vmtrace_op vmtrace_op;
+ struct xen_domctl_fpga_add_dt fpga_add_dt;
struct xen_domctl_fpga_del_dt fpga_del_dt;
uint8_t pad[128];
} u;
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index eb7f645..4c8dec6 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -496,6 +496,7 @@ int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
* Prints all node names.
*/
void dt_print_node_names(struct dt_device_node *dt);
+int fpga_add_node(struct dt_device_node *fpga_node, const char *parent_node);
int fpga_del_node(struct dt_device_node *device_node);
/**
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [XEN][RFC PATCH 10/13] xen/arm: Implement device tree node addition functionalities
2021-09-02 6:06 ` [XEN][RFC PATCH 10/13] xen/arm: Implement device tree node addition functionalities Vikram Garhwal
@ 2021-10-21 18:30 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2021-10-21 18:30 UTC (permalink / raw)
To: Vikram Garhwal, xen-devel
Cc: sstabellini, Volodymyr Babchuk, Andrew Cooper, George Dunlap,
Ian Jackson, Jan Beulich, Wei Liu
Hi Vikram,
On 02/09/2021 07:06, Vikram Garhwal wrote:
> Introduce domctl XEN_DOMCTL_addfpga to add a device-tree node through device
XEN_DOMCTL_* are hypercalls to manage a given domain. However, here you
are modifying the system.
This is similar to managing PCI devices, except here we are dealing with
platforms devices. In the case of PCI devices, we are using PHYSDEVOP_*.
If we only expect the toolstack to the call (e.g. the kernel won't use
it directly), then it would be better to use SYSCTL_* as they are not
stable (so we have more flexibility to modify the layout).
Furthermore, rather than introduciong 2 new sub-hypercalls for
XEN_SYSCTL_* I would introduce a single sub-hypercall that takes a
command (e.g. add/remove).
Regarding the name, in theory this feature is not FPGA specific. So I
would prefer a more generic name, maybe XEN_DOMCTL_dt_overlay?
> tree overlay. This works with a device tree overlay(.dtbo) as input.
>
> Add check_pfdt() to do sanity check on the dtbo.
From my experience with libfdt, the library tends to consider the DTB
to be sound. So we would have to trust what the toolstack is provided us.
So I think we need to make clear that this is basic sanity check and
there are no expectation that we will be able to deal with a random blob.
>
> Also, added overlay_get_node_info() to get the node's full name with path. This
> comes handy when checking node for duplication.
>
> Each time a overlay node is added, a new fdt(memcpy of device_tree_flattened) is
> created and updated with overlay node. This updated fdt is further unflattened
> to a dt_host_new. Next, it checks if overlay node already exists in the dt_host.
> If overlay node doesn't exist then find the overlay node in dt_host_new, find
> the overlay node's parent in dt_host and add the node as child under parent in
> the dt_host. The node is attached as the last node under target parent.
>
> Finally, add IRQs, add device to IOMMUs, set permissions and map MMIO for the
> overlay node.
>
> When a node is added using overlay, a new entry is allocated in the
> overlay_track to keep the track of memory allocation due to addition of overlay
> node. This is helpful for freeing the memory allocated when a device tree node
> is removed with domctl XEN_DOMCTL_delfpga domctl.
>
> Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
> ---
> xen/arch/arm/domctl.c | 262 ++++++++++++++++++++++++++++++++++++++++++
> xen/common/device_tree.c | 54 +++++++++
> xen/include/public/domctl.h | 7 ++
> xen/include/xen/device_tree.h | 1 +
> 4 files changed, 324 insertions(+)
>
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index 5986934..0ac635f 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -15,6 +15,8 @@
> #include <xen/types.h>
> #include <xsm/xsm.h>
> #include <public/domctl.h>
> +/* Included for FPGA dt add. */
We don't usually write done why an header is included. So I would remove
this comment.
> +#include <xen/libfdt/libfdt.h>
> #include <xen/xmalloc.h>
> #include <xen/device_tree.h>
> #include <asm/domain_build.h>
> @@ -68,6 +70,61 @@ static int handle_vuart_init(struct domain *d,
> return rc;
> }
>
> +static int check_pfdt(void *pfdt, uint32_t pfdt_size)
> +{
> + if ( fdt_totalsize(pfdt) != pfdt_size )
> + {
> + printk(XENLOG_ERR "Partial FDT is not a valid Flat Device Tree\n");
> + return -EFAULT;
I think -EINVAL is more suitable.
> + }
> +
> + if ( fdt_check_header(pfdt) )
> + {
> + printk(XENLOG_ERR "Partial FDT is not a valid Flat Device Tree\n");
The printk() is exactly the same as above. So how about combining the
two check?
Furthemore, from the commit message, the pfdt is an overlay fdt. So
shouldn't we have write "The overlay FDT" rather than "Partial FDT"?
> + return -EFAULT;
> + }
> +
> + return 0;
> +}
> +
> +static void overlay_get_node_info(void *fdto, char *node_full_path)
> +{
> + int fragment;
> +
> + /*
> + * Handle overlay nodes. But for now we are just handling one node.
Is this a limitation you plan to handle before this series is going to
be committed?
> + */
> + fdt_for_each_subnode(fragment, fdto, 0)
> + {
> + int target;
> + int overlay;
> + int subnode;
> + const char *target_path;
> +
> + target = overlay_get_target(device_tree_flattened, fdto, fragment,
> + &target_path);
> + overlay = fdt_subnode_offset(fdto, fragment, "__overlay__");
> +
> + fdt_for_each_subnode(subnode, fdto, overlay)
> + {
> + const char *node_name = fdt_get_name(fdto, subnode, NULL);
> + int node_name_len = strlen(node_name);
> + int target_path_len = strlen(target_path);
> +
> + memcpy(node_full_path, target_path, target_path_len);
Looking at the caller (handle_add_fpga_overlay()), the node_full_path is
a fixed array. What does guarantee that the array is large enough to
target_path?
> +
> + node_full_path[target_path_len] = '/';
> +
> + memcpy(node_full_path + target_path_len + 1, node_name,
> + node_name_len);
> +
> + node_full_path[target_path_len + 1 + node_name_len] = '\0';
> +
> + return;
> + }
> + }
> +}
> +
> /*
> * First finds the device node to remove. Check if the device is being used by
> * any dom and finally remove it from dt_host. IOMMU is already being taken care
> @@ -194,6 +251,181 @@ out:
> return rc;
> }
>
> +/*
> + * Adds only one device node at a time under target node.
> + * We use dt_host_new to unflatten the updated device_tree_flattened. This is
> + * done to avoid the removal of device_tree generation, iomem regions mapping to
> + * DOM0 done by handle_node().
Below you are using "hardware_domain" which may or may not be dom0. So
replace "dom0" with "hardware domain".
> + */
> +static long handle_add_fpga_overlay(void *pfdt, uint32_t pfdt_size)
> +{
> + int rc = 0;
> + struct dt_device_node *fpga_node;
> + char node_full_path[128];
> + void *fdt = xmalloc_bytes(fdt_totalsize(device_tree_flattened));
I would prefer if the xmalloc_bytes() happens closer to the check "fdt
== NULL" below. This makes easier to link the two.
In addition to that, device_tree_flattened will be NULL when Xen is
booting with ACPI. So we need to return an error if one try to use this
feature on such platform.
Lastly, I think we should consider to protect all the new code under a
config so this is not compiled by default.
> + struct dt_device_node *dt_host_new;
> + struct domain *d = hardware_domain;
> + struct overlay_track *tr = NULL;
> + int node_full_path_namelen;
> + unsigned int naddr;
> + unsigned int i;
> + u64 addr, size;
> +
> + if ( fdt == NULL )
> + return ENOMEM;
We usually add - in from of the errno.
> +
> + spin_lock(&overlay_lock);
> +
> + memcpy(fdt, device_tree_flattened, fdt_totalsize(device_tree_flattened));
> +
> + rc = check_pfdt(pfdt, pfdt_size);
> +
> + if ( rc )
> + goto err;
> +
> + overlay_get_node_info(pfdt, node_full_path);
You don't really need this info until the overlay has been applied. So I
would suggest to move this after fdt_overlay_apply().
> +
> + rc = fdt_overlay_apply(fdt, pfdt);
> +
> + if ( rc )
> + {
> + printk(XENLOG_ERR "Adding overlay node %s failed with error %d\n",
> + node_full_path, rc);
> + goto err;
> + }
> +
> + /* Check if node already exists in dt_host. */
> + fpga_node = dt_find_node_by_path(node_full_path);
> +
> + if ( fpga_node != NULL )
> + {
> + printk(XENLOG_ERR "node %s exists in device tree\n", node_full_path);
> + rc = -EINVAL;
> + goto err;
> + }
> +
> + /* Unflatten the fdt into a new dt_host. */
> + unflatten_device_tree(fdt, &dt_host_new);
Looking at this function, there is at least a potential allocation
failure. So it needs to be updated to propagate any error in order to
use it in an hypercall.
Furthemore, AFAICT, the function will continue even if there are
corrupted node. This brings back to my point above that this hypercall
will only work with trusted DT. I am OK with that so long we written it
clearly in the interface and documentation.
> +
> + /* Find the newly added node in dt_host_new by it's full path. */
> + fpga_node = _dt_find_node_by_path(dt_host_new, node_full_path);
> +
> + if ( fpga_node == NULL )
> + {
> + dt_dprintk("%s node not found\n", node_full_path);
> + rc = -EFAULT;
> + xfree(dt_host_new);
> + goto err;
> + }
> +
> + /* Just keep the node we intend to add. Remove every other node in list. */
> + fpga_node->allnext = NULL;
> + fpga_node->sibling = NULL;
IMHO, the two lines belong to fpga_add_node().
> +
> + /* Add the node to dt_host. */
> + rc = fpga_add_node(fpga_node, fpga_node->parent->full_name);
> +
> + if ( rc )
> + {
> + /* Node not added in dt_host. Safe to free dt_host_new. */
> + xfree(dt_host_new);
> + goto err;
> + }
> +
> + /* Get the node from dt_host and add interrupt and IOMMUs. */
> + fpga_node = dt_find_node_by_path(fpga_node->full_name);
> +
> + if ( fpga_node == NULL )
> + {
> + /* Sanity check. But code will never come in this loop. */
There is no loop here. So what did you mean?
> + printk(XENLOG_ERR "Cannot find %s node under updated dt_host\n",
> + fpga_node->name);
> + goto remove_node;
> + }
> +
The code below is pretty much a copy of handle_device(). Can you look to
re-use it?
> + /* First let's handle the interrupts. */
> + rc = handle_device_interrupts(d, fpga_node, false);
> +
> + if ( rc )
> + {
> + printk(XENLOG_G_ERR "Interrupt failed\n");
> + goto remove_node;
> + }
> +
> + /* Add device to IOMMUs */
> + rc = iommu_add_dt_device(fpga_node);
> +
> + if ( rc < 0 )
> + {
> + printk(XENLOG_G_ERR "Failed to add %s to the IOMMU\n",
> + dt_node_full_name(fpga_node));
> + goto remove_node;
> + }
> +
> + /* Set permissions. */
> + naddr = dt_number_of_address(fpga_node);
> +
> + dt_dprintk("%s passthrough = %d naddr = %u\n",
> + dt_node_full_name(fpga_node), false, naddr);
> +
> + /* Give permission and map MMIOs */
> + for ( i = 0; i < naddr; i++ )
> + {
> + struct map_range_data mr_data = { .d = d, .p2mt = p2m_mmio_direct_c };
> + rc = dt_device_get_address(fpga_node, i, &addr, &size);
> + if ( rc )
> + {
> + printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
> + i, dt_node_full_name(fpga_node));
> + goto remove_node;
> + }
> +
> + rc = map_range_to_domain(fpga_node, addr, size, &mr_data);
> + if ( rc )
> + goto remove_node;
> + }
> +
> + /* This will happen if everything above goes right. */
> + tr = xzalloc(struct overlay_track);
xzalloc() can fail. So you need to check the return.
> + tr->dt_host_new = dt_host_new;
Looking at the code above, dt_host_new is pretty much a full copy of the
DT + the overlay. From my understanding, the flat device-tree can be up
to 2MB, therefore the unflatten one is likely bigger.
So to me this seems quite a waste of memory. Can we look to only keep in
memory the nodes we need?
> + node_full_path_namelen = strlen(node_full_path);
> + tr->node_fullname = xmalloc_bytes(node_full_path_namelen + 1);
> +
> + if ( tr->node_fullname == NULL )
> + {
> + rc = -ENOMEM;
> + goto remove_node;
> + }
> +
> + memcpy(tr->node_fullname, node_full_path, node_full_path_namelen);
> + tr->node_fullname[node_full_path_namelen] = '\0';
> +
> + INIT_LIST_HEAD(&tr->entry);
> + list_add_tail(&tr->entry, &overlay_tracker);
> +
> +err:
> + spin_unlock(&overlay_lock);
> + xfree(fdt);
> + return rc;
> +
> +/*
> + * Failure case. We need to remove the node, free tracker(if tr exists) and
> + * dt_host_new. As the tracker is not in list yet so it doesn't get freed in
> + * handle_del_fpga_nodes() and due to that dt_host_new will not get freed so we
> + * we free tracker and dt_host_new here.
> + */
> +remove_node:
> + spin_unlock(&overlay_lock);
> + handle_del_fpga_nodes(node_full_path);
> + xfree(dt_host_new);
> +
> + if ( tr )
> + xfree(tr);
> +
> + xfree(fdt);
> + return rc;
> +}
> +
> long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> {
> @@ -323,6 +555,36 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> return rc;
> }
>
> + case XEN_DOMCTL_addfpga:
> + {
> + void *pfdt;
> + int rc;
> +
> + if ( domctl->u.fpga_add_dt.pfdt_size > 0 )
> + pfdt = xmalloc_bytes(domctl->u.fpga_add_dt.pfdt_size);
Can we limit the size of the blob?
> + else
> + return -EINVAL;
> +
> + if ( pfdt == NULL )
> + return -ENOMEM;
> +
> + rc = copy_from_guest(pfdt, domctl->u.fpga_add_dt.pfdt,
> + domctl->u.fpga_add_dt.pfdt_size);
> + if ( rc )
> + {
> + gprintk(XENLOG_ERR, "copy from guest failed\n");
> + xfree(pfdt);
> +
> + return -EFAULT;
> + }
> +
> + rc = handle_add_fpga_overlay(pfdt, domctl->u.fpga_add_dt.pfdt_size);
> +
> + xfree(pfdt);
> +
> + return rc;
> + }
> +
> case XEN_DOMCTL_delfpga:
> {
> char *full_dt_node_path;
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 04f2578..d062c17 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -324,6 +324,60 @@ void dt_print_node_names(struct dt_device_node *dt)
> return;
> }
>
> +int fpga_add_node(struct dt_device_node *fpga_node,
> + const char *parent_node_path)
> +{
> + struct dt_device_node *parent_node;
> + struct dt_device_node *np;
> + struct dt_device_node *next_node;
> + struct dt_device_node *new_node;
> +
> + parent_node = dt_find_node_by_path(parent_node_path);
> +
> + new_node = fpga_node;
> +
> + if ( new_node == NULL )
> + return -EINVAL;
> +
> + if ( parent_node == NULL )
> + {
> + dt_dprintk("Node not found. Partial dtb will not be added");
> + return -EINVAL;
> + }
> +
> + /*
> + * If node is found. We can attach the fpga_node as a child of the
> + * parent node.
> + */
> +
> + for ( np = parent_node->child; np->sibling != NULL; np = np->sibling )
> + {
I would suggest to clearify in the comment what this loop is for. This
would make easier to understand that the body of the loop is left empty
on purpose.
> + }
> +
> + /*
> + * Before attaching also check if the parent node of fpga_node is also
> + * same named as parent.
> + */
> + next_node = np->allnext;
> +
> + new_node->parent = parent_node;
> + np->sibling = new_node;
> + np->allnext = new_node;
> +
> + /*
> + * Reach at the end of fpga_node.
> + * TODO: Remove this loop as we are just adding one node for now.
For clarification, by "one node", do you mean top-level node or are you
saying this node has no child?
> + */
> + for ( np = new_node; np->allnext != NULL; np = np->allnext )
> + {
> + }
> +
> + /* Now plug next_node at the end of fpga_node. */
> + np->allnext = next_node;
> +
> + return 0;
> +}
> +
> int fpga_del_node(struct dt_device_node *device_node)
> {
> struct dt_device_node *np;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index b1b8efd..ce4667e 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1175,6 +1175,11 @@ struct xen_domctl_fpga_del_dt {
> uint32_t size;
> };
>
> +/* XEN_DOMCTL_fpga_add. */
Please add some documentation about the new hypercall.
> +struct xen_domctl_fpga_add_dt {
> + XEN_GUEST_HANDLE_64(void) pfdt;
> + uint32_t pfdt_size; /* Partial dtb size. */
> +};
>
> struct xen_domctl {
> uint32_t cmd;
> @@ -1261,6 +1266,7 @@ struct xen_domctl {
> #define XEN_DOMCTL_get_cpu_policy 82
> #define XEN_DOMCTL_set_cpu_policy 83
> #define XEN_DOMCTL_vmtrace_op 84
> +#define XEN_DOMCTL_addfpga 85
> #define XEN_DOMCTL_delfpga 86
> #define XEN_DOMCTL_gdbsx_guestmemio 1000
> #define XEN_DOMCTL_gdbsx_pausevcpu 1001
> @@ -1323,6 +1329,7 @@ struct xen_domctl {
> struct xen_domctl_psr_alloc psr_alloc;
> struct xen_domctl_vuart_op vuart_op;
> struct xen_domctl_vmtrace_op vmtrace_op;
> + struct xen_domctl_fpga_add_dt fpga_add_dt;
> struct xen_domctl_fpga_del_dt fpga_del_dt;
> uint8_t pad[128];
> } u;
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index eb7f645..4c8dec6 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -496,6 +496,7 @@ int dt_find_node_by_gpath(XEN_GUEST_HANDLE(char) u_path, uint32_t u_plen,
> * Prints all node names.
> */
> void dt_print_node_names(struct dt_device_node *dt);
> +int fpga_add_node(struct dt_device_node *fpga_node, const char *parent_node);
> int fpga_del_node(struct dt_device_node *device_node);
>
> /**
>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [XEN][RFC PATCH 11/13] tools/libs/ctrl: Implement new xc interfaces for fpga-add and fpga-del
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (9 preceding siblings ...)
2021-09-02 6:06 ` [XEN][RFC PATCH 10/13] xen/arm: Implement device tree node addition functionalities Vikram Garhwal
@ 2021-09-02 6:06 ` Vikram Garhwal
2021-09-02 6:06 ` [XEN][RFC PATCH 12/13] tools/libs/light: Implement new libxl functions " Vikram Garhwal
2021-09-02 6:06 ` [XEN][RFC PATCH 13/13] tools/xl: Add new xl commands " Vikram Garhwal
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:06 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Ian Jackson, Wei Liu, Juergen Gross
xc_domain_add_fpga() sends the device tree binary overlay and size of .dtbo to
xen.
xc_domain_del_fpga() sends full path for the node to be removed.
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
tools/include/xenctrl.h | 4 +++
tools/libs/ctrl/Makefile | 1 +
tools/libs/ctrl/xc_fpga.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 87 insertions(+)
create mode 100644 tools/libs/ctrl/xc_fpga.c
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index b77726e..d14b3df 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2679,6 +2679,10 @@ 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);
+int xc_domain_add_fpga(xc_interface *xch, void *pfdt, int pdft_size);
+int xc_domain_del_fpga(xc_interface *xch, char *full_dt_node_path);
+
+
/* Compat shims */
#include "xenctrl_compat.h"
diff --git a/tools/libs/ctrl/Makefile b/tools/libs/ctrl/Makefile
index 519246b..95021b9 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_ARM) += xc_fpga.c
SRCS-y += xc_domain.c
SRCS-y += xc_evtchn.c
SRCS-y += xc_gnttab.c
diff --git a/tools/libs/ctrl/xc_fpga.c b/tools/libs/ctrl/xc_fpga.c
new file mode 100644
index 0000000..41c37b5
--- /dev/null
+++ b/tools/libs/ctrl/xc_fpga.c
@@ -0,0 +1,82 @@
+/*
+ *
+ * FPGA 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_domain_add_fpga(xc_interface *xch, void *pfdt, int pfdt_size)
+{
+ int err;
+ DECLARE_DOMCTL;
+
+ DECLARE_HYPERCALL_BOUNCE(pfdt, pfdt_size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+ if ( (err = xc_hypercall_bounce_pre(xch, pfdt)) )
+ goto err;
+
+ domctl.cmd = XEN_DOMCTL_addfpga;
+ /* Adding the device to hardware domain by default. */
+ domctl.domain = 0;
+ domctl.u.fpga_add_dt.pfdt_size = pfdt_size;
+
+ set_xen_guest_handle(domctl.u.fpga_add_dt.pfdt, pfdt);
+
+ if ( (err = do_domctl(xch, &domctl)) != 0 )
+ PERROR("%s failed\n", __func__);
+
+err:
+ xc_hypercall_bounce_post(xch, pfdt);
+
+ return err;
+}
+
+int xc_domain_del_fpga(xc_interface *xch, char *full_dt_node_path)
+{
+ int err;
+ DECLARE_DOMCTL;
+ size_t size = strlen(full_dt_node_path) + 1;
+
+ DECLARE_HYPERCALL_BOUNCE(full_dt_node_path, size,
+ XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+ if ( (err = xc_hypercall_bounce_pre(xch, full_dt_node_path)) )
+ goto err;
+
+ domctl.cmd = XEN_DOMCTL_delfpga;
+ /*
+ * Remove the device from the dt_host, setting hardware domain by
+ * default.
+ */
+ domctl.domain = 0;
+ domctl.u.fpga_del_dt.size = size;
+
+ set_xen_guest_handle(domctl.u.fpga_del_dt.full_dt_node_path,
+ full_dt_node_path);
+
+ if ( (err = do_domctl(xch, &domctl)) != 0 )
+ PERROR("%s failed\n", __func__);
+
+err:
+ xc_hypercall_bounce_post(xch, full_dt_node_path);
+
+ return err;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [XEN][RFC PATCH 12/13] tools/libs/light: Implement new libxl functions for fpga-add and fpga-del
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (10 preceding siblings ...)
2021-09-02 6:06 ` [XEN][RFC PATCH 11/13] tools/libs/ctrl: Implement new xc interfaces for fpga-add and fpga-del Vikram Garhwal
@ 2021-09-02 6:06 ` Vikram Garhwal
2021-09-02 6:06 ` [XEN][RFC PATCH 13/13] tools/xl: Add new xl commands " Vikram Garhwal
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:06 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Ian Jackson, Wei Liu,
Anthony PERARD, Juergen Gross
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
tools/include/libxl.h | 5 +++
tools/libs/light/Makefile | 1 +
tools/libs/light/libxl_fpga.c | 73 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+)
create mode 100644 tools/libs/light/libxl_fpga.c
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index b9ba16d..896cbcf 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -2386,6 +2386,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);
+/* FPGA device add. */
+int libxl_add_fpga_node(libxl_ctx *ctx, void *pfdt, int pfdt_size);
+/* FPGA device remove. */
+int libxl_del_fpga_node(libxl_ctx *ctx, char *full_dt_node_path);
+
/*
* 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 7d8c51d..b17d4a6 100644
--- a/tools/libs/light/Makefile
+++ b/tools/libs/light/Makefile
@@ -115,6 +115,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_fpga.o
ifeq ($(CONFIG_LIBNL),y)
CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
diff --git a/tools/libs/light/libxl_fpga.c b/tools/libs/light/libxl_fpga.c
new file mode 100644
index 0000000..a33d00f
--- /dev/null
+++ b/tools/libs/light/libxl_fpga.c
@@ -0,0 +1,73 @@
+/*
+ * 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_partial_fdt(libxl__gc *gc, void *fdt, size_t size)
+{
+ int r;
+
+ if (fdt_magic(fdt) != FDT_MAGIC) {
+ LOG(ERROR, "Partial FDT is not a valid Flat Device Tree");
+ return ERROR_FAIL;
+ }
+
+ r = fdt_check_header(fdt);
+ if (r) {
+ LOG(ERROR, "Failed to check the partial FDT (%d)", r);
+ return ERROR_FAIL;
+ }
+
+ if (fdt_totalsize(fdt) > size) {
+ LOG(ERROR, "Partial FDT totalsize is too big");
+ return ERROR_FAIL;
+ }
+
+ return 0;
+}
+
+int libxl_add_fpga_node(libxl_ctx *ctx, void *pfdt, int pfdt_size)
+{
+ int rc = 0;
+ GC_INIT(ctx);
+
+ if (check_partial_fdt(gc, pfdt, pfdt_size)) {
+ LOG(ERROR, "Partial DTB check failed\n");
+ return ERROR_FAIL;
+ } else
+ LOG(DEBUG, "Partial DTB check passed\n");
+
+ /* We don't need to do xc_interface_open here. */
+ rc = xc_domain_add_fpga(ctx->xch, pfdt, pfdt_size);
+
+ if (rc)
+ LOG(ERROR, "%s: Adding partial dtb failed.\n", __func__);
+
+ return rc;
+}
+
+int libxl_del_fpga_node(libxl_ctx *ctx, char *device_path)
+{
+ int rc = 0;
+
+ /* We don't need to do xc_interface_open here. */
+ rc = xc_domain_del_fpga(ctx->xch, device_path);
+
+ return rc;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [XEN][RFC PATCH 13/13] tools/xl: Add new xl commands fpga-add and fpga-del
2021-09-02 6:05 [XEN][RFC PATCH 00/13] Add Support for dynamic Programming Vikram Garhwal
` (11 preceding siblings ...)
2021-09-02 6:06 ` [XEN][RFC PATCH 12/13] tools/libs/light: Implement new libxl functions " Vikram Garhwal
@ 2021-09-02 6:06 ` Vikram Garhwal
12 siblings, 0 replies; 21+ messages in thread
From: Vikram Garhwal @ 2021-09-02 6:06 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, julien, Vikram Garhwal, Ian Jackson, Wei Liu,
Anthony PERARD
Signed-off-by: Vikram Garhwal <fnu.vikram@xilinx.com>
---
tools/xl/xl.h | 2 ++
tools/xl/xl_cmdtable.c | 12 ++++++++++++
tools/xl/xl_vmcontrol.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+)
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 7e23f30..63be31e 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -140,6 +140,8 @@ int main_shutdown(int argc, char **argv);
int main_reboot(int argc, char **argv);
int main_list(int argc, char **argv);
int main_vm_list(int argc, char **argv);
+int main_fpga_add(int argc, char **argv);
+int main_fpga_del(int argc, char **argv);
int main_create(int argc, char **argv);
int main_config_update(int argc, char **argv);
int main_button_press(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 661323d..135fe6a 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -20,6 +20,18 @@
#include "xl.h"
const struct cmd_spec cmd_table[] = {
+ { "fpga-add",
+ &main_fpga_add, 1, 1,
+ "Add a PL block",
+ "<.dtbo>"
+ "-h print this help\n"
+ },
+ { "fpga-del",
+ &main_fpga_del, 1, 1,
+ "Remove a PL block",
+ "<full_node_path>"
+ "-h print this help\n"
+ },
{ "create",
&main_create, 1, 1,
"Create a domain from config file <filename>",
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 435155a..f5bfdbc 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -1262,6 +1262,57 @@ int main_create(int argc, char **argv)
return 0;
}
+int main_fpga_add(int argc, char **argv)
+{
+ const char *fpga_config_file = argv[1];
+ void *pfdt = NULL;
+ int rc;
+ int pfdt_size = 0;
+
+ if (fpga_config_file) {
+ rc = libxl_read_file_contents(ctx, fpga_config_file,
+ &pfdt, &pfdt_size);
+
+ if (rc) {
+ fprintf(stderr, "failed to read the fpga-partial device file %s\n",
+ fpga_config_file);
+ free(pfdt);
+ return ERROR_FAIL;
+ }
+ } else {
+ fprintf(stderr, "FPGA config file is not provided\n");
+ return ERROR_FAIL;
+ }
+
+ rc = libxl_add_fpga_node(ctx, pfdt, pfdt_size);
+ if (rc)
+ fprintf(stderr, "Adding FPGA node failed\n");
+
+ free(pfdt);
+ return rc;
+}
+
+int main_fpga_del(int argc, char **argv)
+{
+ char *full_dt_node_path = argv[1];
+ int rc = 0;
+
+ if (full_dt_node_path) {
+ rc = libxl_del_fpga_node(ctx, full_dt_node_path);
+
+ fprintf(stdout, "fpga-del called for device = %s\n", full_dt_node_path);
+
+ if (rc)
+ fprintf(stderr, "Removing FPGA node failed\n");
+
+ } else {
+ fprintf(stderr, "No device node path provided\n");
+ return ERROR_FAIL;
+ }
+
+ return rc;
+}
+
/*
* Local variables:
* mode: C
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread