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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ