sysbus_add_io() just wraps memory_region_add_subregion() while also
obscuring where the memory is attached. So use
memory_region_add_subregion() directly and attach it to the existing
memory region s->mch.address_space_io which is set as an alias to
get_system_io() by the q35 machine.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/pci-host/q35.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 26390863d6..fa05844319 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
     Q35PCIHost *s = Q35_HOST_DEVICE(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
+    memory_region_add_subregion(s->mch.address_space_io,
+                                MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
 
-    sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
+    memory_region_add_subregion(s->mch.address_space_io,
+                                MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
     sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
     /* register q35 0xcf8 port as coalesced pio */
-- 
2.39.1

On 4/2/23 16:10, Bernhard Beschow wrote:
> sysbus_add_io() just wraps memory_region_add_subregion() while also
> obscuring where the memory is attached. So use
> memory_region_add_subregion() directly and attach it to the existing
> memory region s->mch.address_space_io which is set as an alias to
> get_system_io() by the q35 machine.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/pci-host/q35.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 26390863d6..fa05844319 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>       Q35PCIHost *s = Q35_HOST_DEVICE(dev);
>       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>   
> -    sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
> +    memory_region_add_subregion(s->mch.address_space_io,
> +                                MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
>       sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);

This makes me wonder why MCH_PCI_DEVICE doesn't use the bus I/O space
via pci_address_space_io(). IOW, why the MR like is in MCH_PCI_DEVICE
and not Q35_HOST_DEVICE?

Am 5. Februar 2023 11:12:26 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>On 4/2/23 16:10, Bernhard Beschow wrote:
>> sysbus_add_io() just wraps memory_region_add_subregion() while also
>> obscuring where the memory is attached. So use
>> memory_region_add_subregion() directly and attach it to the existing
>> memory region s->mch.address_space_io which is set as an alias to
>> get_system_io() by the q35 machine.
>> 
>> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   hw/pci-host/q35.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
>> index 26390863d6..fa05844319 100644
>> --- a/hw/pci-host/q35.c
>> +++ b/hw/pci-host/q35.c
>> @@ -50,10 +50,12 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>>       Q35PCIHost *s = Q35_HOST_DEVICE(dev);
>>       SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>   -    sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
>> +    memory_region_add_subregion(s->mch.address_space_io,
>> +                                MCH_HOST_BRIDGE_CONFIG_ADDR, &pci->conf_mem);
>>       sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_ADDR, 4);
>
>This makes me wonder why MCH_PCI_DEVICE doesn't use the bus I/O space
>via pci_address_space_io(). IOW, why the MR like is in MCH_PCI_DEVICE
>and not Q35_HOST_DEVICE?

I think I have follow-up patches in the pipeline moving the MemoryRegion pointers to the host device. Interestingly, these pointers are only used during the realize phase and just needlessly occupy memory during the rest of the device's lifetime...