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:   "Zhang, Tianfei" <tianfei.zhang@intel.com>
To:     "Wu, Hao" <hao.wu@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: AQHYOQUcVwr1NO7I9EaBR7STlp8buqzC1K+AgABHRmA=
Date:   Thu, 17 Mar 2022 07:34:04 +0000
Message-ID: <BN9PR11MB54839B3ABC5844DF45602995E3129@BN9PR11MB5483.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>
In-Reply-To: <BYAPR11MB3816EAB6CC0047B2DBB5086685129@BYAPR11MB3816.namprd11.prod.outlook.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.401.20
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?2SB0Vb49tvoKN6nP6jquW2kAisiDjR5QmVe8yWKd0rrKK3QgJ+9cl/Uuqwkl?=
 =?us-ascii?Q?Dyd1FXdGGPpnCpFZQCtCUw1LKSRLTMFPHsIwmbwNJaRFzzsDod7IluKxTCIl?=
 =?us-ascii?Q?Wrzm0oqL4szcPrkBBXtVD/VtwyesIZg0txtEBfrS56TdQ0SZ2L+RdHU77T3d?=
 =?us-ascii?Q?PGCDjr32LGboz8j6ys/pCoYLqS3Ds3tchU3DipMxgHxHFgNCDCUS2UNTnvrL?=
 =?us-ascii?Q?a5yB4o8sS48Tf4ut64nUbPF5KdTurYsr8rTjgh50KaWsHl/2nX3shchJ1Ply?=
 =?us-ascii?Q?JfUQfngyE7NIxnIEgV6Xo8VN7YtM3lmRaKLz4pa8RxyrWs5zN/eUZpy11Wo9?=
 =?us-ascii?Q?m6he53mlgVOaN9yxtYSlfRETzMsYYlvbadgEYmEA4WNXmYPeqhefUEd+bg27?=
 =?us-ascii?Q?FmiRlUZZzGor6TEQwgmoR77WNhEbe7NqpOcv3gwpbUVuqYIoXmg8yimFzeS6?=
 =?us-ascii?Q?1cf6q4CI4A7FpjBWOqpHGRvBbdIy6Fq7LNEGr8PThwPhR/gCESphIZtEshan?=
 =?us-ascii?Q?a8cw98S1Sj+Pc4wBi1BiYzgjG465XV213I/8S8DEXVinevy/FVeZvrqiTZml?=
 =?us-ascii?Q?4FS6TNV5c6UXJ1Nr4s0zNEuzg9bm4rjHufeVhH+e2h7/l33Jbg0VJe7hBUHz?=
 =?us-ascii?Q?nPW6MTN3w2aijgKPW51L6ahr6TaG16nrSMYgQ4+/P3OBYnClZQsS9lTLlFFh?=
 =?us-ascii?Q?cYyXggPOsvhUlnCiWdaCs2R4/B3O1dfS0hCLr/SVv46yDn3MR+I7DBKWjBGd?=
 =?us-ascii?Q?kRAh+Xxm9YgiAgyjQTf2dYgVwfpGbgyrfWMmTZobFsVxkecOpSqF+RP9F8u6?=
 =?us-ascii?Q?hsSPse9vNnPkpmeQDSnoEPazAJ7NPNKQLJtHxco6Z9/AJM9VVdCe+J0uftAK?=
 =?us-ascii?Q?IogCjfQjgDfZAIWD+v3p7uDivQnHiUIAwTpnMJg7fNcFLJegKT/NFjcLvklp?=
 =?us-ascii?Q?I5RuahrXCzuU56uzJh8BojlWmH2w5FEBP1bggzaXgBSN8zJ55SyzTWTZ40gG?=
 =?us-ascii?Q?GimHZT0sqMDQRrMeYKKyHiFZpkQ0XUujqUmxujFyl0rrcKryyII0MA4p8UOE?=
 =?us-ascii?Q?mktv75fR32ouVQv14ZYZRImB19xYafbLz6Gq51+4Pm7N3oNtgUF2bSFTzr+g?=
 =?us-ascii?Q?l4WBmhkN0x2lFQIWc6k9l2Jh0GVUzx9W8jQsXDtFwgmTX8Y4FddGxiPGVIbJ?=
 =?us-ascii?Q?XHBjaJP0leramCmmEXT2fVDaj+mPeWsqFEqw35Wpamb2pENZ/aoVO3MvqCt5?=
 =?us-ascii?Q?3bdCLvgbnNyZ6UVMCENVAO8NAep+beY+liwKYJ9jNJoKtFcYZs0m+ERyvSnY?=
 =?us-ascii?Q?Czp37ph9ZqlpKFPUH3/YXUa8FZw8f0JbgHS/9Zn2ocwJLaTR0hAqMYRTUZhV?=
 =?us-ascii?Q?L0X9seoWyIfWqyWUnkIPdAOs3IjV5QToUMqTyQGtvN/fkXhwDeP0VYWuWjqE?=
 =?us-ascii?Q?E+DGHWaXmc6QGONAJkjMv8CxhzzOLzqP?=
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: BN9PR11MB5483.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1852c1fd-5458-413b-4307-08da07e8833a
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Mar 2022 07:34:04.0225
 (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: 7E7moZdU+4o3fCDw+6xsiLAtACA58QuhjPyU9654FkKYPzNQFWVkzlEOK8K1Ay2jmTaWNBxzLuGEbVykF6rI4A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4414
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.or=
g;
> 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 spac=
e.
>=20
> > -----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 space.
> >
> > 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, 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.
>=20
> The commit message is not matching the change, it's not related to AFU...
>=20
> Current usage (FME DFL and PORT DFL are not linked together)

This usage is only on Intel PAC N3000 and N5000 card.=20
In my understand, the space of Port can put into any PCI bar space.=20
In the previous use case, the space of port was located on Bar 2.
For OFS, it allows the port without specific bar space.

>=20
> FME DFL
> PORT DFL (located by FME's PORTn_OFFSET register, BAR + offset)
>=20
> Your proposed new usage is (FME DFL and PORT DFL are linked together)
>=20
> FME DFL -> PORT DFL
> So FME's PORTn_OFFSET can be marked, then driver could skip it.
>=20
> 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" ?

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 (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=20
the Port device. The value 0b111 (FME_HDR_NO_PORT_BAR) of BarID means that =
no specific PCI bar space=20
was associated with the port device.

>=20
> Again, the change you did in dfl core code, is not only impacting your OF=
S
> device, but also future DFL devices, it's an extension to DFL.

Yes, I agree that is an extended usage.

>=20
> Thanks
> Hao
>=20
> >
> > ---
> > 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 *pc=
idev,
> >  			 */
> >  			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