Re: [PATCH v3 2/9] hw/pci-host/q35: Inline sysbus_add_io()

[Bernhard Beschow]

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...