RE: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a document describing XRT Alveo drivers

[Sonal Santan]

Hello Moritz,

> -Original Message-
> From: Moritz Fischer 
> Sent: Saturday, March 6, 2021 9:19 AM
> To: Sonal Santan 
> Cc: Tom Rix ; Lizhi Hou ; linux-
> ker...@vger.kernel.org; linux-f...@vger.kernel.org; Max Zhen
> ; Michal Simek ; Stefano Stabellini
> ; devicet...@vger.kernel.org; m...@kernel.org;
> r...@kernel.org
> Subject: Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a
> document describing XRT Alveo drivers
> 
> On Mon, Mar 01, 2021 at 06:48:46AM +, Sonal Santan wrote:
> > Hello Tom,
> >
> > > -Original Message-
> > > From: Tom Rix 
> > > Sent: Friday, February 19, 2021 2:26 PM
> > > To: Lizhi Hou ; linux-kernel@vger.kernel.org
> > > Cc: Lizhi Hou ; linux-f...@vger.kernel.org; Max
> > > Zhen ; Sonal Santan ; Michal
> > > Simek ; Stefano Stabellini
> > > ; devicet...@vger.kernel.org; m...@kernel.org;
> > > r...@kernel.org; Max Zhen 
> > > Subject: Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a
> > > document describing XRT Alveo drivers
> > >
> > > From the documentation, there are a couple of big questions and a
> > > bunch of word smithing.
> > >
> > > pseudo-bus : do we need a bus ?
> > We are looking for guidance here.
> > >
> > > xrt-lib real platform devices that aren't fpga, do they need to move
> > > to another subsystem ?
> > >
> >
> > Drivers for the IPs that show up in the Alveo shell are not generic
> > enough. They fit into the framework that XRT uses. Is the idea that
> > that they can be used in a different context?
> >
> > > Overall looks good, love the ascii art!
> > >
> > > On 2/17/21 10:40 PM, Lizhi Hou wrote:
> > > > Describe XRT driver architecture and provide basic overview of
> > > > Xilinx Alveo platform.
> > > >
> > > > Signed-off-by: Sonal Santan 
> > > > Signed-off-by: Max Zhen 
> > > > Signed-off-by: Lizhi Hou 
> > > > ---
> > > >  Documentation/fpga/index.rst |   1 +
> > > >  Documentation/fpga/xrt.rst   | 842
> > > +++
> > > >  2 files changed, 843 insertions(+)  create mode 100644
> > > > Documentation/fpga/xrt.rst
> > > >
> > > > diff --git a/Documentation/fpga/index.rst
> > > > b/Documentation/fpga/index.rst index f80f95667ca2..30134357b70d
> > > > 100644
> > > > --- a/Documentation/fpga/index.rst
> > > > +++ b/Documentation/fpga/index.rst
> > > > @@ -8,6 +8,7 @@ fpga
> > > >  :maxdepth: 1
> > > >
> > > >  dfl
> > > > +xrt
> > > >
> > > >  .. only::  subproject and html
> > > >
> > > > diff --git a/Documentation/fpga/xrt.rst
> > > > b/Documentation/fpga/xrt.rst new file mode 100644 index
> > > > ..9bc2d2785cb9
> > > > --- /dev/null
> > > > +++ b/Documentation/fpga/xrt.rst
> > > > @@ -0,0 +1,842 @@
> > > > +.. SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +==
> > > > +XRTV2 Linux Kernel Driver Overview
> > > > +==
> > > > +
> > > > +Authors:
> > > > +
> > > > +* Sonal Santan 
> > > > +* Max Zhen 
> > > > +* Lizhi Hou 
> > > > +
> > > > +XRTV2 drivers are second generation `XRT
> > > > +<https://github.com/Xilinx/XRT>`_ drivers which support `Alveo
> > > > +<https://www.xilinx.com/products/boards-and-kits/alveo.html>`_
> > > > +PCIe platforms from Xilinx.
> > > > +
> > > > +XRTV2 drivers support *subsystem* style data driven platforms
> > > > +where driver's
> > > where the driver's
> > > > +configuration and behavior is determined by meta data provided by
> > > > +the platform (in *device tree* format). Primary management
> > > > +physical function (MPF) driver is called **xmgmt**. Primary user
> > > > +physical function (UPF) driver is called
> > > > +**xuser** and is under development. xrt driver framework and HW
> > > > +subsystem drivers are packaged into a library module called
> > > > +**xrt-lib**, which is shared by **xmgmt** and **xuser** (under
> > > > +development). The xrt driver framework
> > > xuser still under development ?
> > > > +implements 

Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a document describing XRT Alveo drivers

[Moritz Fischer]

On Mon, Mar 01, 2021 at 06:48:46AM +, Sonal Santan wrote:
> Hello Tom,
> 
> > -Original Message-
> > From: Tom Rix 
> > Sent: Friday, February 19, 2021 2:26 PM
> > To: Lizhi Hou ; linux-kernel@vger.kernel.org
> > Cc: Lizhi Hou ; linux-f...@vger.kernel.org; Max Zhen
> > ; Sonal Santan ; Michal Simek
> > ; Stefano Stabellini ;
> > devicet...@vger.kernel.org; m...@kernel.org; r...@kernel.org; Max Zhen
> > 
> > Subject: Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a
> > document describing XRT Alveo drivers
> > 
> > From the documentation, there are a couple of big questions and a bunch of
> > word smithing.
> > 
> > pseudo-bus : do we need a bus ?
> We are looking for guidance here. 
> > 
> > xrt-lib real platform devices that aren't fpga, do they need to move to 
> > another
> > subsystem ?
> > 
> 
> Drivers for the IPs that show up in the Alveo shell are not generic enough. 
> They 
> fit into the framework that XRT uses. Is the idea that that they can be used 
> in a 
> different context?
> 
> > Overall looks good, love the ascii art!
> > 
> > On 2/17/21 10:40 PM, Lizhi Hou wrote:
> > > Describe XRT driver architecture and provide basic overview of Xilinx
> > > Alveo platform.
> > >
> > > Signed-off-by: Sonal Santan 
> > > Signed-off-by: Max Zhen 
> > > Signed-off-by: Lizhi Hou 
> > > ---
> > >  Documentation/fpga/index.rst |   1 +
> > >  Documentation/fpga/xrt.rst   | 842
> > +++
> > >  2 files changed, 843 insertions(+)
> > >  create mode 100644 Documentation/fpga/xrt.rst
> > >
> > > diff --git a/Documentation/fpga/index.rst
> > > b/Documentation/fpga/index.rst index f80f95667ca2..30134357b70d 100644
> > > --- a/Documentation/fpga/index.rst
> > > +++ b/Documentation/fpga/index.rst
> > > @@ -8,6 +8,7 @@ fpga
> > >  :maxdepth: 1
> > >
> > >  dfl
> > > +xrt
> > >
> > >  .. only::  subproject and html
> > >
> > > diff --git a/Documentation/fpga/xrt.rst b/Documentation/fpga/xrt.rst
> > > new file mode 100644 index ..9bc2d2785cb9
> > > --- /dev/null
> > > +++ b/Documentation/fpga/xrt.rst
> > > @@ -0,0 +1,842 @@
> > > +.. SPDX-License-Identifier: GPL-2.0
> > > +
> > > +==
> > > +XRTV2 Linux Kernel Driver Overview
> > > +==
> > > +
> > > +Authors:
> > > +
> > > +* Sonal Santan 
> > > +* Max Zhen 
> > > +* Lizhi Hou 
> > > +
> > > +XRTV2 drivers are second generation `XRT
> > > +<https://github.com/Xilinx/XRT>`_ drivers which support `Alveo
> > > +<https://www.xilinx.com/products/boards-and-kits/alveo.html>`_
> > > +PCIe platforms from Xilinx.
> > > +
> > > +XRTV2 drivers support *subsystem* style data driven platforms where
> > > +driver's
> > where the driver's
> > > +configuration and behavior is determined by meta data provided by the
> > > +platform (in *device tree* format). Primary management physical
> > > +function (MPF) driver is called **xmgmt**. Primary user physical
> > > +function (UPF) driver is called
> > > +**xuser** and is under development. xrt driver framework and HW
> > > +subsystem drivers are packaged into a library module called
> > > +**xrt-lib**, which is shared by **xmgmt** and **xuser** (under
> > > +development). The xrt driver framework
> > xuser still under development ?
> > > +implements a pseudo-bus which is used to discover HW subsystems and
> > > +facilitate
> > 
> > A pseudo-bus.
> > 
> > It would be good if this was close to what was done for dfl here
> > 
> > https://lore.kernel.org/linux-fpga/1605159759-3439-1-git-send-email-
> > yilun...@intel.com/
> > 
> 
> I am wondering if we can phase in the migration to formal bus architecture 
> based on struct bus_type as a follow on set of patches?

I'd rather have it done early on. If we know there's a better way of
doing something and we don't the code should go into staging.

This gives us an out if it doesn't happen, otherwise the kernel
community would depend on corporate goodwill to appropriately staff it.

Note, I'm not trying to say there's any ill will anywhere, Xilinx has
been traditionally good about this :)
> 
> > > +inter HW subsystem interaction.
>

RE: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a document describing XRT Alveo drivers

[Sonal Santan]

Hello Tom,

> -Original Message-
> From: Tom Rix 
> Sent: Friday, February 19, 2021 2:26 PM
> To: Lizhi Hou ; linux-kernel@vger.kernel.org
> Cc: Lizhi Hou ; linux-f...@vger.kernel.org; Max Zhen
> ; Sonal Santan ; Michal Simek
> ; Stefano Stabellini ;
> devicet...@vger.kernel.org; m...@kernel.org; r...@kernel.org; Max Zhen
> 
> Subject: Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a
> document describing XRT Alveo drivers
> 
> From the documentation, there are a couple of big questions and a bunch of
> word smithing.
> 
> pseudo-bus : do we need a bus ?
We are looking for guidance here. 
> 
> xrt-lib real platform devices that aren't fpga, do they need to move to 
> another
> subsystem ?
> 

Drivers for the IPs that show up in the Alveo shell are not generic enough. 
They 
fit into the framework that XRT uses. Is the idea that that they can be used in 
a 
different context?

> Overall looks good, love the ascii art!
> 
> On 2/17/21 10:40 PM, Lizhi Hou wrote:
> > Describe XRT driver architecture and provide basic overview of Xilinx
> > Alveo platform.
> >
> > Signed-off-by: Sonal Santan 
> > Signed-off-by: Max Zhen 
> > Signed-off-by: Lizhi Hou 
> > ---
> >  Documentation/fpga/index.rst |   1 +
> >  Documentation/fpga/xrt.rst   | 842
> +++
> >  2 files changed, 843 insertions(+)
> >  create mode 100644 Documentation/fpga/xrt.rst
> >
> > diff --git a/Documentation/fpga/index.rst
> > b/Documentation/fpga/index.rst index f80f95667ca2..30134357b70d 100644
> > --- a/Documentation/fpga/index.rst
> > +++ b/Documentation/fpga/index.rst
> > @@ -8,6 +8,7 @@ fpga
> >  :maxdepth: 1
> >
> >  dfl
> > +xrt
> >
> >  .. only::  subproject and html
> >
> > diff --git a/Documentation/fpga/xrt.rst b/Documentation/fpga/xrt.rst
> > new file mode 100644 index ..9bc2d2785cb9
> > --- /dev/null
> > +++ b/Documentation/fpga/xrt.rst
> > @@ -0,0 +1,842 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +==
> > +XRTV2 Linux Kernel Driver Overview
> > +==
> > +
> > +Authors:
> > +
> > +* Sonal Santan 
> > +* Max Zhen 
> > +* Lizhi Hou 
> > +
> > +XRTV2 drivers are second generation `XRT
> > +<https://github.com/Xilinx/XRT>`_ drivers which support `Alveo
> > +<https://www.xilinx.com/products/boards-and-kits/alveo.html>`_
> > +PCIe platforms from Xilinx.
> > +
> > +XRTV2 drivers support *subsystem* style data driven platforms where
> > +driver's
> where the driver's
> > +configuration and behavior is determined by meta data provided by the
> > +platform (in *device tree* format). Primary management physical
> > +function (MPF) driver is called **xmgmt**. Primary user physical
> > +function (UPF) driver is called
> > +**xuser** and is under development. xrt driver framework and HW
> > +subsystem drivers are packaged into a library module called
> > +**xrt-lib**, which is shared by **xmgmt** and **xuser** (under
> > +development). The xrt driver framework
> xuser still under development ?
> > +implements a pseudo-bus which is used to discover HW subsystems and
> > +facilitate
> 
> A pseudo-bus.
> 
> It would be good if this was close to what was done for dfl here
> 
> https://lore.kernel.org/linux-fpga/1605159759-3439-1-git-send-email-
> yilun...@intel.com/
> 

I am wondering if we can phase in the migration to formal bus architecture 
based on struct bus_type as a follow on set of patches?

> > +inter HW subsystem interaction.
> > +
> > +Driver Modules
> > +==
> > +
> > +xrt-lib.ko
> > +--
> > +
> > +Repository of all subsystem drivers and pure software modules that
> > +can potentially
> 
> subsystem drivers
> 
> drivers in fpga/ should be for managing just the fpganess of the fpga.
> 
> soft devices ex/ a soft tty should go to their respective subsystem location
> 
> Are there any in this patchset you think might move ?

We have already shrunk the patch to only include FPGA centric pieces 
necessary to get the bitstream download implemented. Should we explore
the question of subsystem drivers when we add support for more features of 
the Alveo shell?

> 
> Maybe we can defer reviewing those now.
> 
> > +be shared between xmgmt and xuser. All these drivers are structured
> > +as Linux *platform driver* and are instantiated by xmgmt (or xuser
> > +under development) based on meta data associated with

Re: [PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a document describing XRT Alveo drivers

[Tom Rix]

>From the documentation, there are a couple of big questions and a bunch of 
>word smithing.

pseudo-bus : do we need a bus ?

xrt-lib real platform devices that aren't fpga, do they need to move to another 
subsystem ?

Overall looks good, love the ascii art!

On 2/17/21 10:40 PM, Lizhi Hou wrote:
> Describe XRT driver architecture and provide basic overview of
> Xilinx Alveo platform.
>
> Signed-off-by: Sonal Santan 
> Signed-off-by: Max Zhen 
> Signed-off-by: Lizhi Hou 
> ---
>  Documentation/fpga/index.rst |   1 +
>  Documentation/fpga/xrt.rst   | 842 +++
>  2 files changed, 843 insertions(+)
>  create mode 100644 Documentation/fpga/xrt.rst
>
> diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
> index f80f95667ca2..30134357b70d 100644
> --- a/Documentation/fpga/index.rst
> +++ b/Documentation/fpga/index.rst
> @@ -8,6 +8,7 @@ fpga
>  :maxdepth: 1
>  
>  dfl
> +xrt
>  
>  .. only::  subproject and html
>  
> diff --git a/Documentation/fpga/xrt.rst b/Documentation/fpga/xrt.rst
> new file mode 100644
> index ..9bc2d2785cb9
> --- /dev/null
> +++ b/Documentation/fpga/xrt.rst
> @@ -0,0 +1,842 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==
> +XRTV2 Linux Kernel Driver Overview
> +==
> +
> +Authors:
> +
> +* Sonal Santan 
> +* Max Zhen 
> +* Lizhi Hou 
> +
> +XRTV2 drivers are second generation `XRT `_
> +drivers which support `Alveo 
> `_
> +PCIe platforms from Xilinx.
> +
> +XRTV2 drivers support *subsystem* style data driven platforms where driver's
where the driver's
> +configuration and behavior is determined by meta data provided by the 
> platform
> +(in *device tree* format). Primary management physical function (MPF) driver
> +is called **xmgmt**. Primary user physical function (UPF) driver is called
> +**xuser** and is under development. xrt driver framework and HW subsystem
> +drivers are packaged into a library module called **xrt-lib**, which is
> +shared by **xmgmt** and **xuser** (under development). The xrt driver 
> framework
xuser still under development ?
> +implements a pseudo-bus which is used to discover HW subsystems and 
> facilitate

A pseudo-bus.

It would be good if this was close to what was done for dfl here

https://lore.kernel.org/linux-fpga/1605159759-3439-1-git-send-email-yilun...@intel.com/

> +inter HW subsystem interaction.
> +
> +Driver Modules
> +==
> +
> +xrt-lib.ko
> +--
> +
> +Repository of all subsystem drivers and pure software modules that can 
> potentially

subsystem drivers

drivers in fpga/ should be for managing just the fpganess of the fpga.

soft devices ex/ a soft tty should go to their respective subsystem location

Are there any in this patchset you think might move ?

Maybe we can defer reviewing those now.

> +be shared between xmgmt and xuser. All these drivers are structured as Linux
> +*platform driver* and are instantiated by xmgmt (or xuser under development) 
> based
> +on meta data associated with hardware. The metadata is in the form of device 
> tree

with the hardware

form of a device tree

> +as mentioned before. Each platform driver statically defines a subsystem node
> +array by using node name or a string in its ``compatible`` property. And this
> +array is eventually translated to IOMEM resources of the platform device.
> +
> +The xrt-lib core infrastructure provides hooks to platform drivers for 
> device node
> +management, user file operations and ioctl callbacks. The core also provides 
> pseudo-bus
> +functionality for platform driver registration, discovery and inter platform 
> driver
> +ioctl calls.

core infrastructure.

The interfaces to the infrastructure are not in include/linux/fpga/

Maybe this needs to change.

> +
> +.. note::
> +   See code in ``include/xleaf.h``
> +
> +
> +xmgmt.ko
> +
> +
> +The xmgmt driver is a PCIe device driver driving MPF found on Xilinx's Alveo
> +PCIE device. It consists of one *root* driver, one or more *group* drivers
> +and one or more *xleaf* drivers. The root and MPF specific xleaf drivers are
> +in xmgmt.ko. The group driver and other xleaf drivers are in xrt-lib.ko.
I am not sure if *.ko is correct, these will also be intree.
> +
> +The instantiation of specific group driver or xleaf driver is completely data
of a specific
> +driven based on meta data (mostly in device tree format) found through VSEC
mostly ? what is the deviation from device tree ?
> +capability and inside firmware files, such as platform xsabin or user xclbin 
> file.
> +The root driver manages life cycle of multiple group drivers, which, in turn,
the life cycle
> +manages multiple xleaf drivers. This allows a single set of driver code to 
> support

set of drivers

drop 'code'

> +all kinds of subsystems exposed by different shells. The difference among all
> 

[PATCH V3 XRT Alveo 01/18] Documentation: fpga: Add a document describing XRT Alveo drivers

[Lizhi Hou]

Describe XRT driver architecture and provide basic overview of
Xilinx Alveo platform.

Signed-off-by: Sonal Santan 
Signed-off-by: Max Zhen 
Signed-off-by: Lizhi Hou 
---
 Documentation/fpga/index.rst |   1 +
 Documentation/fpga/xrt.rst   | 842 +++
 2 files changed, 843 insertions(+)
 create mode 100644 Documentation/fpga/xrt.rst

diff --git a/Documentation/fpga/index.rst b/Documentation/fpga/index.rst
index f80f95667ca2..30134357b70d 100644
--- a/Documentation/fpga/index.rst
+++ b/Documentation/fpga/index.rst
@@ -8,6 +8,7 @@ fpga
 :maxdepth: 1
 
 dfl
+xrt
 
 .. only::  subproject and html
 
diff --git a/Documentation/fpga/xrt.rst b/Documentation/fpga/xrt.rst
new file mode 100644
index ..9bc2d2785cb9
--- /dev/null
+++ b/Documentation/fpga/xrt.rst
@@ -0,0 +1,842 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==
+XRTV2 Linux Kernel Driver Overview
+==
+
+Authors:
+
+* Sonal Santan 
+* Max Zhen 
+* Lizhi Hou 
+
+XRTV2 drivers are second generation `XRT `_
+drivers which support `Alveo 
`_
+PCIe platforms from Xilinx.
+
+XRTV2 drivers support *subsystem* style data driven platforms where driver's
+configuration and behavior is determined by meta data provided by the platform
+(in *device tree* format). Primary management physical function (MPF) driver
+is called **xmgmt**. Primary user physical function (UPF) driver is called
+**xuser** and is under development. xrt driver framework and HW subsystem
+drivers are packaged into a library module called **xrt-lib**, which is
+shared by **xmgmt** and **xuser** (under development). The xrt driver framework
+implements a pseudo-bus which is used to discover HW subsystems and facilitate
+inter HW subsystem interaction.
+
+Driver Modules
+==
+
+xrt-lib.ko
+--
+
+Repository of all subsystem drivers and pure software modules that can 
potentially
+be shared between xmgmt and xuser. All these drivers are structured as Linux
+*platform driver* and are instantiated by xmgmt (or xuser under development) 
based
+on meta data associated with hardware. The metadata is in the form of device 
tree
+as mentioned before. Each platform driver statically defines a subsystem node
+array by using node name or a string in its ``compatible`` property. And this
+array is eventually translated to IOMEM resources of the platform device.
+
+The xrt-lib core infrastructure provides hooks to platform drivers for device 
node
+management, user file operations and ioctl callbacks. The core also provides 
pseudo-bus
+functionality for platform driver registration, discovery and inter platform 
driver
+ioctl calls.
+
+.. note::
+   See code in ``include/xleaf.h``
+
+
+xmgmt.ko
+
+
+The xmgmt driver is a PCIe device driver driving MPF found on Xilinx's Alveo
+PCIE device. It consists of one *root* driver, one or more *group* drivers
+and one or more *xleaf* drivers. The root and MPF specific xleaf drivers are
+in xmgmt.ko. The group driver and other xleaf drivers are in xrt-lib.ko.
+
+The instantiation of specific group driver or xleaf driver is completely data
+driven based on meta data (mostly in device tree format) found through VSEC
+capability and inside firmware files, such as platform xsabin or user xclbin 
file.
+The root driver manages life cycle of multiple group drivers, which, in turn,
+manages multiple xleaf drivers. This allows a single set of driver code to 
support
+all kinds of subsystems exposed by different shells. The difference among all
+these subsystems will be handled in xleaf drivers with root and group drivers
+being part of the infrastructure and provide common services for all leaves
+found on all platforms.
+
+The driver object model looks like the following::
+
++---+
+|   xroot   |
++-+-+
+  |
+  +---+---+
+  |   |
+  v   v
++---+  +---+
+|   group   |...   |   group   |
++-+-+  +--++
+  |   |
+  |   |
++-+++-++
+|  ||  |
+v  vv  v
++---+  +---++---+  +---+
+| xleaf |..| xleaf || xleaf |..| xleaf |
++---+  +---++---+  +---+
+
+As an example for Xilinx Alveo U50 before user xclbin download, the tree
+looks like the following::
+
++---+
+|   xmgmt   |
++-+-+
+  |
++-+-