Re: [PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
On 3/17/21 2:08 PM, Tom Rix wrote:
On 3/16/21 1:29 PM, Max Zhen wrote:
Hi Tom,
On 2/26/21 7:01 AM, Tom Rix wrote:
A question i do not know the answer to.
Seems like 'golden' is linked to a manufacturing (diagnostics?) image.
If the public will never see it, should handling it here be done ?
Moritz, do you know ?
Golden image is preloaded on the device when it is shipped to customer. Then,
customer can load other shells (from Xilinx or some other vendor). If something
goes wrong with the shell, customer can always go back to golden and start over
again. So, golden image is going to be used in public, not just internally by
Xilinx.
Thanks for the explanation.
On 2/17/21 10:40 PM, Lizhi Hou wrote:
The PCIE device driver which attaches to management function on Alveo
to the management
Sure.
devices. It instantiates one or more partition drivers which in turn
more fpga partition / group ?
Group driver.
instantiate platform drivers. The instantiation of partition and platform
drivers is completely data driven.
data driven ? everything is data driven. do you mean dtb driven ?
Data driven means not hard-coded. Here data means meta data which is presented
in device tree format, dtb.
Signed-off-by: Sonal Santan
Signed-off-by: Max Zhen
Signed-off-by: Lizhi Hou
---
drivers/fpga/xrt/include/xroot.h | 114 +++
drivers/fpga/xrt/mgmt/root.c | 342 +++
2 files changed, 456 insertions(+)
create mode 100644 drivers/fpga/xrt/include/xroot.h
create mode 100644 drivers/fpga/xrt/mgmt/root.c
diff --git a/drivers/fpga/xrt/include/xroot.h b/drivers/fpga/xrt/include/xroot.h
new file mode 100644
index ..752e10daa85e
--- /dev/null
+++ b/drivers/fpga/xrt/include/xroot.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for Xilinx Runtime (XRT) driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Cheng Zhen
+ */
+
+#ifndef _XRT_ROOT_H_
+#define _XRT_ROOT_H_
+
+#include
+#include "subdev_id.h"
+#include "events.h"
+
+typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
+ struct platform_device *, void *);
+#define XRT_SUBDEV_MATCH_PREV((xrt_subdev_match_t)-1)
+#define XRT_SUBDEV_MATCH_NEXT((xrt_subdev_match_t)-2)
+
+/*
+ * Root IOCTL calls.
+ */
+enum xrt_root_ioctl_cmd {
+ /* Leaf actions. */
+ XRT_ROOT_GET_LEAF = 0,
+ XRT_ROOT_PUT_LEAF,
+ XRT_ROOT_GET_LEAF_HOLDERS,
+
+ /* Group actions. */
+ XRT_ROOT_CREATE_GROUP,
+ XRT_ROOT_REMOVE_GROUP,
+ XRT_ROOT_LOOKUP_GROUP,
+ XRT_ROOT_WAIT_GROUP_BRINGUP,
+
+ /* Event actions. */
+ XRT_ROOT_EVENT,
should this be XRT_ROOT_EVENT_SYNC ?
Sure.
+ XRT_ROOT_EVENT_ASYNC,
+
+ /* Device info. */
+ XRT_ROOT_GET_RESOURCE,
+ XRT_ROOT_GET_ID,
+
+ /* Misc. */
+ XRT_ROOT_HOT_RESET,
+ XRT_ROOT_HWMON,
+};
+
+struct xrt_root_ioctl_get_leaf {
+ struct platform_device *xpigl_pdev; /* caller's pdev */
xpigl_ ? unneeded suffix in element names
It's needed since the it might be included and used in > 1 .c files. I'd like
to keep it's name unique.
This is an element name, the variable name sound be unique enough to make it
clear.
This is not a critical issue, ok as-is.
+ xrt_subdev_match_t xpigl_match_cb;
+ void *xpigl_match_arg;
+ struct platform_device *xpigl_leaf; /* target leaf pdev */
+};
+
+struct xrt_root_ioctl_put_leaf {
+ struct platform_device *xpipl_pdev; /* caller's pdev */
+ struct platform_device *xpipl_leaf; /* target's pdev */
caller_pdev;
target_pdev;
Sure.
+};
+
+struct xrt_root_ioctl_lookup_group {
+ struct platform_device *xpilp_pdev; /* caller's pdev */
+ xrt_subdev_match_t xpilp_match_cb;
+ void *xpilp_match_arg;
+ int xpilp_grp_inst;
+};
+
+struct xrt_root_ioctl_get_holders {
+ struct platform_device *xpigh_pdev; /* caller's pdev */
+ char *xpigh_holder_buf;
+ size_t xpigh_holder_buf_len;
+};
+
+struct xrt_root_ioctl_get_res {
+ struct resource *xpigr_res;
+};
+
+struct xrt_root_ioctl_get_id {
+ unsigned short xpigi_vendor_id;
+ unsigned short xpigi_device_id;
+ unsigned short xpigi_sub_vendor_id;
+ unsigned short xpigi_sub_device_id;
+};
+
+struct xrt_root_ioctl_hwmon {
+ bool xpih_register;
+ const char *xpih_name;
+ void *xpih_drvdata;
+ const struct attribute_group **xpih_groups;
+ struct device *xpih_hwmon_dev;
+};
+
+typedef int (*xrt_subdev_root_cb_t)(struct device *, void *, u32, void *);
This function pointer type is important, please add a comment about its use and
expected parameters
Added.
+int xrt_subdev_root_request(struct platform_device *self, u32 cmd, void *arg);
+
+/*
+ * Defines physical function (MPF / UPF) specific operations
+ * needed in common root driver.
+ */
+struct xroot_pf_cb {
+ void (*xpc_hot_reset)(struct pci_dev *pdev);
This is only ever set to xmgmt_root_hot_reset, why is this
Re: [PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
On 3/16/21 1:29 PM, Max Zhen wrote:
> Hi Tom,
>
>
> On 2/26/21 7:01 AM, Tom Rix 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.
>>
>>
>> A question i do not know the answer to.
>>
>> Seems like 'golden' is linked to a manufacturing (diagnostics?) image.
>>
>> If the public will never see it, should handling it here be done ?
>>
>> Moritz, do you know ?
>
>
> Golden image is preloaded on the device when it is shipped to customer. Then,
> customer can load other shells (from Xilinx or some other vendor). If
> something goes wrong with the shell, customer can always go back to golden
> and start over again. So, golden image is going to be used in public, not
> just internally by Xilinx.
>
>
Thanks for the explanation.
>>
>>
>> On 2/17/21 10:40 PM, Lizhi Hou wrote:
>>> The PCIE device driver which attaches to management function on Alveo
>> to the management
>
>
> Sure.
>
>
>>> devices. It instantiates one or more partition drivers which in turn
>> more fpga partition / group ?
>
>
> Group driver.
>
>
>>> instantiate platform drivers. The instantiation of partition and platform
>>> drivers is completely data driven.
>> data driven ? everything is data driven. do you mean dtb driven ?
>
>
> Data driven means not hard-coded. Here data means meta data which is
> presented in device tree format, dtb.
>
>
>>> Signed-off-by: Sonal Santan
>>> Signed-off-by: Max Zhen
>>> Signed-off-by: Lizhi Hou
>>> ---
>>> drivers/fpga/xrt/include/xroot.h | 114 +++
>>> drivers/fpga/xrt/mgmt/root.c | 342 +++
>>> 2 files changed, 456 insertions(+)
>>> create mode 100644 drivers/fpga/xrt/include/xroot.h
>>> create mode 100644 drivers/fpga/xrt/mgmt/root.c
>>>
>>> diff --git a/drivers/fpga/xrt/include/xroot.h
>>> b/drivers/fpga/xrt/include/xroot.h
>>> new file mode 100644
>>> index ..752e10daa85e
>>> --- /dev/null
>>> +++ b/drivers/fpga/xrt/include/xroot.h
>>> @@ -0,0 +1,114 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Header file for Xilinx Runtime (XRT) driver
>>> + *
>>> + * Copyright (C) 2020-2021 Xilinx, Inc.
>>> + *
>>> + * Authors:
>>> + * Cheng Zhen
>>> + */
>>> +
>>> +#ifndef _XRT_ROOT_H_
>>> +#define _XRT_ROOT_H_
>>> +
>>> +#include
>>> +#include "subdev_id.h"
>>> +#include "events.h"
>>> +
>>> +typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
>>> + struct platform_device *, void *);
>>> +#define XRT_SUBDEV_MATCH_PREV ((xrt_subdev_match_t)-1)
>>> +#define XRT_SUBDEV_MATCH_NEXT ((xrt_subdev_match_t)-2)
>>> +
>>> +/*
>>> + * Root IOCTL calls.
>>> + */
>>> +enum xrt_root_ioctl_cmd {
>>> + /* Leaf actions. */
>>> + XRT_ROOT_GET_LEAF = 0,
>>> + XRT_ROOT_PUT_LEAF,
>>> + XRT_ROOT_GET_LEAF_HOLDERS,
>>> +
>>> + /* Group actions. */
>>> + XRT_ROOT_CREATE_GROUP,
>>> + XRT_ROOT_REMOVE_GROUP,
>>> + XRT_ROOT_LOOKUP_GROUP,
>>> + XRT_ROOT_WAIT_GROUP_BRINGUP,
>>> +
>>> + /* Event actions. */
>>> + XRT_ROOT_EVENT,
>> should this be XRT_ROOT_EVENT_SYNC ?
>
>
> Sure.
>
>
>>> + XRT_ROOT_EVENT_ASYNC,
>>> +
>>> + /* Device info. */
>>> + XRT_ROOT_GET_RESOURCE,
>>> + XRT_ROOT_GET_ID,
>>> +
>>> + /* Misc. */
>>> + XRT_ROOT_HOT_RESET,
>>> + XRT_ROOT_HWMON,
>>> +};
>>> +
>>> +struct xrt_root_ioctl_get_leaf {
>>> + struct platform_device *xpigl_pdev; /* caller's pdev */
>> xpigl_ ? unneeded suffix in element names
>
>
> It's needed since the it might be included and used in > 1 .c files. I'd like
> to keep it's name unique.
This is an element name, the variable name sound be unique enough to make it
clear.
This is not a critical issue, ok as-is.
>
>
>>> + xrt_subdev_match_t xpigl_match_cb;
>>> + void *xpigl_match_arg;
>>> + struct platform_device *xpigl_leaf; /* target leaf pdev */
>>> +};
>>> +
>>> +struct xrt_root_ioctl_put_leaf {
>>> + struct platform_device *xpipl_pdev; /* caller's pdev */
>>> + struct platform_device *xpipl_leaf; /* target's pdev */
>> caller_pdev;
>>
>> target_pdev;
>
>
> Sure.
>
>
>>
>>> +};
>>> +
>>> +struct xrt_root_ioctl_lookup_group {
>>> + struct platform_device *xpilp_pdev; /* caller's pdev */
>>> + xrt_subdev_match_t xpilp_match_cb;
>>> + void *xpilp_match_arg;
>>> + int xpilp_grp_inst;
>>> +};
>>> +
>>> +struct xrt_root_ioctl_get_holders {
>>> + struct platform_device *xpigh_pdev; /* caller's pdev */
>>> + char *xpigh_holder_buf;
>>> + size_t xpigh_holder_buf_len;
>>> +};
>>> +
>>> +struct xrt_root_ioctl_get_res {
>>> + struct resource *xpigr_res;
>>> +};
>>> +
>>> +struct xrt_root_ioctl_get_id {
>>> + unsigned short xpigi_vendor_id;
>>> + unsigned short xpigi_device_id;
>>> + unsigned short xpigi_sub_vendor_id;
>>> + unsigned short xpigi_sub_device_id;
>>> +};
>>> +
>>> +struct xrt_root_ioctl_hwmon {
>>> +
Re: [PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
Hi Tom,
On 2/26/21 7:01 AM, Tom Rix 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.
A question i do not know the answer to.
Seems like 'golden' is linked to a manufacturing (diagnostics?) image.
If the public will never see it, should handling it here be done ?
Moritz, do you know ?
Golden image is preloaded on the device when it is shipped to customer.
Then, customer can load other shells (from Xilinx or some other vendor).
If something goes wrong with the shell, customer can always go back to
golden and start over again. So, golden image is going to be used in
public, not just internally by Xilinx.
On 2/17/21 10:40 PM, Lizhi Hou wrote:
The PCIE device driver which attaches to management function on Alveo
to the management
Sure.
devices. It instantiates one or more partition drivers which in turn
more fpga partition / group ?
Group driver.
instantiate platform drivers. The instantiation of partition and platform
drivers is completely data driven.
data driven ? everything is data driven. do you mean dtb driven ?
Data driven means not hard-coded. Here data means meta data which is
presented in device tree format, dtb.
Signed-off-by: Sonal Santan
Signed-off-by: Max Zhen
Signed-off-by: Lizhi Hou
---
drivers/fpga/xrt/include/xroot.h | 114 +++
drivers/fpga/xrt/mgmt/root.c | 342 +++
2 files changed, 456 insertions(+)
create mode 100644 drivers/fpga/xrt/include/xroot.h
create mode 100644 drivers/fpga/xrt/mgmt/root.c
diff --git a/drivers/fpga/xrt/include/xroot.h b/drivers/fpga/xrt/include/xroot.h
new file mode 100644
index ..752e10daa85e
--- /dev/null
+++ b/drivers/fpga/xrt/include/xroot.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for Xilinx Runtime (XRT) driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Cheng Zhen
+ */
+
+#ifndef _XRT_ROOT_H_
+#define _XRT_ROOT_H_
+
+#include
+#include "subdev_id.h"
+#include "events.h"
+
+typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
+ struct platform_device *, void *);
+#define XRT_SUBDEV_MATCH_PREV((xrt_subdev_match_t)-1)
+#define XRT_SUBDEV_MATCH_NEXT((xrt_subdev_match_t)-2)
+
+/*
+ * Root IOCTL calls.
+ */
+enum xrt_root_ioctl_cmd {
+ /* Leaf actions. */
+ XRT_ROOT_GET_LEAF = 0,
+ XRT_ROOT_PUT_LEAF,
+ XRT_ROOT_GET_LEAF_HOLDERS,
+
+ /* Group actions. */
+ XRT_ROOT_CREATE_GROUP,
+ XRT_ROOT_REMOVE_GROUP,
+ XRT_ROOT_LOOKUP_GROUP,
+ XRT_ROOT_WAIT_GROUP_BRINGUP,
+
+ /* Event actions. */
+ XRT_ROOT_EVENT,
should this be XRT_ROOT_EVENT_SYNC ?
Sure.
+ XRT_ROOT_EVENT_ASYNC,
+
+ /* Device info. */
+ XRT_ROOT_GET_RESOURCE,
+ XRT_ROOT_GET_ID,
+
+ /* Misc. */
+ XRT_ROOT_HOT_RESET,
+ XRT_ROOT_HWMON,
+};
+
+struct xrt_root_ioctl_get_leaf {
+ struct platform_device *xpigl_pdev; /* caller's pdev */
xpigl_ ? unneeded suffix in element names
It's needed since the it might be included and used in > 1 .c files. I'd
like to keep it's name unique.
+ xrt_subdev_match_t xpigl_match_cb;
+ void *xpigl_match_arg;
+ struct platform_device *xpigl_leaf; /* target leaf pdev */
+};
+
+struct xrt_root_ioctl_put_leaf {
+ struct platform_device *xpipl_pdev; /* caller's pdev */
+ struct platform_device *xpipl_leaf; /* target's pdev */
caller_pdev;
target_pdev;
Sure.
+};
+
+struct xrt_root_ioctl_lookup_group {
+ struct platform_device *xpilp_pdev; /* caller's pdev */
+ xrt_subdev_match_t xpilp_match_cb;
+ void *xpilp_match_arg;
+ int xpilp_grp_inst;
+};
+
+struct xrt_root_ioctl_get_holders {
+ struct platform_device *xpigh_pdev; /* caller's pdev */
+ char *xpigh_holder_buf;
+ size_t xpigh_holder_buf_len;
+};
+
+struct xrt_root_ioctl_get_res {
+ struct resource *xpigr_res;
+};
+
+struct xrt_root_ioctl_get_id {
+ unsigned short xpigi_vendor_id;
+ unsigned short xpigi_device_id;
+ unsigned short xpigi_sub_vendor_id;
+ unsigned short xpigi_sub_device_id;
+};
+
+struct xrt_root_ioctl_hwmon {
+ bool xpih_register;
+ const char *xpih_name;
+ void *xpih_drvdata;
+ const struct attribute_group **xpih_groups;
+ struct device *xpih_hwmon_dev;
+};
+
+typedef int (*xrt_subdev_root_cb_t)(struct device *, void *, u32, void *);
This function pointer type is important, please add a comment about its use and
expected parameters
Added.
+int xrt_subdev_root_request(struct platform_device *self, u32 cmd, void *arg);
+
+/*
+ * Defines physical function (MPF / UPF) specific operations
+ * needed in common root driver.
+ */
+struct xroot_pf_cb {
+ void (*xpc_hot_reset)(struct pci_dev *pdev);
This is only ever set to xmgmt_root_hot_reset, why is this abstraction needed ?
As comment says, hot reset
Re: [PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
On Fri, Feb 26, 2021 at 07:01:05AM -0800, Tom Rix wrote:
> A question i do not know the answer to.
>
> Seems like 'golden' is linked to a manufacturing (diagnostics?) image.
>From my brief history with Xilinx Ultrascale+ PCI cards I recall the golden
image being a sort of known good recovery image.
If we can't tell it should probably be explained better :)
>
> If the public will never see it, should handling it here be done ?
Yes. We do want people to run their entire stack using mainline linux,
not just a part of it, if code is needed to get from recovery image to
full image or similar, then we should support that.
> Moritz, do you know ?
>
>
> On 2/17/21 10:40 PM, Lizhi Hou wrote:
> > The PCIE device driver which attaches to management function on Alveo
> to the management
> > devices. It instantiates one or more partition drivers which in turn
> more fpga partition / group ?
> > instantiate platform drivers. The instantiation of partition and platform
> > drivers is completely data driven.
> data driven ? everything is data driven. do you mean dtb driven ?
> >
> > Signed-off-by: Sonal Santan
> > Signed-off-by: Max Zhen
> > Signed-off-by: Lizhi Hou
> > ---
> > drivers/fpga/xrt/include/xroot.h | 114 +++
> > drivers/fpga/xrt/mgmt/root.c | 342 +++
> > 2 files changed, 456 insertions(+)
> > create mode 100644 drivers/fpga/xrt/include/xroot.h
> > create mode 100644 drivers/fpga/xrt/mgmt/root.c
> >
> > diff --git a/drivers/fpga/xrt/include/xroot.h
> > b/drivers/fpga/xrt/include/xroot.h
> > new file mode 100644
> > index ..752e10daa85e
> > --- /dev/null
> > +++ b/drivers/fpga/xrt/include/xroot.h
> > @@ -0,0 +1,114 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Header file for Xilinx Runtime (XRT) driver
> > + *
> > + * Copyright (C) 2020-2021 Xilinx, Inc.
> > + *
> > + * Authors:
> > + * Cheng Zhen
> > + */
> > +
> > +#ifndef _XRT_ROOT_H_
> > +#define _XRT_ROOT_H_
> > +
> > +#include
> > +#include "subdev_id.h"
> > +#include "events.h"
> > +
> > +typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
> > + struct platform_device *, void *);
> > +#define XRT_SUBDEV_MATCH_PREV ((xrt_subdev_match_t)-1)
> > +#define XRT_SUBDEV_MATCH_NEXT ((xrt_subdev_match_t)-2)
> > +
> > +/*
> > + * Root IOCTL calls.
> > + */
> > +enum xrt_root_ioctl_cmd {
> > + /* Leaf actions. */
> > + XRT_ROOT_GET_LEAF = 0,
> > + XRT_ROOT_PUT_LEAF,
> > + XRT_ROOT_GET_LEAF_HOLDERS,
> > +
> > + /* Group actions. */
> > + XRT_ROOT_CREATE_GROUP,
> > + XRT_ROOT_REMOVE_GROUP,
> > + XRT_ROOT_LOOKUP_GROUP,
> > + XRT_ROOT_WAIT_GROUP_BRINGUP,
> > +
> > + /* Event actions. */
> > + XRT_ROOT_EVENT,
> should this be XRT_ROOT_EVENT_SYNC ?
> > + XRT_ROOT_EVENT_ASYNC,
> > +
> > + /* Device info. */
> > + XRT_ROOT_GET_RESOURCE,
> > + XRT_ROOT_GET_ID,
> > +
> > + /* Misc. */
> > + XRT_ROOT_HOT_RESET,
> > + XRT_ROOT_HWMON,
> > +};
> > +
> > +struct xrt_root_ioctl_get_leaf {
> > + struct platform_device *xpigl_pdev; /* caller's pdev */
> xpigl_ ? unneeded suffix in element names
> > + xrt_subdev_match_t xpigl_match_cb;
> > + void *xpigl_match_arg;
> > + struct platform_device *xpigl_leaf; /* target leaf pdev */
> > +};
> > +
> > +struct xrt_root_ioctl_put_leaf {
> > + struct platform_device *xpipl_pdev; /* caller's pdev */
> > + struct platform_device *xpipl_leaf; /* target's pdev */
>
> caller_pdev;
>
> target_pdev;
>
> > +};
> > +
> > +struct xrt_root_ioctl_lookup_group {
> > + struct platform_device *xpilp_pdev; /* caller's pdev */
> > + xrt_subdev_match_t xpilp_match_cb;
> > + void *xpilp_match_arg;
> > + int xpilp_grp_inst;
> > +};
> > +
> > +struct xrt_root_ioctl_get_holders {
> > + struct platform_device *xpigh_pdev; /* caller's pdev */
> > + char *xpigh_holder_buf;
> > + size_t xpigh_holder_buf_len;
> > +};
> > +
> > +struct xrt_root_ioctl_get_res {
> > + struct resource *xpigr_res;
> > +};
> > +
> > +struct xrt_root_ioctl_get_id {
> > + unsigned short xpigi_vendor_id;
> > + unsigned short xpigi_device_id;
> > + unsigned short xpigi_sub_vendor_id;
> > + unsigned short xpigi_sub_device_id;
> > +};
> > +
> > +struct xrt_root_ioctl_hwmon {
> > + bool xpih_register;
> > + const char *xpih_name;
> > + void *xpih_drvdata;
> > + const struct attribute_group **xpih_groups;
> > + struct device *xpih_hwmon_dev;
> > +};
> > +
> > +typedef int (*xrt_subdev_root_cb_t)(struct device *, void *, u32, void *);
> This function pointer type is important, please add a comment about its use
> and expected parameters
> > +int xrt_subdev_root_request(struct platform_device *self, u32 cmd, void
> > *arg);
> > +
> > +/*
> > + * Defines physical function (MPF / UPF) specific operations
> > + * needed in common root driver.
> > + */
> > +struct xroot_pf_cb {
> > + void (*xpc_hot_reset)(struct pci_dev *pdev);
> This is only ever set to xmgmt_root_hot_reset, why is this abstraction needed
> ?
Re: [PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
A question i do not know the answer to.
Seems like 'golden' is linked to a manufacturing (diagnostics?) image.
If the public will never see it, should handling it here be done ?
Moritz, do you know ?
On 2/17/21 10:40 PM, Lizhi Hou wrote:
> The PCIE device driver which attaches to management function on Alveo
to the management
> devices. It instantiates one or more partition drivers which in turn
more fpga partition / group ?
> instantiate platform drivers. The instantiation of partition and platform
> drivers is completely data driven.
data driven ? everything is data driven. do you mean dtb driven ?
>
> Signed-off-by: Sonal Santan
> Signed-off-by: Max Zhen
> Signed-off-by: Lizhi Hou
> ---
> drivers/fpga/xrt/include/xroot.h | 114 +++
> drivers/fpga/xrt/mgmt/root.c | 342 +++
> 2 files changed, 456 insertions(+)
> create mode 100644 drivers/fpga/xrt/include/xroot.h
> create mode 100644 drivers/fpga/xrt/mgmt/root.c
>
> diff --git a/drivers/fpga/xrt/include/xroot.h
> b/drivers/fpga/xrt/include/xroot.h
> new file mode 100644
> index ..752e10daa85e
> --- /dev/null
> +++ b/drivers/fpga/xrt/include/xroot.h
> @@ -0,0 +1,114 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Header file for Xilinx Runtime (XRT) driver
> + *
> + * Copyright (C) 2020-2021 Xilinx, Inc.
> + *
> + * Authors:
> + * Cheng Zhen
> + */
> +
> +#ifndef _XRT_ROOT_H_
> +#define _XRT_ROOT_H_
> +
> +#include
> +#include "subdev_id.h"
> +#include "events.h"
> +
> +typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
> + struct platform_device *, void *);
> +#define XRT_SUBDEV_MATCH_PREV((xrt_subdev_match_t)-1)
> +#define XRT_SUBDEV_MATCH_NEXT((xrt_subdev_match_t)-2)
> +
> +/*
> + * Root IOCTL calls.
> + */
> +enum xrt_root_ioctl_cmd {
> + /* Leaf actions. */
> + XRT_ROOT_GET_LEAF = 0,
> + XRT_ROOT_PUT_LEAF,
> + XRT_ROOT_GET_LEAF_HOLDERS,
> +
> + /* Group actions. */
> + XRT_ROOT_CREATE_GROUP,
> + XRT_ROOT_REMOVE_GROUP,
> + XRT_ROOT_LOOKUP_GROUP,
> + XRT_ROOT_WAIT_GROUP_BRINGUP,
> +
> + /* Event actions. */
> + XRT_ROOT_EVENT,
should this be XRT_ROOT_EVENT_SYNC ?
> + XRT_ROOT_EVENT_ASYNC,
> +
> + /* Device info. */
> + XRT_ROOT_GET_RESOURCE,
> + XRT_ROOT_GET_ID,
> +
> + /* Misc. */
> + XRT_ROOT_HOT_RESET,
> + XRT_ROOT_HWMON,
> +};
> +
> +struct xrt_root_ioctl_get_leaf {
> + struct platform_device *xpigl_pdev; /* caller's pdev */
xpigl_ ? unneeded suffix in element names
> + xrt_subdev_match_t xpigl_match_cb;
> + void *xpigl_match_arg;
> + struct platform_device *xpigl_leaf; /* target leaf pdev */
> +};
> +
> +struct xrt_root_ioctl_put_leaf {
> + struct platform_device *xpipl_pdev; /* caller's pdev */
> + struct platform_device *xpipl_leaf; /* target's pdev */
caller_pdev;
target_pdev;
> +};
> +
> +struct xrt_root_ioctl_lookup_group {
> + struct platform_device *xpilp_pdev; /* caller's pdev */
> + xrt_subdev_match_t xpilp_match_cb;
> + void *xpilp_match_arg;
> + int xpilp_grp_inst;
> +};
> +
> +struct xrt_root_ioctl_get_holders {
> + struct platform_device *xpigh_pdev; /* caller's pdev */
> + char *xpigh_holder_buf;
> + size_t xpigh_holder_buf_len;
> +};
> +
> +struct xrt_root_ioctl_get_res {
> + struct resource *xpigr_res;
> +};
> +
> +struct xrt_root_ioctl_get_id {
> + unsigned short xpigi_vendor_id;
> + unsigned short xpigi_device_id;
> + unsigned short xpigi_sub_vendor_id;
> + unsigned short xpigi_sub_device_id;
> +};
> +
> +struct xrt_root_ioctl_hwmon {
> + bool xpih_register;
> + const char *xpih_name;
> + void *xpih_drvdata;
> + const struct attribute_group **xpih_groups;
> + struct device *xpih_hwmon_dev;
> +};
> +
> +typedef int (*xrt_subdev_root_cb_t)(struct device *, void *, u32, void *);
This function pointer type is important, please add a comment about its use and
expected parameters
> +int xrt_subdev_root_request(struct platform_device *self, u32 cmd, void
> *arg);
> +
> +/*
> + * Defines physical function (MPF / UPF) specific operations
> + * needed in common root driver.
> + */
> +struct xroot_pf_cb {
> + void (*xpc_hot_reset)(struct pci_dev *pdev);
This is only ever set to xmgmt_root_hot_reset, why is this abstraction needed ?
> +};
> +
> +int xroot_probe(struct pci_dev *pdev, struct xroot_pf_cb *cb, void **root);
> +void xroot_remove(void *root);
> +bool xroot_wait_for_bringup(void *root);
> +int xroot_add_vsec_node(void *root, char *dtb);
> +int xroot_create_group(void *xr, char *dtb);
> +int xroot_add_simple_node(void *root, char *dtb, const char *endpoint);
> +void xroot_broadcast(void *root, enum xrt_events evt);
> +
> +#endif /* _XRT_ROOT_H_ */
> diff --git a/drivers/fpga/xrt/mgmt/root.c b/drivers/fpga/xrt/mgmt/root.c
> new file mode 100644
> index ..583a37c9d30c
> --- /dev/null
> +++ b/drivers/fpga/xrt/mgmt/root.c
> @@ -0,0 +1,342 @@
>
[PATCH V3 XRT Alveo 07/18] fpga: xrt: management physical function driver (root)
The PCIE device driver which attaches to management function on Alveo
devices. It instantiates one or more partition drivers which in turn
instantiate platform drivers. The instantiation of partition and platform
drivers is completely data driven.
Signed-off-by: Sonal Santan
Signed-off-by: Max Zhen
Signed-off-by: Lizhi Hou
---
drivers/fpga/xrt/include/xroot.h | 114 +++
drivers/fpga/xrt/mgmt/root.c | 342 +++
2 files changed, 456 insertions(+)
create mode 100644 drivers/fpga/xrt/include/xroot.h
create mode 100644 drivers/fpga/xrt/mgmt/root.c
diff --git a/drivers/fpga/xrt/include/xroot.h b/drivers/fpga/xrt/include/xroot.h
new file mode 100644
index ..752e10daa85e
--- /dev/null
+++ b/drivers/fpga/xrt/include/xroot.h
@@ -0,0 +1,114 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for Xilinx Runtime (XRT) driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Cheng Zhen
+ */
+
+#ifndef _XRT_ROOT_H_
+#define _XRT_ROOT_H_
+
+#include
+#include "subdev_id.h"
+#include "events.h"
+
+typedef bool (*xrt_subdev_match_t)(enum xrt_subdev_id,
+ struct platform_device *, void *);
+#define XRT_SUBDEV_MATCH_PREV ((xrt_subdev_match_t)-1)
+#define XRT_SUBDEV_MATCH_NEXT ((xrt_subdev_match_t)-2)
+
+/*
+ * Root IOCTL calls.
+ */
+enum xrt_root_ioctl_cmd {
+ /* Leaf actions. */
+ XRT_ROOT_GET_LEAF = 0,
+ XRT_ROOT_PUT_LEAF,
+ XRT_ROOT_GET_LEAF_HOLDERS,
+
+ /* Group actions. */
+ XRT_ROOT_CREATE_GROUP,
+ XRT_ROOT_REMOVE_GROUP,
+ XRT_ROOT_LOOKUP_GROUP,
+ XRT_ROOT_WAIT_GROUP_BRINGUP,
+
+ /* Event actions. */
+ XRT_ROOT_EVENT,
+ XRT_ROOT_EVENT_ASYNC,
+
+ /* Device info. */
+ XRT_ROOT_GET_RESOURCE,
+ XRT_ROOT_GET_ID,
+
+ /* Misc. */
+ XRT_ROOT_HOT_RESET,
+ XRT_ROOT_HWMON,
+};
+
+struct xrt_root_ioctl_get_leaf {
+ struct platform_device *xpigl_pdev; /* caller's pdev */
+ xrt_subdev_match_t xpigl_match_cb;
+ void *xpigl_match_arg;
+ struct platform_device *xpigl_leaf; /* target leaf pdev */
+};
+
+struct xrt_root_ioctl_put_leaf {
+ struct platform_device *xpipl_pdev; /* caller's pdev */
+ struct platform_device *xpipl_leaf; /* target's pdev */
+};
+
+struct xrt_root_ioctl_lookup_group {
+ struct platform_device *xpilp_pdev; /* caller's pdev */
+ xrt_subdev_match_t xpilp_match_cb;
+ void *xpilp_match_arg;
+ int xpilp_grp_inst;
+};
+
+struct xrt_root_ioctl_get_holders {
+ struct platform_device *xpigh_pdev; /* caller's pdev */
+ char *xpigh_holder_buf;
+ size_t xpigh_holder_buf_len;
+};
+
+struct xrt_root_ioctl_get_res {
+ struct resource *xpigr_res;
+};
+
+struct xrt_root_ioctl_get_id {
+ unsigned short xpigi_vendor_id;
+ unsigned short xpigi_device_id;
+ unsigned short xpigi_sub_vendor_id;
+ unsigned short xpigi_sub_device_id;
+};
+
+struct xrt_root_ioctl_hwmon {
+ bool xpih_register;
+ const char *xpih_name;
+ void *xpih_drvdata;
+ const struct attribute_group **xpih_groups;
+ struct device *xpih_hwmon_dev;
+};
+
+typedef int (*xrt_subdev_root_cb_t)(struct device *, void *, u32, void *);
+int xrt_subdev_root_request(struct platform_device *self, u32 cmd, void *arg);
+
+/*
+ * Defines physical function (MPF / UPF) specific operations
+ * needed in common root driver.
+ */
+struct xroot_pf_cb {
+ void (*xpc_hot_reset)(struct pci_dev *pdev);
+};
+
+int xroot_probe(struct pci_dev *pdev, struct xroot_pf_cb *cb, void **root);
+void xroot_remove(void *root);
+bool xroot_wait_for_bringup(void *root);
+int xroot_add_vsec_node(void *root, char *dtb);
+int xroot_create_group(void *xr, char *dtb);
+int xroot_add_simple_node(void *root, char *dtb, const char *endpoint);
+void xroot_broadcast(void *root, enum xrt_events evt);
+
+#endif /* _XRT_ROOT_H_ */
diff --git a/drivers/fpga/xrt/mgmt/root.c b/drivers/fpga/xrt/mgmt/root.c
new file mode 100644
index ..583a37c9d30c
--- /dev/null
+++ b/drivers/fpga/xrt/mgmt/root.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Alveo Management Function Driver
+ *
+ * Copyright (C) 2020-2021 Xilinx, Inc.
+ *
+ * Authors:
+ * Cheng Zhen
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "xroot.h"
+#include "main-impl.h"
+#include "metadata.h"
+
+#define XMGMT_MODULE_NAME "xmgmt"
+#define XMGMT_DRIVER_VERSION "4.0.0"
+
+#define XMGMT_PDEV(xm) ((xm)->pdev)
+#define XMGMT_DEV(xm) (&(XMGMT_PDEV(xm)->dev))
+#define xmgmt_err(xm, fmt, args...)\
+ dev_err(XMGMT_DEV(xm), "%s: " fmt, __func__, ##args)
+#define xmgmt_warn(xm, fmt, args...) \
+ dev_warn(XMGMT_DEV(xm), "%s: " fmt, __func__, ##args)
+#define xmgmt_info(xm, fmt, args...) \
+ dev_info(XMGMT_DEV(xm), "%s: " fmt, __func__, ##args)
+#define xmgmt_dbg(xm, fmt, args...)\
+
