1. Patchew
  2. QEMU
  3. [PATCH 0/4] hw/pci: Have functions acess memory regions via pci_address_space()
  4. View patch

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

Philippe Mathieu-Daudé posted 4 patches 2 years, 4 months ago
Maintainers: "Hervé Poussineau" <hpoussin@reactos.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>
[PATCH 3/4] hw/pci-host/bonito: Access memory regions via pci_address_space[_io]()
Posted by Philippe Mathieu-Daudé 2 years, 4 months ago 
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]()
Posted by Bernhard Beschow 2 years, 3 months ago 

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]()
Posted by Philippe Mathieu-Daudé 2 years, 3 months ago 
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]()
Posted by Philippe Mathieu-Daudé 2 years ago 
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]()
Posted by Bernhard Beschow 2 years ago 

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.