Re: [PATCH v5 13/15] hw/riscv/device-common, virt.c: add flash helpers

[Chao Liu via qemu development <qemu-devel@nongnu.org>]

On Fri, Aug 28, 2026 at 05:39:15PM +0800, Daniel Henrique Barboza wrote:
> Move the flash device creating and flash map initialization to
> device-common.c.
> 
> Signed-off-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Reviewed-by: Chao Liu <chao.liu@processmission.com>

Thanks,
Chao

> ---
>  hw/riscv/device-common.c         | 44 ++++++++++++++++++++++++
>  hw/riscv/virt.c                  | 57 +++++---------------------------
>  include/hw/riscv/device-common.h |  6 ++++
>  3 files changed, 58 insertions(+), 49 deletions(-)
> 
> diff --git a/hw/riscv/device-common.c b/hw/riscv/device-common.c
> index d3c68a9e71..a054cd7114 100644
> --- a/hw/riscv/device-common.c
> +++ b/hw/riscv/device-common.c
> @@ -10,11 +10,13 @@
>  
>  #include "hw/riscv/device-common.h"
>  
> +#include "hw/block/flash.h"
>  #include "hw/core/platform-bus.h"
>  #include "hw/core/qdev.h"
>  #include "hw/core/qdev-properties.h"
>  #include "hw/core/sysbus.h"
>  #include "qapi/error.h"
> +#include "qom/object.h"
>  #include "system/address-spaces.h"
>  #include "system/memory.h"
>  
> @@ -46,3 +48,45 @@ DeviceState *riscv_create_platform_bus(DeviceState *irqchip,
>  
>      return dev;
>  }
> +
> +PFlashCFI01 *riscv_flash_create(Object *parent, const char *name,
> +                                const char *alias_prop_name,
> +                                int flash_sector_size)
> +{
> +    /*
> +     * Create a single flash device.  We use the same parameters as
> +     * the flash devices on the ARM virt board.
> +     */
> +    DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01);
> +
> +    qdev_prop_set_uint64(dev, "sector-length", 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);
> +
> +    object_property_add_child(parent, name, OBJECT(dev));
> +    object_property_add_alias(parent, alias_prop_name,
> +                              OBJECT(dev), "drive");
> +
> +    return PFLASH_CFI01(dev);
> +}
> +
> +void riscv_init_flash_map(PFlashCFI01 *flash, hwaddr base, hwaddr size,
> +                          MemoryRegion *sysmem, int flash_sector_size)
> +{
> +    DeviceState *dev = DEVICE(flash);
> +
> +    assert(QEMU_IS_ALIGNED(size, flash_sector_size));
> +    assert(size / flash_sector_size <= UINT32_MAX);
> +
> +    qdev_prop_set_uint32(dev, "num-blocks", size / flash_sector_size);
> +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> +
> +    memory_region_add_subregion(sysmem, base,
> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
> +}
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index a2671ae341..9ff5beae77 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -120,53 +120,12 @@ static MemMapEntry virt_high_pcie_memmap;
>  
>  #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_new(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);
> -
> -    object_property_add_child(OBJECT(s), name, OBJECT(dev));
> -    object_property_add_alias(OBJECT(s), alias_prop_name,
> -                              OBJECT(dev), "drive");
> -
> -    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");
> -}
> -
> -static void virt_flash_map1(PFlashCFI01 *flash,
> -                            hwaddr base, hwaddr size,
> -                            MemoryRegion *sysmem)
> -{
> -    DeviceState *dev = DEVICE(flash);
> -
> -    assert(QEMU_IS_ALIGNED(size, VIRT_FLASH_SECTOR_SIZE));
> -    assert(size / VIRT_FLASH_SECTOR_SIZE <= UINT32_MAX);
> -    qdev_prop_set_uint32(dev, "num-blocks", size / VIRT_FLASH_SECTOR_SIZE);
> -    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> -
> -    memory_region_add_subregion(sysmem, base,
> -                                sysbus_mmio_get_region(SYS_BUS_DEVICE(dev),
> -                                                       0));
> +    s->flash[0] = riscv_flash_create(OBJECT(s), "virt.flash0", "pflash0",
> +                                     VIRT_FLASH_SECTOR_SIZE);
> +    s->flash[1] = riscv_flash_create(OBJECT(s), "virt.flash1", "pflash1",
> +                                     VIRT_FLASH_SECTOR_SIZE);
>  }
>  
>  static void virt_flash_map(RISCVVirtState *s,
> @@ -175,10 +134,10 @@ static void virt_flash_map(RISCVVirtState *s,
>      hwaddr flashsize = s->memmap[VIRT_FLASH].size / 2;
>      hwaddr flashbase = s->memmap[VIRT_FLASH].base;
>  
> -    virt_flash_map1(s->flash[0], flashbase, flashsize,
> -                    sysmem);
> -    virt_flash_map1(s->flash[1], flashbase + flashsize, flashsize,
> -                    sysmem);
> +    riscv_init_flash_map(s->flash[0], flashbase, flashsize,
> +                         sysmem, VIRT_FLASH_SECTOR_SIZE);
> +    riscv_init_flash_map(s->flash[1], flashbase + flashsize, flashsize,
> +                         sysmem, VIRT_FLASH_SECTOR_SIZE);
>  }
>  
>  static void create_fdt_socket_plic(RISCVVirtState *s,
> diff --git a/include/hw/riscv/device-common.h b/include/hw/riscv/device-common.h
> index 12af031bbf..9f8bc67a1d 100644
> --- a/include/hw/riscv/device-common.h
> +++ b/include/hw/riscv/device-common.h
> @@ -10,10 +10,16 @@
>  #define RISCV_DEVICE_COMMON_H
>  
>  #include "exec/hwaddr.h"
> +#include "hw/block/flash.h"
>  #include "hw/core/qdev.h"
>  
>  DeviceState *riscv_create_platform_bus(DeviceState *irqchip,
>                                         const MemMapEntry *platform_bus_mem,
>                                         int platform_bus_base_irq,
>                                         int platform_bus_num_irqs);
> +PFlashCFI01 *riscv_flash_create(Object *parent, const char *name,
> +                                const char *alias_prop_name,
> +                                int flash_sector_size);
> +void riscv_init_flash_map(PFlashCFI01 *flash, hwaddr base, hwaddr size,
> +                          MemoryRegion *sysmem, int flash_sector_size);
>  #endif
> -- 
> 2.43.0
>