[v2] hw/boards: Remove legacy MachineClass::pci_allow_0_address flag

[PATCH-for-10.0 v2 01/13] hw/pci: Do not declare PCIBus::flags mask as enum

Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago

We use PCIBus::flags to mask various flags. It is not
an enum, and doing so confuses static analyzers. Rename
the enum as singular. Use a generic unsigned type for
the mask.

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

diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 22613125462..6ecfe2e06d5 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -19,7 +19,7 @@ struct PCIBusClass {
     uint16_t (*numa_node)(PCIBus *bus);
 };
 
-enum PCIBusFlags {
+enum PCIBusFlag {
     /* This bus is the root of a PCI domain */
     PCI_BUS_IS_ROOT                                         = 0x0001,
     /* PCIe extended configuration space is accessible on this bus */
@@ -32,7 +32,7 @@ enum PCIBusFlags {
 
 struct PCIBus {
     BusState qbus;
-    enum PCIBusFlags flags;
+    unsigned flags;
     const PCIIOMMUOps *iommu_ops;
     void *iommu_opaque;
     uint8_t devfn_min;
-- 
2.45.2

Re: [PATCH-for-10.0 v2 01/13] hw/pci: Do not declare PCIBus::flags mask as enum

Posted by Philippe Mathieu-Daudé 11 months, 3 weeks ago

On 26/11/24 12:22, Philippe Mathieu-Daudé wrote:
> We use PCIBus::flags to mask various flags. It is not
> an enum, and doing so confuses static analyzers. Rename
> the enum as singular. Use a generic unsigned type for
> the mask.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/pci/pci_bus.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 22613125462..6ecfe2e06d5 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -19,7 +19,7 @@ struct PCIBusClass {
>       uint16_t (*numa_node)(PCIBus *bus);
>   };
>   
> -enum PCIBusFlags {
> +enum PCIBusFlag {
>       /* This bus is the root of a PCI domain */
>       PCI_BUS_IS_ROOT                                         = 0x0001,
>       /* PCIe extended configuration space is accessible on this bus */

(more diff context:)

         PCI_BUS_EXTENDED_CONFIG_SPACE                           = 0x0002,
        /* This is a CXL Type BUS */
        PCI_BUS_CXL                                          = 0x0004,

Enum would be the [0, 1, 2] bits. Since we define bitmask and use
bitmask arguments in the code, shouldn't we simply replace that
enum by #define?

> @@ -32,7 +32,7 @@ enum PCIBusFlags {
>   
>   struct PCIBus {
>       BusState qbus;
> -    enum PCIBusFlags flags;
> +    unsigned flags;
>       const PCIIOMMUOps *iommu_ops;
>       void *iommu_opaque;
>       uint8_t devfn_min;

Re: [PATCH-for-10.0 v2 01/13] hw/pci: Do not declare PCIBus::flags mask as enum

Posted by Thomas Huth 11 months, 2 weeks ago

On 27/11/2024 10.37, Philippe Mathieu-Daudé wrote:
> On 26/11/24 12:22, Philippe Mathieu-Daudé wrote:
>> We use PCIBus::flags to mask various flags. It is not
>> an enum, and doing so confuses static analyzers. Rename
>> the enum as singular. Use a generic unsigned type for
>> the mask.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/pci/pci_bus.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
>> index 22613125462..6ecfe2e06d5 100644
>> --- a/include/hw/pci/pci_bus.h
>> +++ b/include/hw/pci/pci_bus.h
>> @@ -19,7 +19,7 @@ struct PCIBusClass {
>>       uint16_t (*numa_node)(PCIBus *bus);
>>   };
>> -enum PCIBusFlags {
>> +enum PCIBusFlag {
>>       /* This bus is the root of a PCI domain */
>>       PCI_BUS_IS_ROOT                                         = 0x0001,
>>       /* PCIe extended configuration space is accessible on this bus */
> 
> (more diff context:)
> 
>          PCI_BUS_EXTENDED_CONFIG_SPACE                           = 0x0002,
>         /* This is a CXL Type BUS */
>         PCI_BUS_CXL                                          = 0x0004,
> 
> Enum would be the [0, 1, 2] bits. Since we define bitmask and use
> bitmask arguments in the code, shouldn't we simply replace that
> enum by #define?

Agreed, this rather sounds like #defines than an enum to me, too.

  Thomas