Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates
 149.199.62.198 as permitted sender) receiver=protection.outlook.com;
 client-ip=149.199.62.198; helo=xsj-pvapexch02.xlnx.xilinx.com;
Envelope-to: michal.simek@xilinx.com,
 sonal.santan@xilinx.com,
 lizhi.hou@xilinx.com,
 stefanos@xilinx.com,
 maxz@xilinx.com,
 robh@kernel.org,
 trix@redhat.com,
 devicetree@vger.kernel.org,
 linux-fpga@vger.kernel.org,
 linux-kernel@vger.kernel.org,
 mdf@kernel.org
Subject: Re: [PATCH V3 XRT Alveo 00/18] XRT Alveo driver overview
To:     Moritz Fischer <mdf@kernel.org>, Lizhi Hou <lizhi.hou@xilinx.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-fpga@vger.kernel.org>,
        <maxz@xilinx.com>, <sonal.santan@xilinx.com>,
        <michal.simek@xilinx.com>, <stefanos@xilinx.com>,
        <devicetree@vger.kernel.org>, <trix@redhat.com>, <robh@kernel.org>
References: <20210218064019.29189-1-lizhih@xilinx.com>
 <YDLF2Bi3oEhP6A7Q@archbook>
From:   Lizhi Hou <lizhi.hou@xilinx.com>
Message-ID: <f85b48e2-155a-c42c-9b47-5098421c554f@xilinx.com>
Date:   Mon, 1 Mar 2021 10:29:11 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <YDLF2Bi3oEhP6A7Q@archbook>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Mar 2021 18:29:18.1872
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 26cb8096-2eb1-4951-80b3-08d8dcdfecdd
X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c;Ip=[149.199.62.198];Helo=[xsj-pvapexch02.xlnx.xilinx.com]
X-MS-Exchange-CrossTenant-AuthSource: SN1NAM02FT055.eop-nam02.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR02MB6679
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Moritz,


On 02/21/2021 12:43 PM, Moritz Fischer wrote:
> Lizhi,
>
> On Wed, Feb 17, 2021 at 10:40:01PM -0800, Lizhi Hou wrote:
>> Hello,
>>
>> This is V3 of patch series which adds management physical function driver for Xilinx
>> Alveo PCIe accelerator cards, https://www.xilinx.com/products/boards-and-kits/alveo.html
>> This driver is part of Xilinx Runtime (XRT) open source stack.
>>
>> XILINX ALVEO PLATFORM ARCHITECTURE
>>
>> Alveo PCIe FPGA based platforms have a static *shell* partition and a partial
>> re-configurable *user* partition. The shell partition is automatically loaded from
>> flash when host is booted and PCIe is enumerated by BIOS. Shell cannot be changed
>> till the next cold reboot. The shell exposes two PCIe physical functions:
>>
>> 1. management physical function
>> 2. user physical function
>>
>> The patch series includes Documentation/xrt.rst which describes Alveo platform,
>> XRT driver architecture and deployment model in more detail.
>>
>> Users compile their high level design in C/C++/OpenCL or RTL into FPGA image using
>> Vitis https://www.xilinx.com/products/design-tools/vitis/vitis-platform.html
>> tools. The compiled image is packaged as xclbin which contains partial bitstream
>> for the user partition and necessary metadata. Users can dynamically swap the image
>> running on the user partition in order to switch between different workloads by
>> loading different xclbins.
>>
>> XRT DRIVERS FOR XILINX ALVEO
>>
>> XRT Linux kernel driver *xmgmt* binds to management physical function of Alveo
>> platform. The modular driver framework is organized into several platform drivers
>> which primarily handle the following functionality:
>>
>> 1.  Loading firmware container also called xsabin at driver attach time
>> 2.  Loading of user compiled xclbin with FPGA Manager integration
>> 3.  Clock scaling of image running on user partition
>> 4.  In-band sensors: temp, voltage, power, etc.
>> 5.  Device reset and rescan
>>
>> The platform drivers are packaged into *xrt-lib* helper module with well
>> defined interfaces. The module provides a pseudo-bus implementation for the
>> platform drivers. More details on the driver model can be found in
>> Documentation/xrt.rst.
>>
>> User physical function driver is not included in this patch series.
>>
>> LIBFDT REQUIREMENT
>>
>> XRT driver infrastructure uses Device Tree as a metadata format to discover
>> HW subsystems in the Alveo PCIe device. The Device Tree schema used by XRT
>> is documented in Documentation/xrt.rst. Unlike previous V1 and V2 version
>> of patch series, V3 version does not require export of libfdt symbols.
>>
>> TESTING AND VALIDATION
>>
>> xmgmt driver can be tested with full XRT open source stack which includes user
>> space libraries, board utilities and (out of tree) first generation user physical
>> function driver xocl. XRT open source runtime stack is available at
>> https://github.com/Xilinx/XRT
>>
>> Complete documentation for XRT open source stack including sections on Alveo/XRT
>> security and platform architecture can be found here:
>>
>> https://xilinx.github.io/XRT/master/html/index.html
>> https://xilinx.github.io/XRT/master/html/security.html
>> https://xilinx.github.io/XRT/master/html/platforms_partitions.html
>>
>> Changes since v2:
>> - Streamlined the driver framework into *xleaf*, *group* and *xroot*
>> - Updated documentation to show the driver model with examples
>> - Addressed kernel test robot errors
>> - Added a selftest for basic driver framework
>> - Documented device tree schema
>> - Removed need to export libfdt symbols
>>
>> Changes since v1:
>> - Updated the driver to use fpga_region and fpga_bridge for FPGA
>>    programming
>> - Dropped platform drivers not related to PR programming to focus on XRT
>>    core framework
>> - Updated Documentation/fpga/xrt.rst with information on XRT core framework
>> - Addressed checkpatch issues
>> - Dropped xrt- prefix from some header files
>>
>> For reference V1 version of patch series can be found here:
>>
>> https://lore.kernel.org/lkml/20201217075046.28553-1-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-2-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-3-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-4-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-5-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-6-sonals@xilinx.com/
>> https://lore.kernel.org/lkml/20201217075046.28553-7-sonals@xilinx.com/
>>
>> Lizhi Hou (18):
>>    Documentation: fpga: Add a document describing XRT Alveo drivers
>>    fpga: xrt: driver metadata helper functions
>>    fpga: xrt: xclbin file helper functions
>>    fpga: xrt: xrt-lib platform driver manager
>>    fpga: xrt: group platform driver
>>    fpga: xrt: platform driver infrastructure
>>    fpga: xrt: management physical function driver (root)
>>    fpga: xrt: main platform driver for management function device
>>    fpga: xrt: fpga-mgr and region implementation for xclbin download
>>    fpga: xrt: VSEC platform driver
>>    fpga: xrt: UCS platform driver
>>    fpga: xrt: ICAP platform driver
>>    fpga: xrt: devctl platform driver
>>    fpga: xrt: clock platform driver
>>    fpga: xrt: clock frequence counter platform driver
>>    fpga: xrt: DDR calibration platform driver
>>    fpga: xrt: partition isolation platform driver
>>    fpga: xrt: Kconfig and Makefile updates for XRT drivers
>>
>>   Documentation/fpga/index.rst             |   1 +
>>   Documentation/fpga/xrt.rst               | 842 ++++++++++++++++++++++
>>   MAINTAINERS                              |  11 +
>>   drivers/Makefile                         |   1 +
>>   drivers/fpga/Kconfig                     |   2 +
>>   drivers/fpga/Makefile                    |   4 +
>>   drivers/fpga/xrt/Kconfig                 |   8 +
>>   drivers/fpga/xrt/include/events.h        |  48 ++
>>   drivers/fpga/xrt/include/group.h         |  27 +
>>   drivers/fpga/xrt/include/metadata.h      | 229 ++++++
>>   drivers/fpga/xrt/include/subdev_id.h     |  43 ++
>>   drivers/fpga/xrt/include/xclbin-helper.h |  52 ++
>>   drivers/fpga/xrt/include/xleaf.h         | 276 +++++++
>>   drivers/fpga/xrt/include/xleaf/axigate.h |  25 +
>>   drivers/fpga/xrt/include/xleaf/calib.h   |  30 +
>>   drivers/fpga/xrt/include/xleaf/clkfreq.h |  23 +
>>   drivers/fpga/xrt/include/xleaf/clock.h   |  31 +
>>   drivers/fpga/xrt/include/xleaf/devctl.h  |  43 ++
>>   drivers/fpga/xrt/include/xleaf/icap.h    |  29 +
>>   drivers/fpga/xrt/include/xleaf/ucs.h     |  24 +
>>   drivers/fpga/xrt/include/xmgmt-main.h    |  37 +
>>   drivers/fpga/xrt/include/xroot.h         | 114 +++
>>   drivers/fpga/xrt/lib/Kconfig             |  16 +
>>   drivers/fpga/xrt/lib/Makefile            |  30 +
>>   drivers/fpga/xrt/lib/cdev.c              | 231 ++++++
>>   drivers/fpga/xrt/lib/group.c             | 265 +++++++
>>   drivers/fpga/xrt/lib/main.c              | 274 +++++++
>>   drivers/fpga/xrt/lib/main.h              |  17 +
>>   drivers/fpga/xrt/lib/subdev.c            | 871 +++++++++++++++++++++++
>>   drivers/fpga/xrt/lib/subdev_pool.h       |  53 ++
>>   drivers/fpga/xrt/lib/xclbin.c            | 394 ++++++++++
>>   drivers/fpga/xrt/lib/xleaf/axigate.c     | 298 ++++++++
>>   drivers/fpga/xrt/lib/xleaf/calib.c       | 226 ++++++
>>   drivers/fpga/xrt/lib/xleaf/clkfreq.c     | 221 ++++++
>>   drivers/fpga/xrt/lib/xleaf/clock.c       | 648 +++++++++++++++++
>>   drivers/fpga/xrt/lib/xleaf/devctl.c      | 206 ++++++
>>   drivers/fpga/xrt/lib/xleaf/icap.c        | 317 +++++++++
>>   drivers/fpga/xrt/lib/xleaf/ucs.c         | 235 ++++++
>>   drivers/fpga/xrt/lib/xleaf/vsec.c        | 359 ++++++++++
>>   drivers/fpga/xrt/lib/xroot.c             | 598 ++++++++++++++++
>>   drivers/fpga/xrt/metadata/Kconfig        |  12 +
>>   drivers/fpga/xrt/metadata/Makefile       |  16 +
>>   drivers/fpga/xrt/metadata/metadata.c     | 524 ++++++++++++++
>>   drivers/fpga/xrt/mgmt/Kconfig            |  15 +
>>   drivers/fpga/xrt/mgmt/Makefile           |  19 +
>>   drivers/fpga/xrt/mgmt/fmgr-drv.c         | 187 +++++
>>   drivers/fpga/xrt/mgmt/fmgr.h             |  28 +
>>   drivers/fpga/xrt/mgmt/main-impl.h        |  37 +
>>   drivers/fpga/xrt/mgmt/main-region.c      | 471 ++++++++++++
>>   drivers/fpga/xrt/mgmt/main.c             | 693 ++++++++++++++++++
>>   drivers/fpga/xrt/mgmt/root.c             | 342 +++++++++
>>   include/uapi/linux/xrt/xclbin.h          | 408 +++++++++++
>>   include/uapi/linux/xrt/xmgmt-ioctl.h     |  46 ++
>>   53 files changed, 9957 insertions(+)
>>   create mode 100644 Documentation/fpga/xrt.rst
>>   create mode 100644 drivers/fpga/xrt/Kconfig
>>   create mode 100644 drivers/fpga/xrt/include/events.h
>>   create mode 100644 drivers/fpga/xrt/include/group.h
>>   create mode 100644 drivers/fpga/xrt/include/metadata.h
>>   create mode 100644 drivers/fpga/xrt/include/subdev_id.h
>>   create mode 100644 drivers/fpga/xrt/include/xclbin-helper.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/axigate.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/calib.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/clkfreq.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/clock.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/devctl.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/icap.h
>>   create mode 100644 drivers/fpga/xrt/include/xleaf/ucs.h
>>   create mode 100644 drivers/fpga/xrt/include/xmgmt-main.h
>>   create mode 100644 drivers/fpga/xrt/include/xroot.h
>>   create mode 100644 drivers/fpga/xrt/lib/Kconfig
>>   create mode 100644 drivers/fpga/xrt/lib/Makefile
>>   create mode 100644 drivers/fpga/xrt/lib/cdev.c
>>   create mode 100644 drivers/fpga/xrt/lib/group.c
>>   create mode 100644 drivers/fpga/xrt/lib/main.c
>>   create mode 100644 drivers/fpga/xrt/lib/main.h
>>   create mode 100644 drivers/fpga/xrt/lib/subdev.c
>>   create mode 100644 drivers/fpga/xrt/lib/subdev_pool.h
>>   create mode 100644 drivers/fpga/xrt/lib/xclbin.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/axigate.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/calib.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/clkfreq.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/clock.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/devctl.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/icap.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/ucs.c
>>   create mode 100644 drivers/fpga/xrt/lib/xleaf/vsec.c
>>   create mode 100644 drivers/fpga/xrt/lib/xroot.c
>>   create mode 100644 drivers/fpga/xrt/metadata/Kconfig
>>   create mode 100644 drivers/fpga/xrt/metadata/Makefile
>>   create mode 100644 drivers/fpga/xrt/metadata/metadata.c
>>   create mode 100644 drivers/fpga/xrt/mgmt/Kconfig
>>   create mode 100644 drivers/fpga/xrt/mgmt/Makefile
>>   create mode 100644 drivers/fpga/xrt/mgmt/fmgr-drv.c
>>   create mode 100644 drivers/fpga/xrt/mgmt/fmgr.h
>>   create mode 100644 drivers/fpga/xrt/mgmt/main-impl.h
>>   create mode 100644 drivers/fpga/xrt/mgmt/main-region.c
>>   create mode 100644 drivers/fpga/xrt/mgmt/main.c
>>   create mode 100644 drivers/fpga/xrt/mgmt/root.c
>>   create mode 100644 include/uapi/linux/xrt/xclbin.h
>>   create mode 100644 include/uapi/linux/xrt/xmgmt-ioctl.h
>>
>> --
>> 2.18.4
>>
> Please fix the indents all across this patchset. Doesn't checkpatch with
> --strict complain about this?
checkpatch --strict did not complain. And we will fix this.
>
> Also more generally this looks like it should be a bus. Look at DFL for
> reference.
Tom asked the same question in 01/18 and Sonal replied. We may discuss 
with that thread.

Thanks,
Lizhi
>
> - Moritz