mbox series

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

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

Message

Philippe Mathieu-Daudé Oct. 18, 2023, 2:11 p.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.

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          | 29 ++++++++++++++---------------
 hw/s390x/css-bridge.c         |  7 +++----
 11 files changed, 63 insertions(+), 43 deletions(-)

Comments

Michael S. Tsirkin Oct. 18, 2023, 4:11 p.m. UTC | #1
On Wed, Oct 18, 2023 at 04:11:38PM +0200, 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.
> 
> First we fix some abuse, then we enforce in qdev/sysbus
> core code.


pc things:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> 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          | 29 ++++++++++++++---------------
>  hw/s390x/css-bridge.c         |  7 +++----
>  11 files changed, 63 insertions(+), 43 deletions(-)
> 
> -- 
> 2.41.0
Thomas Huth Oct. 18, 2023, 4:24 p.m. UTC | #2
On 18/10/2023 16.11, 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.
> 
> First we fix some abuse, then we enforce in qdev/sysbus
> core code.

I like the idea, and just had a try with "make check-qtest" with the patches 
here, but seems like there are more spots that need attention:

  10/433 qemu:qtest+qtest-ppc64 / qtest-ppc64/qom-test 
              ERROR           0.72s   killed by signal 6 SIGABRT
 >>> MALLOC_PERTURB_=217 QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
QTEST_QEMU_BINARY=./qemu-system-ppc64 
/home/thuth/tmp/qemu-build/tests/qtest/qom-test --tap -k
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ 
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
qemu-system-ppc64: sysbus_mmio_map(type:power9_v2.2-pnv-chip, index:0, 
addr:0x603fc00000000, prio:0) but object is not realized
Broken pipe
../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU death 
from signal 6 (Aborted) (core dumped)

(test program exited with status code -6)

TAP parsing error: Too few tests run (expected 17, got 0)
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

...

  24/433 qemu:qtest+qtest-aarch64 / qtest-aarch64/qom-test 
              ERROR           5.94s   killed by signal 6 SIGABRT
 >>> QTEST_QEMU_BINARY=./qemu-system-aarch64 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
QTEST_QEMU_IMG=./qemu-img 
PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
MALLOC_PERTURB_=105 /home/thuth/tmp/qemu-build/tests/qtest/qom-test --tap -k
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ 
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
qemu-system-aarch64: sysbus_init_mmio(type:pxa2xx_pic) but object is realized
Broken pipe
../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU death 
from signal 6 (Aborted) (core dumped)

(test program exited with status code -6)

TAP parsing error: Too few tests run (expected 95, got 3)
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

...

  73/433 qemu:qtest+qtest-ppc64 / qtest-ppc64/boot-serial-test 
              ERROR           2.65s   killed by signal 6 SIGABRT
 >>> QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
MALLOC_PERTURB_=129 QTEST_QEMU_BINARY=./qemu-system-ppc64 
/home/thuth/tmp/qemu-build/tests/qtest/boot-serial-test --tap -k
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ 
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
qemu-system-ppc64: sysbus_mmio_map(type:spapr-xive, index:0, 
addr:0x6010000000000, prio:0) but object is not realized
Broken pipe
../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU death 
from signal 6 (Aborted) (core dumped)

(test program exited with status code -6)

TAP parsing error: Too few tests run (expected 7, got 3)
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

...

270/433 qemu:qtest+qtest-ppc64 / qtest-ppc64/cpu-plug-test 
             ERROR           0.40s   killed by signal 6 SIGABRT
 >>> QTEST_QEMU_IMG=./qemu-img 
G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
MALLOC_PERTURB_=175 QTEST_QEMU_BINARY=./qemu-system-ppc64 
/home/thuth/tmp/qemu-build/tests/qtest/cpu-plug-test --tap -k
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ 
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
qemu-system-ppc64: qbus_new(type:spapr-vio-bus parent:spapr-vio-bridge, 
name:spapr-vio) but parent realized
Broken pipe
../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU death 
from signal 6 (Aborted) (core dumped)

(test program exited with status code -6)

TAP parsing error: Too few tests run (expected 3, got 0)
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

...

  HTH,
   Thomas
Philippe Mathieu-Daudé Oct. 18, 2023, 6:25 p.m. UTC | #3
On 18/10/23 18:24, Thomas Huth wrote:
> On 18/10/2023 16.11, 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.
>>
>> First we fix some abuse, then we enforce in qdev/sysbus
>> core code.
> 
> I like the idea, and just had a try with "make check-qtest" with the 
> patches here, but seems like there are more spots that need attention:
> 
>   10/433 qemu:qtest+qtest-ppc64 / qtest-ppc64/qom-test              
> ERROR           0.72s   killed by signal 6 SIGABRT
>  >>> MALLOC_PERTURB_=217 QTEST_QEMU_IMG=./qemu-img 
> G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
> PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
> QTEST_QEMU_BINARY=./qemu-system-ppc64 
> /home/thuth/tmp/qemu-build/tests/qtest/qom-test --tap -k
> ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― 
> ✀ ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> stderr:
> qemu-system-ppc64: sysbus_mmio_map(type:power9_v2.2-pnv-chip, index:0, 
> addr:0x603fc00000000, prio:0) but object is not realized
> Broken pipe
> ../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU 
> death from signal 6 (Aborted) (core dumped)
> 
> (test program exited with status code -6)
> 
> TAP parsing error: Too few tests run (expected 17, got 0)

Sorry, I forgot to add:

Based-on: <20231018133059.85765-1-philmd@linaro.org>
           "hw/ppc: SysBus simplifications"
https://lore.kernel.org/qemu-devel/20231018133059.85765-1-philmd@linaro.org/
Philippe Mathieu-Daudé Oct. 18, 2023, 6:32 p.m. UTC | #4
On 18/10/23 18:24, Thomas Huth wrote:
> On 18/10/2023 16.11, 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.
>>
>> First we fix some abuse, then we enforce in qdev/sysbus
>> core code.
> 
> I like the idea, and just had a try with "make check-qtest" with the 
> patches here, but seems like there are more spots that need attention:


>   24/433 qemu:qtest+qtest-aarch64 / qtest-aarch64/qom-test              
> ERROR           5.94s   killed by signal 6 SIGABRT
>  >>> QTEST_QEMU_BINARY=./qemu-system-aarch64 
> G_TEST_DBUS_DAEMON=/home/thuth/devel/qemu/tests/dbus-vmstate-daemon.sh 
> QTEST_QEMU_IMG=./qemu-img 
> PYTHON=/home/thuth/tmp/qemu-build/pyvenv/bin/python3 
> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon 
> MALLOC_PERTURB_=105 /home/thuth/tmp/qemu-build/tests/qtest/qom-test 
> --tap -k
> ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― 
> ✀ ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> stderr:
> qemu-system-aarch64: sysbus_init_mmio(type:pxa2xx_pic) but object is 
> realized
> Broken pipe
> ../../devel/qemu/tests/qtest/libqtest.c:203: kill_qemu() detected QEMU 
> death from signal 6 (Aborted) (core dumped)

I neglected to mention that in the cover letter. This is mentioned here
https://lore.kernel.org/qemu-devel/1b159c7a-f52c-3705-8757-c2b80a04965b@linaro.org/

I ping'd Peter on IRC because I'm not sure how to fix this PXA2xx code.
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.

Regards,

Phil.