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

From: Sonal Santan
Date: Tue Oct 19 2021 - 00:32:41 EST

Next message: NeilBrown: "Re: [PATCH 2/6] MM: improve documentation for __GFP_NOFAIL"
Previous message: Chengming Zhou: "Re: [External] Re: [PATCH] bpf: use count for prealloc hashtab too"
Next in thread: Rob Herring: "Re: [PATCH V9 XRT Alveo 01/14] Documentation: fpga: Add a document describing XRT Alveo drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hello Rob,

> -----Original Message-----
> From: Lizhi Hou <lizhi.hou@xxxxxxxxxx>
> Sent: Thursday, October 14, 2021 9:13 AM
> To: robh@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-fpga@xxxxxxxxxxxxxxx; Max Zhen
> <maxz@xxxxxxxxxx>; Sonal Santan <sonals@xxxxxxxxxx>; Yu Liu
> <yliu@xxxxxxxxxx>; Michal Simek <michals@xxxxxxxxxx>; Stefano Stabellini
> <stefanos@xxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; trix@xxxxxxxxxx;
> mdf@xxxxxxxxxx; Max Zhen <maxz@xxxxxxxxxx>; Lizhi Hou <lizhih@xxxxxxxxxx>;
> Xu Yilun <yilun.xu@xxxxxxxxx>
> Subject: Re: [PATCH V9 XRT Alveo 01/14] Documentation: fpga: Add a
> document describing XRT Alveo drivers
>
> Hello Rob,
>
> Please help with the review of the proposed FDT usage by Alveo/XRT drivers.
>
> Thanks,
> Lizhi
>
> On 10/13/21 7:21 PM, Xu Yilun wrote:
> > CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
> >
> >
> >>>> +.. _device_tree_usage:
> >>>> +
> >>>> +Device Tree Usage
> >>>> +-----------------
> >>>> +
> >>>> +The xsabin file stores metadata which advertise HW subsystems
> >>>> +present in a partition. The metadata is stored in device tree
> >>>> +format with a well defined schema. XRT management driver uses this
> >>>> +information to bind *xrt_drivers* to the subsystem instantiations.
> >>>> +The xrt_drivers are found in **xrt-lib.ko** kernel module.
> >>> I'm still catching up the patchset from the very beginning, and just
> >>> finished the Documentation part. So far, I see the DT usage concern
> >>> which may impact the architecure a lot, so I should raise it ASAP.
> >>>
> >>> The concern raised by the DT maintainer:
> >>> https://lore.kernel.org/linux-fpga/CAL_JsqLod6FBGFhu7WXtMrB_z7wj8-up
> >>> 0EetM1QS9M3gjm8d7Q@xxxxxxxxxxxxxx/
> >>>
> >>> First of all, directly parsing FDT in device drivers is not a normal
> >>> usage of DT in linux. It is out of the current DT usage model. So it
> >>> should be agreed by DT maintainers.

Friendly reminder that we are waiting for your feedback on the proposed FDT use model by the Alveo/XRT drivers. Please advise on how to proceed forward.

Thanks,
-Sonal
> >> Thanks for reviewing XRT document and providing feedback.
> >> Here is the reply from Sonal for Rob’s question:
> >> https://lore.kernel.org/linux-
> fpga/BY5PR02MB62604B87C66A1AD139A6F153B
> >> BF40@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> >> Overall, libfdt is used by XRT driver to parse the metadata which
> >> comes with an Alveo board.
> >> When XRT driver discovers an Alveo board, it will read a fdt blob
> >> from board firmware file resident on the host.
> >> By parsing the fdt blob, XRT driver gets information about this Alveo
> >> board, such as version, uuid, IPs exposed to PCI BAR, interrupt binding etc.
> >> So libfdt is used simply as Alveo metadata parser here. XRT drivers
> >> do not interact with system wide DT or present the Alveo device tree
> >> to host. For many systems like x86_64, system wide DT is not present
> >> but libfdt parsing services will still be needed.
> > Yes, I understand the use case.
> >
> > My concern is, directly parsing an isolated FDT in device driver and
> > populate sub devices, skipping the unflattening, this is a new working
> > model of device tree usage, but for the same purpose as the existing
> > one.
> >
> > So I really need the confirmation of DT maintainers.
> >
> >>> Current FPGA framework modifies kernel's live tree by DT overlay,
> >>> when FPGA is dynamically reprogrammed and new HW devices appear. See
> >>> Documentation/devicetree/bindings/fpga/fpga-region.txt.
> >>>
> >>> Then something less important:
> >>>
> >>> 1. The bindings should be documented in
> Documentation/devicetree/bindings/.
> >>> 2. Are all the example DT usage conform to the exsiting bindings? I
> >>> didn't go through all device classes, but remember like the
> >>> interrupt-controller should have a "interrupt-controller" property, and
> >>> the PCI properties are also different from PCI bindings.
> >> The fdt properties are defined for Alveo firmware files. XRT driver
> >> is the only consumer of this data. I am wondering if
> > Personally I don't like the idea of a different binding definition
> > set, even if the new device tree usage is accepted. We all use device
> > tree for device enumeration, so why the different definitions.
> >
> > Thanks,
> > Yilun
> >
> >> Documentation/devicetree/bindings is the right place for Alveo/XRT
> >> private format or should it be documented as part of XRT driver
> documentation?
> >> Looking for guidance here.
> >>
> >>
> >> Thanks,
> >>
> >> Lizhi
> >>
> >>> Thanks,
> >>> Yilun
> >>>
> >>>> +
> >>>> +Logic UUID
> >>>> +^^^^^^^^^^
> >>>> +A partition is identified uniquely through ``logic_uuid`` property::
> >>>> +
> >>>> + /dts-v1/;
> >>>> + / {
> >>>> + logic_uuid = "0123456789abcdef0123456789abcdef";
> >>>> + ...
> >>>> + }
> >>>> +
> >>>> +Schema Version
> >>>> +^^^^^^^^^^^^^^
> >>>> +Schema version is defined through the ``schema_version`` node. It
> >>>> +contains ``major`` and ``minor`` properties as below::
> >>>> +
> >>>> + /dts-v1/;
> >>>> + / {
> >>>> + schema_version {
> >>>> + major = <0x01>;
> >>>> + minor = <0x00>;
> >>>> + };
> >>>> + ...
> >>>> + }
> >>>> +
> >>>> +.. _partition_uuids:
> >>>> +
> >>>> +Partition UUIDs
> >>>> +^^^^^^^^^^^^^^^
> >>>> +Each partition may have parent and child UUIDs. These UUIDs are
> >>>> +defined by ``interfaces`` node and ``interface_uuid`` property::
> >>>> +
> >>>> + /dts-v1/;
> >>>> + / {
> >>>> + interfaces {
> >>>> + @0 {
> >>>> + interface_uuid = "0123456789abcdef0123456789abcdef";
> >>>> + };
> >>>> + @1 {
> >>>> + interface_uuid = "fedcba9876543210fedcba9876543210";
> >>>> + };
> >>>> + ...
> >>>> + };
> >>>> + ...
> >>>> + }
> >>>> +
> >>>> +
> >>>> +Subsystem Instantiations
> >>>> +^^^^^^^^^^^^^^^^^^^^^^^^
> >>>> +Subsystem instantiations are captured as children of
> >>>> +``addressable_endpoints``
> >>>> +node::
> >>>> +
> >>>> + /dts-v1/;
> >>>> + / {
> >>>> + addressable_endpoints {
> >>>> + abc {
> >>>> + ...
> >>>> + };
> >>>> + def {
> >>>> + ...
> >>>> + };
> >>>> + ...
> >>>> + }
> >>>> + }
> >>>> +
> >>>> +Subnode 'abc' and 'def' are the name of subsystem nodes
> >>>> +
> >>>> +Subsystem Node
> >>>> +^^^^^^^^^^^^^^
> >>>> +Each subsystem node and its properties define a hardware instance::
> >>>> +
> >>>> +
> >>>> + addressable_endpoints {
> >>>> + abc {
> >>>> + reg = <0x00 0x1f05000 0x00 0x1000>>
> >>>> + pcie_physical_function = <0x0>;
> >>>> + pcie_bar_mapping = <0x2>;
> >>>> + compatible = "abc def";
> >>>> + interrupts = <0x09 0x0c>;
> >>>> + firmware {
> >>>> + firmware_product_name = "abc"
> >>>> + firmware_branch_name = "def"
> >>>> + firmware_version_major = <1>
> >>>> + firmware_version_minor = <2>
> >>>> + };
> >>>> + }
> >>>> + ...
> >>>> + }
> >>>> +
> >>>> +:reg:
> >>>> + Property defines an address range. `<0x00 0x1f05000 0x00 0x1000>`
> >>>> +indicates
> >>>> + *0x00 0x1f05000* as BAR offset and *0x00 0x1000* as address length.
> >>>> +:pcie_physical_function:
> >>>> + Property specifies which PCIe physical function the subsystem node
> resides.
> >>>> + `<0x0>` implies physical function 0.
> >>>> +:pcie_bar_mapping:
> >>>> + Property specifies which PCIe BAR the subsystem node resides.
> >>>> +`<0x2>` implies BAR 2. A value of 0 means the property is not defined.
> >>>> +:compatible:
> >>>> + Property is a list of strings. The first string in the list
> >>>> +specifies the exact subsystem node. The following strings
> >>>> +represent other devices that the device is compatible with.
> >>>> +:interrupts:
> >>>> + Property specifies start and end interrupts for this subsystem node.
> >>>> + `<0x09 0x0c>` implies interrupts 9 to 13 are used by this subsystem.
> >>>> +:firmware:
> >>>> + Subnode defines the firmware required by this subsystem node.
> >>>> +
> >>>> +Alveo U50 Platform Example
> >>>> +^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>>> +::
> >>>> +
> >>>> + /dts-v1/;
> >>>> +
> >>>> + /{
> >>>> + logic_uuid = "f465b0a3ae8c64f619bc150384ace69b";
> >>>> +
> >>>> + schema_version {
> >>>> + major = <0x01>;
> >>>> + minor = <0x00>;
> >>>> + };
> >>>> +
> >>>> + interfaces {
> >>>> +
> >>>> + @0 {
> >>>> + interface_uuid =
> "862c7020a250293e32036f19956669e5";
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + addressable_endpoints {
> >>>> +
> >>>> + ep_blp_rom_00 {
> >>>> + reg = <0x00 0x1f04000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_bram_ctrl-
> 1.0\0axi_bram_ctrl";
> >>>> + };
> >>>> +
> >>>> + ep_card_flash_program_00 {
> >>>> + reg = <0x00 0x1f06000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_quad_spi-
> 1.0\0axi_quad_spi";
> >>>> + interrupts = <0x03 0x03>;
> >>>> + };
> >>>> +
> >>>> + ep_cmc_firmware_mem_00 {
> >>>> + reg = <0x00 0x1e20000 0x00 0x20000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible =
> >>>> + "xilinx.com,reg_abs-axi_bram_ctrl-1.0\0axi_bram_ctrl";
> >>>> +
> >>>> + firmware {
> >>>> + firmware_product_name = "cmc";
> >>>> + firmware_branch_name = "u50";
> >>>> + firmware_version_major = <0x01>;
> >>>> + firmware_version_minor = <0x00>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + ep_cmc_intc_00 {
> >>>> + reg = <0x00 0x1e03000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_intc-1.0\0axi_intc";
> >>>> + interrupts = <0x04 0x04>;
> >>>> + };
> >>>> +
> >>>> + ep_cmc_mutex_00 {
> >>>> + reg = <0x00 0x1e02000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_cmc_regmap_00 {
> >>>> + reg = <0x00 0x1e08000 0x00 0x2000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible =
> >>>> + "xilinx.com,reg_abs-axi_bram_ctrl-1.0\0axi_bram_ctrl";
> >>>> +
> >>>> + firmware {
> >>>> + firmware_product_name = "sc-fw";
> >>>> + firmware_branch_name = "u50";
> >>>> + firmware_version_major = <0x05>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + ep_cmc_reset_00 {
> >>>> + reg = <0x00 0x1e01000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_ddr_mem_calib_00 {
> >>>> + reg = <0x00 0x63000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_debug_bscan_mgmt_00 {
> >>>> + reg = <0x00 0x1e90000 0x00 0x10000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-debug_bridge-
> 1.0\0debug_bridge";
> >>>> + };
> >>>> +
> >>>> + ep_ert_base_address_00 {
> >>>> + reg = <0x00 0x21000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_ert_command_queue_mgmt_00 {
> >>>> + reg = <0x00 0x40000 0x00 0x10000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-ert_command_queue-
> 1.0\0ert_command_queue";
> >>>> + };
> >>>> +
> >>>> + ep_ert_command_queue_user_00 {
> >>>> + reg = <0x00 0x40000 0x00 0x10000>;
> >>>> + pcie_physical_function = <0x01>;
> >>>> + compatible = "xilinx.com,reg_abs-ert_command_queue-
> 1.0\0ert_command_queue";
> >>>> + };
> >>>> +
> >>>> + ep_ert_firmware_mem_00 {
> >>>> + reg = <0x00 0x30000 0x00 0x8000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible =
> >>>> + "xilinx.com,reg_abs-axi_bram_ctrl-1.0\0axi_bram_ctrl";
> >>>> +
> >>>> + firmware {
> >>>> + firmware_product_name = "ert";
> >>>> + firmware_branch_name = "v20";
> >>>> + firmware_version_major = <0x01>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + ep_ert_intc_00 {
> >>>> + reg = <0x00 0x23000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_intc-1.0\0axi_intc";
> >>>> + interrupts = <0x05 0x05>;
> >>>> + };
> >>>> +
> >>>> + ep_ert_reset_00 {
> >>>> + reg = <0x00 0x22000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_ert_sched_00 {
> >>>> + reg = <0x00 0x50000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x01>;
> >>>> + compatible = "xilinx.com,reg_abs-ert_sched-
> 1.0\0ert_sched";
> >>>> + interrupts = <0x09 0x0c>;
> >>>> + };
> >>>> +
> >>>> + ep_fpga_configuration_00 {
> >>>> + reg = <0x00 0x1e88000 0x00 0x8000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_hwicap-
> 1.0\0axi_hwicap";
> >>>> + interrupts = <0x02 0x02>;
> >>>> + };
> >>>> +
> >>>> + ep_icap_reset_00 {
> >>>> + reg = <0x00 0x1f07000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_msix_00 {
> >>>> + reg = <0x00 0x00 0x00 0x20000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-msix-1.0\0msix";
> >>>> + pcie_bar_mapping = <0x02>;
> >>>> + };
> >>>> +
> >>>> + ep_pcie_link_mon_00 {
> >>>> + reg = <0x00 0x1f05000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_pr_isolate_plp_00 {
> >>>> + reg = <0x00 0x1f01000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_pr_isolate_ulp_00 {
> >>>> + reg = <0x00 0x1000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_gpio-1.0\0axi_gpio";
> >>>> + };
> >>>> +
> >>>> + ep_uuid_rom_00 {
> >>>> + reg = <0x00 0x64000 0x00 0x1000>;
> >>>> + pcie_physical_function = <0x00>;
> >>>> + compatible = "xilinx.com,reg_abs-axi_bram_ctrl-
> 1.0\0axi_bram_ctrl";
> >>>> + };
> >>>> +
> >>>> + ep_xdma_00 {
> >>>> + reg = <0x00 0x00 0x00 0x10000>;
> >>>> + pcie_physical_function = <0x01>;
> >>>> + compatible = "xilinx.com,reg_abs-xdma-1.0\0xdma";
> >>>> + pcie_bar_mapping = <0x02>;
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + }
> >>>> +
> >>>> +
> >>>> +
> >>>> +Deployment Models
> >>>> +=================
> >>>> +
> >>>> +Baremetal
> >>>> +---------
> >>>> +
> >>>> +In bare-metal deployments, both MPF and UPF are visible and
> >>>> +accessible. The xrt-mgmt driver binds to MPF. The xrt-mgmt driver
> >>>> +operations are privileged and available to system administrator. The full
> stack is illustrated below::
> >>>> +
> >>>> + HOST
> >>>> +
> >>>> + [XRT-MGMT] [XRT-USER]
> >>>> + | |
> >>>> + | |
> >>>> + +-----+ +-----+
> >>>> + | MPF | | UPF |
> >>>> + | | | |
> >>>> + | PF0 | | PF1 |
> >>>> + +--+--+ +--+--+
> >>>> + ......... ^................. ^..........
> >>>> + | |
> >>>> + | PCIe DEVICE |
> >>>> + | |
> >>>> + +--+------------------+--+
> >>>> + | SHELL |
> >>>> + | |
> >>>> + +------------------------+
> >>>> + | USER |
> >>>> + | |
> >>>> + | |
> >>>> + | |
> >>>> + | |
> >>>> + +------------------------+
> >>>> +
> >>>> +
> >>>> +
> >>>> +Virtualized
> >>>> +-----------
> >>>> +
> >>>> +In virtualized deployments, the privileged MPF is assigned to the
> >>>> +host but the unprivileged UPF is assigned to a guest VM via PCIe
> >>>> +pass-through. The xrt-mgmt driver in host binds to MPF. The
> >>>> +xrt-mgmt driver operations are privileged and only accessible to the
> MPF. The full stack is illustrated below::
> >>>> +
> >>>> +
> >>>> + ..............
> >>>> + HOST . VM .
> >>>> + . .
> >>>> + [XRT-MGMT] . [XRT-USER] .
> >>>> + | . | .
> >>>> + | . | .
> >>>> + +-----+ . +-----+ .
> >>>> + | MPF | . | UPF | .
> >>>> + | | . | | .
> >>>> + | PF0 | . | PF1 | .
> >>>> + +--+--+ . +--+--+ .
> >>>> + ......... ^................. ^..........
> >>>> + | |
> >>>> + | PCIe DEVICE |
> >>>> + | |
> >>>> + +--+------------------+--+
> >>>> + | SHELL |
> >>>> + | |
> >>>> + +------------------------+
> >>>> + | USER |
> >>>> + | |
> >>>> + | |
> >>>> + | |
> >>>> + | |
> >>>> + +------------------------+
> >>>> +
> >>>> +
> >>>> +
> >>>> +
> >>>> +
> >>>> +Platform Security Considerations
> >>>> +================================
> >>>> +
> >>>> +`Security of Alveo Platform
> >>>> +<https://xilinx.github.io/XRT/master/html/security.html>`_
> >>>> +discusses the deployment options and security implications in great
> detail.
> >>>> diff --git a/MAINTAINERS b/MAINTAINERS index
> >>>> 056966c9aac9..beeaf0257364 100644
> >>>> --- a/MAINTAINERS
> >>>> +++ b/MAINTAINERS
> >>>> @@ -7274,6 +7274,17 @@ F: Documentation/fpga/
> >>>> F: drivers/fpga/
> >>>> F: include/linux/fpga/
> >>>>
> >>>> +FPGA XRT DRIVERS
> >>>> +M: Lizhi Hou <lizhi.hou@xxxxxxxxxx>
> >>>> +R: Max Zhen <max.zhen@xxxxxxxxxx>
> >>>> +R: Sonal Santan <sonal.santan@xxxxxxxxxx>
> >>>> +L: linux-fpga@xxxxxxxxxxxxxxx
> >>>> +S: Supported
> >>>> +W: https://github.com/Xilinx/XRT
> >>>> +F: Documentation/fpga/xrt.rst
> >>>> +F: drivers/fpga/xrt/
> >>>> +F: include/uapi/linux/xrt/
> >>>> +
> >>>> FPU EMULATOR
> >>>> M: Bill Metzenthen <billm@xxxxxxxxxxxxx>
> >>>> S: Maintained
> >>>> --
> >>>> 2.27.0

Next message: NeilBrown: "Re: [PATCH 2/6] MM: improve documentation for __GFP_NOFAIL"
Previous message: Chengming Zhou: "Re: [External] Re: [PATCH] bpf: use count for prealloc hashtab too"
Next in thread: Rob Herring: "Re: [PATCH V9 XRT Alveo 01/14] Documentation: fpga: Add a document describing XRT Alveo drivers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]