hw/pci: Use correct bus aperture for device container size

[PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago

Before commit 3716d5902d7 ("pci: introduce a bus master container")
device view of the bus address space was correctly sized to the
bus aperture, but then it always used a 64-bit aperture. Restore
the previous behavior, so devices on 32-bit PCI only see 32-bit
address space.

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

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b1eba348e06..64f33b9de04 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
     pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
 
     memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
-                       "bus master container", UINT64_MAX);
+                       "bus master container",
+                       memory_region_size(bus->address_space_mem));
     address_space_init(&pci_dev->bus_master_as,
                        &pci_dev->bus_master_container_region, pci_dev->name);
     pci_dev->bus_master_as.max_bounce_buffer_size =
-- 
2.51.0

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Michael S. Tsirkin 2 days, 21 hours ago

On Mon, Nov 24, 2025 at 12:47:54PM +0100, Philippe Mathieu-Daudé wrote:
> Before commit 3716d5902d7 ("pci: introduce a bus master container")
> device view of the bus address space was correctly sized to the
> bus aperture, but then it always used a 64-bit aperture. Restore
> the previous behavior, so devices on 32-bit PCI only see 32-bit
> address space.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Wait I am confused. Is there a problem this is trying to fix?


> ---
>  hw/pci/pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b1eba348e06..64f33b9de04 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>      pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
>  
>      memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
> -                       "bus master container", UINT64_MAX);
> +                       "bus master container",
> +                       memory_region_size(bus->address_space_mem));

as far as I can tell this does more than it says it does.


Look at raven.c:
                                                                                                                                         
  h->bus = pci_register_root_bus(d, NULL, raven_set_irq, raven_map_irq,                                                                  
                                 s, &s->pci_memory, &s->pci_io, 0, 4,                                                                    
                                 TYPE_PCI_BUS);                                                                                          
                                                                                                                                         
  The bus is created with &s->pci_memory (1G) as the address space.                                                  
                                                                                                                                         
  But the actual bus master space (&s->bm at line 285) is 4 G.                                                                         
                                                                                                                                         
  Your patch limits DMA to 1G. No?

failures here (definitely the ppc one)

https://gitlab.com/mstredhat/qemu/-/jobs/12984084992

reverting fixed it:

https://gitlab.com/mstredhat/qemu/-/jobs/12984187692



                                                                                                                                         
>      address_space_init(&pci_dev->bus_master_as,
>                         &pci_dev->bus_master_container_region, pci_dev->name);
>      pci_dev->bus_master_as.max_bounce_buffer_size =
> -- 
> 2.51.0

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Yodel Eldar 2 days, 11 hours ago


On 04/02/2026 06:29, Michael S. Tsirkin wrote:
> On Mon, Nov 24, 2025 at 12:47:54PM +0100, Philippe Mathieu-Daudé wrote:
>> Before commit 3716d5902d7 ("pci: introduce a bus master container")
>> device view of the bus address space was correctly sized to the
>> bus aperture, but then it always used a 64-bit aperture. Restore
>> the previous behavior, so devices on 32-bit PCI only see 32-bit
>> address space.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 
> Wait I am confused. Is there a problem this is trying to fix?
> 
> 
>> ---
>>   hw/pci/pci.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index b1eba348e06..64f33b9de04 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>>       pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
>>   
>>       memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
>> -                       "bus master container", UINT64_MAX);
>> +                       "bus master container",
>> +                       memory_region_size(bus->address_space_mem));
> 
> as far as I can tell this does more than it says it does.
> 
> 
> Look at raven.c:
>                                                                                                                                           
>    h->bus = pci_register_root_bus(d, NULL, raven_set_irq, raven_map_irq,
>                                   s, &s->pci_memory, &s->pci_io, 0, 4,
>                                   TYPE_PCI_BUS);
>                                                                                                                                           
>    The bus is created with &s->pci_memory (1G) as the address space.
>                                                                                                                                           
>    But the actual bus master space (&s->bm at line 285) is 4 G.
>                                                                                                                                           
>    Your patch limits DMA to 1G. No?
> 
> failures here (definitely the ppc one)
> 
> https://gitlab.com/mstredhat/qemu/-/jobs/12984084992
> 
> reverting fixed it:
> 
> https://gitlab.com/mstredhat/qemu/-/jobs/12984187692
> 
> 
> 
>

Thanks for the catch, Michael.

Sorry, I wasn't aware of this quirk in raven when I acked. Looking into
it now, it seems that prior to 97db0466788 its pci_memory had size
0xff000000 as a workaround, and that would indeed "fix" the regression
here, too, but is probably not preferable to leaving the bus master
container maxed out. What do you think, Philippe?

  

>>       address_space_init(&pci_dev->bus_master_as,
>>                          &pci_dev->bus_master_container_region, pci_dev->name);
>>       pci_dev->bus_master_as.max_bounce_buffer_size =
>> -- 
>> 2.51.0
> 
>

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago

ping?

On 24/11/25 12:47, Philippe Mathieu-Daudé wrote:
> Before commit 3716d5902d7 ("pci: introduce a bus master container")
> device view of the bus address space was correctly sized to the
> bus aperture, but then it always used a 64-bit aperture. Restore
> the previous behavior, so devices on 32-bit PCI only see 32-bit
> address space.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci/pci.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b1eba348e06..64f33b9de04 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>       pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
>   
>       memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
> -                       "bus master container", UINT64_MAX);
> +                       "bus master container",
> +                       memory_region_size(bus->address_space_mem));
>       address_space_init(&pci_dev->bus_master_as,
>                          &pci_dev->bus_master_container_region, pci_dev->name);
>       pci_dev->bus_master_as.max_bounce_buffer_size =

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Michael S. Tsirkin 1 month, 2 weeks ago

On Tue, Dec 23, 2025 at 02:52:20PM +0100, Philippe Mathieu-Daudé wrote:
> ping?


picked it up.

> On 24/11/25 12:47, Philippe Mathieu-Daudé wrote:
> > Before commit 3716d5902d7 ("pci: introduce a bus master container")
> > device view of the bus address space was correctly sized to the
> > bus aperture, but then it always used a 64-bit aperture. Restore
> > the previous behavior, so devices on 32-bit PCI only see 32-bit
> > address space.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/pci/pci.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index b1eba348e06..64f33b9de04 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
> >       pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
> >       memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
> > -                       "bus master container", UINT64_MAX);
> > +                       "bus master container",
> > +                       memory_region_size(bus->address_space_mem));
> >       address_space_init(&pci_dev->bus_master_as,
> >                          &pci_dev->bus_master_container_region, pci_dev->name);
> >       pci_dev->bus_master_as.max_bounce_buffer_size =

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Philippe Mathieu-Daudé 3 weeks, 2 days ago

On 23/12/25 22:52, Michael S. Tsirkin wrote:
> On Tue, Dec 23, 2025 at 02:52:20PM +0100, Philippe Mathieu-Daudé wrote:
>> ping?
> 
> 
> picked it up.

Thank you :)

> 
>> On 24/11/25 12:47, Philippe Mathieu-Daudé wrote:
>>> Before commit 3716d5902d7 ("pci: introduce a bus master container")
>>> device view of the bus address space was correctly sized to the
>>> bus aperture, but then it always used a 64-bit aperture. Restore
>>> the previous behavior, so devices on 32-bit PCI only see 32-bit
>>> address space.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/pci/pci.c | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)

Re: [PATCH] hw/pci: Use correct bus aperture for device container size

Posted by Yodel Eldar via 2 months, 1 week ago

Hi, Philippe!

On 24/11/2025 05:47, Philippe Mathieu-Daudé wrote:
> Before commit 3716d5902d7 ("pci: introduce a bus master container")
> device view of the bus address space was correctly sized to the
> bus aperture, but then it always used a 64-bit aperture. Restore
> the previous behavior, so devices on 32-bit PCI only see 32-bit
> address space.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/pci/pci.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b1eba348e06..64f33b9de04 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1375,7 +1375,8 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>       pstrcpy(pci_dev->name, sizeof(pci_dev->name), name);
>   
>       memory_region_init(&pci_dev->bus_master_container_region, OBJECT(pci_dev),
> -                       "bus master container", UINT64_MAX);
> +                       "bus master container",
> +                       memory_region_size(bus->address_space_mem));
>       address_space_init(&pci_dev->bus_master_as,
>                          &pci_dev->bus_master_container_region, pci_dev->name);
>       pci_dev->bus_master_as.max_bounce_buffer_size =

Acked-by: Yodel Eldar <yodel.eldar@yodel.dev>

Thanks,
Yodel