Re: [PATCH] net/gve: fix meson build failure on non-Linux platforms

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 10/26/2022 10:33 AM, David Marchand wrote:
> On Wed, Oct 26, 2022 at 11:10 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>>>> diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
>>>> index b0f7b98daa..e968317737 100644
>>>> --- a/drivers/net/gve/gve_ethdev.c
>>>> +++ b/drivers/net/gve/gve_ethdev.c
>>>> @@ -1,12 +1,24 @@
>>>>    /* SPDX-License-Identifier: BSD-3-Clause
>>>>     * Copyright(C) 2022 Intel Corporation
>>>>     */
>>>> -#include <linux/pci_regs.h>
>>>>
>>>>    #include "gve_ethdev.h"
>>>>    #include "base/gve_adminq.h"
>>>>    #include "base/gve_register.h"
>>>>
>>>> +/*
>>>> + * Following macros are derived from linux/pci_regs.h, however,
>>>> + * we can't simply include that header here, as there is no such
>>>> + * file for non-Linux platform.
>>>> + */
>>>> +#define PCI_CFG_SPACE_SIZE     256
>>>> +#define PCI_CAPABILITY_LIST    0x34    /* Offset of first capability list entry */
>>>> +#define PCI_STD_HEADER_SIZEOF  64
>>>> +#define PCI_CAP_SIZEOF         4
>>>> +#define PCI_CAP_ID_MSIX                0x11    /* MSI-X */
>>>> +#define PCI_MSIX_FLAGS         2       /* Message Control */
>>>> +#define PCI_MSIX_FLAGS_QSIZE   0x07FF  /* Table size */
>>>

@Junfeng,

Can you please move these defines to 'gve_ethdev.h'?
Beginning of 'gve_ethdev.c' seems too unrelated for PCI defines.

>>> No, don't introduce such defines in a driver.
>>> We have a PCI library, that provides some defines.
>>> So fix your driver to use them.
>>>
> 
> After chat with Ferruh, I am ok if we take this compilation fix for
> now and unblock the CI..
> 

ack

> 
>>
>> I can see 'bnx2x' driver is using some flags from 'dev/pci/pcireg.h'
>> header for FreeBSD, unfortunately macro names are not same so a #ifdef
>> is required. Also I don't know it has all macros or if the header is
>> available in all versions etc..
>>
>> Other option can be to define them all for DPDK in common pci header? As
>> far as I can see we don't have all defined right now.
>>
> 
> If there are some missing definitions we can add them in the pci header indeed.
> Those are constants, defined in the PCIE standard.
> 
> I can post a cleanup later, but I would be happy if someone else handles it.
> 

@Junfeng,

Can you have bandwidth for this cleanup?

Task is to define PCI macros in common pci header, ('bus_pci_driver.h' I 
guess ?), and update drivers to use new common ones, remove local 
definition from drivers.

DPDK macros can use Linux macro names with RTE_ prefix and with a define 
guard, as done in following:
https://elixir.bootlin.com/dpdk/v22.07/source/drivers/bus/pci/linux/pci_init.h#L50