Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Wu, Hao" <hao.wu@intel.com>
To:     "Zhang, Tianfei" <tianfei.zhang@intel.com>,
        "trix@redhat.com" <trix@redhat.com>,
        "mdf@kernel.org" <mdf@kernel.org>,
        "Xu, Yilun" <yilun.xu@intel.com>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "rdunlap@infradead.org" <rdunlap@infradead.org>
CC:     "corbet@lwn.net" <corbet@lwn.net>,
        Matthew Gerlach <matthew.gerlach@linux.intel.com>
Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
Thread-Topic: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar space.
Thread-Index: AQHYOQUc6KfnoIn0pECU3HlxJyPskazC0EmAgABgbgCAAAKekA==
Date:   Thu, 17 Mar 2022 08:17:51 +0000
Message-ID: <DM6PR11MB381901ED86051F8D3117799A85129@DM6PR11MB3819.namprd11.prod.outlook.com>
References: <20220316070814.1916017-1-tianfei.zhang@intel.com>
 <20220316070814.1916017-2-tianfei.zhang@intel.com>
 <BYAPR11MB3816EAB6CC0047B2DBB5086685129@BYAPR11MB3816.namprd11.prod.outlook.com>
 <BN9PR11MB54839B3ABC5844DF45602995E3129@BN9PR11MB5483.namprd11.prod.outlook.com>
In-Reply-To: <BN9PR11MB54839B3ABC5844DF45602995E3129@BN9PR11MB5483.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.6.401.20
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?gj54rCJw+xPS0rWBzQotKHKYNeDq86CeWlaHIME2o6wet70S1JHrwdiv6Ddv?=
 =?us-ascii?Q?bsfUWaHqb7cp9vCuNqgpe04PNnvZ8jNd2GVF3N/mwdcPnTSXw6Tb6ShEfhvu?=
 =?us-ascii?Q?lX5gVAB/tuXo8hlIDVdx6DRjOFMdoLs1IazQCDMXTp72nUjRb+YtnqB/n3Px?=
 =?us-ascii?Q?dsHc7BkBmTe/zCpWIkNCHlAgc3kWGvnYLRFrMD48HFupHHAR0OdOJzvr+mHu?=
 =?us-ascii?Q?8D5vLe9+avBEVFwzYs/oWHjpl/CwV0+yW4N2wDz+9ZAR/EmVPgH2uvqQTfuL?=
 =?us-ascii?Q?qSn7pstupY5FsHoRRS7JMZbiB5PPh/yBwmDBDXaPyX2YI6AqEFeQOUaM+/m0?=
 =?us-ascii?Q?I3txOyBddD7A+gJiaZ8tvev2vR1C4Obw76qHx+1kEsm0UplT1ZeaoXcjXkYs?=
 =?us-ascii?Q?ARlr0nASHK6pATSMtmBQOb4ud4na7UlE2u8qA2GijQvFOD26mNsygQf3kSjT?=
 =?us-ascii?Q?0gdvQuvki27RSRp34jN90TX5bWQ9ZJtJ5CIdjt6Dik49lMXR3eCXKVuswQJE?=
 =?us-ascii?Q?WYftM3hc7hdDPSokVLhUQGyre3C7KTaEcKkuUaUK8wrEtCc+PY2GcidHM4Qj?=
 =?us-ascii?Q?zy8GvuJNuiC0vfM3f8rjrAT6A2gc3JLzTlDTc/UjWQ+VIq7vWEZAeCtaXETq?=
 =?us-ascii?Q?t6PhvRfHjkFMRNmFYQiyiduGbdwz8ctOkHzERjEpxyznr7Afc+1zu7gIisA0?=
 =?us-ascii?Q?Qkz21wYYHemsI25dF9UjtXt7G+qS0ifU8oQahSYQQt6R6lq23B9pb5N0L7j/?=
 =?us-ascii?Q?6P8u/IadsgHQwS7KM0iGuDPZpjulZLoieqzd/gBl1K89afmBAATTQX8cMzvO?=
 =?us-ascii?Q?CAYJG7PEJfYOG7ewpR88+kL6PdyfRW1o5BryQuvbI+MGpmwqmdOPtc+Txn9u?=
 =?us-ascii?Q?L8tZZ8fT0hPvHptlntYUx8s6HAnIhftnMAAZOl9k/nCD2qTch4OEHHD29HlI?=
 =?us-ascii?Q?GLRuD9JSwmr+d8FH6XC9LPmaI44JXhb8xXi3Ayu9/zpsvkKCHx5Uvsp84EDH?=
 =?us-ascii?Q?imEJ3bJNtolWruYMZvCFP9LnXyCNJixkUJ0gMssioRI+SA86GJKqyRRHBN4P?=
 =?us-ascii?Q?p5ZaKkshwcCUIf0zYv40UUXU8vB0RyUXX3rzJwXFTPV5FXtKjFRQ/+HfHruc?=
 =?us-ascii?Q?IpRI76uDyoPsVRA4SjuQq+PCaRDxlsDPaEjRv/gqfEjH66f8gEU9qEfK5P3M?=
 =?us-ascii?Q?fY3H/jUIXnKGzXUI40/HCfk25lsfAl9QwkhTGaxXGpC5dzkEZq3iXd/gQh4Y?=
 =?us-ascii?Q?eZck+z0yJd9rJl4XdrdfUn788WTGe3tLZE/gg9le0Keag+X1HebhTceuXCqq?=
 =?us-ascii?Q?vkNZKK/6a+GJwM+Je/Ma7lps2x9pXRZ9dIsQL/HJq7FvVkMe5W79oDIbXDsM?=
 =?us-ascii?Q?KzlAyX5jfI/66+ZTck08xL+f7Ign3ePC8xDp6pEo4BOIEjmS0TilBRZcqEYg?=
 =?us-ascii?Q?KAgDkmZAJ3o=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3819.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8d294cc8-1a17-4c62-eb71-08da07eea1a0
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Mar 2022 08:17:52.0229
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: NdP5dKfiie5CFh9bO+Hb33Yv2WLKZeg+Z6bjxzWKquMkvvxjRmgwHOLFg3fl3BjCc4Pm+8/7kXo2NhbkIh+QqA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4153
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > -----Original Message-----
> > From: Wu, Hao <hao.wu@intel.com>
> > Sent: Thursday, March 17, 2022 10:05 AM
> > To: Zhang, Tianfei <tianfei.zhang@intel.com>; trix@redhat.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;
> > rdunlap@infradead.org
> > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>
> > Subject: RE: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar sp=
ace.
> >
> > > -----Original Message-----
> > > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > > Sent: Wednesday, March 16, 2022 3:08 PM
> > > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.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;
> > > rdunlap@infradead.org
> > > Cc: corbet@lwn.net; Matthew Gerlach <matthew.gerlach@linux.intel.com>=
;
> > > Zhang, Tianfei <tianfei.zhang@intel.com>
> > > Subject: [PATCH v6 1/6] fpga: dfl: Allow ports without local bar spac=
e.
> > >
> > > From: Matthew Gerlach <matthew.gerlach@linux.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, th=
e
> > > 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)
>=20
> 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"?

>=20
> >
> > 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 commi=
t
> > message, and add some comments in code as well.
>=20
> From DLF perspective, I think it is yes.
>=20
> 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.

>=20
> I will also add some comments in code.
> Here is the new git commit for this patch, any comments?
>=20
> In previous FPGA platform like Intel PAC N3000 and N5000, The BarID (3bit=
s field)
> in PORTn_OFFSET
> register indicated which PCI bar space was associated with this port devi=
ce. In
> this case, the DFL of Port device
> was located in the specific PCI bar space, and then the FME and Port's DF=
L were
> not linked. But in OFS, we extend
> the usage, it allows the FME and Port's DFL  linked together when there w=
as 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),=
=20
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.

>=20
> >
> > 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.
>=20
> Yes, I agree that is an extended usage.

Please make sure related changes documented in DFL spec as well.

>=20
> >
> > 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@linux.intel.com>
> > > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.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 =3D FIELD_GET(FME_PORT_OFST_BAR_ID, v);
> > >  			offset =3D FIELD_GET(FME_PORT_OFST_DFH_OFST, v);
> > > +			if (bar >=3D PCI_STD_NUM_BARS ||
> > > +			    bar =3D=3D FME_HDR_NO_PORT_BAR) {
> > > +				dev_dbg(&pcidev->dev, "skipping port without
> > > local BAR space %d\n",
> > > +					bar);
> > > +				continue;
> > > +			}
> > > +
> > >  			start =3D pci_resource_start(pcidev, bar) + offset;
> > >  			len =3D 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