On Sat, Sep 21, 2019 at 7:15 PM Bin Meng <bmeng...@gmail.com> wrote: > > On Sat, Sep 21, 2019 at 6:16 AM Alistair Francis <alistai...@gmail.com> wrote: > > > > On Thu, Sep 19, 2019 at 10:15 PM Bin Meng <bmeng...@gmail.com> wrote: > > > > > > On Fri, Sep 20, 2019 at 6:36 AM Alistair Francis > > > <alistair.fran...@wdc.com> wrote: > > > > > > > > Add the CFI01 PFlash to the RISC-V virt board. This is the same PFlash > > > > from the ARM Virt board and the implementation is based on the ARM Virt > > > > board. This allows users to specify flash files from the command line. > > > > > > > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > > > > --- > > > > hw/riscv/Kconfig | 1 + > > > > hw/riscv/virt.c | 81 +++++++++++++++++++++++++++++++++++++++++ > > > > include/hw/riscv/virt.h | 3 ++ > > > > 3 files changed, 85 insertions(+) > > > > > > > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig > > > > index fb19b2df3a..b12660b9f8 100644 > > > > --- a/hw/riscv/Kconfig > > > > +++ b/hw/riscv/Kconfig > > > > @@ -36,4 +36,5 @@ config RISCV_VIRT > > > > select SERIAL > > > > select VIRTIO_MMIO > > > > select PCI_EXPRESS_GENERIC_BRIDGE > > > > + select PFLASH_CFI01 > > > > select SIFIVE > > > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > > > > index d36f5625ec..ca002ecea7 100644 > > > > --- a/hw/riscv/virt.c > > > > +++ b/hw/riscv/virt.c > > > > @@ -26,6 +26,7 @@ > > > > #include "hw/boards.h" > > > > #include "hw/loader.h" > > > > #include "hw/sysbus.h" > > > > +#include "hw/qdev-properties.h" > > > > #include "hw/char/serial.h" > > > > #include "target/riscv/cpu.h" > > > > #include "hw/riscv/riscv_hart.h" > > > > @@ -61,12 +62,72 @@ static const struct MemmapEntry { > > > > [VIRT_PLIC] = { 0xc000000, 0x4000000 }, > > > > [VIRT_UART0] = { 0x10000000, 0x100 }, > > > > [VIRT_VIRTIO] = { 0x10001000, 0x1000 }, > > > > + [VIRT_FLASH] = { 0x20000000, 0x2000000 }, > > > > [VIRT_DRAM] = { 0x80000000, 0x0 }, > > > > [VIRT_PCIE_MMIO] = { 0x40000000, 0x40000000 }, > > > > [VIRT_PCIE_PIO] = { 0x03000000, 0x00010000 }, > > > > [VIRT_PCIE_ECAM] = { 0x30000000, 0x10000000 }, > > > > }; > > > > > > > > +#define VIRT_FLASH_SECTOR_SIZE (256 * KiB) > > > > + > > > > +static PFlashCFI01 *virt_flash_create1(RISCVVirtState *s, > > > > + const char *name, > > > > + const char *alias_prop_name) > > > > +{ > > > > + /* > > > > + * Create a single flash device. We use the same parameters as > > > > + * the flash devices on the ARM virt board. > > > > + */ > > > > + DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01); > > > > + > > > > + qdev_prop_set_uint64(dev, "sector-length", VIRT_FLASH_SECTOR_SIZE); > > > > + qdev_prop_set_uint8(dev, "width", 4); > > > > + qdev_prop_set_uint8(dev, "device-width", 2); > > > > + qdev_prop_set_bit(dev, "big-endian", false); > > > > + qdev_prop_set_uint16(dev, "id0", 0x89); > > > > + qdev_prop_set_uint16(dev, "id1", 0x18); > > > > + qdev_prop_set_uint16(dev, "id2", 0x00); > > > > + qdev_prop_set_uint16(dev, "id3", 0x00); > > > > + qdev_prop_set_string(dev, "name", name); > > > > > > alias_prop_name is unused? ARM virt has 2 more calls in the same function > > > here. > > > > Yep, you are right. I have removed this. > > Any reason of removing this?
The way the virt machine was structured didn't work with the options. I have fixed that in v2 of the series. Alistair > > > > > > > > > > + > > > > + return PFLASH_CFI01(dev); > > > > +} > > > > + > > > > +static void virt_flash_create(RISCVVirtState *s) > > > > +{ > > > > + s->flash[0] = virt_flash_create1(s, "virt.flash0", "pflash0"); > > > > + s->flash[1] = virt_flash_create1(s, "virt.flash1", "pflash1"); > > > > > > I don't think we should mirror what is used on ARM virt board to > > > create 2 flash for sifive_u. For ARM virt, there are 2 flashes because > > > they need distinguish secure and non-secure. For sifive_u, only one is > > > enough. > > > > I went back and forward about 1 or 2. Two seems more usable as maybe > > someone wants to include two pflash files? The Xilinx machine also has > > two so I'm kind of used to 2, but I'm not really fussed. > > > > Unless anyone else wants two I will change it to 1. > > Regards, > Bin