[PATCH 0/4] hw/pci: Have functions acess memory regions via pci_address_space()

[PATCH 0/4] hw/pci: Have functions acess memory regions via pci_address_space()

[Philippe Mathieu-Daudé]

PCI functions are plugged on a PCI bus. They can only
access external memory regions via the bus. Use the
corresponding pci_address_space() / pci_address_space_io()
for that.

Philippe Mathieu-Daudé (4):
  hw/isa/i82378: Access memory regions via pci_address_space()
  hw/isa/lpc_ich9: Access memory regions via pci_address_space[_io]()
  hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()
  hw/sparc64/ebus: Access memory regions via pci_address_space_io()

 hw/isa/i82378.c      | 2 +-
 hw/isa/lpc_ich9.c    | 4 ++--
 hw/pci-host/bonito.c | 6 +++---
 hw/sparc64/sun4u.c   | 8 ++++----
 4 files changed, 10 insertions(+), 10 deletions(-)

-- 
2.41.0

[PATCH 1/4] hw/isa/i82378: Access memory regions via pci_address_space()

[Philippe Mathieu-Daudé]

PCI functions are plugged on a PCI bus. They can only access
external memory regions via the bus.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/i82378.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 63e0857208..95b45d0178 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -76,7 +76,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
 
     pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
 
-    isabus = isa_bus_new(dev, get_system_memory(),
+    isabus = isa_bus_new(dev, pci_address_space(pci),
                          pci_address_space_io(pci), errp);
     if (!isabus) {
         return;
-- 
2.41.0

Re: [PATCH 1/4] hw/isa/i82378: Access memory regions via pci_address_space()

[Bernhard Beschow]

Am 11. Oktober 2023 18:59:51 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>PCI functions are plugged on a PCI bus. They can only access
>external memory regions via the bus.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

With `info mtree` and `info mtree -f` in the QEMU console before and after this patch I get the same results for `qemu-system-alpha -M clipper -S` and `qemu-system-ppc64 -M 40p -S`. So:

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

>---
> hw/isa/i82378.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
>index 63e0857208..95b45d0178 100644
>--- a/hw/isa/i82378.c
>+++ b/hw/isa/i82378.c
>@@ -76,7 +76,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
> 
>     pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
> 
>-    isabus = isa_bus_new(dev, get_system_memory(),
>+    isabus = isa_bus_new(dev, pci_address_space(pci),
>                          pci_address_space_io(pci), errp);
>     if (!isabus) {
>         return;

[PATCH 2/4] hw/isa/lpc_ich9: Access memory regions via pci_address_space[_io]()

[Philippe Mathieu-Daudé]

PCI functions are plugged on a PCI bus. They can only access
external memory regions via the bus.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/lpc_ich9.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 3f59980aa0..213b667f9b 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -707,8 +707,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
         return;
     }
 
-    isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), get_system_io(),
-                          errp);
+    isa_bus = isa_bus_new(DEVICE(d), pci_address_space(d),
+                          pci_address_space_io(d), errp);
     if (!isa_bus) {
         return;
     }
-- 
2.41.0

Re: [PATCH 2/4] hw/isa/lpc_ich9: Access memory regions via pci_address_space[_io]()

[Bernhard Beschow]

Am 11. Oktober 2023 18:59:52 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>PCI functions are plugged on a PCI bus. They can only access
>external memory regions via the bus.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

With `info mtree` and `info mtree -f` in the QEMU console before and after this patch I get the same results for `qemu-system-x86_64 -M q35 -S`. So:

Reviewed-by: Bernhard Beschow <shentey@gmail.com>

>---
> hw/isa/lpc_ich9.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
>index 3f59980aa0..213b667f9b 100644
>--- a/hw/isa/lpc_ich9.c
>+++ b/hw/isa/lpc_ich9.c
>@@ -707,8 +707,8 @@ static void ich9_lpc_realize(PCIDevice *d, Error **errp)
>         return;
>     }
> 
>-    isa_bus = isa_bus_new(DEVICE(d), get_system_memory(), get_system_io(),
>-                          errp);
>+    isa_bus = isa_bus_new(DEVICE(d), pci_address_space(d),
>+                          pci_address_space_io(d), errp);
>     if (!isa_bus) {
>         return;
>     }

[PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()

[Philippe Mathieu-Daudé]

PCI functions are plugged on a PCI bus. They can only access
external memory regions via the bus.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/pci-host/bonito.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index ee6cb85e97..50bde57862 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -700,14 +700,14 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
 
     /* Map PCI IO Space  0x1fd0 0000 - 0x1fd1 0000 */
     memory_region_init_alias(&s->bonito_pciio, OBJECT(s), "isa_mmio",
-                             get_system_io(), 0, BONITO_PCIIO_SIZE);
+                             pci_address_space_io(dev), 0, BONITO_PCIIO_SIZE);
     sysbus_init_mmio(sysbus, &s->bonito_pciio);
     sysbus_mmio_map(sysbus, 5, BONITO_PCIIO_BASE);
 
     /* add pci local io mapping */
 
     memory_region_init_alias(&s->bonito_localio, OBJECT(s), "IOCS[0]",
-                             get_system_io(), 0, 256 * KiB);
+                             pci_address_space_io(dev), 0, 256 * KiB);
     sysbus_init_mmio(sysbus, &s->bonito_localio);
     sysbus_mmio_map(sysbus, 6, BONITO_DEV_BASE);
     create_unimplemented_device("IOCS[1]", BONITO_DEV_BASE + 1 * 256 * KiB,
@@ -719,7 +719,7 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
 
     memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
                              &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
-    memory_region_add_subregion(get_system_memory(),
+    memory_region_add_subregion(pci_address_space(dev),
                                 BONITO_PCIHI_BASE, pcimem_alias);
     create_unimplemented_device("PCI_2",
                                 (hwaddr)BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE,
-- 
2.41.0

Re: [PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()

[Bernhard Beschow]

Am 11. Oktober 2023 18:59:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>PCI functions are plugged on a PCI bus. They can only access
>external memory regions via the bus.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> hw/pci-host/bonito.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>index ee6cb85e97..50bde57862 100644
>--- a/hw/pci-host/bonito.c
>+++ b/hw/pci-host/bonito.c
>@@ -700,14 +700,14 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
> 
>     /* Map PCI IO Space  0x1fd0 0000 - 0x1fd1 0000 */
>     memory_region_init_alias(&s->bonito_pciio, OBJECT(s), "isa_mmio",
>-                             get_system_io(), 0, BONITO_PCIIO_SIZE);
>+                             pci_address_space_io(dev), 0, BONITO_PCIIO_SIZE);
>     sysbus_init_mmio(sysbus, &s->bonito_pciio);
>     sysbus_mmio_map(sysbus, 5, BONITO_PCIIO_BASE);
> 
>     /* add pci local io mapping */
> 
>     memory_region_init_alias(&s->bonito_localio, OBJECT(s), "IOCS[0]",
>-                             get_system_io(), 0, 256 * KiB);
>+                             pci_address_space_io(dev), 0, 256 * KiB);
>     sysbus_init_mmio(sysbus, &s->bonito_localio);
>     sysbus_mmio_map(sysbus, 6, BONITO_DEV_BASE);
>     create_unimplemented_device("IOCS[1]", BONITO_DEV_BASE + 1 * 256 * KiB,
>@@ -719,7 +719,7 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
> 
>     memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
>                              &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
>-    memory_region_add_subregion(get_system_memory(),
>+    memory_region_add_subregion(pci_address_space(dev),

I need to keep `get_system_memory()` here to get the same results for `info mtree` in the QEMU console before and after this patch when running `qemu-system-mips64el -M fuloong2e -S`. The other two changes above seem to work as expected.

Best regards,
Bernhard

>                                 BONITO_PCIHI_BASE, pcimem_alias);
>     create_unimplemented_device("PCI_2",
>                                 (hwaddr)BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE,

Re: [PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()

[Philippe Mathieu-Daudé]

On 16/10/23 00:19, Bernhard Beschow wrote:
> Am 11. Oktober 2023 18:59:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>> PCI functions are plugged on a PCI bus. They can only access
>> external memory regions via the bus.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/pci-host/bonito.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)


>> @@ -719,7 +719,7 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
>>
>>      memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
>>                               &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
>> -    memory_region_add_subregion(get_system_memory(),
>> +    memory_region_add_subregion(pci_address_space(dev),
> 
> I need to keep `get_system_memory()` here to get the same results for `info mtree` in the QEMU console before and after this patch when running `qemu-system-mips64el -M fuloong2e -S`. The other two changes above seem to work as expected.

Good catch, thank you Bernhard!

Re: [PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()

[Philippe Mathieu-Daudé]

Hi Bernhard,

On 16/10/23 09:04, Philippe Mathieu-Daudé wrote:
> On 16/10/23 00:19, Bernhard Beschow wrote:
>> Am 11. Oktober 2023 18:59:53 UTC schrieb "Philippe Mathieu-Daudé" 
>> <philmd@linaro.org>:
>>> PCI functions are plugged on a PCI bus. They can only access
>>> external memory regions via the bus.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> hw/pci-host/bonito.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> 
>>> @@ -719,7 +719,7 @@ static void bonito_pci_realize(PCIDevice *dev, 
>>> Error **errp)
>>>
>>>      memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
>>>                               &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
>>> -    memory_region_add_subregion(get_system_memory(),
>>> +    memory_region_add_subregion(pci_address_space(dev),
>>
>> I need to keep `get_system_memory()` here to get the same results for 
>> `info mtree` in the QEMU console before and after this patch when 
>> running `qemu-system-mips64el -M fuloong2e -S`. The other two changes 
>> above seem to work as expected.

Can I use add R-b tag after dropping this change?

Re: [PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()

[Bernhard Beschow]

Am 6. Februar 2024 15:45:13 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>Hi Bernhard,
>
>On 16/10/23 09:04, Philippe Mathieu-Daudé wrote:
>> On 16/10/23 00:19, Bernhard Beschow wrote:
>>> Am 11. Oktober 2023 18:59:53 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>>>> PCI functions are plugged on a PCI bus. They can only access
>>>> external memory regions via the bus.
>>>> 
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>> hw/pci-host/bonito.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> 
>>>> @@ -719,7 +719,7 @@ static void bonito_pci_realize(PCIDevice *dev, Error **errp)
>>>> 
>>>>      memory_region_init_alias(pcimem_alias, NULL, "pci.mem.alias",
>>>>                               &bs->pci_mem, 0, BONITO_PCIHI_SIZE);
>>>> -    memory_region_add_subregion(get_system_memory(),
>>>> +    memory_region_add_subregion(pci_address_space(dev),
>>> 
>>> I need to keep `get_system_memory()` here to get the same results for `info mtree` in the QEMU console before and after this patch when running `qemu-system-mips64el -M fuloong2e -S`. The other two changes above seem to work as expected.

With that resolved:
Reviewed-by: Bernhard Beschow <shentey@gmail.com>

>
>Can I use add R-b tag after dropping this change?

Yes, formally added above.

While at it, could you possibly double check if you really queued https://patchew.org/QEMU/20230105154440.259361-1-shentey@gmail.com/ into mips-next?

Thanks,
Bernhard

[PATCH 4/4] hw/sparc64/ebus: Access memory regions via pci_address_space_io()

[Philippe Mathieu-Daudé]

PCI functions are plugged on a PCI bus. They can only access
external memory regions via the bus.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/sparc64/sun4u.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d908a38f73..c871170378 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -360,11 +360,11 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
     pci_dev->config[0x09] = 0x00; // programming i/f
     pci_dev->config[0x0D] = 0x0a; // latency_timer
 
-    memory_region_init_alias(&s->bar0, OBJECT(s), "bar0", get_system_io(),
-                             0, 0x1000000);
+    memory_region_init_alias(&s->bar0, OBJECT(s), "bar0",
+                             pci_address_space_io(pci_dev), 0, 0x1000000);
     pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar0);
-    memory_region_init_alias(&s->bar1, OBJECT(s), "bar1", get_system_io(),
-                             0, 0x8000);
+    memory_region_init_alias(&s->bar1, OBJECT(s), "bar1",
+                             pci_address_space_io(pci_dev), 0, 0x8000);
     pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
 }
 
-- 
2.41.0

Re: [PATCH 4/4] hw/sparc64/ebus: Access memory regions via pci_address_space_io()

[Mark Cave-Ayland]

On 11/10/2023 19:59, Philippe Mathieu-Daudé wrote:

> PCI functions are plugged on a PCI bus. They can only access
> external memory regions via the bus.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/sparc64/sun4u.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
> index d908a38f73..c871170378 100644
> --- a/hw/sparc64/sun4u.c
> +++ b/hw/sparc64/sun4u.c
> @@ -360,11 +360,11 @@ static void ebus_realize(PCIDevice *pci_dev, Error **errp)
>       pci_dev->config[0x09] = 0x00; // programming i/f
>       pci_dev->config[0x0D] = 0x0a; // latency_timer
>   
> -    memory_region_init_alias(&s->bar0, OBJECT(s), "bar0", get_system_io(),
> -                             0, 0x1000000);
> +    memory_region_init_alias(&s->bar0, OBJECT(s), "bar0",
> +                             pci_address_space_io(pci_dev), 0, 0x1000000);
>       pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->bar0);
> -    memory_region_init_alias(&s->bar1, OBJECT(s), "bar1", get_system_io(),
> -                             0, 0x8000);
> +    memory_region_init_alias(&s->bar1, OBJECT(s), "bar1",
> +                             pci_address_space_io(pci_dev), 0, 0x8000);
>       pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->bar1);
>   }

Nice!

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.