[v1,7/7] fpga: dfl: pci: Add generic OFS PCI PID

Commit Message

Zhang, Tianfei Feb. 14, 2022, 11:26 a.m. UTC

From: Matthew Gerlach <[e-mail]>

Add the PCI product id for an Open FPGA Stack PCI card.

Signed-off-by: Matthew Gerlach <[e-mail]>
Signed-off-by: Tianfei Zhang <[e-mail]>
---
 drivers/fpga/dfl-pci.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Tom Rix Feb. 15, 2022, 4:16 p.m. UTC | #1

On 2/14/22 3:26 AM, Tianfei zhang wrote:
> From: Matthew Gerlach <[e-mail]>
>
> Add the PCI product id for an Open FPGA Stack PCI card.
Is there a URL to the card ?
>
> Signed-off-by: Matthew Gerlach <[e-mail]>
> Signed-off-by: Tianfei Zhang <[e-mail]>
> ---
>   drivers/fpga/dfl-pci.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index 83b604d6dbe6..cb2fbf3eb918 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
>   #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
>   #define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
>   #define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
> +#define PCIE_DEVICE_ID_INTEL_OFS		0xbcce

INTEL_OFS is a generic name, pci id's map to specific cards

Is there a more specific name for this card ?

Tom

>   
>   /* VF Device */
>   #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
>   #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
>   #define PCIE_DEVICE_ID_VF_DSC_1_X		0x09C5
>   #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF	0x0B2C
> +#define PCIE_DEVICE_ID_INTEL_OFS_VF		0xbccf
>   
>   static struct pci_device_id cci_pcie_id_tbl[] = {
>   	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
> @@ -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>   	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>   	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>   	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS_VF),},
>   	{0,}
>   };
>   MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);

Zhang, Tianfei Feb. 18, 2022, 9:03 a.m. UTC | #2

> -----Original Message-----
> From: Tom Rix <[e-mail]>
> Sent: Wednesday, February 16, 2022 12:16 AM
> To: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
> [e-mail]; Xu, Yilun <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]
> Cc: [e-mail]; Matthew Gerlach <[e-mail]>
> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> 
> 
> On 2/14/22 3:26 AM, Tianfei zhang wrote:
> > From: Matthew Gerlach <[e-mail]>
> >
> > Add the PCI product id for an Open FPGA Stack PCI card.
> Is there a URL to the card ?

This PCIe Device IDs have registered by Intel.

> >
> > Signed-off-by: Matthew Gerlach <[e-mail]>
> > Signed-off-by: Tianfei Zhang <[e-mail]>
> > ---
> >   drivers/fpga/dfl-pci.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > 83b604d6dbe6..cb2fbf3eb918 100644
> > --- a/drivers/fpga/dfl-pci.c
> > +++ b/drivers/fpga/dfl-pci.c
> > @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
> >   #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
> >   #define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
> >   #define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
> > +#define PCIE_DEVICE_ID_INTEL_OFS		0xbcce
> 
> INTEL_OFS is a generic name, pci id's map to specific cards
> 
> Is there a more specific name for this card ?

I think using INTEL_OFS is better, because INTEL_OFS is the Generic development platform can support multiple cards which using OFS specification,
like Intel PAC N6000 card. 

> 
> Tom
> 
> >
> >   /* VF Device */
> >   #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
> >   #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
> >   #define PCIE_DEVICE_ID_VF_DSC_1_X		0x09C5
> >   #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF	0x0B2C
> > +#define PCIE_DEVICE_ID_INTEL_OFS_VF		0xbccf
> >
> >   static struct pci_device_id cci_pcie_id_tbl[] = {
> >   	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
> @@
> > -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
> >   	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
> >   	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
> >   	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> > PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
> > +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
> > +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_OFS_VF),},
> >   	{0,}
> >   };
> >   MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);

Tom Rix Feb. 18, 2022, 3:27 p.m. UTC | #3

On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>
>> -----Original Message-----
>> From: Tom Rix <[e-mail]>
>> Sent: Wednesday, February 16, 2022 12:16 AM
>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
>> [e-mail]; Xu, Yilun <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]
>> Cc: [e-mail]; Matthew Gerlach <[e-mail]>
>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>
>>
>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>> From: Matthew Gerlach <[e-mail]>
>>>
>>> Add the PCI product id for an Open FPGA Stack PCI card.
>> Is there a URL to the card ?
> This PCIe Device IDs have registered by Intel.
A URL is useful to introduce the board, Is there one ?
>
>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>> ---
>>>    drivers/fpga/dfl-pci.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>> --- a/drivers/fpga/dfl-pci.c
>>> +++ b/drivers/fpga/dfl-pci.c
>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
>>> +#define PCIE_DEVICE_ID_INTEL_OFS		0xbcce
>> INTEL_OFS is a generic name, pci id's map to specific cards
>>
>> Is there a more specific name for this card ?
> I think using INTEL_OFS is better, because INTEL_OFS is the Generic development platform can support multiple cards which using OFS specification,
> like Intel PAC N6000 card.

I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because it 
follows an existing pattern.  Make it easy on a developer, they will 
look at their board or box, see X and try to find something similar in 
the driver source.

To use OSF_ * the name needs a suffix to differentiate it from future 
cards that will also use ofs.

If this really is a generic id please explain in the doc patch how every 
future board with use this single id and how a driver could work around 
a hw problem in a specific board with a pci id covering multiple boards.

Tom

>
>> Tom
>>
>>>    /* VF Device */
>>>    #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
>>>    #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
>>>    #define PCIE_DEVICE_ID_VF_DSC_1_X		0x09C5
>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF	0x0B2C
>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF		0xbccf
>>>
>>>    static struct pci_device_id cci_pcie_id_tbl[] = {
>>>    	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
>> @@
>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>    	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>    	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>    	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
>>> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>    	{0,}
>>>    };
>>>    MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);

Matthew Gerlach Feb. 21, 2022, 5:50 p.m. UTC | #4

On Fri, 18 Feb 2022, Tom Rix wrote:

>
> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>> 
>>> -----Original Message-----
>>> From: Tom Rix <[e-mail]>
>>> Sent: Wednesday, February 16, 2022 12:16 AM
>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
>>> [e-mail]; Xu, Yilun <[e-mail]>; 
>>> [e-mail];
>>> [e-mail]; [e-mail]
>>> Cc: [e-mail]; Matthew Gerlach <[e-mail]>
>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>> 
>>> 
>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>>> From: Matthew Gerlach <[e-mail]>
>>>> 
>>>> Add the PCI product id for an Open FPGA Stack PCI card.
>>> Is there a URL to the card ?
>> This PCIe Device IDs have registered by Intel.
> A URL is useful to introduce the board, Is there one ?
>> 
>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>>> ---
>>>>    drivers/fpga/dfl-pci.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>> 
>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>>> --- a/drivers/fpga/dfl-pci.c
>>>> +++ b/drivers/fpga/dfl-pci.c
>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
>>>> +#define PCIE_DEVICE_ID_INTEL_OFS		0xbcce
>>> INTEL_OFS is a generic name, pci id's map to specific cards
>>> 
>>> Is there a more specific name for this card ?
>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic 
>> development platform can support multiple cards which using OFS 
>> specification,
>> like Intel PAC N6000 card.
>
> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because it 
> follows an existing pattern.  Make it easy on a developer, they will look at 
> their board or box, see X and try to find something similar in the driver 
> source.
>
> To use OSF_ * the name needs a suffix to differentiate it from future cards 
> that will also use ofs.
>
> If this really is a generic id please explain in the doc patch how every 
> future board with use this single id and how a driver could work around a hw 
> problem in a specific board with a pci id covering multiple boards.
>
> Tom

Hi Tom,

The intent is to have a generic device id that can be used with many 
different boards.  Currently, we have FPGA implementations for 3 different 
boards using this generic id.  We may need a better name for device id 
than OFS.  More precisely this generic device id means a PCI function that 
is described by a Device Feature List (DFL).  How about 
PCIE_DEVICE_ID_INTEL_DFL?

With a DFL device id, the functionality of the PF/VF is determined by the 
contents of the DFL.  Each Device Feature Header (DFH) in the DFL has a 
revision field that can be used identify "broken" hw, or new functionality 
added to a feature.  Additionally, since the DFL is typically used in a 
FPGA, the broken hardware, can and should be fixed in most cases.

Matthew
>
>> 
>>> Tom
>>>
>>>>    /* VF Device */
>>>>    #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
>>>>    #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
>>>>    #define PCIE_DEVICE_ID_VF_DSC_1_X		0x09C5
>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF	0x0B2C
>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF		0xbccf
>>>>
>>>>    static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>    	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
>>> @@
>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>    	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>>    	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>>    	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>>> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
>>>> +	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>>    	{0,}
>>>>    };
>>>>    MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
>
>

Tom Rix Feb. 21, 2022, 6:09 p.m. UTC | #5

On 2/21/22 9:50 AM, [e-mail] wrote:
>
>
> On Fri, 18 Feb 2022, Tom Rix wrote:
>
>>
>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>>>
>>>> -----Original Message-----
>>>> From: Tom Rix <[e-mail]>
>>>> Sent: Wednesday, February 16, 2022 12:16 AM
>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao 
>>>> <[e-mail]>;
>>>> [e-mail]; Xu, Yilun <[e-mail]>; 
>>>> [e-mail];
>>>> [e-mail]; [e-mail]
>>>> Cc: [e-mail]; Matthew Gerlach <[e-mail]>
>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>>>
>>>>
>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>>>> From: Matthew Gerlach <[e-mail]>
>>>>>
>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
>>>> Is there a URL to the card ?
>>> This PCIe Device IDs have registered by Intel.
>> A URL is useful to introduce the board, Is there one ?
>>>
>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>>>> ---
>>>>>    drivers/fpga/dfl-pci.c | 4 ++++
>>>>>    1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>>>> --- a/drivers/fpga/dfl-pci.c
>>>>> +++ b/drivers/fpga/dfl-pci.c
>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev 
>>>>> *pcidev)
>>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
>>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
>>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
>>>> INTEL_OFS is a generic name, pci id's map to specific cards
>>>>
>>>> Is there a more specific name for this card ?
>>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic 
>>> development platform can support multiple cards which using OFS 
>>> specification,
>>> like Intel PAC N6000 card.
>>
>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because 
>> it follows an existing pattern.  Make it easy on a developer, they 
>> will look at their board or box, see X and try to find something 
>> similar in the driver source.
>>
>> To use OSF_ * the name needs a suffix to differentiate it from future 
>> cards that will also use ofs.
>>
>> If this really is a generic id please explain in the doc patch how 
>> every future board with use this single id and how a driver could 
>> work around a hw problem in a specific board with a pci id covering 
>> multiple boards.
>>
>> Tom
>
> Hi Tom,
>
> The intent is to have a generic device id that can be used with many 
> different boards.  Currently, we have FPGA implementations for 3 
> different boards using this generic id.  We may need a better name for 
> device id than OFS.  More precisely this generic device id means a PCI 
> function that is described by a Device Feature List (DFL).  How about 
> PCIE_DEVICE_ID_INTEL_DFL?
>
> With a DFL device id, the functionality of the PF/VF is determined by 
> the contents of the DFL.  Each Device Feature Header (DFH) in the DFL 
> has a revision field that can be used identify "broken" hw, or new 
> functionality added to a feature.  Additionally, since the DFL is 
> typically used in a FPGA, the broken hardware, can and should be fixed 
> in most cases.

How is lspci supposed to work ?

A dfl set can change with fw updates and in theory different boards 
could have the same set.

Tom

>
> Matthew
>>
>>>
>>>> Tom
>>>>
>>>>>    /* VF Device */
>>>>>    #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
>>>>>    #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
>>>>>    #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
>>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
>>>>>
>>>>>    static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>        {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
>>>> @@
>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>        {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>>>        {0,}
>>>>>    };
>>>>>    MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
>>
>>

Zhang, Tianfei Feb. 22, 2022, 3:11 a.m. UTC | #6

> -----Original Message-----
> From: Tom Rix <[e-mail]>
> Sent: Tuesday, February 22, 2022 2:10 AM
> To: [e-mail]
> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
> [e-mail]; Xu, Yilun <[e-mail]>; [e-mail];
> [e-mail]; [e-mail]; [e-mail]
> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> 
> 
> On 2/21/22 9:50 AM, [e-mail] wrote:
> >
> >
> > On Fri, 18 Feb 2022, Tom Rix wrote:
> >
> >>
> >> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: Tom Rix <[e-mail]>
> >>>> Sent: Wednesday, February 16, 2022 12:16 AM
> >>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
> >>>> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
> >>>> [e-mail]; [e-mail];
> >>>> [e-mail]
> >>>> Cc: [e-mail]; Matthew Gerlach
> >>>> <[e-mail]>
> >>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> >>>>
> >>>>
> >>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
> >>>>> From: Matthew Gerlach <[e-mail]>
> >>>>>
> >>>>> Add the PCI product id for an Open FPGA Stack PCI card.
> >>>> Is there a URL to the card ?
> >>> This PCIe Device IDs have registered by Intel.
> >> A URL is useful to introduce the board, Is there one ?
> >>>
> >>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
> >>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
> >>>>> ---
> >>>>>    drivers/fpga/dfl-pci.c | 4 ++++
> >>>>>    1 file changed, 4 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> >>>>> 83b604d6dbe6..cb2fbf3eb918 100644
> >>>>> --- a/drivers/fpga/dfl-pci.c
> >>>>> +++ b/drivers/fpga/dfl-pci.c
> >>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
> >>>>> *pcidev)
> >>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
> >>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
> >>>>>    #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
> >>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
> >>>> INTEL_OFS is a generic name, pci id's map to specific cards
> >>>>
> >>>> Is there a more specific name for this card ?
> >>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic
> >>> development platform can support multiple cards which using OFS
> >>> specification, like Intel PAC N6000 card.
> >>
> >> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because
> >> it follows an existing pattern.  Make it easy on a developer, they
> >> will look at their board or box, see X and try to find something
> >> similar in the driver source.
> >>
> >> To use OSF_ * the name needs a suffix to differentiate it from future
> >> cards that will also use ofs.
> >>
> >> If this really is a generic id please explain in the doc patch how
> >> every future board with use this single id and how a driver could
> >> work around a hw problem in a specific board with a pci id covering
> >> multiple boards.
> >>
> >> Tom
> >
> > Hi Tom,
> >
> > The intent is to have a generic device id that can be used with many
> > different boards.  Currently, we have FPGA implementations for 3
> > different boards using this generic id.  We may need a better name for
> > device id than OFS.  More precisely this generic device id means a PCI
> > function that is described by a Device Feature List (DFL).  How about
> > PCIE_DEVICE_ID_INTEL_DFL?
> >
> > With a DFL device id, the functionality of the PF/VF is determined by
> > the contents of the DFL.  Each Device Feature Header (DFH) in the DFL
> > has a revision field that can be used identify "broken" hw, or new
> > functionality added to a feature.  Additionally, since the DFL is
> > typically used in a FPGA, the broken hardware, can and should be fixed
> > in most cases.
> 
> How is lspci supposed to work ?

There is an example for one card using IOFS and DFL.

# lspci | grep acc
b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev 01)
b1:00.1 Processing accelerators: Intel Corporation Device bcce
b1:00.2 Processing accelerators: Intel Corporation Device bcce
b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
b1:00.4 Processing accelerators: Intel Corporation Device bcce

Note: There 5 PFs in this card, it exports the management functions via PF0(b1:00.0),
Other PFs like b1:00.1, b1:00.2, b1:00.4, are using for testing, which depends on RTL designer
or project requirement. The PF3 instance a VirtIO net device for example, will bind with virtio-net driver
presenting itself as a network interface to the OS.

> 
> A dfl set can change with fw updates and in theory different boards could have
> the same set.
> 
> Tom
> 
> >
> > Matthew
> >>
> >>>
> >>>> Tom
> >>>>
> >>>>>    /* VF Device */
> >>>>>    #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
> >>>>>    #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
> >>>>>    #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
> >>>>>    #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
> >>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
> >>>>>
> >>>>>    static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>        {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
> >>>> @@
> >>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>        {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
> >>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
> >>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
> >>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
> >>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
> >>>>>        {0,}
> >>>>>    };
> >>>>>    MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
> >>
> >>

Tom Rix Feb. 22, 2022, 4:11 p.m. UTC | #7

On 2/21/22 7:11 PM, Zhang, Tianfei wrote:
>
>> -----Original Message-----
>> From: Tom Rix <[e-mail]>
>> Sent: Tuesday, February 22, 2022 2:10 AM
>> To: [e-mail]
>> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
>> [e-mail]; Xu, Yilun <[e-mail]>; [e-mail];
>> [e-mail]; [e-mail]; [e-mail]
>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>
>>
>> On 2/21/22 9:50 AM, [e-mail] wrote:
>>>
>>> On Fri, 18 Feb 2022, Tom Rix wrote:
>>>
>>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>>>>>> -----Original Message-----
>>>>>> From: Tom Rix <[e-mail]>
>>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
>>>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
>>>>>> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
>>>>>> [e-mail]; [e-mail];
>>>>>> [e-mail]
>>>>>> Cc: [e-mail]; Matthew Gerlach
>>>>>> <[e-mail]>
>>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>>>>>
>>>>>>
>>>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>>>>>> From: Matthew Gerlach <[e-mail]>
>>>>>>>
>>>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
>>>>>> Is there a URL to the card ?
>>>>> This PCIe Device IDs have registered by Intel.
>>>> A URL is useful to introduce the board, Is there one ?
>>>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>>>>>> ---
>>>>>>>     drivers/fpga/dfl-pci.c | 4 ++++
>>>>>>>     1 file changed, 4 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>>>>>> --- a/drivers/fpga/dfl-pci.c
>>>>>>> +++ b/drivers/fpga/dfl-pci.c
>>>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
>>>>>>> *pcidev)
>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
>>>>>> INTEL_OFS is a generic name, pci id's map to specific cards
>>>>>>
>>>>>> Is there a more specific name for this card ?
>>>>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic
>>>>> development platform can support multiple cards which using OFS
>>>>> specification, like Intel PAC N6000 card.
>>>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because
>>>> it follows an existing pattern.  Make it easy on a developer, they
>>>> will look at their board or box, see X and try to find something
>>>> similar in the driver source.
>>>>
>>>> To use OSF_ * the name needs a suffix to differentiate it from future
>>>> cards that will also use ofs.
>>>>
>>>> If this really is a generic id please explain in the doc patch how
>>>> every future board with use this single id and how a driver could
>>>> work around a hw problem in a specific board with a pci id covering
>>>> multiple boards.
>>>>
>>>> Tom
>>> Hi Tom,
>>>
>>> The intent is to have a generic device id that can be used with many
>>> different boards.  Currently, we have FPGA implementations for 3
>>> different boards using this generic id.  We may need a better name for
>>> device id than OFS.  More precisely this generic device id means a PCI
>>> function that is described by a Device Feature List (DFL).  How about
>>> PCIE_DEVICE_ID_INTEL_DFL?
>>>
>>> With a DFL device id, the functionality of the PF/VF is determined by
>>> the contents of the DFL.  Each Device Feature Header (DFH) in the DFL
>>> has a revision field that can be used identify "broken" hw, or new
>>> functionality added to a feature.  Additionally, since the DFL is
>>> typically used in a FPGA, the broken hardware, can and should be fixed
>>> in most cases.
>> How is lspci supposed to work ?
> There is an example for one card using IOFS and DFL.
>
> # lspci | grep acc
> b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev 01)
> b1:00.1 Processing accelerators: Intel Corporation Device bcce
> b1:00.2 Processing accelerators: Intel Corporation Device bcce
> b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
> b1:00.4 Processing accelerators: Intel Corporation Device bcce
>
> Note: There 5 PFs in this card, it exports the management functions via PF0(b1:00.0),
> Other PFs like b1:00.1, b1:00.2, b1:00.4, are using for testing, which depends on RTL designer
> or project requirement. The PF3 instance a VirtIO net device for example, will bind with virtio-net driver
> presenting itself as a network interface to the OS.

What I mean there is heterogeneous set of cards in one machine, how do 
you tell which card is which ?

Or in a datacenter where the machines are all remote and admin has to 
flash just the n6000's ?

How could she find just the n6000's with lspci ?

How would the driver know ?

Tom

>
>> A dfl set can change with fw updates and in theory different boards could have
>> the same set.
>>
>> Tom
>>
>>> Matthew
>>>>>> Tom
>>>>>>
>>>>>>>     /* VF Device */
>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
>>>>>>>     #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
>>>>>>>
>>>>>>>     static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
>>>>>> @@
>>>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>>>>>         {0,}
>>>>>>>     };
>>>>>>>     MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
>>>>

Zhang, Tianfei Feb. 23, 2022, 1:48 a.m. UTC | #8

> -----Original Message-----
> From: Tom Rix <[e-mail]>
> Sent: Wednesday, February 23, 2022 12:11 AM
> To: Zhang, Tianfei <[e-mail]>;
> [e-mail]
> Cc: Wu, Hao <[e-mail]>; [e-mail]; Xu, Yilun
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> 
> 
> On 2/21/22 7:11 PM, Zhang, Tianfei wrote:
> >
> >> -----Original Message-----
> >> From: Tom Rix <[e-mail]>
> >> Sent: Tuesday, February 22, 2022 2:10 AM
> >> To: [e-mail]
> >> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao
> >> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
> >> [e-mail]; [e-mail];
> >> [e-mail]; [e-mail]
> >> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> >>
> >>
> >> On 2/21/22 9:50 AM, [e-mail] wrote:
> >>>
> >>> On Fri, 18 Feb 2022, Tom Rix wrote:
> >>>
> >>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Tom Rix <[e-mail]>
> >>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
> >>>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
> >>>>>> <[e-mail]>; [e-mail]; Xu, Yilun
> >>>>>> <[e-mail]>; [e-mail];
> >>>>>> [e-mail]; [e-mail]
> >>>>>> Cc: [e-mail]; Matthew Gerlach
> >>>>>> <[e-mail]>
> >>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI
> >>>>>> PID
> >>>>>>
> >>>>>>
> >>>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
> >>>>>>> From: Matthew Gerlach <[e-mail]>
> >>>>>>>
> >>>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
> >>>>>> Is there a URL to the card ?
> >>>>> This PCIe Device IDs have registered by Intel.
> >>>> A URL is useful to introduce the board, Is there one ?
> >>>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
> >>>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
> >>>>>>> ---
> >>>>>>>     drivers/fpga/dfl-pci.c | 4 ++++
> >>>>>>>     1 file changed, 4 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> >>>>>>> index
> >>>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
> >>>>>>> --- a/drivers/fpga/dfl-pci.c
> >>>>>>> +++ b/drivers/fpga/dfl-pci.c
> >>>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
> >>>>>>> *pcidev)
> >>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
> >>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
> >>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
> >>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
> >>>>>> INTEL_OFS is a generic name, pci id's map to specific cards
> >>>>>>
> >>>>>> Is there a more specific name for this card ?
> >>>>> I think using INTEL_OFS is better, because INTEL_OFS is the
> >>>>> Generic development platform can support multiple cards which
> >>>>> using OFS specification, like Intel PAC N6000 card.
> >>>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000
> >>>> because it follows an existing pattern.  Make it easy on a
> >>>> developer, they will look at their board or box, see X and try to
> >>>> find something similar in the driver source.
> >>>>
> >>>> To use OSF_ * the name needs a suffix to differentiate it from
> >>>> future cards that will also use ofs.
> >>>>
> >>>> If this really is a generic id please explain in the doc patch how
> >>>> every future board with use this single id and how a driver could
> >>>> work around a hw problem in a specific board with a pci id covering
> >>>> multiple boards.
> >>>>
> >>>> Tom
> >>> Hi Tom,
> >>>
> >>> The intent is to have a generic device id that can be used with many
> >>> different boards.  Currently, we have FPGA implementations for 3
> >>> different boards using this generic id.  We may need a better name
> >>> for device id than OFS.  More precisely this generic device id means
> >>> a PCI function that is described by a Device Feature List (DFL).
> >>> How about PCIE_DEVICE_ID_INTEL_DFL?
> >>>
> >>> With a DFL device id, the functionality of the PF/VF is determined
> >>> by the contents of the DFL.  Each Device Feature Header (DFH) in the
> >>> DFL has a revision field that can be used identify "broken" hw, or
> >>> new functionality added to a feature.  Additionally, since the DFL
> >>> is typically used in a FPGA, the broken hardware, can and should be
> >>> fixed in most cases.
> >> How is lspci supposed to work ?
> > There is an example for one card using IOFS and DFL.
> >
> > # lspci | grep acc
> > b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev
> > 01)
> > b1:00.1 Processing accelerators: Intel Corporation Device bcce
> > b1:00.2 Processing accelerators: Intel Corporation Device bcce
> > b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
> > b1:00.4 Processing accelerators: Intel Corporation Device bcce
> >
> > Note: There 5 PFs in this card, it exports the management functions
> > via PF0(b1:00.0), Other PFs like b1:00.1, b1:00.2, b1:00.4, are using
> > for testing, which depends on RTL designer or project requirement. The
> > PF3 instance a VirtIO net device for example, will bind with virtio-net driver
> presenting itself as a network interface to the OS.
> 
> What I mean there is heterogeneous set of cards in one machine, how do you
> tell which card is which ?
> 
> Or in a datacenter where the machines are all remote and admin has to flash just
> the n6000's ?
> 
> How could she find just the n6000's with lspci ?

This is good question, we have several methods to distinguish in heterogeneous set of cards：
1. BDF for card, each card or each PF/VF has different BDF on PCIe bus.
When we want to flash the card, it need to  specify the BDF of card. For example, our OPAE userspace tool
" fpgasupdate" need to provide the BDF of the card in the argument.
2. if we have several different cards with the same PCIE PID which implemented the IOFS specification, some of
ID will be different, for example, the Bitstream Id, Pr Interface Id, the BMC FW version, BMC build version and so on,
Those information exposed by sysfs node.  For example, here is the sysfs node for Pr Interface Id:
/sys/class/fpga_region/regionX/dfl-fme.X/dfl-fme-region.X/fpga_region/regionX/ compat_id

The end-user can easy using OPAE tools to show this information, like fpgainfo.

[root@ ]# fpgainfo fme
Intel N6000 Acceleration Development Platform
Board Management Controller, MAX10 NIOS FW version: 3.4.0
Board Management Controller, MAX10 Build version: 3.4.0
//****** FME ******//
Object Id                        : 0xEF00000
PCIe s:b:d.f                     : 0000:B1:00.0
Device Id                        : 0xBCCE
Socket Id                        : 0x00
Ports Num                        : 01
Bitstream Id                     : 0x50104022AC010D2
Bitstream Version                : 5.0.1
Pr Interface Id                  : bb03eb0e-4a61-547f-a0ce-28ffe8ab25f3
Boot Page                        : user1
Factory Image Info               : a0ce28ffe8ab25f3bb03eb0e4a61547f
User1 Image Info                 : a0ce28ffe8ab25f3bb03eb0e4a61547f
User2 Image Info                 : a0ce28ffe8ab25f3bb03eb0e4a61547f

> 
> How would the driver know ?
> 
> Tom
> 
> >
> >> A dfl set can change with fw updates and in theory different boards
> >> could have the same set.
> >>
> >> Tom
> >>
> >>> Matthew
> >>>>>> Tom
> >>>>>>
> >>>>>>>     /* VF Device */
> >>>>>>>     #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
> >>>>>>>     #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
> >>>>>>>     #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
> >>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
> >>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
> >>>>>>>
> >>>>>>>     static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
> >>>>>> @@
> >>>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
> >>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
> >>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
> >>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>>> +PCIE_DEVICE_ID_INTEL_OFS),},
> >>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
> >>>>>>>         {0,}
> >>>>>>>     };
> >>>>>>>     MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
> >>>>

Matthew Gerlach Feb. 24, 2022, 5:54 p.m. UTC | #9

On Tue, 22 Feb 2022, Tom Rix wrote:

>
> On 2/21/22 7:11 PM, Zhang, Tianfei wrote:
>> 
>>> -----Original Message-----
>>> From: Tom Rix <[e-mail]>
>>> Sent: Tuesday, February 22, 2022 2:10 AM
>>> To: [e-mail]
>>> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao <[e-mail]>;
>>> [e-mail]; Xu, Yilun <[e-mail]>; 
>>> [e-mail];
>>> [e-mail]; [e-mail]; [e-mail]
>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>> 
>>> 
>>> On 2/21/22 9:50 AM, [e-mail] wrote:
>>>> 
>>>> On Fri, 18 Feb 2022, Tom Rix wrote:
>>>> 
>>>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Tom Rix <[e-mail]>
>>>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
>>>>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
>>>>>>> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
>>>>>>> [e-mail]; [e-mail];
>>>>>>> [e-mail]
>>>>>>> Cc: [e-mail]; Matthew Gerlach
>>>>>>> <[e-mail]>
>>>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>>>>>> 
>>>>>>> 
>>>>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>>>>>>> From: Matthew Gerlach <[e-mail]>
>>>>>>>> 
>>>>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
>>>>>>> Is there a URL to the card ?
>>>>>> This PCIe Device IDs have registered by Intel.
>>>>> A URL is useful to introduce the board, Is there one ?
>>>>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>>>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>>>>>>> ---
>>>>>>>>     drivers/fpga/dfl-pci.c | 4 ++++
>>>>>>>>     1 file changed, 4 insertions(+)
>>>>>>>> 
>>>>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>>>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>>>>>>> --- a/drivers/fpga/dfl-pci.c
>>>>>>>> +++ b/drivers/fpga/dfl-pci.c
>>>>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
>>>>>>>> *pcidev)
>>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
>>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
>>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
>>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
>>>>>>> INTEL_OFS is a generic name, pci id's map to specific cards
>>>>>>> 
>>>>>>> Is there a more specific name for this card ?
>>>>>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic
>>>>>> development platform can support multiple cards which using OFS
>>>>>> specification, like Intel PAC N6000 card.
>>>>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000 because
>>>>> it follows an existing pattern.  Make it easy on a developer, they
>>>>> will look at their board or box, see X and try to find something
>>>>> similar in the driver source.
>>>>> 
>>>>> To use OSF_ * the name needs a suffix to differentiate it from future
>>>>> cards that will also use ofs.
>>>>> 
>>>>> If this really is a generic id please explain in the doc patch how
>>>>> every future board with use this single id and how a driver could
>>>>> work around a hw problem in a specific board with a pci id covering
>>>>> multiple boards.
>>>>> 
>>>>> Tom
>>>> Hi Tom,
>>>> 
>>>> The intent is to have a generic device id that can be used with many
>>>> different boards.  Currently, we have FPGA implementations for 3
>>>> different boards using this generic id.  We may need a better name for
>>>> device id than OFS.  More precisely this generic device id means a PCI
>>>> function that is described by a Device Feature List (DFL).  How about
>>>> PCIE_DEVICE_ID_INTEL_DFL?
>>>> 
>>>> With a DFL device id, the functionality of the PF/VF is determined by
>>>> the contents of the DFL.  Each Device Feature Header (DFH) in the DFL
>>>> has a revision field that can be used identify "broken" hw, or new
>>>> functionality added to a feature.  Additionally, since the DFL is
>>>> typically used in a FPGA, the broken hardware, can and should be fixed
>>>> in most cases.
>>> How is lspci supposed to work ?
>> There is an example for one card using IOFS and DFL.
>> 
>> # lspci | grep acc
>> b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev 01)
>> b1:00.1 Processing accelerators: Intel Corporation Device bcce
>> b1:00.2 Processing accelerators: Intel Corporation Device bcce
>> b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
>> b1:00.4 Processing accelerators: Intel Corporation Device bcce
>> 
>> Note: There 5 PFs in this card, it exports the management functions via 
>> PF0(b1:00.0),
>> Other PFs like b1:00.1, b1:00.2, b1:00.4, are using for testing, which 
>> depends on RTL designer
>> or project requirement. The PF3 instance a VirtIO net device for example, 
>> will bind with virtio-net driver
>> presenting itself as a network interface to the OS.

Hi Tom,

These are very good questions, and the answers will be addressed in the 
documentation associated with a v2 submission of this patch.

>
> What I mean there is heterogeneous set of cards in one machine, how do you 
> tell which card is which ?

If the PCI PID/VID is generic, indicating only that there is one or more 
DFL, then some other mechanism must be used to differentiate the cards. 
One could use unique PCI sub-PID/sub-VIDs to differentiate specific 
implementations.  One could also use some register in BAR space to help 
identify the card, or one could use PCI Vital Product Data (VPD) to 
provide detailed information about the running FPGA design on the card.

>
> Or in a datacenter where the machines are all remote and admin has to flash 
> just the n6000's ?

This problem exists with the N3000 cards.  Depending on the FPGA 
configuration, the line side of the card could be very different (e.g. 
4x10Gb or 2x2x25Gb).  The network operator must make sure to update a 
particular N3000 card with the correct FPGA image type.  In the case of 
the N3000 there is a register exposed through sysfs containing the 
"Bitstream ID" which contains the line side configuration of the FPGA.

>
> How could she find just the n6000's with lspci ?

If you only wanted to use lspci to determine the card, then 
differentiating PCI sub-VID/sub-PID could be used or VPD could be used.

>
> How would the driver know ?

The dfl-pci driver is fairly generic in that it doesn't really care about 
the PCI PID/VID because all it really does enumerate the DFLs.  It is the 
individual dfl drivers that may need to know hw differences/bugs for that 
component IP.

>
> Tom
>
>> 
>>> A dfl set can change with fw updates and in theory different boards could 
>>> have
>>> the same set.
>>> 
>>> Tom
>>> 
>>>> Matthew
>>>>>>> Tom
>>>>>>>
>>>>>>>>     /* VF Device */
>>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
>>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
>>>>>>>>     #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
>>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
>>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
>>>>>>>>
>>>>>>>>     static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
>>>>>>> @@
>>>>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
>>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>>>>>>         {0,}
>>>>>>>>     };
>>>>>>>>     MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
>>>>> 
>
>

Wu, Hao Feb. 28, 2022, 10:57 a.m. UTC | #10

> >
> > On 2/21/22 7:11 PM, Zhang, Tianfei wrote:
> >>
> >>> -----Original Message-----
> >>> From: Tom Rix <[e-mail]>
> >>> Sent: Tuesday, February 22, 2022 2:10 AM
> >>> To: [e-mail]
> >>> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao
> <[e-mail]>;
> >>> [e-mail]; Xu, Yilun <[e-mail]>;
> >>> [e-mail];
> >>> [e-mail]; [e-mail]; [e-mail]
> >>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> >>>
> >>>
> >>> On 2/21/22 9:50 AM, [e-mail] wrote:
> >>>>
> >>>> On Fri, 18 Feb 2022, Tom Rix wrote:
> >>>>
> >>>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
> >>>>>>> -----Original Message-----
> >>>>>>> From: Tom Rix <[e-mail]>
> >>>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
> >>>>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
> >>>>>>> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
> >>>>>>> [e-mail]; [e-mail];
> >>>>>>> [e-mail]
> >>>>>>> Cc: [e-mail]; Matthew Gerlach
> >>>>>>> <[e-mail]>
> >>>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> >>>>>>>
> >>>>>>>
> >>>>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
> >>>>>>>> From: Matthew Gerlach <[e-mail]>
> >>>>>>>>
> >>>>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
> >>>>>>> Is there a URL to the card ?
> >>>>>> This PCIe Device IDs have registered by Intel.
> >>>>> A URL is useful to introduce the board, Is there one ?
> >>>>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
> >>>>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
> >>>>>>>> ---
> >>>>>>>>     drivers/fpga/dfl-pci.c | 4 ++++
> >>>>>>>>     1 file changed, 4 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> >>>>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
> >>>>>>>> --- a/drivers/fpga/dfl-pci.c
> >>>>>>>> +++ b/drivers/fpga/dfl-pci.c
> >>>>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
> >>>>>>>> *pcidev)
> >>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
> >>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
> >>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
> >>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
> >>>>>>> INTEL_OFS is a generic name, pci id's map to specific cards
> >>>>>>>
> >>>>>>> Is there a more specific name for this card ?
> >>>>>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic
> >>>>>> development platform can support multiple cards which using OFS
> >>>>>> specification, like Intel PAC N6000 card.
> >>>>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000
> because
> >>>>> it follows an existing pattern.  Make it easy on a developer, they
> >>>>> will look at their board or box, see X and try to find something
> >>>>> similar in the driver source.
> >>>>>
> >>>>> To use OSF_ * the name needs a suffix to differentiate it from future
> >>>>> cards that will also use ofs.
> >>>>>
> >>>>> If this really is a generic id please explain in the doc patch how
> >>>>> every future board with use this single id and how a driver could
> >>>>> work around a hw problem in a specific board with a pci id covering
> >>>>> multiple boards.
> >>>>>
> >>>>> Tom
> >>>> Hi Tom,
> >>>>
> >>>> The intent is to have a generic device id that can be used with many
> >>>> different boards.  Currently, we have FPGA implementations for 3
> >>>> different boards using this generic id.  We may need a better name for
> >>>> device id than OFS.  More precisely this generic device id means a PCI
> >>>> function that is described by a Device Feature List (DFL).  How about
> >>>> PCIE_DEVICE_ID_INTEL_DFL?
> >>>>
> >>>> With a DFL device id, the functionality of the PF/VF is determined by
> >>>> the contents of the DFL.  Each Device Feature Header (DFH) in the DFL
> >>>> has a revision field that can be used identify "broken" hw, or new
> >>>> functionality added to a feature.  Additionally, since the DFL is
> >>>> typically used in a FPGA, the broken hardware, can and should be fixed
> >>>> in most cases.
> >>> How is lspci supposed to work ?
> >> There is an example for one card using IOFS and DFL.
> >>
> >> # lspci | grep acc
> >> b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev 01)
> >> b1:00.1 Processing accelerators: Intel Corporation Device bcce
> >> b1:00.2 Processing accelerators: Intel Corporation Device bcce
> >> b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
> >> b1:00.4 Processing accelerators: Intel Corporation Device bcce
> >>
> >> Note: There 5 PFs in this card, it exports the management functions via
> >> PF0(b1:00.0),
> >> Other PFs like b1:00.1, b1:00.2, b1:00.4, are using for testing, which
> >> depends on RTL designer
> >> or project requirement. The PF3 instance a VirtIO net device for example,
> >> will bind with virtio-net driver
> >> presenting itself as a network interface to the OS.
> 
> Hi Tom,
> 
> These are very good questions, and the answers will be addressed in the
> documentation associated with a v2 submission of this patch.
> 
> >
> > What I mean there is heterogeneous set of cards in one machine, how do you
> > tell which card is which ?
> 
> If the PCI PID/VID is generic, indicating only that there is one or more
> DFL, then some other mechanism must be used to differentiate the cards.
> One could use unique PCI sub-PID/sub-VIDs to differentiate specific
> implementations.  One could also use some register in BAR space to help
> identify the card, or one could use PCI Vital Product Data (VPD) to
> provide detailed information about the running FPGA design on the card.

Ideally DFL has different scope than PCI. DFL is a higher layer concept than
PCI, as DFL can be applied to PCI device, platform device or even other devices.
If some PCI level quirks need to be applied before accessing BAR for one card,
then DFL may not be able to help at all. Use PCI level solution should be better,
and different VID/DID may be the easiest solution.

Hao

> 
> >
> > Or in a datacenter where the machines are all remote and admin has to flash
> > just the n6000's ?
> 
> This problem exists with the N3000 cards.  Depending on the FPGA
> configuration, the line side of the card could be very different (e.g.
> 4x10Gb or 2x2x25Gb).  The network operator must make sure to update a
> particular N3000 card with the correct FPGA image type.  In the case of
> the N3000 there is a register exposed through sysfs containing the
> "Bitstream ID" which contains the line side configuration of the FPGA.
> 
> >
> > How could she find just the n6000's with lspci ?
> 
> If you only wanted to use lspci to determine the card, then
> differentiating PCI sub-VID/sub-PID could be used or VPD could be used.
> 
> >
> > How would the driver know ?
> 
> The dfl-pci driver is fairly generic in that it doesn't really care about
> the PCI PID/VID because all it really does enumerate the DFLs.  It is the
> individual dfl drivers that may need to know hw differences/bugs for that
> component IP.
> 
> >
> > Tom
> >
> >>
> >>> A dfl set can change with fw updates and in theory different boards could
> >>> have
> >>> the same set.
> >>>
> >>> Tom
> >>>
> >>>> Matthew
> >>>>>>> Tom
> >>>>>>>
> >>>>>>>>     /* VF Device */
> >>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
> >>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
> >>>>>>>>     #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
> >>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
> >>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
> >>>>>>>>
> >>>>>>>>     static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
> >>>>>>> @@
> >>>>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
> >>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
> >>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
> >>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> >>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
> >>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_OFS),},
> >>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> >>>>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
> >>>>>>>>         {0,}
> >>>>>>>>     };
> >>>>>>>>     MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
> >>>>>
> >
> >

Matthew Gerlach March 1, 2022, 12:25 a.m. UTC | #11

On Mon, 28 Feb 2022, Wu, Hao wrote:

>>>
>>> On 2/21/22 7:11 PM, Zhang, Tianfei wrote:
>>>>
>>>>> -----Original Message-----
>>>>> From: Tom Rix <[e-mail]>
>>>>> Sent: Tuesday, February 22, 2022 2:10 AM
>>>>> To: [e-mail]
>>>>> Cc: Zhang, Tianfei <[e-mail]>; Wu, Hao
>> <[e-mail]>;
>>>>> [e-mail]; Xu, Yilun <[e-mail]>;
>>>>> [e-mail];
>>>>> [e-mail]; [e-mail]; [e-mail]
>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>>>>
>>>>>
>>>>> On 2/21/22 9:50 AM, [e-mail] wrote:
>>>>>>
>>>>>> On Fri, 18 Feb 2022, Tom Rix wrote:
>>>>>>
>>>>>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Tom Rix <[e-mail]>
>>>>>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
>>>>>>>>> To: Zhang, Tianfei <[e-mail]>; Wu, Hao
>>>>>>>>> <[e-mail]>; [e-mail]; Xu, Yilun <[e-mail]>;
>>>>>>>>> [e-mail]; [e-mail];
>>>>>>>>> [e-mail]
>>>>>>>>> Cc: [e-mail]; Matthew Gerlach
>>>>>>>>> <[e-mail]>
>>>>>>>>> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 2/14/22 3:26 AM, Tianfei zhang wrote:
>>>>>>>>>> From: Matthew Gerlach <[e-mail]>
>>>>>>>>>>
>>>>>>>>>> Add the PCI product id for an Open FPGA Stack PCI card.
>>>>>>>>> Is there a URL to the card ?
>>>>>>>> This PCIe Device IDs have registered by Intel.
>>>>>>> A URL is useful to introduce the board, Is there one ?
>>>>>>>>>> Signed-off-by: Matthew Gerlach <[e-mail]>
>>>>>>>>>> Signed-off-by: Tianfei Zhang <[e-mail]>
>>>>>>>>>> ---
>>>>>>>>>>     drivers/fpga/dfl-pci.c | 4 ++++
>>>>>>>>>>     1 file changed, 4 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
>>>>>>>>>> 83b604d6dbe6..cb2fbf3eb918 100644
>>>>>>>>>> --- a/drivers/fpga/dfl-pci.c
>>>>>>>>>> +++ b/drivers/fpga/dfl-pci.c
>>>>>>>>>> @@ -76,12 +76,14 @@ static void cci_pci_free_irq(struct pci_dev
>>>>>>>>>> *pcidev)
>>>>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005        0x0B2B
>>>>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5010    0x1000
>>>>>>>>>>     #define PCIE_DEVICE_ID_SILICOM_PAC_N5011    0x1001
>>>>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS        0xbcce
>>>>>>>>> INTEL_OFS is a generic name, pci id's map to specific cards
>>>>>>>>>
>>>>>>>>> Is there a more specific name for this card ?
>>>>>>>> I think using INTEL_OFS is better, because INTEL_OFS is the Generic
>>>>>>>> development platform can support multiple cards which using OFS
>>>>>>>> specification, like Intel PAC N6000 card.
>>>>>>> I would prefer something like PCIE_DEVICE_ID_INTEL_PAC_N6000
>> because
>>>>>>> it follows an existing pattern.  Make it easy on a developer, they
>>>>>>> will look at their board or box, see X and try to find something
>>>>>>> similar in the driver source.
>>>>>>>
>>>>>>> To use OSF_ * the name needs a suffix to differentiate it from future
>>>>>>> cards that will also use ofs.
>>>>>>>
>>>>>>> If this really is a generic id please explain in the doc patch how
>>>>>>> every future board with use this single id and how a driver could
>>>>>>> work around a hw problem in a specific board with a pci id covering
>>>>>>> multiple boards.
>>>>>>>
>>>>>>> Tom
>>>>>> Hi Tom,
>>>>>>
>>>>>> The intent is to have a generic device id that can be used with many
>>>>>> different boards.  Currently, we have FPGA implementations for 3
>>>>>> different boards using this generic id.  We may need a better name for
>>>>>> device id than OFS.  More precisely this generic device id means a PCI
>>>>>> function that is described by a Device Feature List (DFL).  How about
>>>>>> PCIE_DEVICE_ID_INTEL_DFL?
>>>>>>
>>>>>> With a DFL device id, the functionality of the PF/VF is determined by
>>>>>> the contents of the DFL.  Each Device Feature Header (DFH) in the DFL
>>>>>> has a revision field that can be used identify "broken" hw, or new
>>>>>> functionality added to a feature.  Additionally, since the DFL is
>>>>>> typically used in a FPGA, the broken hardware, can and should be fixed
>>>>>> in most cases.
>>>>> How is lspci supposed to work ?
>>>> There is an example for one card using IOFS and DFL.
>>>>
>>>> # lspci | grep acc
>>>> b1:00.0 Processing accelerators: Intel Corporation Device bcce (rev 01)
>>>> b1:00.1 Processing accelerators: Intel Corporation Device bcce
>>>> b1:00.2 Processing accelerators: Intel Corporation Device bcce
>>>> b1:00.3 Processing accelerators: Red Hat, Inc. Virtio network device
>>>> b1:00.4 Processing accelerators: Intel Corporation Device bcce
>>>>
>>>> Note: There 5 PFs in this card, it exports the management functions via
>>>> PF0(b1:00.0),
>>>> Other PFs like b1:00.1, b1:00.2, b1:00.4, are using for testing, which
>>>> depends on RTL designer
>>>> or project requirement. The PF3 instance a VirtIO net device for example,
>>>> will bind with virtio-net driver
>>>> presenting itself as a network interface to the OS.
>>
>> Hi Tom,
>>
>> These are very good questions, and the answers will be addressed in the
>> documentation associated with a v2 submission of this patch.
>>
>>>
>>> What I mean there is heterogeneous set of cards in one machine, how do you
>>> tell which card is which ?
>>
>> If the PCI PID/VID is generic, indicating only that there is one or more
>> DFL, then some other mechanism must be used to differentiate the cards.
>> One could use unique PCI sub-PID/sub-VIDs to differentiate specific
>> implementations.  One could also use some register in BAR space to help
>> identify the card, or one could use PCI Vital Product Data (VPD) to
>> provide detailed information about the running FPGA design on the card.
>
> Ideally DFL has different scope than PCI. DFL is a higher layer concept than
> PCI, as DFL can be applied to PCI device, platform device or even other devices.
> If some PCI level quirks need to be applied before accessing BAR for one card,
> then DFL may not be able to help at all. Use PCI level solution should be better,
> and different VID/DID may be the easiest solution.
>
> Hao

Very good point Hao.  DFL is a higher layer than PCI.  So PCI level quirks 
would need to be handled at the PCI level.  The VID/DID, optionally 
in conjuction with the Subsytem Vendor ID and Substem ID, would be used to 
determine how the quirks were applied.

Matthew


>>
>>>
>>> Or in a datacenter where the machines are all remote and admin has to flash
>>> just the n6000's ?
>>
>> This problem exists with the N3000 cards.  Depending on the FPGA
>> configuration, the line side of the card could be very different (e.g.
>> 4x10Gb or 2x2x25Gb).  The network operator must make sure to update a
>> particular N3000 card with the correct FPGA image type.  In the case of
>> the N3000 there is a register exposed through sysfs containing the
>> "Bitstream ID" which contains the line side configuration of the FPGA.
>>
>>>
>>> How could she find just the n6000's with lspci ?
>>
>> If you only wanted to use lspci to determine the card, then
>> differentiating PCI sub-VID/sub-PID could be used or VPD could be used.
>>
>>>
>>> How would the driver know ?
>>
>> The dfl-pci driver is fairly generic in that it doesn't really care about
>> the PCI PID/VID because all it really does enumerate the DFLs.  It is the
>> individual dfl drivers that may need to know hw differences/bugs for that
>> component IP.
>>
>>>
>>> Tom
>>>
>>>>
>>>>> A dfl set can change with fw updates and in theory different boards could
>>>>> have
>>>>> the same set.
>>>>>
>>>>> Tom
>>>>>
>>>>>> Matthew
>>>>>>>>> Tom
>>>>>>>>>
>>>>>>>>>>     /* VF Device */
>>>>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_5_X        0xBCBF
>>>>>>>>>>     #define PCIE_DEVICE_ID_VF_INT_6_X        0xBCC1
>>>>>>>>>>     #define PCIE_DEVICE_ID_VF_DSC_1_X        0x09C5
>>>>>>>>>>     #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF    0x0B2C
>>>>>>>>>> +#define PCIE_DEVICE_ID_INTEL_OFS_VF        0xbccf
>>>>>>>>>>
>>>>>>>>>>     static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>>>>> PCIE_DEVICE_ID_PF_INT_5_X),},
>>>>>>>>> @@
>>>>>>>>>> -95,6 +97,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>>>>>>>>>>         {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>>>> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
>>>>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
>>>>>>>>>> {PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
>>>>>>>>>> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>>>>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>> PCIE_DEVICE_ID_INTEL_OFS),},
>>>>>>>>>> +    {PCI_DEVICE(PCI_VENDOR_ID_INTEL,
>>>>>>>>> PCIE_DEVICE_ID_INTEL_OFS_VF),},
>>>>>>>>>>         {0,}
>>>>>>>>>>     };
>>>>>>>>>>     MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
>>>>>>>
>>>
>>>
>

Patch

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index 83b604d6dbe6..cb2fbf3eb918 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -76,12 +76,14 @@  static void cci_pci_free_irq(struct pci_dev *pcidev)
 #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
 #define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
 #define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
+#define PCIE_DEVICE_ID_INTEL_OFS		0xbcce
 
 /* VF Device */
 #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
 #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
 #define PCIE_DEVICE_ID_VF_DSC_1_X		0x09C5
 #define PCIE_DEVICE_ID_INTEL_PAC_D5005_VF	0x0B2C
+#define PCIE_DEVICE_ID_INTEL_OFS_VF		0xbccf
 
 static struct pci_device_id cci_pcie_id_tbl[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
@@ -95,6 +97,8 @@  static struct pci_device_id cci_pcie_id_tbl[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
 	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
 	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_OFS_VF),},
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);