Hi Daniel, Thanks for the review, I will send a V2 addressing the comments.
Regards, Sai Pavan >-----Original Message----- >From: Daniel Henrique Barboza <dbarb...@ventanamicro.com> >Sent: Wednesday, October 16, 2024 1:28 AM >To: Boddu, Sai Pavan <sai.pavan.bo...@amd.com>; qemu-devel@nongnu.org; >qemu-ri...@nongnu.org >Cc: Paolo Bonzini <pbonz...@redhat.com>; Palmer Dabbelt ><pal...@dabbelt.com>; Alistair Francis <alistair.fran...@wdc.com>; Bin Meng ><bmeng...@gmail.com>; Weiwei Li <liwei1...@gmail.com>; Liu Zhiwei ><zhiwei_...@linux.alibaba.com>; Simek, Michal <michal.si...@amd.com> >Subject: Re: [PATCH] hw/riscv: Add Microblaze V 32bit virt board > >Hi, > > >The doc file included in the patch will break the build: > >/home/danielhb/work/qemu/docs/system/riscv/microblaze-v-virt.rst:document isn't >included in any toctree > >You'll need this extra change: > > >$ git diff >diff --git a/docs/system/target-riscv.rst b/docs/system/target-riscv.rst index >ba195f1518..6161f01d21 100644 >--- a/docs/system/target-riscv.rst >+++ b/docs/system/target-riscv.rst >@@ -70,6 +70,7 @@ undocumented; you can get a complete list by running > riscv/shakti-c > riscv/sifive_u > riscv/virt >+ riscv/microblaze-v-virt > > RISC-V CPU firmware > > >More comments below: > >On 10/15/24 10:39 AM, Sai Pavan Boddu 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> >> --- >> MAINTAINERS | 6 + >> docs/system/riscv/microblaze-v-virt.rst | 39 +++++ >> hw/riscv/microblaze-v-virt.c | 182 ++++++++++++++++++++++++ >> hw/riscv/Kconfig | 8 ++ >> hw/riscv/meson.build | 1 + >> 5 files changed, 236 insertions(+) >> create mode 100644 docs/system/riscv/microblaze-v-virt.rst >> create mode 100644 hw/riscv/microblaze-v-virt.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS index d7a11fe6017..b104b6d0f7f >> 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -1274,6 +1274,12 @@ M: Edgar E. Iglesias <edgar.igles...@gmail.com> >> S: Maintained >> F: hw/microblaze/petalogix_ml605_mmu.c >> >> +amd-microblaze-v-virt >> +M: Sai Pavan Boddu <sai.pavan.bo...@amd.com> >> +S: Maintained >> +F: hw/riscv/microblaze-v-virt.c >> +F: docs/system/riscv/microblaze-v-virt.rst >> + >> MIPS Machines >> ------------- >> Overall MIPS Machines >> diff --git a/docs/system/riscv/microblaze-v-virt.rst >> b/docs/system/riscv/microblaze-v-virt.rst >> new file mode 100644 >> index 00000000000..42cac9ac475 >> --- /dev/null >> +++ b/docs/system/riscv/microblaze-v-virt.rst >> @@ -0,0 +1,39 @@ >> +microblaze-v virt board (``amd-microblaze-v-virt``) >> +=================================================== >> +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 RISC-V instruction set >architecture (ISA). >> + >> +More details here: >> +https://docs.amd.com/r/en-US/ug1629-microblaze-v-user-guide/MicroBlaz >> +e-V-Architecture >> + >> +The microblaze-v virt board in QEMU is a virtual board with following >> +supported devices >> + >> +Implemented CPU cores: >> + >> +1 RISCV32 core >> + >> +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-virt \ >> + -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 >> diff --git a/hw/riscv/microblaze-v-virt.c >> b/hw/riscv/microblaze-v-virt.c new file mode 100644 index >> 00000000000..d9cdfe39857 >> --- /dev/null >> +++ b/hw/riscv/microblaze-v-virt.c >> @@ -0,0 +1,182 @@ >> +/* >> + * QEMU model of Microblaze V (32bit version) >> + * >> + * 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_virt_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(TYPE_RISCV_CPU_BASE32)); >> + object_property_set_bool(OBJECT(cpu), "i", true, NULL); >> + object_property_set_bool(OBJECT(cpu), "m", true, NULL); >> + object_property_set_bool(OBJECT(cpu), "a", true, NULL); >> + object_property_set_bool(OBJECT(cpu), "c", true, NULL); >> + qdev_realize(DEVICE(cpu), NULL, &error_abort); > > >We usually do not instantiate the CPU in the middle of the board code. The >idea is >that the board must be able to deal with the '-cpu' option that the user might >pass via >command line, i.e. a board can run with multiple CPU types. > >But the board can determine CPU defaults like the default cpu type, number of >cpus >and so on in the machine class init. In your case, mb_v_virt_machine_init(). > >For example, if you want this board to have only one CPU of the 'rv32' type: > > > mc->max_cpus = 1; > mc->min_cpus = 1; > mc->default_cpu_type = TYPE_RISCV_CPU_BASE32; > mc->default_cpus = 1; > > >This will make sure that if no '-cpu' arg is passed the board will have >exactly one >rv32 CPU. > >Note that rv32 will have by default rv32imafdch, i.e more extensions that >you're >setting (imac). If you want the board to not have access to all these >extensions by >default, you can create a new CPU type in target/riscv/cpu.c and use it as >default_cpu_type for this board. See hw/riscv/sifive_u.c for an example. > >One more thing: > > >> + /* 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_virt_machine_init(MachineClass *mc) { >> + mc->desc = "AMD Microblaze-V Virt platform"; >> + mc->init = mb_v_virt_init; >> + mc->is_default = true; > >Setting mc->is_default will overwrite the current default board for >qemu-system- >riscv32 (spike). I'm not sure if this is your intention here. > > >Thanks, > >Daniel > >> +} >> + >> +DEFINE_MACHINE("amd-microblaze-v-virt", mb_v_virt_machine_init) >> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig index >> 44695ff9f2c..5424803a82e 100644 >> --- a/hw/riscv/Kconfig >> +++ b/hw/riscv/Kconfig >> @@ -22,6 +22,14 @@ config MICROCHIP_PFSOC >> select SIFIVE_PLIC >> select UNIMP >> >> +config MICROBLAZE_V >> + bool >> + default y >> + depends on RISCV32 >> + 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 >> f872674093a..8ea0412a376 100644 >> --- a/hw/riscv/meson.build >> +++ b/hw/riscv/meson.build >> @@ -10,5 +10,6 @@ riscv_ss.add(when: 'CONFIG_SIFIVE_U', if_true: >files('sifive_u.c')) >> 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_MICROBLAZE_V', if_true: >> +files('microblaze-v-virt.c')) >> >> hw_arch += {'riscv': riscv_ss}
