| Message ID | 20230104220449.41337-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | hw: Remove implicit sysbus_mmio_map() from pflash APIs | expand |
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!
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(-)