| 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: Fri, 12 Mar 2021 16:45:33 -0800
From: Lizhi Hou <lizhi.hou@...inx.com>
To: Tom Rix <trix@...hat.com>, Lizhi Hou <lizhi.hou@...inx.com>,
<linux-kernel@...r.kernel.org>
CC: <linux-fpga@...r.kernel.org>, <maxz@...inx.com>,
<sonal.santan@...inx.com>, <michal.simek@...inx.com>,
<stefanos@...inx.com>, <devicetree@...r.kernel.org>,
<mdf@...nel.org>, <robh@...nel.org>, Max Zhen <max.zhen@...inx.com>
Subject: Re: [PATCH V3 XRT Alveo 16/18] fpga: xrt: DDR calibration platform
driver
Hi Tom,
On 03/06/2021 07:34 AM, Tom Rix wrote:
> On 2/17/21 10:40 PM, Lizhi Hou wrote:
>> Add DDR calibration driver. DDR calibration is a hardware function
>> discovered by walking firmware metadata. A platform device node will
>> be created for it. Hardware provides DDR calibration status through
>> this function.
>>
>> 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/include/xleaf/calib.h | 30 ++++
>> drivers/fpga/xrt/lib/xleaf/calib.c | 226 +++++++++++++++++++++++++
>> 2 files changed, 256 insertions(+)
>> create mode 100644 drivers/fpga/xrt/include/xleaf/calib.h
>> create mode 100644 drivers/fpga/xrt/lib/xleaf/calib.c
> calib is not descriptive, change filename to ddr_calibration
Sure.
>> diff --git a/drivers/fpga/xrt/include/xleaf/calib.h b/drivers/fpga/xrt/include/xleaf/calib.h
>> new file mode 100644
>> index 000000000000..f8aba4594c58
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/include/xleaf/calib.h
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Header file for XRT DDR Calibration Leaf Driver
>> + *
>> + * Copyright (C) 2020-2021 Xilinx, Inc.
>> + *
>> + * Authors:
>> + * Cheng Zhen <maxz@...inx.com>
>> + */
>> +
>> +#ifndef _XRT_CALIB_H_
>> +#define _XRT_CALIB_H_
>> +
>> +#include "xleaf.h"
>> +#include <linux/xrt/xclbin.h>
>> +
>> +/*
>> + * Memory calibration driver IOCTL calls.
>> + */
>> +enum xrt_calib_results {
>> + XRT_CALIB_UNKNOWN,
> Initialize ?
Will fix.
>> + XRT_CALIB_SUCCEEDED,
>> + XRT_CALIB_FAILED,
>> +};
>> +
>> +enum xrt_calib_ioctl_cmd {
>> + XRT_CALIB_RESULT = XRT_XLEAF_CUSTOM_BASE, /* See comments in xleaf.h */
>> +};
>> +
>> +#endif /* _XRT_CALIB_H_ */
>> diff --git a/drivers/fpga/xrt/lib/xleaf/calib.c b/drivers/fpga/xrt/lib/xleaf/calib.c
>> new file mode 100644
>> index 000000000000..fbb813636e76
>> --- /dev/null
>> +++ b/drivers/fpga/xrt/lib/xleaf/calib.c
>> @@ -0,0 +1,226 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Xilinx Alveo FPGA memory calibration driver
>> + *
>> + * Copyright (C) 2020-2021 Xilinx, Inc.
>> + *
>> + * memory calibration
>> + *
>> + * Authors:
>> + * Lizhi Hou<Lizhi.Hou@...inx.com>
>> + */
>> +#include <linux/delay.h>
>> +#include "xclbin-helper.h"
>> +#include "metadata.h"
>> +#include "xleaf/calib.h"
>> +
>> +#define XRT_CALIB "xrt_calib"
>> +
>> +struct calib_cache {
>> + struct list_head link;
>> + const char *ep_name;
>> + char *data;
>> + u32 data_size;
>> +};
>> +
>> +struct calib {
>> + struct platform_device *pdev;
>> + void *calib_base;
>> + struct mutex lock; /* calibration dev lock */
>> + struct list_head cache_list;
>> + u32 cache_num;
>> + enum xrt_calib_results result;
>> +};
>> +
>> +#define CALIB_DONE(calib) \
>> + (ioread32((calib)->calib_base) & BIT(0))
>> +
>> +static void calib_cache_clean_nolock(struct calib *calib)
>> +{
>> + struct calib_cache *cache, *temp;
>> +
>> + list_for_each_entry_safe(cache, temp, &calib->cache_list, link) {
>> + vfree(cache->data);
>> + list_del(&cache->link);
>> + vfree(cache);
>> + }
>> + calib->cache_num = 0;
>> +}
>> +
>> +static void calib_cache_clean(struct calib *calib)
>> +{
>> + mutex_lock(&calib->lock);
>> + calib_cache_clean_nolock(calib);
> No lock functions (i believe) should be prefixed with '__'
Will change.
>> + mutex_unlock(&calib->lock);
>> +}
>> +
>> +static int calib_srsr(struct calib *calib, struct platform_device *srsr_leaf)
> what is srsr ?
>
> Why a noop function ?
srsr is save-restore and self-refresh. It will not be supported in this
patch set. I will remove this function.
>
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static int calib_calibration(struct calib *calib)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 20; i++) {
> 20 is a config parameter so should have a #define
>
> There a couple of busy wait blocks in xrt/ some count up, some count down.
>
> It would be good if they were consistent.
Will change these.
>
>> + if (CALIB_DONE(calib))
>> + break;
>> + msleep(500);
> 500 is another config
Will define.
Thanks,
Lizhi
>
> Tom
>
>> + }
>> +
>> + if (i == 20) {
>> + xrt_err(calib->pdev,
>> + "MIG calibration timeout after bitstream download");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + xrt_info(calib->pdev, "took %dms", i * 500);
>> + return 0;
>> +}
>> +
>> +static void xrt_calib_event_cb(struct platform_device *pdev, void *arg)
>> +{
>> + struct calib *calib = platform_get_drvdata(pdev);
>> + struct xrt_event *evt = (struct xrt_event *)arg;
>> + enum xrt_events e = evt->xe_evt;
>> + enum xrt_subdev_id id = evt->xe_subdev.xevt_subdev_id;
>> + int instance = evt->xe_subdev.xevt_subdev_instance;
>> + struct platform_device *leaf;
>> + int ret;
>> +
>> + switch (e) {
>> + case XRT_EVENT_POST_CREATION: {
>> + if (id == XRT_SUBDEV_SRSR) {
>> + leaf = xleaf_get_leaf_by_id(pdev,
>> + XRT_SUBDEV_SRSR,
>> + instance);
>> + if (!leaf) {
>> + xrt_err(pdev, "does not get SRSR subdev");
>> + return;
>> + }
>> + ret = calib_srsr(calib, leaf);
>> + xleaf_put_leaf(pdev, leaf);
>> + calib->result =
>> + ret ? XRT_CALIB_FAILED : XRT_CALIB_SUCCEEDED;
>> + } else if (id == XRT_SUBDEV_UCS) {
>> + ret = calib_calibration(calib);
>> + calib->result =
>> + ret ? XRT_CALIB_FAILED : XRT_CALIB_SUCCEEDED;
>> + }
>> + break;
>> + }
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static int xrt_calib_remove(struct platform_device *pdev)
>> +{
>> + struct calib *calib = platform_get_drvdata(pdev);
>> +
>> + calib_cache_clean(calib);
>> +
>> + if (calib->calib_base)
>> + iounmap(calib->calib_base);
>> +
>> + platform_set_drvdata(pdev, NULL);
>> + devm_kfree(&pdev->dev, calib);
>> +
>> + return 0;
>> +}
>> +
>> +static int xrt_calib_probe(struct platform_device *pdev)
>> +{
>> + struct calib *calib;
>> + struct resource *res;
>> + int err = 0;
>> +
>> + calib = devm_kzalloc(&pdev->dev, sizeof(*calib), GFP_KERNEL);
>> + if (!calib)
>> + return -ENOMEM;
>> +
>> + calib->pdev = pdev;
>> + platform_set_drvdata(pdev, calib);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + goto failed;
>> +
>> + calib->calib_base = ioremap(res->start, res->end - res->start + 1);
>> + if (!calib->calib_base) {
>> + err = -EIO;
>> + xrt_err(pdev, "Map iomem failed");
>> + goto failed;
>> + }
>> +
>> + mutex_init(&calib->lock);
>> + INIT_LIST_HEAD(&calib->cache_list);
>> +
>> + return 0;
>> +
>> +failed:
>> + xrt_calib_remove(pdev);
>> + return err;
>> +}
>> +
>> +static int
>> +xrt_calib_leaf_ioctl(struct platform_device *pdev, u32 cmd, void *arg)
>> +{
>> + struct calib *calib = platform_get_drvdata(pdev);
>> + int ret = 0;
>> +
>> + switch (cmd) {
>> + case XRT_XLEAF_EVENT:
>> + xrt_calib_event_cb(pdev, arg);
>> + break;
>> + case XRT_CALIB_RESULT: {
>> + enum xrt_calib_results *r = (enum xrt_calib_results *)arg;
>> + *r = calib->result;
>> + break;
>> + }
>> + default:
>> + xrt_err(pdev, "unsupported cmd %d", cmd);
>> + ret = -EINVAL;
>> + }
>> + return ret;
>> +}
>> +
>> +static struct xrt_subdev_endpoints xrt_calib_endpoints[] = {
>> + {
>> + .xse_names = (struct xrt_subdev_ep_names[]) {
>> + { .ep_name = XRT_MD_NODE_DDR_CALIB },
>> + { NULL },
>> + },
>> + .xse_min_ep = 1,
>> + },
>> + { 0 },
>> +};
>> +
>> +static struct xrt_subdev_drvdata xrt_calib_data = {
>> + .xsd_dev_ops = {
>> + .xsd_ioctl = xrt_calib_leaf_ioctl,
>> + },
>> +};
>> +
>> +static const struct platform_device_id xrt_calib_table[] = {
>> + { XRT_CALIB, (kernel_ulong_t)&xrt_calib_data },
>> + { },
>> +};
>> +
>> +static struct platform_driver xrt_calib_driver = {
>> + .driver = {
>> + .name = XRT_CALIB,
>> + },
>> + .probe = xrt_calib_probe,
>> + .remove = xrt_calib_remove,
>> + .id_table = xrt_calib_table,
>> +};
>> +
>> +void calib_leaf_init_fini(bool init)
>> +{
>> + if (init)
>> + xleaf_register_driver(XRT_SUBDEV_CALIB, &xrt_calib_driver, xrt_calib_endpoints);
>> + else
>> + xleaf_unregister_driver(XRT_SUBDEV_CALIB);
>> +}
Powered by blists - more mailing lists