Hi Edgar, From: Boddu, Sai Pavan Sent: Friday, June 14, 2024 8:37 PM To: Edgar E. Iglesias <edgar.igles...@gmail.com> Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org; Alistair Francis <alist...@alistair23.me>; Peter Maydell <peter.mayd...@linaro.org>; Iglesias, Francisco <francisco.igles...@amd.com> Subject: RE: [PATCH 2/2] hw/arm/xilinx_zynq: Add boot-mode property
Hi Edgar, I examined -boot switch to configure boot mode. It would become little complex to map zynq boot devices to below expected mapping. * a-b: floppy disk drives * c-f: IDE disk drives * g-m: machine implementation dependent drives * n-p: network devices With above options, we may need to use “g-m” flags for our boot modes, which becomes a little confusing. So in order to make it user friendly, I would convert the proposed boot-mode property to accept strings like “jtag/qspi/sd”. Regards, Sai Pavan From: Edgar E. Iglesias <edgar.igles...@gmail.com<mailto:edgar.igles...@gmail.com>> Sent: Friday, June 14, 2024 4:38 PM To: Boddu, Sai Pavan <sai.pavan.bo...@amd.com<mailto:sai.pavan.bo...@amd.com>> Cc: qemu-...@nongnu.org<mailto:qemu-...@nongnu.org>; qemu-devel@nongnu.org<mailto:qemu-devel@nongnu.org>; Alistair Francis <alist...@alistair23.me<mailto:alist...@alistair23.me>>; Peter Maydell <peter.mayd...@linaro.org<mailto:peter.mayd...@linaro.org>>; Iglesias, Francisco <francisco.igles...@amd.com<mailto:francisco.igles...@amd.com>> Subject: Re: [PATCH 2/2] hw/arm/xilinx_zynq: Add boot-mode property On Thu, Jun 13, 2024 at 5:36 PM Sai Pavan Boddu <sai.pavan.bo...@amd.com<mailto:sai.pavan.bo...@amd.com>> wrote: Read boot-mode value as machine property and propagate that to SLCR.BOOT_MODE register. Hi Sai, Directly exposing the register field to the user to set on the command-line probably makes usability a little too rough (user has to check the register specs in the TRM to change boot-mode). We could perhaps add friendly names that we internally map to the register field values. Another question, can we use the existing -boot command-line arg for this? Something along the lines of what x86 PC does: https://github.com/qemu/qemu/blob/master/hw/i386/pc.c#L395 I don't know if the framework allows for long names but something like the following would be nice: qemu -boot spi,ethernet,jtag,uart,etc [Boddu, Sai Pavan] Ok it makes much sense, Otherwise I would add more details in the description of the new property about the possible boot modes. Would also be great to document a small example, perhaps in https://github.com/qemu/qemu/tree/master/docs/system/arm [Boddu, Sai Pavan] Sure. Regards, Sai Pavan Best regards, Edgar Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@amd.com<mailto:sai.pavan.bo...@amd.com>> --- hw/arm/xilinx_zynq.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index 7f7a3d23fb..4dfa9184ac 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -38,6 +38,7 @@ #include "qom/object.h" #include "exec/tswap.h" #include "target/arm/cpu-qom.h" +#include "qapi/visitor.h" #define TYPE_ZYNQ_MACHINE MACHINE_TYPE_NAME("xilinx-zynq-a9") OBJECT_DECLARE_SIMPLE_TYPE(ZynqMachineState, ZYNQ_MACHINE) @@ -90,6 +91,7 @@ struct ZynqMachineState { MachineState parent; Clock *ps_clk; ARMCPU *cpu[ZYNQ_MAX_CPUS]; + uint8_t BootMode; }; static void zynq_write_board_setup(ARMCPU *cpu, @@ -176,6 +178,19 @@ static inline int zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, return unit; } +static void zynq_set_boot_mode(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ + ZynqMachineState *m = ZYNQ_MACHINE(obj); + uint8_t val; + + if (!visit_type_uint8(v, name, &val, errp)) { + return; + } + m->BootMode = val; +} + static void zynq_init(MachineState *machine) { ZynqMachineState *zynq_machine = ZYNQ_MACHINE(machine); @@ -241,6 +256,7 @@ static void zynq_init(MachineState *machine) /* Create slcr, keep a pointer to connect clocks */ slcr = qdev_new("xilinx-zynq_slcr"); qdev_connect_clock_in(slcr, "ps_clk", zynq_machine->ps_clk); + qdev_prop_set_uint8(slcr, "boot-mode", zynq_machine->BootMode); sysbus_realize_and_unref(SYS_BUS_DEVICE(slcr), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF8000000); @@ -372,6 +388,7 @@ static void zynq_machine_class_init(ObjectClass *oc, void *data) NULL }; MachineClass *mc = MACHINE_CLASS(oc); + ObjectProperty *prop; mc->desc = "Xilinx Zynq Platform Baseboard for Cortex-A9"; mc->init = zynq_init; mc->max_cpus = ZYNQ_MAX_CPUS; @@ -379,6 +396,11 @@ static void zynq_machine_class_init(ObjectClass *oc, void *data) mc->ignore_memory_transaction_failures = true; mc->valid_cpu_types = valid_cpu_types; mc->default_ram_id = "zynq.ext_ram"; + prop = object_class_property_add(oc, "boot-mode", "uint8_t", NULL, + zynq_set_boot_mode, NULL, NULL); + object_class_property_set_description(oc, "boot-mode", + "Update SLCR.BOOT_MODE register"); + object_property_set_default_uint(prop, 1); } static const TypeInfo zynq_machine_type = { -- 2.34.1