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

Zhang, Tianfei
2022-02-23T01:48:52Z

> -----Original Message-----
> From: Tom Rix <trix@redhat.com>
> Sent: Wednesday, February 23, 2022 12:11 AM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>;
> matthew.gerlach@linux.intel.com
> Cc: Wu, Hao <hao.wu@intel.com>; mdf@kernel.org; Xu, Yilun
> <yilun.xu@intel.com>; linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org;
> linux-kernel@vger.kernel.org; corbet@lwn.net
> 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 <trix@redhat.com>
> >> Sent: Tuesday, February 22, 2022 2:10 AM
> >> To: matthew.gerlach@linux.intel.com
> >> Cc: Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao
> >> <hao.wu@intel.com>; mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>;
> >> linux-fpga@vger.kernel.org; linux-doc@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; corbet@lwn.net
> >> Subject: Re: [PATCH v1 7/7] fpga: dfl: pci: Add generic OFS PCI PID
> >>
> >>
> >> On 2/21/22 9:50 AM, matthew.gerlach@linux.intel.com wrote:
> >>>
> >>> On Fri, 18 Feb 2022, Tom Rix wrote:
> >>>
> >>>> On 2/18/22 1:03 AM, Zhang, Tianfei wrote:
> >>>>>> -----Original Message-----
> >>>>>> From: Tom Rix <trix@redhat.com>
> >>>>>> Sent: Wednesday, February 16, 2022 12:16 AM
> >>>>>> To: Zhang, Tianfei <tianfei.zhang@intel.com>; Wu, Hao
> >>>>>> <hao.wu@intel.com>; mdf@kernel.org; Xu, Yilun
> >>>>>> <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> >>>>>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
> >>>>>> Cc: corbet@lwn.net; Matthew Gerlach
> >>>>>> <matthew.gerlach@linux.intel.com>
> >>>>>> 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 <matthew.gerlach@linux.intel.com>
> >>>>>>>
> >>>>>>> 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 <matthew.gerlach@linux.intel.com>
> >>>>>>> Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
> >>>>>>> ---
> >>>>>>>    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);
> >>>>