Paving the road toward heterogeneous QEMU, the limitations of
having a single machine sysbus become more apparent.

The sysbus_mmio_map() API forces the caller to map a sysbus
device to an address on the system bus (system bus here is
the root MemoryRegion returned by get_system_memory() ).

This is not practical when each core has its own address
space and group of cores have access to a part of the
peripherals.

Experimenting with the PFLASH devices. Here the fix is
quite easy, we split the pflash_cfi_register() -- which
does the implicit sysbus mapping -- into an explicit qdev
pflash_cfi_create() followed by the sysbus_mmio_map() call.

Since we were touching the PFLASH API, we restricted the
PFlashCFI0X structures to their models. The API now deals
with a generic qdev pointer (DeviceState*).

First 15 patches deal with the CFI type 1 model, then the
last 5 with the type 2.

The patch logic is mostly:

- extract pflash_cfi_create() from pflash_cfi_register()
- open-code pflash_cfi_register() for each hw/${ARCH}/
- remove unused pflash_cfi_register()
- reduce PFlashCFI0x structure scope

Please review,

Phil.

Philippe Mathieu-Daudé (20):
  hw/block: Pass DeviceState to pflash_cfi01_get_blk()
  hw/block: Use pflash_cfi01_get_blk() in pflash_cfi01_legacy_drive()
  hw/block: Pass DeviceState to pflash_cfi01_get_memory()
  hw/arm: Use generic DeviceState instead of PFlashCFI01
  hw/loongarch: Use generic DeviceState instead of PFlashCFI01
  hw/riscv: Use generic DeviceState instead of PFlashCFI01
  hw/i386: Use generic DeviceState instead of PFlashCFI01
  hw/xtensa: Use generic DeviceState instead of PFlashCFI01
  hw/block: Factor pflash_cfi01_create() out of pflash_cfi01_register()
  hw/arm: Open-code pflash_cfi01_register()
  hw/microblaze: Open-code pflash_cfi01_register()
  hw/mips: Open-code pflash_cfi01_register()
  hw/ppc: Open-code pflash_cfi01_register()
  hw/block: Remove unused pflash_cfi01_register()
  hw/block: Make PFlashCFI01 QOM declaration internal
  hw/block: Factor pflash_cfi02_create() out of pflash_cfi02_register()
  hw/arm: Open-code pflash_cfi02_register()
  hw/sh4: Open-code pflash_cfi02_register()
  hw/block: Remove unused pflash_cfi02_register()
  hw/block: Make PFlashCFI02 QOM declaration internal

 hw/arm/collie.c                          | 15 ++++---
 hw/arm/digic_boards.c                    | 14 +++---
 hw/arm/gumstix.c                         | 19 +++++---
 hw/arm/mainstone.c                       | 13 +++---
 hw/arm/musicpal.c                        | 13 +++---
 hw/arm/omap_sx1.c                        | 22 +++++----
 hw/arm/sbsa-ref.c                        |  8 ++--
 hw/arm/versatilepb.c                     | 13 +++---
 hw/arm/vexpress.c                        | 12 +++--
 hw/arm/virt.c                            |  6 +--
 hw/arm/xilinx_zynq.c                     | 10 ++---
 hw/arm/z2.c                              | 10 +++--
 hw/block/pflash_cfi01.c                  | 35 ++++++++-------
 hw/block/pflash_cfi02.c                  | 25 +++++------
 hw/i386/pc_sysfw.c                       |  6 +--
 hw/loongarch/virt.c                      |  9 ++--
 hw/microblaze/petalogix_ml605_mmu.c      |  8 ++--
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  8 ++--
 hw/mips/malta.c                          | 13 +++---
 hw/ppc/e500.c                            |  2 +-
 hw/ppc/sam460ex.c                        | 12 +++--
 hw/ppc/virtex_ml507.c                    |  7 +--
 hw/riscv/virt.c                          |  7 +--
 hw/sh4/r2d.c                             |  9 ++--
 hw/xtensa/xtfpga.c                       |  6 +--
 include/hw/arm/virt.h                    |  3 +-
 include/hw/block/flash.h                 | 57 ++++++++++++------------
 include/hw/i386/pc.h                     |  3 +-
 include/hw/loongarch/virt.h              |  3 +-
 include/hw/riscv/virt.h                  |  3 +-
 30 files changed, 200 insertions(+), 171 deletions(-)

-- 
2.38.1

On Wed, 4 Jan 2023 at 22:04, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Paving the road toward heterogeneous QEMU, the limitations of
> having a single machine sysbus become more apparent.
>
> The sysbus_mmio_map() API forces the caller to map a sysbus
> device to an address on the system bus (system bus here is
> the root MemoryRegion returned by get_system_memory() ).
>
> This is not practical when each core has its own address
> space and group of cores have access to a part of the
> peripherals.
>
> Experimenting with the PFLASH devices. Here the fix is
> quite easy, we split the pflash_cfi_register() -- which
> does the implicit sysbus mapping -- into an explicit qdev
> pflash_cfi_create() followed by the sysbus_mmio_map() call.

pflash_cfi_register() is a legacy convenience function. If
you don't like the sysbus_mmio_map() it does then you can
create, configure, realize and map the device directly.
This is what hw/arm/virt.c does, for instance (it wants to
map the flash devices into either secure or non secure RAM).
(This also lets you embed the device struct into some other
struct if you want rather than using qdev_new(), though
we don't have any code that does that currently.)

thanks
-- PMM

On 6/1/23 18:51, Peter Maydell wrote:
> On Wed, 4 Jan 2023 at 22:04, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Paving the road toward heterogeneous QEMU, the limitations of
>> having a single machine sysbus become more apparent.
>>
>> The sysbus_mmio_map() API forces the caller to map a sysbus
>> device to an address on the system bus (system bus here is
>> the root MemoryRegion returned by get_system_memory() ).
>>
>> This is not practical when each core has its own address
>> space and group of cores have access to a part of the
>> peripherals.
>>
>> Experimenting with the PFLASH devices. Here the fix is
>> quite easy, we split the pflash_cfi_register() -- which
>> does the implicit sysbus mapping -- into an explicit qdev
>> pflash_cfi_create() followed by the sysbus_mmio_map() call.
> 
> pflash_cfi_register() is a legacy convenience function. If
> you don't like the sysbus_mmio_map() it does then you can
> create, configure, realize and map the device directly.
> This is what hw/arm/virt.c does, for instance (it wants to
> map the flash devices into either secure or non secure RAM).

Good point, thanks!