| 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: Tue, 18 Jul 2017 02:29:57 +0000
From: "Wu, Hao" <hao.wu@...el.com>
To: Alan Tull <atull@...nel.org>
CC: Moritz Fischer <mdf@...nel.org>,
"linux-fpga@...r.kernel.org" <linux-fpga@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
"Kang, Luwei" <luwei.kang@...el.com>,
"Zhang, Yi Z" <yi.z.zhang@...el.com>,
Xiao Guangrong <guangrong.xiao@...ux.intel.com>,
"Whisonant, Tim" <tim.whisonant@...el.com>,
"Luebbers, Enno" <enno.luebbers@...el.com>,
"Rao, Shiva" <shiva.rao@...el.com>,
"Rauer, Christopher" <christopher.rauer@...el.com>
Subject: RE: [PATCH v2 07/22] fpga: intel: pcie: parse feature list and
create platform device for features.
> On Sun, Jun 25, 2017 at 8:52 PM, Wu Hao <hao.wu@...el.com> wrote:
> > From: Xiao Guangrong <guangrong.xiao@...ux.intel.com>
> >
> > Device Feature List structure creates a link list of feature headers
> > within the MMIO space to provide an extensible way of adding features.
> >
> > The Intel FPGA PCIe driver walks through the feature headers to enumerate
> > feature devices, FPGA Management Engine (FME) and FPGA Port for
> Accelerated
> > Function Unit (AFU), and their private sub features. For feature devices,
> > it creates the platform devices and linked the private sub features into
> > their platform data.
> >
> > Signed-off-by: Tim Whisonant <tim.whisonant@...el.com>
> > Signed-off-by: Enno Luebbers <enno.luebbers@...el.com>
> > Signed-off-by: Shiva Rao <shiva.rao@...el.com>
> > Signed-off-by: Christopher Rauer <christopher.rauer@...el.com>
> > Signed-off-by: Kang Luwei <luwei.kang@...el.com>
> > Signed-off-by: Zhang Yi <yi.z.zhang@...el.com>
> > Signed-off-by: Xiao Guangrong <guangrong.xiao@...ux.intel.com>
> > Signed-off-by: Wu Hao <hao.wu@...el.com>
> > ---
> > v2: moved the code to drivers/fpga folder as suggested by Alan Tull.
> > switched to GPLv2 license.
> > fixed comments from Moritz Fischer.
> > fixed kbuild warning, typos and clean up the code.
> > ---
> > drivers/fpga/Makefile | 2 +-
> > drivers/fpga/intel-feature-dev.c | 130 ++++++
> > drivers/fpga/intel-feature-dev.h | 341 ++++++++++++++++
> > drivers/fpga/intel-pcie.c | 841
> ++++++++++++++++++++++++++++++++++++++-
> > 4 files changed, 1311 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/fpga/intel-feature-dev.c
> > create mode 100644 drivers/fpga/intel-feature-dev.h
> >
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 5613133..ad24b3d 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -31,4 +31,4 @@ obj-$(CONFIG_OF_FPGA_REGION) += of-fpga-
> region.o
> > # Intel FPGA Support
> > obj-$(CONFIG_INTEL_FPGA_PCI) += intel-fpga-pci.o
> >
> > -intel-fpga-pci-objs := intel-pcie.o
> > +intel-fpga-pci-objs := intel-pcie.o intel-feature-dev.o
> > diff --git a/drivers/fpga/intel-feature-dev.c b/drivers/fpga/intel-feature-dev.c
> > new file mode 100644
> > index 0000000..68f9cba
> > --- /dev/null
> > +++ b/drivers/fpga/intel-feature-dev.c
> > @@ -0,0 +1,130 @@
> > +/*
> > + * Intel FPGA Feature Device Driver
> > + *
> > + * Copyright (C) 2017 Intel Corporation, Inc.
> > + *
> > + * Authors:
> > + * Kang Luwei <luwei.kang@...el.com>
> > + * Zhang Yi <yi.z.zhang@...el.com>
> > + * Wu Hao <hao.wu@...el.com>
> > + * Xiao Guangrong <guangrong.xiao@...ux.intel.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL version 2. See
> > + * the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "intel-feature-dev.h"
> > +
> > +void feature_platform_data_add(struct feature_platform_data *pdata,
> > + int index, const char *name,
> > + int resource_index, void __iomem *ioaddr)
> > +{
> > + WARN_ON(index >= pdata->num);
> > +
> > + pdata->features[index].name = name;
> > + pdata->features[index].resource_index = resource_index;
> > + pdata->features[index].ioaddr = ioaddr;
> > +}
> > +
> > +struct feature_platform_data *
> > +feature_platform_data_alloc_and_init(struct platform_device *dev, int num)
> > +{
> > + struct feature_platform_data *pdata;
> > +
> > + pdata = kzalloc(feature_platform_data_size(num), GFP_KERNEL);
> > + if (pdata) {
> > + pdata->dev = dev;
> > + pdata->num = num;
> > + mutex_init(&pdata->lock);
> > + }
> > +
> > + return pdata;
> > +}
> > +
> > +int fme_feature_num(void)
> > +{
> > + return FME_FEATURE_ID_MAX;
> > +}
> > +
> > +int port_feature_num(void)
> > +{
> > + return PORT_FEATURE_ID_MAX;
> > +}
> > +
> > +int fpga_port_id(struct platform_device *pdev)
> > +{
> > + struct feature_port_header *port_hdr;
> > + struct feature_port_capability capability;
> > +
> > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev,
> > + PORT_FEATURE_ID_HEADER);
> > + WARN_ON(!port_hdr);
> > +
> > + capability.csr = readq(&port_hdr->capability);
> > + return capability.port_number;
> > +}
> > +EXPORT_SYMBOL_GPL(fpga_port_id);
> > +
> > +/*
> > + * Enable Port by clear the port soft reset bit, which is set by default.
> > + * The User AFU is unable to respond to any MMIO access while in reset.
> > + * __fpga_port_enable function should only be used after
> __fpga_port_disable
> > + * function.
> > + */
> > +void __fpga_port_enable(struct platform_device *pdev)
> > +{
> > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > + struct feature_port_header *port_hdr;
> > + struct feature_port_control control;
> > +
> > + WARN_ON(!pdata->disable_count);
> > +
> > + if (--pdata->disable_count != 0)
> > + return;
> > +
> > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev,
> > + PORT_FEATURE_ID_HEADER);
> > + WARN_ON(!port_hdr);
> > +
> > + control.csr = readq(&port_hdr->control);
> > + control.port_sftrst = 0x0;
> > + writeq(control.csr, &port_hdr->control);
> > +}
> > +EXPORT_SYMBOL_GPL(__fpga_port_enable);
> > +
> > +#define RST_POLL_INVL 10 /* us */
> > +#define RST_POLL_TIMEOUT 1000 /* us */
> > +
> > +int __fpga_port_disable(struct platform_device *pdev)
> > +{
> > + struct feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> > + struct feature_port_header *port_hdr;
> > + struct feature_port_control control;
> > +
> > + if (pdata->disable_count++ != 0)
> > + return 0;
> > +
> > + port_hdr = get_feature_ioaddr_by_index(&pdev->dev,
> > + PORT_FEATURE_ID_HEADER);
> > + WARN_ON(!port_hdr);
> > +
> > + /* Set port soft reset */
> > + control.csr = readq(&port_hdr->control);
> > + control.port_sftrst = 0x1;
> > + writeq(control.csr, &port_hdr->control);
> > +
> > + /*
> > + * HW sets ack bit to 1 when all outstanding requests have been drained
> > + * on this port and minimum soft reset pulse width has elapsed.
> > + * Driver polls port_soft_reset_ack to determine if reset done by HW.
> > + */
> > + if (readq_poll_timeout(&port_hdr->control, control.csr,
> > + (control.port_sftrst_ack == 1),
> > + RST_POLL_INVL, RST_POLL_TIMEOUT)) {
> > + dev_err(&pdev->dev, "timeout, fail to reset device\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(__fpga_port_disable);
> > diff --git a/drivers/fpga/intel-feature-dev.h b/drivers/fpga/intel-feature-dev.h
> > new file mode 100644
> > index 0000000..f67784a
> > --- /dev/null
> > +++ b/drivers/fpga/intel-feature-dev.h
> > @@ -0,0 +1,341 @@
> > +/*
> > + * Intel FPGA Feature Device Driver Header File
> > + *
> > + * Copyright (C) 2017 Intel Corporation, Inc.
> > + *
> > + * Authors:
> > + * Kang Luwei <luwei.kang@...el.com>
> > + * Zhang Yi <yi.z.zhang@...el.com>
> > + * Wu Hao <hao.wu@...el.com>
> > + * Xiao Guangrong <guangrong.xiao@...ux.intel.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL version 2. See
> > + * the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef __INTEL_FPGA_FEATURE_H
> > +#define __INTEL_FPGA_FEATURE_H
> > +
> > +#include <linux/fs.h>
> > +#include <linux/pci.h>
> > +#include <linux/uuid.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/platform_device.h>
> > +
> > +#ifndef readq
> > +static inline u64 readq(void __iomem *addr)
> > +{
> > + return readl(addr) + ((u64)readl(addr + 4) << 32);
> > +}
> > +#endif
> > +
> > +#ifndef writeq
> > +static inline void writeq(u64 val, void __iomem *addr)
> > +{
> > + writel((u32) (val), addr);
> > + writel((u32) (val >> 32), (addr + 4));
> > +}
> > +#endif
>
> Hi Hao,
>
> Was there a reason readq and writeq had to be created for this? Do
> these get used?
>
Hi Alan
Thanks for your review.
Driver uses writeq and readq to access HW registers.
Actually this is a problem reported by kbuild against the patch set v1[1].
Writeq/readq are missed in some arch, so I added writeq/readq definitions to
resolve this problem following the same way done by some existing drivers.
After recheck this today, I found I should use linux/io-64-nonatomic-lo-hi.h
instead of rewriting them. I will fix it in the next version.
[1] http://marc.info/?l=linux-kernel&m=149100396816882&w=2
Thanks
Hao
> Alan
Powered by blists - more mailing lists