| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BN9PR11MB5483D95314F5D82F1EE31174E3129@BN9PR11MB5483.namprd11.prod.outlook.com>
Date: Thu, 17 Mar 2022 08:32:32 +0000
From: "Zhang, Tianfei" <tianfei.zhang@...el.com>
To: "Wu, Hao" <hao.wu@...el.com>, "trix@...hat.com" <trix@...hat.com>,
"mdf@...nel.org" <mdf@...nel.org>,
"Xu, Yilun" <yilun.xu@...el.com>,
"linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"rdunlap@...radead.org" <rdunlap@...radead.org>
CC: "corbet@....net" <corbet@....net>,
Matthew Gerlach <matthew.gerlach@...ux.intel.com>
Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> -----Original Message-----
> From: Wu, Hao <hao.wu@...el.com>
> Sent: Thursday, March 17, 2022 4:18 PM
> To: Zhang, Tianfei <tianfei.zhang@...el.com>; trix@...hat.com;
> mdf@...nel.org; Xu, Yilun <yilun.xu@...el.com>; linux-fpga@...r.kernel.org;
> linux-doc@...r.kernel.org; linux-kernel@...r.kernel.org;
> rdunlap@...radead.org
> Cc: corbet@....net; Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
>
> > > -----Original Message-----
> > > From: Wu, Hao <hao.wu@...el.com>
> > > Sent: Thursday, March 17, 2022 10:05 AM
> > > To: Zhang, Tianfei <tianfei.zhang@...el.com>; trix@...hat.com;
> > > mdf@...nel.org; Xu, Yilun <yilun.xu@...el.com>;
> > > linux-fpga@...r.kernel.org; linux-doc@...r.kernel.org;
> > > linux-kernel@...r.kernel.org; rdunlap@...radead.org
> > > Cc: corbet@....net; Matthew Gerlach
> > > <matthew.gerlach@...ux.intel.com>
> > > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Tianfei <tianfei.zhang@...el.com>
> > > > Sent: Wednesday, March 16, 2022 3:08 PM
> > > > To: Wu, Hao <hao.wu@...el.com>; trix@...hat.com; mdf@...nel.org;
> > > > Xu, Yilun <yilun.xu@...el.com>; linux-fpga@...r.kernel.org;
> > > > linux-doc@...r.kernel.org; linux-kernel@...r.kernel.org;
> > > > rdunlap@...radead.org
> > > > Cc: corbet@....net; Matthew Gerlach
> > > > <matthew.gerlach@...ux.intel.com>;
> > > > Zhang, Tianfei <tianfei.zhang@...el.com>
> > > > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
> > > >
> > > > From: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> > > >
> > > > In OFS, each PR slot (AFU) has one port device which include Port
> > > > control, Port user clock control and Port errors. In legacy model,
> > > > the AFU MMIO space was connected with Port device, so from port
> > > > device point of view, there is a bar space associated with this port device.
> > > > But in "Multiple VFs per PR slot" model, the AFU MMIO space was
> > > > not connected with Port device. The BarID (3bits field) in
> > > > PORTn_OFFSET register indicates which PCI bar space associated
> > > > with this port device, the value 0b111 (FME_HDR_NO_PORT_BAR) means
> > > > that no PCI bar for this port device.
> > >
> > > The commit message is not matching the change, it's not related to AFU...
> > >
> > > Current usage (FME DFL and PORT DFL are not linked together)
> >
> > This usage is only on Intel PAC N3000 and N5000 card.
> > In my understand, the space of Port can put into any PCI bar space.
> > In the previous use case, the space of port was located on Bar 2.
> > For OFS, it allows the port without specific bar space.
>
> I didn't understand what you mean. Without your change, existing driver
> supports Port in any BAR indicated by PORTn_OFFSET, it's fine you put Port to
> BAR 0, or same BAR as FME. What do you mean by "port without specific bar
> space"?
"port with specific bar space" means that the port has a dedicated bar space, including the DFL, AFU, this is use
case in N3000/N5000 card.
"port without specific bar space" means the port without specific bar space, and the Port linked with FME for DFL
perspective.
>
> >
> > >
> > > FME DFL
> > > PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
> > >
> > > Your proposed new usage is (FME DFL and PORT DFL are linked
> > > together)
> > >
> > > FME DFL -> PORT DFL
> > > So FME's PORTn_OFFSET can be marked, then driver could skip it.
> > >
> > > Is my understanding correct? If yes, please update your title and
> > > commit message, and add some comments in code as well.
> >
> > From DLF perspective, I think it is yes.
> >
> > How about the title: "fpga: dfl: Allow Port and FME's DFL link together" ?
>
> "Allow Port to be linked to FME's DFL" should be better, as we don't encourage
> that people to connect FME DFL to Port DFL or any mixed order.
Looks good.
>
> >
> > I will also add some comments in code.
> > Here is the new git commit for this patch, any comments?
> >
> > In previous FPGA platform like Intel PAC N3000 and N5000, The BarID
> > (3bits field) in PORTn_OFFSET register indicated which PCI bar space
> > was associated with this port device. In this case, the DFL of Port
> > device was located in the specific PCI bar space, and then the FME and
> > Port's DFL were not linked. But in OFS, we extend the usage, it allows
> > the FME and Port's DFL linked together when there was no local PCI
> > bar space specified by the Port device. The value 0b111
> > (FME_HDR_NO_PORT_BAR) of BarID means that no specific PCI bar space
> > was associated with the port device.
>
> Currently we use PORTn_OFFSET to locate PORT DFLs, and PORT DFLs are not
> connected FME DFL. But for some cases (e.g. Intel Open FPGA Stack device),
> PORT DFLs are connected to FME DFL directly, so we don't need to search PORT
> DFLs via PORTn_OFFSET again. If BAR value of PORTn_OFFSET is 0x7
> (FME_PORT_OFST_BAR_SKIP/INVALID - depends the description added to DFL
> spec) then driver will skip searching the DFL for that port.
It is good for me.
>
> >
> > >
> > > Again, the change you did in dfl core code, is not only impacting
> > > your OFS device, but also future DFL devices, it's an extension to DFL.
> >
> > Yes, I agree that is an extended usage.
>
> Please make sure related changes documented in DFL spec as well.
I will add it on documentation.
>
> >
> > >
> > > Thanks
> > > Hao
> > >
> > > >
> > > > ---
> > > > v3: add PCI bar number checking with PCI_STD_NUM_BARS.
> > > > v2: use FME_HDR_NO_PORT_BAR instead of PCI_STD_NUM_BARS.
> > > >
> > > > Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> > > > Signed-off-by: Tianfei Zhang <tianfei.zhang@...el.com>
> > > > ---
> > > > drivers/fpga/dfl-pci.c | 7 +++++++
> > > > drivers/fpga/dfl.h | 1 +
> > > > 2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c index
> > > > 4d68719e608f..2e9abeca3625 100644
> > > > --- a/drivers/fpga/dfl-pci.c
> > > > +++ b/drivers/fpga/dfl-pci.c
> > > > @@ -258,6 +258,13 @@ static int find_dfls_by_default(struct
> > > > pci_dev
> > *pcidev,
> > > > */
> > > > bar = FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> > > > offset = FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> > > > + if (bar >= PCI_STD_NUM_BARS ||
> > > > + bar == FME_HDR_NO_PORT_BAR) {
> > > > + dev_dbg(&pcidev->dev, "skipping port without
> > > > local BAR space %d\n",
> > > > + bar);
> > > > + continue;
> > > > + }
> > > > +
> > > > start = pci_resource_start(pcidev, bar) + offset;
> > > > len = pci_resource_len(pcidev, bar) - offset;
> > > >
> > > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index
> > > > 53572c7aced0..1fd493e82dd8 100644
> > > > --- a/drivers/fpga/dfl.h
> > > > +++ b/drivers/fpga/dfl.h
> > > > @@ -91,6 +91,7 @@
> > > > #define FME_HDR_PORT_OFST(n) (0x38 + ((n) * 0x8))
> > > > #define FME_HDR_BITSTREAM_ID 0x60
> > > > #define FME_HDR_BITSTREAM_MD 0x68
> > > > +#define FME_HDR_NO_PORT_BAR 7
> > > >
> > > > /* FME Fab Capability Register Bitfield */
> > > > #define FME_CAP_FABRIC_VERID GENMASK_ULL(7, 0) /* Fabric
> > > > version ID */
> > > > --
> > > > 2.26.2
Powered by blists - more mailing lists