| Message ID | 20200419162727.19148-4-edgar.iglesias@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes | expand |
On Sun, Apr 19, 2020 at 9:27 AM Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Disable unsupported FDT firmware nodes if a user passes us > a DTB with nodes enabled that the machine cannot support > due to lack of EL3 or EL2 support. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/arm/xlnx-zcu102.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c > index 4eb117c755..3332630380 100644 > --- a/hw/arm/xlnx-zcu102.c > +++ b/hw/arm/xlnx-zcu102.c > @@ -23,6 +23,7 @@ > #include "qemu/error-report.h" > #include "qemu/log.h" > #include "sysemu/qtest.h" > +#include "sysemu/device_tree.h" > > typedef struct XlnxZCU102 { > MachineState parent_obj; > @@ -68,6 +69,35 @@ static void zcu102_set_virt(Object *obj, bool value, Error **errp) > s->virt = value; > } > > +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt) > +{ > + XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo); > + bool method_is_hvc; > + char **node_path; > + const char *r; > + int prop_len; > + int i; > + > + /* If EL3 is enabled, we keep all firmware nodes active. */ > + if (!s->secure) { > + node_path = qemu_fdt_node_path(fdt, NULL, > + (char *)"xlnx,zynqmp-firmware", > + &error_fatal); > + > + for (i = 0; node_path && node_path[i]; i++) { > + r = qemu_fdt_getprop(fdt, node_path[i], "method", &prop_len, NULL); > + method_is_hvc = r && !strcmp("hvc", r); > + > + /* Allow HVC based firmware if EL2 is enabled. */ > + if (method_is_hvc && s->virt) { > + continue; > + } > + qemu_fdt_setprop_string(fdt, node_path[i], "status", "disabled"); > + } > + g_strfreev(node_path); > + } > +} > + > static void xlnx_zcu102_init(MachineState *machine) > { > XlnxZCU102 *s = ZCU102_MACHINE(machine); > @@ -169,6 +199,7 @@ static void xlnx_zcu102_init(MachineState *machine) > > s->binfo.ram_size = ram_size; > s->binfo.loader_start = 0; > + s->binfo.modify_dtb = zcu102_modify_dtb; > arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo); > } > > -- > 2.20.1 > >
On Sun, 19 Apr 2020 at 17:27, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Disable unsupported FDT firmware nodes if a user passes us > a DTB with nodes enabled that the machine cannot support > due to lack of EL3 or EL2 support. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > --- > hw/arm/xlnx-zcu102.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt) > +{ > + XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo); > + bool method_is_hvc; > + char **node_path; > + const char *r; > + int prop_len; > + int i; > + > + /* If EL3 is enabled, we keep all firmware nodes active. */ > + if (!s->secure) { > + node_path = qemu_fdt_node_path(fdt, NULL, > + (char *)"xlnx,zynqmp-firmware", > + &error_fatal); Why do we need the 'char *' cast ? thanks -- PMM
On Thu, Apr 23, 2020 at 12:21:11PM +0100, Peter Maydell wrote: > On Sun, 19 Apr 2020 at 17:27, Edgar E. Iglesias > <edgar.iglesias@gmail.com> wrote: > > > > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > > Disable unsupported FDT firmware nodes if a user passes us > > a DTB with nodes enabled that the machine cannot support > > due to lack of EL3 or EL2 support. > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> > > --- > > hw/arm/xlnx-zcu102.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt) > > +{ > > + XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo); > > + bool method_is_hvc; > > + char **node_path; > > + const char *r; > > + int prop_len; > > + int i; > > + > > + /* If EL3 is enabled, we keep all firmware nodes active. */ > > + if (!s->secure) { > > + node_path = qemu_fdt_node_path(fdt, NULL, > > + (char *)"xlnx,zynqmp-firmware", > > + &error_fatal); > > Why do we need the 'char *' cast ? Without it, I see the following warning but compat in qemu_fdt_node_path should probably be changed to const char *. I can make that change in a v2 if you prefer. CC aarch64-softmmu/hw/arm/xlnx-zcu102.o /home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c: In function ‘zcu102_modify_dtb’: /home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c:84:40: error: passing argument 3 of ‘qemu_fdt_node_path’ discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers] 84 | "xlnx,zynqmp-firmware", | ^~~~~~~~~~~~~~~~~~~~~~ In file included from /home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c:26: /home/edgar/src/c/qemu/qemu/include/sysemu/device_tree.h:46:8: note: expected ‘char *’ but argument is of type ‘const char *’ 46 | char **qemu_fdt_node_path(void *fdt, const char *name, char *compat, | ^~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make[1]: *** [/home/edgar/src/c/qemu/qemu/rules.mak:69: hw/arm/xlnx-zcu102.o] Error 1 make: *** [Makefile:527: aarch64-softmmu/all] Error 2 Cheers, Edgar
On Thu, 23 Apr 2020 at 12:43, Edgar E. Iglesias <edgar.iglesias@xilinx.com> wrote: > Without it, I see the following warning but compat in > qemu_fdt_node_path should probably be changed to const char *. > I can make that change in a v2 if you prefer. Yes, I think that would be better. I can't see any reason why the compat argument needs to be non-const. thanks -- PMM
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index 4eb117c755..3332630380 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -23,6 +23,7 @@ #include "qemu/error-report.h" #include "qemu/log.h" #include "sysemu/qtest.h" +#include "sysemu/device_tree.h" typedef struct XlnxZCU102 { MachineState parent_obj; @@ -68,6 +69,35 @@ static void zcu102_set_virt(Object *obj, bool value, Error **errp) s->virt = value; } +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt) +{ + XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo); + bool method_is_hvc; + char **node_path; + const char *r; + int prop_len; + int i; + + /* If EL3 is enabled, we keep all firmware nodes active. */ + if (!s->secure) { + node_path = qemu_fdt_node_path(fdt, NULL, + (char *)"xlnx,zynqmp-firmware", + &error_fatal); + + for (i = 0; node_path && node_path[i]; i++) { + r = qemu_fdt_getprop(fdt, node_path[i], "method", &prop_len, NULL); + method_is_hvc = r && !strcmp("hvc", r); + + /* Allow HVC based firmware if EL2 is enabled. */ + if (method_is_hvc && s->virt) { + continue; + } + qemu_fdt_setprop_string(fdt, node_path[i], "status", "disabled"); + } + g_strfreev(node_path); + } +} + static void xlnx_zcu102_init(MachineState *machine) { XlnxZCU102 *s = ZCU102_MACHINE(machine); @@ -169,6 +199,7 @@ static void xlnx_zcu102_init(MachineState *machine) s->binfo.ram_size = ram_size; s->binfo.loader_start = 0; + s->binfo.modify_dtb = zcu102_modify_dtb; arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo); }