| 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
| ||
|
Date: Mon, 1 Mar 2021 12:34:02 -0800
From: Max Zhen <max.zhen@...inx.com>
To: Moritz Fischer <mdf@...nel.org>, Lizhi Hou <lizhi.hou@...inx.com>
CC: <linux-kernel@...r.kernel.org>, Lizhi Hou <lizhih@...inx.com>,
<linux-fpga@...r.kernel.org>, <sonal.santan@...inx.com>,
<michal.simek@...inx.com>, <stefanos@...inx.com>,
<devicetree@...r.kernel.org>, <trix@...hat.com>, <robh@...nel.org>,
Max Zhen <max.zhen@...inx.com>
Subject: Re: [PATCH V3 XRT Alveo 04/18] fpga: xrt: xrt-lib platform driver
manager
Hi Moritz,
On 2/21/21 12:39 PM, Moritz Fischer 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.
>
>
> Lizhi,
>
> On Wed, Feb 17, 2021 at 10:40:05PM -0800, Lizhi Hou wrote:
>> xrt-lib kernel module infrastructure code to register and manage all
>> leaf driver modules.
>>
>> Signed-off-by: Sonal Santan <sonal.santan@...inx.com>
>> Signed-off-by: Max Zhen <max.zhen@...inx.com>
>> Signed-off-by: Lizhi Hou <lizhih@...inx.com>
>> ---
>> drivers/fpga/xrt/lib/main.c | 274 ++++++++++++++++++++++++++++++++++++
>> drivers/fpga/xrt/lib/main.h | 17 +++
>> 2 files changed, 291 insertions(+)
>> create mode 100644 drivers/fpga/xrt/lib/main.c
>> create mode 100644 drivers/fpga/xrt/lib/main.h
>>
>> diff --git a/drivers/fpga/xrt/lib/main.c b/drivers/fpga/xrt/lib/main.c
>> new file mode 100644
>> index 000000000000..36fb62710843
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/lib/main.c
>> @@ -0,0 +1,274 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for Xilinx Alveo FPGA Support
>> + *
>> + * Copyright (C) 2020-2021 Xilinx, Inc.
>> + *
>> + * Authors:
>> + * Cheng Zhen <maxz@...inx.com>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include "xleaf.h"
>> +#include "xroot.h"
>> +#include "main.h"
>> +
>> +#define XRT_IPLIB_MODULE_NAME "xrt-lib"
>> +#define XRT_IPLIB_MODULE_VERSION "4.0.0"
>> +#define XRT_MAX_DEVICE_NODES 128
>> +#define XRT_DRVNAME(drv) ((drv)->driver.name)
>> +
>> +/*
>> + * Subdev driver is known by ID to others. We map the ID to it's
>> + * struct platform_driver, which contains it's binding name and driver/file ops.
>> + * We also map it to the endpoint name in DTB as well, if it's different
>> + * than the driver's binding name.
>> + */
>> +struct xrt_drv_map {
>> + struct list_head list;
>> + enum xrt_subdev_id id;
>> + struct platform_driver *drv;
>> + struct xrt_subdev_endpoints *eps;
>> + struct ida ida; /* manage driver instance and char dev minor */
>> +};
>> +
>> +static DEFINE_MUTEX(xrt_lib_lock); /* global lock protecting xrt_drv_maps list */
>> +static LIST_HEAD(xrt_drv_maps);
>> +struct class *xrt_class;
>> +
>> +static inline struct xrt_subdev_drvdata *
>> +xrt_drv_map2drvdata(struct xrt_drv_map *map)
>> +{
>> + return (struct xrt_subdev_drvdata *)map->drv->id_table[0].driver_data;
>> +}
>> +
>> +static struct xrt_drv_map *
>> +xrt_drv_find_map_by_id_nolock(enum xrt_subdev_id id)
>> +{
>> + const struct list_head *ptr;
>> +
>> + list_for_each(ptr, &xrt_drv_maps) {
>> + struct xrt_drv_map *tmap = list_entry(ptr, struct xrt_drv_map, list);
> list_for_each_entry, maybe?
Yes!
>> +
>> + if (tmap->id == id)
>> + return tmap;
>> + }
>> + return NULL;
>> +}
>> +
>> +static struct xrt_drv_map *
>> +xrt_drv_find_map_by_id(enum xrt_subdev_id id)
>> +{
>> + struct xrt_drv_map *map;
>> +
>> + mutex_lock(&xrt_lib_lock);
>> + map = xrt_drv_find_map_by_id_nolock(id);
>> + mutex_unlock(&xrt_lib_lock);
>> + /*
>> + * map should remain valid even after lock is dropped since a registered
>> + * driver should only be unregistered when driver module is being unloaded,
>> + * which means that the driver should not be used by then.
>> + */
>> + return map;
>> +}
>> +
>> +static int xrt_drv_register_driver(struct xrt_drv_map *map)
>> +{
>> + struct xrt_subdev_drvdata *drvdata;
>> + int rc = 0;
>> + const char *drvname = XRT_DRVNAME(map->drv);
>> +
>> + rc = platform_driver_register(map->drv);
>> + if (rc) {
>> + pr_err("register %s platform driver failed\n", drvname);
>> + return rc;
>> + }
>> +
>> + drvdata = xrt_drv_map2drvdata(map);
>> + if (drvdata) {
>> + /* Initialize dev_t for char dev node. */
>> + if (xleaf_devnode_enabled(drvdata)) {
>> + rc = alloc_chrdev_region(&drvdata->xsd_file_ops.xsf_dev_t, 0,
>> + XRT_MAX_DEVICE_NODES, drvname);
>> + if (rc) {
>> + platform_driver_unregister(map->drv);
>> + pr_err("failed to alloc dev minor for %s: %d\n", drvname, rc);
>> + return rc;
>> + }
>> + } else {
>> + drvdata->xsd_file_ops.xsf_dev_t = (dev_t)-1;
>> + }
>> + }
>> +
>> + ida_init(&map->ida);
>> +
>> + pr_info("%s registered successfully\n", drvname);
> Is this not xrt_info() then?
xrt_info() is meant for leaf driver to call where struct platform_device
is available. We are initializing the drivers here, so can't call
xrt_info().
>> +
>> + return 0;
>> +}
>> +
>> +static void xrt_drv_unregister_driver(struct xrt_drv_map *map)
>> +{
>> + const char *drvname = XRT_DRVNAME(map->drv);
>> + struct xrt_subdev_drvdata *drvdata;
>> +
>> + ida_destroy(&map->ida);
>> +
>> + drvdata = xrt_drv_map2drvdata(map);
>> + if (drvdata && drvdata->xsd_file_ops.xsf_dev_t != (dev_t)-1) {
>> + unregister_chrdev_region(drvdata->xsd_file_ops.xsf_dev_t,
>> + XRT_MAX_DEVICE_NODES);
>> + }
>> +
>> + platform_driver_unregister(map->drv);
>> +
>> + pr_info("%s unregistered successfully\n", drvname);
>> +}
>> +
>> +int xleaf_register_driver(enum xrt_subdev_id id,
>> + struct platform_driver *drv,
>> + struct xrt_subdev_endpoints *eps)
>> +{
>> + struct xrt_drv_map *map;
>> +
>> + mutex_lock(&xrt_lib_lock);
>> +
>> + map = xrt_drv_find_map_by_id_nolock(id);
>> + if (map) {
>> + mutex_unlock(&xrt_lib_lock);
>> + pr_err("Id %d already has a registered driver, 0x%p\n",
>> + id, map->drv);
>> + return -EEXIST;
>> + }
>> +
>> + map = vzalloc(sizeof(*map));
>> + if (!map) {
>> + mutex_unlock(&xrt_lib_lock);
>> + return -ENOMEM;
>> + }
>> + map->id = id;
>> + map->drv = drv;
>> + map->eps = eps;
>> +
>> + xrt_drv_register_driver(map);
>> +
>> + list_add(&map->list, &xrt_drv_maps);
>> +
>> + mutex_unlock(&xrt_lib_lock);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xleaf_register_driver);
>> +
>> +void xleaf_unregister_driver(enum xrt_subdev_id id)
>> +{
>> + struct xrt_drv_map *map;
>> +
>> + mutex_lock(&xrt_lib_lock);
>> +
>> + map = xrt_drv_find_map_by_id_nolock(id);
>> + if (!map) {
>> + mutex_unlock(&xrt_lib_lock);
>> + pr_err("Id %d has no registered driver\n", id);
>> + return;
>> + }
>> +
>> + list_del(&map->list);
>> +
>> + mutex_unlock(&xrt_lib_lock);
>> +
>> + xrt_drv_unregister_driver(map);
>> + vfree(map);
>> +}
>> +EXPORT_SYMBOL_GPL(xleaf_unregister_driver);
>> +
>> +const char *xrt_drv_name(enum xrt_subdev_id id)
>> +{
>> + struct xrt_drv_map *map = xrt_drv_find_map_by_id(id);
>> +
>> + if (map)
>> + return XRT_DRVNAME(map->drv);
>> + return NULL;
>> +}
>> +
>> +int xrt_drv_get_instance(enum xrt_subdev_id id)
>> +{
>> + struct xrt_drv_map *map = xrt_drv_find_map_by_id(id);
>> +
>> + return ida_alloc_range(&map->ida, 0, XRT_MAX_DEVICE_NODES, GFP_KERNEL);
>> +}
>> +
>> +void xrt_drv_put_instance(enum xrt_subdev_id id, int instance)
>> +{
>> + struct xrt_drv_map *map = xrt_drv_find_map_by_id(id);
>> +
>> + ida_free(&map->ida, instance);
>> +}
>> +
>> +struct xrt_subdev_endpoints *xrt_drv_get_endpoints(enum xrt_subdev_id id)
>> +{
>> + struct xrt_drv_map *map = xrt_drv_find_map_by_id(id);
>> + struct xrt_subdev_endpoints *eps;
>> +
>> + eps = map ? map->eps : NULL;
>> + return eps;
>> +}
>> +
>> +/* Leaf driver's module init/fini callbacks. */
>> +static void (*leaf_init_fini_cbs[])(bool) = {
>> + group_leaf_init_fini,
>> + vsec_leaf_init_fini,
>> + devctl_leaf_init_fini,
>> + axigate_leaf_init_fini,
>> + icap_leaf_init_fini,
>> + calib_leaf_init_fini,
>> + clkfreq_leaf_init_fini,
>> + clock_leaf_init_fini,
>> + ucs_leaf_init_fini,
>> +};
>> +
>> +static __init int xrt_lib_init(void)
>> +{
>> + int i;
>> +
>> + xrt_class = class_create(THIS_MODULE, XRT_IPLIB_MODULE_NAME);
>> + if (IS_ERR(xrt_class))
>> + return PTR_ERR(xrt_class);
>> +
>> + for (i = 0; i < ARRAY_SIZE(leaf_init_fini_cbs); i++)
>> + leaf_init_fini_cbs[i](true);
>> + return 0;
>> +}
>> +
>> +static __exit void xrt_lib_fini(void)
>> +{
>> + struct xrt_drv_map *map;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(leaf_init_fini_cbs); i++)
>> + leaf_init_fini_cbs[i](false);
>> +
>> + mutex_lock(&xrt_lib_lock);
>> +
>> + while (!list_empty(&xrt_drv_maps)) {
>> + map = list_first_entry_or_null(&xrt_drv_maps, struct xrt_drv_map, list);
>> + pr_err("Unloading module with %s still registered\n", XRT_DRVNAME(map->drv));
>> + list_del(&map->list);
>> + mutex_unlock(&xrt_lib_lock);
>> + xrt_drv_unregister_driver(map);
>> + vfree(map);
>> + mutex_lock(&xrt_lib_lock);
>> + }
>> +
>> + mutex_unlock(&xrt_lib_lock);
>> +
>> + class_destroy(xrt_class);
>> +}
>> +
>> +module_init(xrt_lib_init);
>> +module_exit(xrt_lib_fini);
>> +
>> +MODULE_VERSION(XRT_IPLIB_MODULE_VERSION);
>> +MODULE_AUTHOR("XRT Team <runtime@...inx.com>");
>> +MODULE_DESCRIPTION("Xilinx Alveo IP Lib driver");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/fpga/xrt/lib/main.h b/drivers/fpga/xrt/lib/main.h
>> new file mode 100644
>> index 000000000000..f3bfc87ee614
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/lib/main.h
>> @@ -0,0 +1,17 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) 2020-2021 Xilinx, Inc.
>> + *
>> + * Authors:
>> + * Cheng Zhen <maxz@...inx.com>
>> + */
>> +
>> +#ifndef _XRT_MAIN_H_
>> +#define _XRT_MAIN_H_
>> +
>> +const char *xrt_drv_name(enum xrt_subdev_id id);
>> +int xrt_drv_get_instance(enum xrt_subdev_id id);
>> +void xrt_drv_put_instance(enum xrt_subdev_id id, int instance);
>> +struct xrt_subdev_endpoints *xrt_drv_get_endpoints(enum xrt_subdev_id id);
>> +
>> +#endif /* _XRT_MAIN_H_ */
>> --
>> 2.18.4
>>
> This smells like it should be a bus to me...
This is discussed in another email thread. I will not add more comment here.
Thanks,
Max
>
> - Moritz
Powered by blists - more mailing lists