Hi Alistair, Thanks for Review. I will address below comments in V4.
Regards, Sai Pavan >-----Original Message----- >From: Alistair Francis <alistai...@gmail.com> >Sent: Monday, November 18, 2024 11:37 AM >To: Boddu, Sai Pavan <sai.pavan.bo...@amd.com> >Cc: qemu-devel@nongnu.org; qemu-ri...@nongnu.org; Paolo Bonzini ><pbonz...@redhat.com>; Palmer Dabbelt <pal...@dabbelt.com>; Bin Meng ><bmeng...@gmail.com>; Weiwei Li <liwei1...@gmail.com>; Daniel Henrique >Barboza <dbarb...@ventanamicro.com>; Liu Zhiwei ><zhiwei_...@linux.alibaba.com>; Simek, Michal <michal.si...@amd.com>; Philippe >Mathieu-Daudé <phi...@linaro.org>; Iglesias, Francisco ><francisco.igles...@amd.com> >Subject: Re: [PATCH v3] hw/riscv: Add Microblaze V generic board > >On Tue, Nov 5, 2024 at 3:43 AM Sai Pavan Boddu <sai.pavan.bo...@amd.com> >wrote: >> >> Add a basic board with interrupt controller (intc), timer, serial >> (uartlite), small memory called LMB@0 (128kB) and DDR@0x80000000 >> (configured via command line eg. -m 2g). >> This is basic configuration which matches HW generated out of AMD >> Vivado (design tools). But initial configuration is going beyond what >> it is configured by default because validation should be done on other >> configurations too. That's why wire also additional uart16500, axi >> ethernet(with axi dma). >> GPIOs, i2c and qspi is also listed for completeness. >> >> IRQ map is: (addr) >> 0 - timer (0x41c00000) >> 1 - uartlite (0x40600000) >> 2 - i2c (0x40800000) >> 3 - qspi (0x44a00000) >> 4 - uart16550 (0x44a10000) >> 5 - emaclite (0x40e00000) >> 6 - timer2 (0x41c10000) >> 7 - axi emac (0x40c00000) >> 8 - axi dma (0x41e00000) >> 9 - axi dma >> 10 - gpio (0x40000000) >> 11 - gpio2 (0x40010000) >> 12 - gpio3 (0x40020000) >> >> Signed-off-by: Sai Pavan Boddu <sai.pavan.bo...@amd.com> >> Signed-off-by: Michal Simek <michal.si...@amd.com> >> Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com> >> --- >> Changes for V2: >> Make changes to support -cpu switch >> Remove setting of default board >> Include doc to toctree >> Remove setting of 'imac' extensions as they are available by >> default. >> Chages for V3: >> Replace virt with generic >> Update doc with supported riscv extensions >> Change base CPU to TYPE_RISCV_CPU_BASE >> >> >> MAINTAINERS | 6 + >> docs/system/riscv/microblaze-v-generic.rst | 45 +++++ >> docs/system/target-riscv.rst | 1 + >> hw/riscv/microblaze-v-generic.c | 182 +++++++++++++++++++++ >> hw/riscv/Kconfig | 8 + >> hw/riscv/meson.build | 1 + >> 6 files changed, 243 insertions(+) >> create mode 100644 docs/system/riscv/microblaze-v-generic.rst >> create mode 100644 hw/riscv/microblaze-v-generic.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS index 03741d41e58..9830a46d819 >> 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -1290,6 +1290,12 @@ M: Edgar E. Iglesias <edgar.igles...@gmail.com> >> S: Maintained >> F: hw/microblaze/petalogix_ml605_mmu.c >> >> +AMD Microblaze-V Generic Board >> +M: Sai Pavan Boddu <sai.pavan.bo...@amd.com> >> +S: Maintained >> +F: hw/riscv/microblaze-v-generic.c >> +F: docs/system/riscv/microblaze-v-generic.rst > >This should be with the other RISC-V machines > >> + >> MIPS Machines >> ------------- >> Overall MIPS Machines >> diff --git a/docs/system/riscv/microblaze-v-generic.rst >> b/docs/system/riscv/microblaze-v-generic.rst >> new file mode 100644 >> index 00000000000..71e9e655f66 >> --- /dev/null >> +++ b/docs/system/riscv/microblaze-v-generic.rst >> @@ -0,0 +1,45 @@ >> +Microblaze-V generic board (``amd-microblaze-v-generic``) >> +========================================================= >> +The AMD MicroBlaze™ V processor is a soft-core RISC-V processor IP for AMD >adaptive SoCs and FPGAs. >> +The MicroBlaze V processor is based on a 32-bit / 64-bit RISC-V >> +instruction set architecture (ISA) and its fully hardware compatible with >> the >classic MicroBlaze processor. > >I'm not sure "fully hardware compatible" is the right thing to say here as >it's a different >ISA. > >Maybe just say that it works with the existing Microblaze IP > >> + >> +More details here: >> +https://docs.amd.com/r/en-US/ug1629-microblaze-v-user-guide/MicroBlaz >> +e-V-Architecture >> + >> +The microblaze-v generic board in QEMU has following supported >> +devices > >The supported devices should probably be listed here > >> + >> +Implemented CPU cores: >> + >> +1 RISCV core >> + * RV32I base integer instruction set > >Maybe just say RV32I/RV64I and drop the configurable part later on > >> + * "Zicsr" Control and Status register instructions >> + * "Zifencei" instruction-fetch >> + * Configurable features: >> + - RV64I Base Integer Instruction Set >> + - Extensions supported: M, A, C, F, Zba, Zbb, Zbc, Zbs > >Is this configurable? > >> + >> +Implemented devices: >> + >> + - timer >> + - uartlite >> + - uart16550 >> + - emaclite >> + - timer2 >> + - axi emac >> + - axi dma >> + >> +Running >> +""""""" >> +Running U-boot >> + >> +.. code-block:: bash >> + >> + >> + $ qemu-system-riscv32 -M amd-microblaze-v-generic \ >> + -display none \ >> + -device loader,addr=0x80000000,file=u-boot-spl.bin,cpu-num=0 \ >> + -device loader,addr=0x80200000,file=u-boot.img \ >> + -serial mon:stdio \ >> + -device loader,addr=0x83000000,file=system.dtb \ >> + -m 2g > >Are there any details on where these images come from? > >Alistair > >> diff --git a/docs/system/target-riscv.rst >> b/docs/system/target-riscv.rst index ba195f1518a..95457af130b 100644 >> --- a/docs/system/target-riscv.rst >> +++ b/docs/system/target-riscv.rst >> @@ -66,6 +66,7 @@ undocumented; you can get a complete list by running >> .. toctree:: >> :maxdepth: 1 >> >> + riscv/microblaze-v-generic >> riscv/microchip-icicle-kit >> riscv/shakti-c >> riscv/sifive_u >> diff --git a/hw/riscv/microblaze-v-generic.c >> b/hw/riscv/microblaze-v-generic.c new file mode 100644 index >> 00000000000..93d648f902b >> --- /dev/null >> +++ b/hw/riscv/microblaze-v-generic.c >> @@ -0,0 +1,182 @@ >> +/* >> + * QEMU model of Microblaze V generic board. >> + * >> + * based on hw/microblaze/petalogix_ml605_mmu.c >> + * >> + * Copyright (c) 2011 Michal Simek <mon...@monstr.eu> >> + * Copyright (c) 2011 PetaLogix >> + * Copyright (c) 2009 Edgar E. Iglesias. >> + * Copyright (C) 2024, Advanced Micro Devices, Inc. >> + * SPDX-License-Identifier: GPL-2.0-or-later >> + * >> + * Written by Sai Pavan Boddu <sai.pavan.bo...@amd.com >> + * and by Michal Simek <michal.si...@amd.com>. >> + */ >> + >> +#include "qemu/osdep.h" >> +#include "qemu/units.h" >> +#include "qapi/error.h" >> +#include "cpu.h" >> +#include "hw/sysbus.h" >> +#include "sysemu/sysemu.h" >> +#include "net/net.h" >> +#include "hw/boards.h" >> +#include "hw/char/serial-mm.h" >> +#include "exec/address-spaces.h" >> +#include "hw/char/xilinx_uartlite.h" >> +#include "hw/misc/unimp.h" >> + >> +#define LMB_BRAM_SIZE (128 * KiB) >> +#define MEMORY_BASEADDR 0x80000000 >> +#define INTC_BASEADDR 0x41200000 >> +#define TIMER_BASEADDR 0x41c00000 >> +#define TIMER_BASEADDR2 0x41c10000 >> +#define UARTLITE_BASEADDR 0x40600000 >> +#define ETHLITE_BASEADDR 0x40e00000 >> +#define UART16550_BASEADDR 0x44a10000 #define AXIENET_BASEADDR >> +0x40c00000 #define AXIDMA_BASEADDR 0x41e00000 #define >GPIO_BASEADDR >> +0x40000000 #define GPIO_BASEADDR2 0x40010000 #define >GPIO_BASEADDR3 >> +0x40020000 #define I2C_BASEADDR 0x40800000 #define QSPI_BASEADDR >> +0x44a00000 >> + >> +#define TIMER_IRQ 0 >> +#define UARTLITE_IRQ 1 >> +#define UART16550_IRQ 4 >> +#define ETHLITE_IRQ 5 >> +#define TIMER_IRQ2 6 >> +#define AXIENET_IRQ 7 >> +#define AXIDMA_IRQ1 8 >> +#define AXIDMA_IRQ0 9 >> + >> +static void mb_v_generic_init(MachineState *machine) { >> + ram_addr_t ram_size = machine->ram_size; >> + DeviceState *dev, *dma, *eth0; >> + Object *ds, *cs; >> + int i; >> + RISCVCPU *cpu; >> + hwaddr ddr_base = MEMORY_BASEADDR; >> + MemoryRegion *phys_lmb_bram = g_new(MemoryRegion, 1); >> + MemoryRegion *phys_ram = g_new(MemoryRegion, 1); >> + qemu_irq irq[32]; >> + MemoryRegion *sysmem = get_system_memory(); >> + >> + cpu = RISCV_CPU(object_new(machine->cpu_type)); >> + qdev_realize(DEVICE(cpu), NULL, &error_abort); >> + /* Attach emulated BRAM through the LMB. */ >> + memory_region_init_ram(phys_lmb_bram, NULL, >> + "mb_v.lmb_bram", LMB_BRAM_SIZE, >> + &error_fatal); >> + memory_region_add_subregion(sysmem, 0x00000000, phys_lmb_bram); >> + >> + memory_region_init_ram(phys_ram, NULL, "mb_v.ram", >> + ram_size, &error_fatal); >> + memory_region_add_subregion(sysmem, ddr_base, phys_ram); >> + >> + dev = qdev_new("xlnx.xps-intc"); >> + qdev_prop_set_uint32(dev, "kind-of-intr", >> + 1 << UARTLITE_IRQ); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, INTC_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, >> + qdev_get_gpio_in(DEVICE(cpu), 11)); >> + for (i = 0; i < 32; i++) { >> + irq[i] = qdev_get_gpio_in(dev, i); >> + } >> + >> + /* Uartlite */ >> + dev = qdev_new(TYPE_XILINX_UARTLITE); >> + qdev_prop_set_chr(dev, "chardev", serial_hd(0)); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[UARTLITE_IRQ]); >> + >> + /* Full uart */ >> + serial_mm_init(sysmem, UART16550_BASEADDR + 0x1000, 2, >> + irq[UART16550_IRQ], 115200, serial_hd(1), >> + DEVICE_LITTLE_ENDIAN); >> + >> + /* 2 timers at irq 0 @ 100 Mhz. */ >> + dev = qdev_new("xlnx.xps-timer"); >> + qdev_prop_set_uint32(dev, "one-timer-only", 0); >> + qdev_prop_set_uint32(dev, "clock-frequency", 100000000); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ]); >> + >> + /* 2 timers at irq 3 @ 100 Mhz. */ >> + dev = qdev_new("xlnx.xps-timer"); >> + qdev_prop_set_uint32(dev, "one-timer-only", 0); >> + qdev_prop_set_uint32(dev, "clock-frequency", 100000000); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, TIMER_BASEADDR2); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[TIMER_IRQ2]); >> + >> + /* Emaclite */ >> + dev = qdev_new("xlnx.xps-ethernetlite"); >> + qemu_configure_nic_device(dev, true, NULL); >> + qdev_prop_set_uint32(dev, "tx-ping-pong", 0); >> + qdev_prop_set_uint32(dev, "rx-ping-pong", 0); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]); >> + >> + /* axi ethernet and dma initialization. */ >> + eth0 = qdev_new("xlnx.axi-ethernet"); >> + dma = qdev_new("xlnx.axi-dma"); >> + >> + /* FIXME: attach to the sysbus instead */ >> + object_property_add_child(qdev_get_machine(), "xilinx-eth", >> OBJECT(eth0)); >> + object_property_add_child(qdev_get_machine(), "xilinx-dma", >> + OBJECT(dma)); >> + >> + ds = object_property_get_link(OBJECT(dma), >> + "axistream-connected-target", NULL); >> + cs = object_property_get_link(OBJECT(dma), >> + "axistream-control-connected-target", >> NULL); >> + qemu_configure_nic_device(eth0, true, NULL); >> + qdev_prop_set_uint32(eth0, "rxmem", 0x1000); >> + qdev_prop_set_uint32(eth0, "txmem", 0x1000); >> + object_property_set_link(OBJECT(eth0), "axistream-connected", ds, >> + &error_abort); >> + object_property_set_link(OBJECT(eth0), "axistream-control-connected", >> cs, >> + &error_abort); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(eth0), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(eth0), 0, AXIENET_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(eth0), 0, irq[AXIENET_IRQ]); >> + >> + ds = object_property_get_link(OBJECT(eth0), >> + "axistream-connected-target", NULL); >> + cs = object_property_get_link(OBJECT(eth0), >> + "axistream-control-connected-target", >> NULL); >> + qdev_prop_set_uint32(dma, "freqhz", 100000000); >> + object_property_set_link(OBJECT(dma), "axistream-connected", ds, >> + &error_abort); >> + object_property_set_link(OBJECT(dma), "axistream-control-connected", cs, >> + &error_abort); >> + sysbus_realize_and_unref(SYS_BUS_DEVICE(dma), &error_fatal); >> + sysbus_mmio_map(SYS_BUS_DEVICE(dma), 0, AXIDMA_BASEADDR); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dma), 0, irq[AXIDMA_IRQ0]); >> + sysbus_connect_irq(SYS_BUS_DEVICE(dma), 1, irq[AXIDMA_IRQ1]); >> + >> + /* unimplemented devices */ >> + create_unimplemented_device("gpio", GPIO_BASEADDR, 0x10000); >> + create_unimplemented_device("gpio2", GPIO_BASEADDR2, 0x10000); >> + create_unimplemented_device("gpio3", GPIO_BASEADDR3, 0x10000); >> + create_unimplemented_device("i2c", I2C_BASEADDR, 0x10000); >> + create_unimplemented_device("qspi", QSPI_BASEADDR, 0x10000); } >> + >> +static void mb_v_generic_machine_init(MachineClass *mc) { >> + mc->desc = "AMD Microblaze-V generic platform"; >> + mc->init = mb_v_generic_init; >> + mc->min_cpus = 1; >> + mc->max_cpus = 1; >> + mc->default_cpu_type = TYPE_RISCV_CPU_BASE; >> + mc->default_cpus = 1; >> +} >> + >> +DEFINE_MACHINE("amd-microblaze-v-generic", mb_v_generic_machine_init) >> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig index >> 2e88467c4ab..e6a0ac1fa1d 100644 >> --- a/hw/riscv/Kconfig >> +++ b/hw/riscv/Kconfig >> @@ -25,6 +25,14 @@ config MICROCHIP_PFSOC >> select SIFIVE_PLIC >> select UNIMP >> >> +config MICROBLAZE_V >> + bool >> + default y >> + depends on RISCV32 || RISCV64 >> + select XILINX >> + select XILINX_AXI >> + select XILINX_ETHLITE >> + >> config OPENTITAN >> bool >> default y >> diff --git a/hw/riscv/meson.build b/hw/riscv/meson.build index >> adbef8a9b2d..140bcb55d64 100644 >> --- a/hw/riscv/meson.build >> +++ b/hw/riscv/meson.build >> @@ -11,5 +11,6 @@ riscv_ss.add(when: 'CONFIG_SPIKE', if_true: >> files('spike.c')) >> riscv_ss.add(when: 'CONFIG_MICROCHIP_PFSOC', if_true: >> files('microchip_pfsoc.c')) >> riscv_ss.add(when: 'CONFIG_ACPI', if_true: >> files('virt-acpi-build.c')) >> riscv_ss.add(when: 'CONFIG_RISCV_IOMMU', if_true: >> files('riscv-iommu.c', 'riscv-iommu-pci.c')) >> +riscv_ss.add(when: 'CONFIG_MICROBLAZE_V', if_true: >> +files('microblaze-v-generic.c')) >> >> hw_arch += {'riscv': riscv_ss} >> -- >> 2.34.1 >>