mbox series

[v2,00/12] hw: Strengthen SysBus & QBus API

Message ID 20231019071611.98885-1-philmd@linaro.org
Headers show
Series hw: Strengthen SysBus & QBus API | expand

Message

Philippe Mathieu-Daudé Oct. 19, 2023, 7:15 a.m. UTC
Hi,

This series ensure:

- qbus_new() and sysbus_init_mmio() are called *before*
  a device is realized,
- sysbus_mmio_map() is called *after* it is realized.

First we fix some abuse, then we enforce in qdev/sysbus
core code.

There is still a failure in PXA2xx:

  qemu-system-aarch64: sysbus_init_mmio(type:pxa2xx_pic) but object is realized

Apparently it cames from commit 3f6c925f37 ("Use i2c_slave_init() to
allocate the PXA (dummy) I2C slave"), which I presume was how to model
slave<->master transactions *before* I2C bus modelling.

Based-on: <20231018133059.85765-1-philmd@linaro.org>
          "hw/ppc: SysBus simplifications" [1]
Based-on: <20231018131220.84380-1-philmd@linaro.org>
          "hw/arm/pxa2xx: SysBus/QDev fixes" [2]

v1: https://lore.kernel.org/qemu-devel/20231018141151.87466-1-philmd@linaro.org/
[1] https://lore.kernel.org/qemu-devel/20231018133059.85765-1-philmd@linaro.org/
[2] https://lore.kernel.org/qemu-devel/20231018131220.84380-1-philmd@linaro.org/

Philippe Mathieu-Daudé (12):
  hw/i386/amd_iommu: Do not use SysBus API to map local MMIO region
  hw/i386/intel_iommu: Do not use SysBus API to map local MMIO region
  hw/misc/allwinner-dramc: Move sysbus_mmio_map call from init ->
    realize
  hw/misc/allwinner-dramc: Do not use SysBus API to map local MMIO
    region
  hw/pci-host/bonito: Do not use SysBus API to map local MMIO region
  hw/acpi: Realize ACPI_GED sysbus device before accessing it
  hw/arm/virt: Realize ARM_GICV2M sysbus device before accessing it
  hw/isa: Realize ISA BUS sysbus device before accessing it
  hw/s390x/css-bridge: Realize sysbus device before accessing it
  hw/qdev: Ensure parent device is not realized before adding bus
  hw/sysbus: Ensure device is not realized before adding MMIO region
  hw/sysbus: Ensure device is realized before mapping it

 hw/arm/virt.c                 |  5 ++---
 hw/core/bus.c                 |  7 +++++++
 hw/core/sysbus.c              | 13 +++++++++++++
 hw/i386/amd_iommu.c           |  5 ++---
 hw/i386/intel_iommu.c         |  5 ++---
 hw/i386/microvm.c             |  2 +-
 hw/isa/isa-bus.c              | 11 +++++++++--
 hw/loongarch/virt.c           |  2 +-
 hw/misc/allwinner-r40-dramc.c | 20 +++++++++-----------
 hw/pci-host/bonito.c          | 32 ++++++++++++++++----------------
 hw/s390x/css-bridge.c         |  7 +++----
 11 files changed, 65 insertions(+), 44 deletions(-)

Comments

Thomas Huth Oct. 19, 2023, 7:32 a.m. UTC | #1
On 19/10/2023 09.15, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series ensure:
> 
> - qbus_new() and sysbus_init_mmio() are called *before*
>    a device is realized,
> - sysbus_mmio_map() is called *after* it is realized.

Just an additional idea that came up while reading your series:
I think we should also add proper function descriptions for 
sysbus_init_mmio() and sysbus_mmio_map() in include/hw/sysbus.h, to make 
people aware that the first function should be used within the device to 
expose the mmio region, while the latter should be used in the machine code 
that wires the devices to the correct location in the sysbus memory. What do 
you think?

  Thomas
Philippe Mathieu-Daudé Oct. 19, 2023, 7:35 a.m. UTC | #2
On 19/10/23 09:32, Thomas Huth wrote:
> On 19/10/2023 09.15, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This series ensure:
>>
>> - qbus_new() and sysbus_init_mmio() are called *before*
>>    a device is realized,
>> - sysbus_mmio_map() is called *after* it is realized.
> 
> Just an additional idea that came up while reading your series:
> I think we should also add proper function descriptions for 
> sysbus_init_mmio() and sysbus_mmio_map() in include/hw/sysbus.h, to make 
> people aware that the first function should be used within the device to 
> expose the mmio region, while the latter should be used in the machine 
> code that wires the devices to the correct location in the sysbus 
> memory. What do you think?

I've been thinking about this, but I plan to eventually merge these
calls to the QDev API (after removing more unuseful / duplicated
SysBus functions). So I was planning to document there. But yeah,
better to start doing that now.
Philippe Mathieu-Daudé Oct. 19, 2023, 9:40 p.m. UTC | #3
On 19/10/23 09:15, Philippe Mathieu-Daudé wrote:

> Philippe Mathieu-Daudé (12):
>    hw/i386/amd_iommu: Do not use SysBus API to map local MMIO region
>    hw/i386/intel_iommu: Do not use SysBus API to map local MMIO region
>    hw/misc/allwinner-dramc: Move sysbus_mmio_map call from init ->
>      realize
>    hw/misc/allwinner-dramc: Do not use SysBus API to map local MMIO
>      region
>    hw/pci-host/bonito: Do not use SysBus API to map local MMIO region
>    hw/acpi: Realize ACPI_GED sysbus device before accessing it
>    hw/arm/virt: Realize ARM_GICV2M sysbus device before accessing it
>    hw/isa: Realize ISA BUS sysbus device before accessing it
>    hw/s390x/css-bridge: Realize sysbus device before accessing it

Patches 1-9 queued to hw-misc.