| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0677c3b8-87a8-7092-0c63-36de55a4de4d@redhat.com>
Date: Thu, 1 Jul 2021 13:32:29 -0700
From: Tom Rix <trix@...hat.com>
To: Lizhi Hou <lizhi.hou@...inx.com>, linux-kernel@...r.kernel.org,
"mdf@...nel.org" <mdf@...nel.org>
Cc: linux-fpga@...r.kernel.org, maxz@...inx.com,
sonal.santan@...inx.com, yliu@...inx.com, michal.simek@...inx.com,
stefanos@...inx.com, devicetree@...r.kernel.org, robh@...nel.org
Subject: Re: [PATCH V7 XRT Alveo 00/20] XRT Alveo driver overview
Lizhi,
Sorry for the delay in reviewing v7.
Is it too early to blame it on the July 4 holiday here ?!? :)
All the small stuff looks fine to me. In this pass I looked at issues
that would need a refactoring. Since it would be a lot of work and I am
not the final word on this, it would be good if some others to chime in.
Also a couple of new spelling fixes at the end.
Tom
Having xrt/ dir
ok with it or it will follow the subdir reorg of fpga/, afaik not a blocker
Location of xrt_bus_type
ok, similar to dfl_bus_type
Non fpga subdevices should go to other subsystems.
looking in drivers/fpga/xrt/lib/xleaf
clock clkfrq ucs these are clocks
should move to drivers/clk/xilinx/
axigate, for fpga partitioning
ok to stay
ddr_calibaration, a memory status checker
should move drivers/memory dfl-emif is similar
devctl, a general purpose misc driver
should move to drivers/mfd
icap, for fpga bitstream writing
ok to stay
vsec, misc small drivers discovered via pci config vsec
should move to drivers/mfd
For include/uapi/linux
collapse include/uapi/linux/xrt/*.h into include/uapi/linux/fpga-xrt.h
There are only 2 files, one really small. fpga-xrt.h follows fpga-dfl.h
The comments are pretty messy, user should be able to scan them.
Try cleaning them up.
Spelling mistakes
diff --git a/Documentation/fpga/xrt.rst b/Documentation/fpga/xrt.rst
index 5a5b4d5a3bc6..84eb41be9ac1 100644
--- a/Documentation/fpga/xrt.rst
+++ b/Documentation/fpga/xrt.rst
@@ -275,7 +275,7 @@ fpga_bridge and fpga_region for the next region in
the chain.
fpga_bridge
-----------
-Like the fpga_region, a fpga_bridge is created by walking the device tree
+Like the fpga_region, an fpga_bridge is created by walking the device tree
of the parent group. The bridge is used for isolation between a parent and
its child.
@@ -416,7 +416,7 @@ xclbin is compiled by end user using
`Vitis
<https://www.xilinx.com/products/design-tools/vitis/vitis-platform.html>`_
tool set from Xilinx. The xclbin contains sections describing user
compiled
acceleration engines/kernels, memory subsystems, clocking information
etc. It also
-contains a FPGA bitstream for the user partition, UUIDs, platform name,
etc.
+contains an FPGA bitstream for the user partition, UUIDs, platform
name, etc.
.. _xsabin_xclbin_container_format:
diff --git a/drivers/fpga/xrt/include/metadata.h
b/drivers/fpga/xrt/include/metadata.h
index c4df88262f8a..f48d6d42f5ef 100644
--- a/drivers/fpga/xrt/include/metadata.h
+++ b/drivers/fpga/xrt/include/metadata.h
@@ -194,7 +194,7 @@ int xrt_md_get_interface_uuids(struct device *dev,
const char *blob,
/*
* The firmware provides a 128 bit hash string as a unique id to the
* partition/interface.
- * Existing hw does not yet use the cononical form, so it is necessary to
+ * Existing hw does not yet use the canonical form, so it is necessary to
* use a translation function.
*/
static inline void xrt_md_trans_uuid2str(const uuid_t *uuid, char
*uuidstr)
diff --git a/drivers/fpga/xrt/lib/xroot.c b/drivers/fpga/xrt/lib/xroot.c
index 7b3e540dd6c0..f324a25e1d4d 100644
--- a/drivers/fpga/xrt/lib/xroot.c
+++ b/drivers/fpga/xrt/lib/xroot.c
@@ -427,7 +427,7 @@ static void xroot_bringup_group_work(struct
work_struct *work)
r = xleaf_call(xdev, XRT_GROUP_INIT_CHILDREN, NULL);
xroot_put_group(xr, xdev);
if (r == -EEXIST)
- continue; /* Already brough up, nothing to do. */
+ continue; /* Already brought up, nothing to do. */
if (r)
atomic_inc(&xr->groups.bringup_failed_cnt);
diff --git a/drivers/fpga/xrt/mgmt/xmgmt-main.c
b/drivers/fpga/xrt/mgmt/xmgmt-main.c
index 820c888e7918..9077254e0f8a 100644
--- a/drivers/fpga/xrt/mgmt/xmgmt-main.c
+++ b/drivers/fpga/xrt/mgmt/xmgmt-main.c
@@ -142,7 +142,7 @@ static ssize_t VBNV_show(struct device *dev, struct
device_attribute *da, char *
}
static DEVICE_ATTR_RO(VBNV);
-/* logic uuid is the uuid uniquely identfy the partition */
+/* logic uuid is the uuid uniquely identify the partition */
static ssize_t logic_uuids_show(struct device *dev, struct
device_attribute *da, char *buf)
{
struct xrt_device *xdev = to_xrt_dev(dev);
diff --git a/drivers/fpga/xrt/mgmt/xrt-mgr.c
b/drivers/fpga/xrt/mgmt/xrt-mgr.c
index 41263a033d9d..ab253b516e8d 100644
--- a/drivers/fpga/xrt/mgmt/xrt-mgr.c
+++ b/drivers/fpga/xrt/mgmt/xrt-mgr.c
@@ -115,7 +115,7 @@ static int xmgmt_pr_write_init(struct fpga_manager *mgr,
}
/*
- * The implementation requries full xclbin image before we can start
+ * The implementation requires full xclbin image before we can start
* programming the hardware via ICAP subsystem. The full image is required
* for checking the validity of xclbin and walking the sections to
* discover the bitstream.
On 5/27/21 5:49 PM, Lizhi Hou wrote:
> Hello,
>
> This is V7 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 tools.
> https://www.xilinx.com/products/design-tools/vitis/vitis-platform.html
>
> 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 *xrt-mgmt* 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.
>
> TESTING AND VALIDATION
>
> xrt-mgmt 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 v6:
> - Resolved grammatical errors and cleaned up taxonomy in xrt.rst
> documentation.
> - Fixed clang warnings.
> - Updated code base to include v6 code review comments.
>
> Changes since v5:
> - Revert all changes 'mgnt/MGNT' back to 'mgmt/MGMT'
> - Updated code base to include v5 code review comments.
> xrt.rst: address grammar and taxonomy
> subdev_id.h: defines XRT_SUBDEV_INVALID = 0
> xclbin.c: change shift operation to be_to_cpu
> - Resolved kernel test robot errors.
>
> Changes since v4:
> - Added xrt_bus_type and xrt_device. All sub devices were changed from
> platform_bus_type/platform_device to xrt_bus_type/xrt_device.
> - Renamed xrt-mgmt driver to xrt-mgnt driver.
> - Replaced 'MGMT' with 'MGNT' and 'mgmt' with 'mgnt' in code and file names
> - Moved pci function calls from infrastructure to xrt-mgnt driver.
> - Renamed files: mgmt/main.c -> mgnt/xmgnt-main.c
> mgmt/main-region.c -> mgnt/xmgnt-main-region.c
> include/xmgmt-main.h -> include/xmgnt-main.h
> mgmt/fmgr-drv.c -> mgnt/xrt-mgr.c
> mgmt/fmgr.h -> mgnt/xrt-mgr.h
> - Updated code base to include v4 code review comments.
>
> Changes since v3:
> - Leaf drivers use regmap-mmio to access hardware registers.
> - Renamed driver module: xmgmt.ko -> xrt-mgmt.ko
> - Renamed files: calib.[c|h] -> ddr_calibration.[c|h],
> lib/main.[c|h] -> lib/lib-drv.[c|h],
> mgmt/main-impl.h - > mgmt/xmgnt.h
> - Updated code base to include v3 code review comments.
>
> 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 V6 version of patch series can be found here:
>
> https://lore.kernel.org/lkml/20210512015339.5649-1-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-2-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-3-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-4-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-5-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-6-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-7-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-8-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-9-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-10-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-11-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-12-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-13-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-14-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-15-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-16-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-17-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-18-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-19-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-20-lizhi.hou@xilinx.com/
> https://lore.kernel.org/lkml/20210512015339.5649-21-lizhi.hou@xilinx.com/
>
> Lizhi Hou (20):
> 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 driver manager
> fpga: xrt: group driver
> fpga: xrt: char dev node helper functions
> fpga: xrt: root driver infrastructure
> fpga: xrt: driver infrastructure
> fpga: xrt: management physical function driver (root)
> fpga: xrt: main driver for management function device
> fpga: xrt: fpga-mgr and region implementation for xclbin download
> fpga: xrt: VSEC driver
> fpga: xrt: User Clock Subsystem driver
> fpga: xrt: ICAP driver
> fpga: xrt: devctl xrt driver
> fpga: xrt: clock driver
> fpga: xrt: clock frequency counter driver
> fpga: xrt: DDR calibration driver
> fpga: xrt: partition isolation driver
> fpga: xrt: Kconfig and Makefile updates for XRT drivers
>
> Documentation/fpga/index.rst | 1 +
> Documentation/fpga/xrt.rst | 870 ++++++++++++++++++
> MAINTAINERS | 11 +
> drivers/Makefile | 1 +
> drivers/fpga/Kconfig | 2 +
> drivers/fpga/Makefile | 5 +
> drivers/fpga/xrt/Kconfig | 8 +
> drivers/fpga/xrt/include/events.h | 45 +
> drivers/fpga/xrt/include/group.h | 25 +
> drivers/fpga/xrt/include/metadata.h | 236 +++++
> drivers/fpga/xrt/include/subdev_id.h | 39 +
> drivers/fpga/xrt/include/xclbin-helper.h | 48 +
> drivers/fpga/xrt/include/xdevice.h | 131 +++
> drivers/fpga/xrt/include/xleaf.h | 205 +++++
> drivers/fpga/xrt/include/xleaf/axigate.h | 23 +
> drivers/fpga/xrt/include/xleaf/clkfreq.h | 21 +
> drivers/fpga/xrt/include/xleaf/clock.h | 29 +
> .../fpga/xrt/include/xleaf/ddr_calibration.h | 28 +
> drivers/fpga/xrt/include/xleaf/devctl.h | 40 +
> drivers/fpga/xrt/include/xleaf/icap.h | 27 +
> drivers/fpga/xrt/include/xmgmt-main.h | 34 +
> drivers/fpga/xrt/include/xroot.h | 117 +++
> drivers/fpga/xrt/lib/Kconfig | 17 +
> drivers/fpga/xrt/lib/Makefile | 30 +
> drivers/fpga/xrt/lib/cdev.c | 209 +++++
> drivers/fpga/xrt/lib/group.c | 278 ++++++
> drivers/fpga/xrt/lib/lib-drv.c | 328 +++++++
> drivers/fpga/xrt/lib/lib-drv.h | 21 +
> drivers/fpga/xrt/lib/subdev.c | 859 +++++++++++++++++
> drivers/fpga/xrt/lib/subdev_pool.h | 53 ++
> drivers/fpga/xrt/lib/xclbin.c | 381 ++++++++
> drivers/fpga/xrt/lib/xleaf/axigate.c | 325 +++++++
> drivers/fpga/xrt/lib/xleaf/clkfreq.c | 223 +++++
> drivers/fpga/xrt/lib/xleaf/clock.c | 652 +++++++++++++
> drivers/fpga/xrt/lib/xleaf/ddr_calibration.c | 210 +++++
> drivers/fpga/xrt/lib/xleaf/devctl.c | 169 ++++
> drivers/fpga/xrt/lib/xleaf/icap.c | 328 +++++++
> drivers/fpga/xrt/lib/xleaf/ucs.c | 152 +++
> drivers/fpga/xrt/lib/xleaf/vsec.c | 372 ++++++++
> drivers/fpga/xrt/lib/xroot.c | 536 +++++++++++
> drivers/fpga/xrt/metadata/Kconfig | 12 +
> drivers/fpga/xrt/metadata/Makefile | 16 +
> drivers/fpga/xrt/metadata/metadata.c | 578 ++++++++++++
> drivers/fpga/xrt/mgmt/Kconfig | 15 +
> drivers/fpga/xrt/mgmt/Makefile | 19 +
> drivers/fpga/xrt/mgmt/root.c | 420 +++++++++
> drivers/fpga/xrt/mgmt/xmgmt-main-region.c | 483 ++++++++++
> drivers/fpga/xrt/mgmt/xmgmt-main.c | 662 +++++++++++++
> drivers/fpga/xrt/mgmt/xmgmt.h | 33 +
> drivers/fpga/xrt/mgmt/xrt-mgr.c | 190 ++++
> drivers/fpga/xrt/mgmt/xrt-mgr.h | 16 +
> include/uapi/linux/xrt/xclbin.h | 409 ++++++++
> include/uapi/linux/xrt/xmgmt-ioctl.h | 46 +
> 53 files changed, 9988 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/xdevice.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/clkfreq.h
> create mode 100644 drivers/fpga/xrt/include/xleaf/clock.h
> create mode 100644 drivers/fpga/xrt/include/xleaf/ddr_calibration.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/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/lib-drv.c
> create mode 100644 drivers/fpga/xrt/lib/lib-drv.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/clkfreq.c
> create mode 100644 drivers/fpga/xrt/lib/xleaf/clock.c
> create mode 100644 drivers/fpga/xrt/lib/xleaf/ddr_calibration.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/root.c
> create mode 100644 drivers/fpga/xrt/mgmt/xmgmt-main-region.c
> create mode 100644 drivers/fpga/xrt/mgmt/xmgmt-main.c
> create mode 100644 drivers/fpga/xrt/mgmt/xmgmt.h
> create mode 100644 drivers/fpga/xrt/mgmt/xrt-mgr.c
> create mode 100644 drivers/fpga/xrt/mgmt/xrt-mgr.h
> create mode 100644 include/uapi/linux/xrt/xclbin.h
> create mode 100644 include/uapi/linux/xrt/xmgmt-ioctl.h
>
Powered by blists - more mailing lists