| Message ID | 20231011185954.10337-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/pci: Have functions acess memory regions via pci_address_space() | expand |
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,
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!
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?
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
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,
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(-)