diff mbox

[v3,08/21] fpga: add Intel FPGA DFL PCIe device

Message ID 1511764948-20972-9-git-send-email-hao.wu@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Wu, Hao Nov. 27, 2017, 6:42 a.m. UTC
From: Zhang Yi <yi.z.zhang@intel.com>

The Intel FPGA device appears as a PCIe device on the system. This patch
implements the basic framework of the driver for Intel PCIe device which
is located between CPU and Accelerated Function Units (AFUs), and has
the Device Feature List (DFL) implemented in its MMIO space.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Signed-off-by: Enno Luebbers <enno.luebbers@intel.com>
Signed-off-by: Shiva Rao <shiva.rao@intel.com>
Signed-off-by: Christopher Rauer <christopher.rauer@intel.com>
Signed-off-by: Zhang Yi <yi.z.zhang@intel.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
----
v2: move the code to drivers/fpga folder as suggested by Alan Tull.
    switch to GPLv2 license.
    fix comments from Moritz Fischer.
v3: switch to pci_set_dma_mask/consistent_dma_mask() function.
    remove pci_save_state() in probe function.
    rename driver to INTEL_FPGA_DFL_PCI and intel-dfl-pci.c to indicate
    this driver supports Intel FPGA PCI devices which implement DFL.
    improve Kconfig description for INTEL_FPGA_DFL_PCI
---
 drivers/fpga/Kconfig         |  18 ++++++
 drivers/fpga/Makefile        |   3 +
 drivers/fpga/intel-dfl-pci.c | 129 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 150 insertions(+)
 create mode 100644 drivers/fpga/intel-dfl-pci.c

Comments

David Laight Nov. 27, 2017, 10:28 a.m. UTC | #1
From: Wu Hao
> Sent: 27 November 2017 06:42
> From: Zhang Yi <yi.z.zhang@intel.com>
> 
> The Intel FPGA device appears as a PCIe device on the system. This patch
> implements the basic framework of the driver for Intel PCIe device which
> is located between CPU and Accelerated Function Units (AFUs), and has
> the Device Feature List (DFL) implemented in its MMIO space.

This ought to have a better name than 'Intel FPGA'.
An fpga can be used for all sorts of things, this looks like
a very specific architecture using a common VHDL environment to
allow certain types of user VHDL be accessed over PCIe.

	David

--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao Nov. 28, 2017, 3:15 a.m. UTC | #2
On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
> From: Wu Hao
> > Sent: 27 November 2017 06:42
> > From: Zhang Yi <yi.z.zhang@intel.com>
> > 
> > The Intel FPGA device appears as a PCIe device on the system. This patch
> > implements the basic framework of the driver for Intel PCIe device which
> > is located between CPU and Accelerated Function Units (AFUs), and has
> > the Device Feature List (DFL) implemented in its MMIO space.
> 
> This ought to have a better name than 'Intel FPGA'.
> An fpga can be used for all sorts of things, this looks like
> a very specific architecture using a common VHDL environment to
> allow certain types of user VHDL be accessed over PCIe.

Hi David

This patch adds a pcie device driver for Intel FPGA devices which implements
the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
Acceleration Cards. They are pcie devices, and all have DFL implemented in
the MMIO space, so we would like to use one kernel driver to handle them.

With this full patchset, it just provides user the interfaces to configure
and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
users can develop and build their own logics via tools provided by Intel,
program them to accelerators on these Intel FPGA devices, and access them
for their workloads.

Thanks
Hao
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull Dec. 4, 2017, 7:46 p.m. UTC | #3
On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
> On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
>> From: Wu Hao
>> > Sent: 27 November 2017 06:42
>> > From: Zhang Yi <yi.z.zhang@intel.com>
>> >
>> > The Intel FPGA device appears as a PCIe device on the system. This patch
>> > implements the basic framework of the driver for Intel PCIe device which
>> > is located between CPU and Accelerated Function Units (AFUs), and has
>> > the Device Feature List (DFL) implemented in its MMIO space.
>>
>> This ought to have a better name than 'Intel FPGA'.
>> An fpga can be used for all sorts of things, this looks like
>> a very specific architecture using a common VHDL environment to
>> allow certain types of user VHDL be accessed over PCIe.
>
> Hi David
>
> This patch adds a pcie device driver for Intel FPGA devices which implements
> the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> Acceleration Cards. They are pcie devices, and all have DFL implemented in
> the MMIO space, so we would like to use one kernel driver to handle them.
>
> With this full patchset, it just provides user the interfaces to configure
> and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
> users can develop and build their own logics via tools provided by Intel,
> program them to accelerators on these Intel FPGA devices, and access them
> for their workloads.

I don't see anything Intel specific here.  This could all be named dfl-*

Alan

>
> Thanks
> Hao
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao Dec. 5, 2017, 3:33 a.m. UTC | #4
On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
> >> From: Wu Hao
> >> > Sent: 27 November 2017 06:42
> >> > From: Zhang Yi <yi.z.zhang@intel.com>
> >> >
> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
> >> > implements the basic framework of the driver for Intel PCIe device which
> >> > is located between CPU and Accelerated Function Units (AFUs), and has
> >> > the Device Feature List (DFL) implemented in its MMIO space.
> >>
> >> This ought to have a better name than 'Intel FPGA'.
> >> An fpga can be used for all sorts of things, this looks like
> >> a very specific architecture using a common VHDL environment to
> >> allow certain types of user VHDL be accessed over PCIe.
> >
> > Hi David
> >
> > This patch adds a pcie device driver for Intel FPGA devices which implements
> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
> > the MMIO space, so we would like to use one kernel driver to handle them.
> >
> > With this full patchset, it just provides user the interfaces to configure
> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
> > users can develop and build their own logics via tools provided by Intel,
> > program them to accelerators on these Intel FPGA devices, and access them
> > for their workloads.
> 
> I don't see anything Intel specific here.  This could all be named dfl-*

The maybe some device specific things, e.g Intel FPGA devices supported by this
driver always have FME DFL at the beginning on the BAR0 for PF device.

But I think this should be the right direction for better code reuse, it could
save efforts for other vendors who want to use DFL and follow the same way.

Thanks for the comments. I will rename this driver in the next version.

Hao
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull Dec. 5, 2017, 5 p.m. UTC | #5
On Mon, Dec 4, 2017 at 9:33 PM, Wu Hao <hao.wu@intel.com> wrote:
> On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
>> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
>> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
>> >> From: Wu Hao
>> >> > Sent: 27 November 2017 06:42
>> >> > From: Zhang Yi <yi.z.zhang@intel.com>
>> >> >
>> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
>> >> > implements the basic framework of the driver for Intel PCIe device which
>> >> > is located between CPU and Accelerated Function Units (AFUs), and has
>> >> > the Device Feature List (DFL) implemented in its MMIO space.
>> >>
>> >> This ought to have a better name than 'Intel FPGA'.
>> >> An fpga can be used for all sorts of things, this looks like
>> >> a very specific architecture using a common VHDL environment to
>> >> allow certain types of user VHDL be accessed over PCIe.
>> >
>> > Hi David
>> >
>> > This patch adds a pcie device driver for Intel FPGA devices which implements
>> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
>> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
>> > the MMIO space, so we would like to use one kernel driver to handle them.
>> >
>> > With this full patchset, it just provides user the interfaces to configure
>> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
>> > users can develop and build their own logics via tools provided by Intel,
>> > program them to accelerators on these Intel FPGA devices, and access them
>> > for their workloads.
>>
>> I don't see anything Intel specific here.  This could all be named dfl-*
>
> The maybe some device specific things, e.g Intel FPGA devices supported by this
> driver always have FME DFL at the beginning on the BAR0 for PF device.
>
> But I think this should be the right direction for better code reuse, it could
> save efforts for other vendors who want to use DFL and follow the same way.
>
> Thanks for the comments. I will rename this driver in the next version.

Thanks!

Regarding file names, it seems like the files added to drivers/fpga
could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
other are currently dfl-*.[ch] currently.

Alan

>
> Hao
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao Dec. 6, 2017, 5:30 a.m. UTC | #6
On Tue, Dec 05, 2017 at 11:00:22AM -0600, Alan Tull wrote:
> On Mon, Dec 4, 2017 at 9:33 PM, Wu Hao <hao.wu@intel.com> wrote:
> > On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
> >> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
> >> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
> >> >> From: Wu Hao
> >> >> > Sent: 27 November 2017 06:42
> >> >> > From: Zhang Yi <yi.z.zhang@intel.com>
> >> >> >
> >> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
> >> >> > implements the basic framework of the driver for Intel PCIe device which
> >> >> > is located between CPU and Accelerated Function Units (AFUs), and has
> >> >> > the Device Feature List (DFL) implemented in its MMIO space.
> >> >>
> >> >> This ought to have a better name than 'Intel FPGA'.
> >> >> An fpga can be used for all sorts of things, this looks like
> >> >> a very specific architecture using a common VHDL environment to
> >> >> allow certain types of user VHDL be accessed over PCIe.
> >> >
> >> > Hi David
> >> >
> >> > This patch adds a pcie device driver for Intel FPGA devices which implements
> >> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> >> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
> >> > the MMIO space, so we would like to use one kernel driver to handle them.
> >> >
> >> > With this full patchset, it just provides user the interfaces to configure
> >> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
> >> > users can develop and build their own logics via tools provided by Intel,
> >> > program them to accelerators on these Intel FPGA devices, and access them
> >> > for their workloads.
> >>
> >> I don't see anything Intel specific here.  This could all be named dfl-*
> >
> > The maybe some device specific things, e.g Intel FPGA devices supported by this
> > driver always have FME DFL at the beginning on the BAR0 for PF device.
> >
> > But I think this should be the right direction for better code reuse, it could
> > save efforts for other vendors who want to use DFL and follow the same way.
> >
> > Thanks for the comments. I will rename this driver in the next version.
> 
> Thanks!
> 
> Regarding file names, it seems like the files added to drivers/fpga
> could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
> other are currently dfl-*.[ch] currently.

Sure, will have all related drivers files renamed to dfl-*.[ch].

Thanks
Hao

> 
> Alan
> 
> >
> > Hao
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight Dec. 6, 2017, 9:31 a.m. UTC | #7
RnJvbTogQWxhbiBUdWxsDQo+IFNlbnQ6IDA0IERlY2VtYmVyIDIwMTcgMTk6NDcNCj4gDQo+IE9u
IE1vbiwgTm92IDI3LCAyMDE3IGF0IDk6MTUgUE0sIFd1IEhhbyA8aGFvLnd1QGludGVsLmNvbT4g
d3JvdGU6DQo+ID4gT24gTW9uLCBOb3YgMjcsIDIwMTcgYXQgMTA6Mjg6MDRBTSArMDAwMCwgRGF2
aWQgTGFpZ2h0IHdyb3RlOg0KPiA+PiBGcm9tOiBXdSBIYW8NCj4gPj4gPiBTZW50OiAyNyBOb3Zl
bWJlciAyMDE3IDA2OjQyDQo+ID4+ID4gRnJvbTogWmhhbmcgWWkgPHlpLnouemhhbmdAaW50ZWwu
Y29tPg0KPiA+PiA+DQo+ID4+ID4gVGhlIEludGVsIEZQR0EgZGV2aWNlIGFwcGVhcnMgYXMgYSBQ
Q0llIGRldmljZSBvbiB0aGUgc3lzdGVtLiBUaGlzIHBhdGNoDQo+ID4+ID4gaW1wbGVtZW50cyB0
aGUgYmFzaWMgZnJhbWV3b3JrIG9mIHRoZSBkcml2ZXIgZm9yIEludGVsIFBDSWUgZGV2aWNlIHdo
aWNoDQo+ID4+ID4gaXMgbG9jYXRlZCBiZXR3ZWVuIENQVSBhbmQgQWNjZWxlcmF0ZWQgRnVuY3Rp
b24gVW5pdHMgKEFGVXMpLCBhbmQgaGFzDQo+ID4+ID4gdGhlIERldmljZSBGZWF0dXJlIExpc3Qg
KERGTCkgaW1wbGVtZW50ZWQgaW4gaXRzIE1NSU8gc3BhY2UuDQo+ID4+DQo+ID4+IFRoaXMgb3Vn
aHQgdG8gaGF2ZSBhIGJldHRlciBuYW1lIHRoYW4gJ0ludGVsIEZQR0EnLg0KPiA+PiBBbiBmcGdh
IGNhbiBiZSB1c2VkIGZvciBhbGwgc29ydHMgb2YgdGhpbmdzLCB0aGlzIGxvb2tzIGxpa2UNCj4g
Pj4gYSB2ZXJ5IHNwZWNpZmljIGFyY2hpdGVjdHVyZSB1c2luZyBhIGNvbW1vbiBWSERMIGVudmly
b25tZW50IHRvDQo+ID4+IGFsbG93IGNlcnRhaW4gdHlwZXMgb2YgdXNlciBWSERMIGJlIGFjY2Vz
c2VkIG92ZXIgUENJZS4NCj4gPg0KPiA+IEhpIERhdmlkDQo+ID4NCj4gPiBUaGlzIHBhdGNoIGFk
ZHMgYSBwY2llIGRldmljZSBkcml2ZXIgZm9yIEludGVsIEZQR0EgZGV2aWNlcyB3aGljaCBpbXBs
ZW1lbnRzDQo+ID4gdGhlIERGTCwgZS5nIEludGVsIFNlcnZlciBQbGF0Zm9ybSB3aXRoIEluLXBh
Y2thZ2UgRlBHQSBhbmQgSW50ZWwgRlBHQSBQQ0llDQo+ID4gQWNjZWxlcmF0aW9uIENhcmRzLiBU
aGV5IGFyZSBwY2llIGRldmljZXMsIGFuZCBhbGwgaGF2ZSBERkwgaW1wbGVtZW50ZWQgaW4NCj4g
PiB0aGUgTU1JTyBzcGFjZSwgc28gd2Ugd291bGQgbGlrZSB0byB1c2Ugb25lIGtlcm5lbCBkcml2
ZXIgdG8gaGFuZGxlIHRoZW0uDQo+ID4NCj4gPiBXaXRoIHRoaXMgZnVsbCBwYXRjaHNldCwgaXQg
anVzdCBwcm92aWRlcyB1c2VyIHRoZSBpbnRlcmZhY2VzIHRvIGNvbmZpZ3VyZQ0KPiA+IGFuZCBh
Y2Nlc3MgdGhlIEZQR0EgYWNjZWxlcmF0b3JzIG9uIEludGVsIERGTCBiYXNlZCBGUEdBIGRldmlj
ZXMuIEZvciBzdXJlLA0KPiA+IHVzZXJzIGNhbiBkZXZlbG9wIGFuZCBidWlsZCB0aGVpciBvd24g
bG9naWNzIHZpYSB0b29scyBwcm92aWRlZCBieSBJbnRlbCwNCj4gPiBwcm9ncmFtIHRoZW0gdG8g
YWNjZWxlcmF0b3JzIG9uIHRoZXNlIEludGVsIEZQR0EgZGV2aWNlcywgYW5kIGFjY2VzcyB0aGVt
DQo+ID4gZm9yIHRoZWlyIHdvcmtsb2Fkcy4NCj4gDQo+IEkgZG9uJ3Qgc2VlIGFueXRoaW5nIElu
dGVsIHNwZWNpZmljIGhlcmUuICBUaGlzIGNvdWxkIGFsbCBiZSBuYW1lZCBkZmwtKg0KDQpJbmRl
ZWQsIGRvZXNuJ3QgZXZlbiBzZWVtIHRvIGhhdmUgdG8gYmUgaW1wbGVtZW50ZWQgaW4gYW4gZnBn
YS4NCkl0IG1pZ2h0IGFsc28gbm90IGJlIHRoZSBvbmx5IGRldmljZSB0aGF0IGltcGxlbWVudHMg
REZMLg0KWW91IHJlYWxseSBuZWVkIGEgbmFtZSBmb3IgeW91ciBERkwgYWNjZWxlcmF0aW9uIGlt
cGxlbWVudGF0aW9uL2ludGVyZmFjZS4NCg0KV2UgbWFrZSBhIGJvYXJkIHRoYXQgdXNlcyBhbiBJ
bnRlbC9BbHRlcmEgZnBnYSBhcyBhIFBDSWUgZGV2aWNlLCB3b24ndCBsb29rDQphbnl0aGluZyBs
aWtlIHlvdXIgb25lIQ0KDQoJRGF2aWQNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight Dec. 6, 2017, 9:34 a.m. UTC | #8
From: Wu Hao
> Sent: 05 December 2017 03:34
...
> > I don't see anything Intel specific here.  This could all be named dfl-*
> 
> The maybe some device specific things, e.g Intel FPGA devices supported by this
> driver always have FME DFL at the beginning on the BAR0 for PF device.

Since when has that been a method for specifying what the card does?
You need to allocate a PCI-id for your DFL accelerator.

	David

--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight Dec. 6, 2017, 9:44 a.m. UTC | #9
From: Wu Hao
> Sent: 06 December 2017 05:30
...
> > Regarding file names, it seems like the files added to drivers/fpga
> > could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
> > other are currently dfl-*.[ch] currently.

They don't even want to do into a drivers/fgpa directory.
Maybe drivers/dfl or drivers/dfl/intel

	David

--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull Dec. 6, 2017, 3:29 p.m. UTC | #10
On Wed, Dec 6, 2017 at 3:44 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Wu Hao
>> Sent: 06 December 2017 05:30
> ...
>> > Regarding file names, it seems like the files added to drivers/fpga
>> > could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
>> > other are currently dfl-*.[ch] currently.
>
> They don't even want to do into a drivers/fgpa directory.
> Maybe drivers/dfl or drivers/dfl/intel

It's plugged into the fpga framework in drivers/fpga.  This patchset
also handles reprogramming the fpga, not just the dfl style
enumeration.  But your points about this being not just for FPGA are
interesting to me.  Do you have a use for this that isn't
FPGA-centric?

Alan

>
>         David
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Laight Dec. 6, 2017, 4:28 p.m. UTC | #11
From: Alan Tull

> Sent: 06 December 2017 15:30

> On Wed, Dec 6, 2017 at 3:44 AM, David Laight <David.Laight@aculab.com> wrote:

> > From: Wu Hao

> >> Sent: 06 December 2017 05:30

> > ...

> >> > Regarding file names, it seems like the files added to drivers/fpga

> >> > could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while

> >> > other are currently dfl-*.[ch] currently.

> >

> > They don't even want to do into a drivers/fgpa directory.

> > Maybe drivers/dfl or drivers/dfl/intel

> 

> It's plugged into the fpga framework in drivers/fpga.  This patchset

> also handles reprogramming the fpga, not just the dfl style

> enumeration.  But your points about this being not just for FPGA are

> interesting to me.  Do you have a use for this that isn't

> FPGA-centric?


That all just seems wrong to me.
If you've managed to invent some common code for reprogramming fpga
I'd have though it would be library functions.

The driver ought to sit somewhere related to its functionality.

Our fpga loads from a serial EEPROM, the image is about 6.5MB.
We can rewrite it from userspace by mmap()ing part of one of the BARs
to access some very locally written (by me) VHDL that does most of
the required bit-banging for 32it word accesses.
You really wouldn't want to load 6.5MB into kernel space!

We also had to solve the problem of 9 separate driver modules that
want to access different parts of the BARs.
I think we have 46 separate slaves in the fpgas BARs (most are in 1 BAR).
Some of these are common between different boards (or completely different
memory maps for the same board.

I can imagine some generic method of having a 'board' driver for a
specific PCI-id that knows the BAR offsets of various functions so that
other sub-drivers could be loaded to access those functions.
But that is some kind of pseudo-bus not fpga specific in any way.

	David
Alan Tull Dec. 6, 2017, 10:39 p.m. UTC | #12
On Wed, Dec 6, 2017 at 10:28 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Alan Tull
>> Sent: 06 December 2017 15:30
>> On Wed, Dec 6, 2017 at 3:44 AM, David Laight <David.Laight@aculab.com> wrote:
>> > From: Wu Hao
>> >> Sent: 06 December 2017 05:30
>> > ...
>> >> > Regarding file names, it seems like the files added to drivers/fpga
>> >> > could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
>> >> > other are currently dfl-*.[ch] currently.
>> >
>> > They don't even want to do into a drivers/fgpa directory.
>> > Maybe drivers/dfl or drivers/dfl/intel
>>
>> It's plugged into the fpga framework in drivers/fpga.  This patchset
>> also handles reprogramming the fpga, not just the dfl style
>> enumeration.  But your points about this being not just for FPGA are
>> interesting to me.  Do you have a use for this that isn't
>> FPGA-centric?
>
> That all just seems wrong to me.
> If you've managed to invent some common code for reprogramming fpga
> I'd have though it would be library functions.

Why don't you familiarize yourself with the fpga framework before
commenting on it? ;)

>
> The driver ought to sit somewhere related to its functionality.
>
> Our fpga loads from a serial EEPROM, the image is about 6.5MB.
> We can rewrite it from userspace by mmap()ing part of one of the BARs
> to access some very locally written (by me) VHDL that does most of
> the required bit-banging for 32it word accesses.

The fpga framework is intended to handle cases where the it is desired
to reprogram the fpga a lot without having to reboot.  It doesn't
sound like that is your use case.

> You really wouldn't want to load 6.5MB into kernel space!

No, and there have been proposals, shot down so far regarding
streaming firmware images a page at a time.

>
> We also had to solve the problem of 9 separate driver modules that
> want to access different parts of the BARs.
> I think we have 46 separate slaves in the fpgas BARs (most are in 1 BAR).
> Some of these are common between different boards (or completely different
> memory maps for the same board.
>
> I can imagine some generic method of having a 'board' driver for a
> specific PCI-id that knows the BAR offsets of various functions so that
> other sub-drivers could be loaded to access those functions.
> But that is some kind of pseudo-bus not fpga specific in any way.
>
>         David
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao Dec. 7, 2017, 3:47 a.m. UTC | #13
On Wed, Dec 06, 2017 at 09:34:14AM +0000, David Laight wrote:
> From: Wu Hao
> > Sent: 05 December 2017 03:34
> ...
> > > I don't see anything Intel specific here.  This could all be named dfl-*
> > 
> > The maybe some device specific things, e.g Intel FPGA devices supported by this
> > driver always have FME DFL at the beginning on the BAR0 for PF device.
> 
> Since when has that been a method for specifying what the card does?
> You need to allocate a PCI-id for your DFL accelerator.

This driver only supports Intel FPGA devices (see PCI Device Ids table
in this patch) as mentioned above now, per my current understanding, if
other vendors follow the same hardware design on using DFL for their PCIe
based devices, then it's possible for them to fully reuse this code,
otherwise they need to develop new drivers for their own designs or extend
this driver in some ways (it depends on actual hardware implementation).

Thanks
Hao

> 
> 	David
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull Feb. 1, 2018, 9:59 p.m. UTC | #14
On Tue, Dec 5, 2017 at 11:30 PM, Wu Hao <hao.wu@intel.com> wrote:
> On Tue, Dec 05, 2017 at 11:00:22AM -0600, Alan Tull wrote:
>> On Mon, Dec 4, 2017 at 9:33 PM, Wu Hao <hao.wu@intel.com> wrote:
>> > On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
>> >> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
>> >> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
>> >> >> From: Wu Hao
>> >> >> > Sent: 27 November 2017 06:42
>> >> >> > From: Zhang Yi <yi.z.zhang@intel.com>
>> >> >> >
>> >> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
>> >> >> > implements the basic framework of the driver for Intel PCIe device which
>> >> >> > is located between CPU and Accelerated Function Units (AFUs), and has
>> >> >> > the Device Feature List (DFL) implemented in its MMIO space.
>> >> >>
>> >> >> This ought to have a better name than 'Intel FPGA'.
>> >> >> An fpga can be used for all sorts of things, this looks like
>> >> >> a very specific architecture using a common VHDL environment to
>> >> >> allow certain types of user VHDL be accessed over PCIe.
>> >> >
>> >> > Hi David
>> >> >
>> >> > This patch adds a pcie device driver for Intel FPGA devices which implements
>> >> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
>> >> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
>> >> > the MMIO space, so we would like to use one kernel driver to handle them.
>> >> >
>> >> > With this full patchset, it just provides user the interfaces to configure
>> >> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
>> >> > users can develop and build their own logics via tools provided by Intel,
>> >> > program them to accelerators on these Intel FPGA devices, and access them
>> >> > for their workloads.
>> >>
>> >> I don't see anything Intel specific here.  This could all be named dfl-*
>> >
>> > The maybe some device specific things, e.g Intel FPGA devices supported by this
>> > driver always have FME DFL at the beginning on the BAR0 for PF device.

I'm thinking that another user could add their PCI id's and a static
FPGA image that has a DFL in the right place for this to work for
them.

>> >
>> > But I think this should be the right direction for better code reuse, it could
>> > save efforts for other vendors who want to use DFL and follow the same way.

I appreciate your understanding here.

>> >
>> > Thanks for the comments. I will rename this driver in the next version.
>>
>> Thanks!
>>
>> Regarding file names, it seems like the files added to drivers/fpga
>> could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
>> other are currently dfl-*.[ch] currently.
>
> Sure, will have all related drivers files renamed to dfl-*.[ch].

Actually, I'll reverse that a bit.  The enumeration code, including
the pcie part is all sufficiently general to run on anything that has
a DFL struct located in the right place.  But individual feature
drivers (currently only the fme-mgr) will be vendor specific and could
be named intel-*.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wu, Hao Feb. 13, 2018, 9:36 a.m. UTC | #15
On Thu, Feb 01, 2018 at 03:59:21PM -0600, Alan Tull wrote:
> On Tue, Dec 5, 2017 at 11:30 PM, Wu Hao <hao.wu@intel.com> wrote:
> > On Tue, Dec 05, 2017 at 11:00:22AM -0600, Alan Tull wrote:
> >> On Mon, Dec 4, 2017 at 9:33 PM, Wu Hao <hao.wu@intel.com> wrote:
> >> > On Mon, Dec 04, 2017 at 01:46:59PM -0600, Alan Tull wrote:
> >> >> On Mon, Nov 27, 2017 at 9:15 PM, Wu Hao <hao.wu@intel.com> wrote:
> >> >> > On Mon, Nov 27, 2017 at 10:28:04AM +0000, David Laight wrote:
> >> >> >> From: Wu Hao
> >> >> >> > Sent: 27 November 2017 06:42
> >> >> >> > From: Zhang Yi <yi.z.zhang@intel.com>
> >> >> >> >
> >> >> >> > The Intel FPGA device appears as a PCIe device on the system. This patch
> >> >> >> > implements the basic framework of the driver for Intel PCIe device which
> >> >> >> > is located between CPU and Accelerated Function Units (AFUs), and has
> >> >> >> > the Device Feature List (DFL) implemented in its MMIO space.
> >> >> >>
> >> >> >> This ought to have a better name than 'Intel FPGA'.
> >> >> >> An fpga can be used for all sorts of things, this looks like
> >> >> >> a very specific architecture using a common VHDL environment to
> >> >> >> allow certain types of user VHDL be accessed over PCIe.
> >> >> >
> >> >> > Hi David
> >> >> >
> >> >> > This patch adds a pcie device driver for Intel FPGA devices which implements
> >> >> > the DFL, e.g Intel Server Platform with In-package FPGA and Intel FPGA PCIe
> >> >> > Acceleration Cards. They are pcie devices, and all have DFL implemented in
> >> >> > the MMIO space, so we would like to use one kernel driver to handle them.
> >> >> >
> >> >> > With this full patchset, it just provides user the interfaces to configure
> >> >> > and access the FPGA accelerators on Intel DFL based FPGA devices. For sure,
> >> >> > users can develop and build their own logics via tools provided by Intel,
> >> >> > program them to accelerators on these Intel FPGA devices, and access them
> >> >> > for their workloads.
> >> >>
> >> >> I don't see anything Intel specific here.  This could all be named dfl-*
> >> >
> >> > The maybe some device specific things, e.g Intel FPGA devices supported by this
> >> > driver always have FME DFL at the beginning on the BAR0 for PF device.
> 
> I'm thinking that another user could add their PCI id's and a static
> FPGA image that has a DFL in the right place for this to work for
> them.
> 
> >> >
> >> > But I think this should be the right direction for better code reuse, it could
> >> > save efforts for other vendors who want to use DFL and follow the same way.
> 
> I appreciate your understanding here.
> 
> >> >
> >> > Thanks for the comments. I will rename this driver in the next version.
> >>
> >> Thanks!
> >>
> >> Regarding file names, it seems like the files added to drivers/fpga
> >> could be uniformly named dfl-*.[ch].  Some are fpga-dfl-*.[ch] while
> >> other are currently dfl-*.[ch] currently.
> >
> > Sure, will have all related drivers files renamed to dfl-*.[ch].
> 
> Actually, I'll reverse that a bit.  The enumeration code, including
> the pcie part is all sufficiently general to run on anything that has
> a DFL struct located in the right place.  But individual feature
> drivers (currently only the fme-mgr) will be vendor specific and could
> be named intel-*.

Hi Alan,

I unified all the drivers to use dfl-* in the v4 patchset, including fme-mgr.
As I feel it's hard to say which FME functions (sub features, registers) are
vendor specific and which FME functions are not, from the spec, they all belong
to FME, and people can be reused for sure. So I didn't rename it back to Intel
driver in the v4 patchset. :)

Thanks
Hao


> 
> Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 01ad31f..cc35d12 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -140,4 +140,22 @@  config FPGA_DFL
 	  Gate Array (FPGA) solutions which implement Device Feature List.
 	  It provides enumeration APIs, and feature device infrastructure.
 
+config INTEL_FPGA_DFL_PCI
+	tristate "Intel FPGA DFL PCIe Device Driver"
+	depends on PCI && FPGA_DFL
+	help
+	  Select this option to enable PCIe driver for Intel(R) PCIe based
+	  Field-Programmable Gate Array (FPGA) solutions which implemented
+	  the Device Feature List (DFL). It supports both integrated (e.g
+	  Intel Server Platform with In-package FPGA) and discrete (e.g
+	  Intel FPGA PCIe Acceleration Cards) solutions. This driver
+	  provides interfaces for userspace applications to configure,
+	  enumerate, open and access FPGA accelerators on platforms
+	  equipped with Intel(R) FPGA solutions and enables system level
+	  management functions such as FPGA partial reconfiguration, power
+	  management, and virtualization via DFL framework and DFL feature
+	  device drivers.
+
+	  To compile this as a module, choose M here.
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 447ba2b..d39a431 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -30,3 +30,6 @@  obj-$(CONFIG_OF_FPGA_REGION)		+= of-fpga-region.o
 
 # FPGA Device Feature List Support
 obj-$(CONFIG_FPGA_DFL)			+= fpga-dfl.o
+
+# Drivers for FPGAs which implement DFL
+obj-$(CONFIG_INTEL_FPGA_DFL_PCI)	+= intel-dfl-pci.o
diff --git a/drivers/fpga/intel-dfl-pci.c b/drivers/fpga/intel-dfl-pci.c
new file mode 100644
index 0000000..4774a77
--- /dev/null
+++ b/drivers/fpga/intel-dfl-pci.c
@@ -0,0 +1,129 @@ 
+/*
+ * Driver for Intel FPGA DFL PCIe device
+ *
+ * Copyright (C) 2017 Intel Corporation, Inc.
+ *
+ * Authors:
+ *   Zhang Yi <Yi.Z.Zhang@intel.com>
+ *   Xiao Guangrong <guangrong.xiao@linux.intel.com>
+ *   Joseph Grecco <joe.grecco@intel.com>
+ *   Enno Luebbers <enno.luebbers@intel.com>
+ *   Tim Whisonant <tim.whisonant@intel.com>
+ *   Ananda Ravuri <ananda.ravuri@intel.com>
+ *   Henry Mitchel <henry.mitchel@intel.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <linux/pci.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/stddef.h>
+#include <linux/errno.h>
+#include <linux/aer.h>
+
+#define DRV_VERSION	"0.8"
+#define DRV_NAME	"intel-dfl-pci"
+
+/* PCI Device ID */
+#define PCIE_DEVICE_ID_PF_INT_5_X	0xBCBD
+#define PCIE_DEVICE_ID_PF_INT_6_X	0xBCC0
+#define PCIE_DEVICE_ID_PF_DSC_1_X	0x09C4
+/* 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
+
+static struct pci_device_id cci_pcie_id_tbl[] = {
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_5_X),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_VF_INT_5_X),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_INT_6_X),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_VF_INT_6_X),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_PF_DSC_1_X),},
+	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_VF_DSC_1_X),},
+	{0,}
+};
+MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
+
+static
+int cci_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *pcidevid)
+{
+	int ret;
+
+	ret = pci_enable_device(pcidev);
+	if (ret < 0) {
+		dev_err(&pcidev->dev, "Failed to enable device %d.\n", ret);
+		return ret;
+	}
+
+	ret = pci_enable_pcie_error_reporting(pcidev);
+	if (ret && ret != -EINVAL)
+		dev_info(&pcidev->dev, "PCIE AER unavailable %d.\n", ret);
+
+	ret = pci_request_regions(pcidev, DRV_NAME);
+	if (ret) {
+		dev_err(&pcidev->dev, "Failed to request regions.\n");
+		goto disable_error_report_exit;
+	}
+
+	pci_set_master(pcidev);
+
+	if (!pci_set_dma_mask(pcidev, DMA_BIT_MASK(64))) {
+		ret = pci_set_consistent_dma_mask(pcidev, DMA_BIT_MASK(64));
+		if (ret)
+			goto release_region_exit;
+	} else if (!pci_set_dma_mask(pcidev, DMA_BIT_MASK(32))) {
+		ret = pci_set_consistent_dma_mask(pcidev, DMA_BIT_MASK(32));
+		if (ret)
+			goto release_region_exit;
+	} else {
+		ret = -EIO;
+		dev_err(&pcidev->dev, "No suitable DMA support available.\n");
+		goto release_region_exit;
+	}
+
+	/* TODO: create and add the platform device per feature list */
+	return 0;
+
+release_region_exit:
+	pci_release_regions(pcidev);
+disable_error_report_exit:
+	pci_disable_pcie_error_reporting(pcidev);
+	pci_disable_device(pcidev);
+	return ret;
+}
+
+static void cci_pci_remove(struct pci_dev *pcidev)
+{
+	pci_release_regions(pcidev);
+	pci_disable_pcie_error_reporting(pcidev);
+	pci_disable_device(pcidev);
+}
+
+static struct pci_driver cci_pci_driver = {
+	.name = DRV_NAME,
+	.id_table = cci_pcie_id_tbl,
+	.probe = cci_pci_probe,
+	.remove = cci_pci_remove,
+};
+
+static int __init ccidrv_init(void)
+{
+	pr_info("Intel(R) FPGA PCIe Driver: Version %s\n", DRV_VERSION);
+
+	return pci_register_driver(&cci_pci_driver);
+}
+
+static void __exit ccidrv_exit(void)
+{
+	pci_unregister_driver(&cci_pci_driver);
+}
+
+module_init(ccidrv_init);
+module_exit(ccidrv_exit);
+
+MODULE_DESCRIPTION("Intel FPGA DFL PCIe Device Driver");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");