diff mbox series

[PATCH-for-10.0,v2,05/13] hw/pci: Propagate bar_at_addr_0_refused to pci_root_bus_internal_init()

Message ID 20241126112212.64524-6-philmd@linaro.org
State New
Headers show
Series hw/boards: Remove legacy MachineClass::pci_allow_0_address flag | expand

Commit Message

Philippe Mathieu-Daudé Nov. 26, 2024, 11:22 a.m. UTC 
Have pci_root_bus_internal_init() callers set the
'bar_at_addr_0_refused' argument. No logical change.

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

Comments

Peter Maydell Dec. 5, 2024, 4:44 p.m. UTC | #1 
On Tue, 26 Nov 2024 at 11:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Have pci_root_bus_internal_init() callers set the
> 'bar_at_addr_0_refused' argument. No logical change.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/pci/pci.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 27b66583e54..8eacb8f82fc 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -529,7 +529,8 @@ static bool machine_refuses_bar_at_addr_0(void)
>
>  static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
>                                         MemoryRegion *mem, MemoryRegion *io,
> -                                       uint8_t devfn_min)
> +                                       uint8_t devfn_min,
> +                                       bool bar_at_addr_0_refused)
>  {
>      assert(PCI_FUNC(devfn_min) == 0);
>      bus->devfn_min = devfn_min;
> @@ -537,7 +538,7 @@ static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
>      bus->address_space_mem = mem;
>      bus->address_space_io = io;
>      bus->flags |= PCI_BUS_IS_ROOT;
> -    if (machine_refuses_bar_at_addr_0()) {
> +    if (bar_at_addr_0_refused && machine_refuses_bar_at_addr_0()) {

Should this be || rather than &&  ? If I understand the
intent correctly, we want to prevent a BAR at address 0
if either:
 * the MachineClass field says we don't want one
   (legacy handling, eventually goes away)
 * the new command line argument says we don't want one

rather than only if *both* say "no address 0" ?

thanks
-- PMM
Philippe Mathieu-Daudé Dec. 5, 2024, 9:24 p.m. UTC | #2 
On 5/12/24 17:44, Peter Maydell wrote:
> On Tue, 26 Nov 2024 at 11:22, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Have pci_root_bus_internal_init() callers set the
>> 'bar_at_addr_0_refused' argument. No logical change.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/pci/pci.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 27b66583e54..8eacb8f82fc 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -529,7 +529,8 @@ static bool machine_refuses_bar_at_addr_0(void)
>>
>>   static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
>>                                          MemoryRegion *mem, MemoryRegion *io,
>> -                                       uint8_t devfn_min)
>> +                                       uint8_t devfn_min,
>> +                                       bool bar_at_addr_0_refused)
>>   {
>>       assert(PCI_FUNC(devfn_min) == 0);
>>       bus->devfn_min = devfn_min;
>> @@ -537,7 +538,7 @@ static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
>>       bus->address_space_mem = mem;
>>       bus->address_space_io = io;
>>       bus->flags |= PCI_BUS_IS_ROOT;
>> -    if (machine_refuses_bar_at_addr_0()) {
>> +    if (bar_at_addr_0_refused && machine_refuses_bar_at_addr_0()) {
> 
> Should this be || rather than &&  ? If I understand the
> intent correctly, we want to prevent a BAR at address 0
> if either:
>   * the MachineClass field says we don't want one
>     (legacy handling, eventually goes away)
>   * the new command line argument says we don't want one
> 
> rather than only if *both* say "no address 0" ?

Oops :)

> 
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 27b66583e54..8eacb8f82fc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -529,7 +529,8 @@  static bool machine_refuses_bar_at_addr_0(void)
 
 static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
                                        MemoryRegion *mem, MemoryRegion *io,
-                                       uint8_t devfn_min)
+                                       uint8_t devfn_min,
+                                       bool bar_at_addr_0_refused)
 {
     assert(PCI_FUNC(devfn_min) == 0);
     bus->devfn_min = devfn_min;
@@ -537,7 +538,7 @@  static void pci_root_bus_internal_init(PCIBus *bus, DeviceState *parent,
     bus->address_space_mem = mem;
     bus->address_space_io = io;
     bus->flags |= PCI_BUS_IS_ROOT;
-    if (machine_refuses_bar_at_addr_0()) {
+    if (bar_at_addr_0_refused && machine_refuses_bar_at_addr_0()) {
         bus->flags |= PCI_BUS_BAR_AT_ADDR0_REFUSED;
     }
 
@@ -563,7 +564,8 @@  void pci_root_bus_init(PCIBus *bus, size_t bus_size, DeviceState *parent,
                        uint8_t devfn_min, const char *typename)
 {
     qbus_init(bus, bus_size, typename, parent, name);
-    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
+    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min,
+                               true);
 }
 
 PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
@@ -573,7 +575,8 @@  PCIBus *pci_root_bus_new(DeviceState *parent, const char *name,
     PCIBus *bus;
 
     bus = PCI_BUS(qbus_new(typename, parent, name));
-    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min);
+    pci_root_bus_internal_init(bus, parent, mem, io, devfn_min,
+                               true);
     return bus;
 }