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
>>

Reply via email to