[PATCH 0/2] hwrng/PCI/IOV: Add driver for Cavium Thunder RNG

[PATCH 0/2] hwrng/PCI/IOV: Add driver for Cavium Thunder RNG

[Omer Khaliq]

There is a hardware error rendering the FDL field incorrect for the some Thunder RNG devices.  The first patch adds infrastructure to fix the problem.

The second patch adds the driver.

David Daney (1):
  PCI/IOV: Add function to allow Function Dependency Link override.

Omer Khaliq (1):
  hwrng: thunderx: Add Cavium HWRNG driver for ThunderX SoC.

 drivers/char/hw_random/Kconfig         |  13 +++++
 drivers/char/hw_random/Makefile        |   1 +
 drivers/char/hw_random/cavium-rng-vf.c | 102 ++++++++++++++++++++++++++++++++
 drivers/char/hw_random/cavium-rng.c    | 103 +++++++++++++++++++++++++++++++++
 drivers/pci/iov.c                      |  14 +++++
 include/linux/pci.h                    |   1 +
 6 files changed, 234 insertions(+)
 create mode 100644 drivers/char/hw_random/cavium-rng-vf.c
 create mode 100644 drivers/char/hw_random/cavium-rng.c

-- 
1.9.1

[PATCH 1/2] PCI/IOV: Add function to allow Function Dependency Link override.

[Omer Khaliq]

From: David Daney <david.daney@cavium.com>

Some hardware presents an incorrect SR-IOV Function Dependency Link,
add a function to allow this to be overridden in the PF driver for
such devices.

Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
---
 drivers/pci/iov.c   | 14 ++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 2194b44..81f0672 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -640,6 +640,20 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
 EXPORT_SYMBOL_GPL(pci_enable_sriov);
 
 /**
+ * pci_sriov_fdl_override - fix incorrect Function Dependency Link
+ * @dev: the PCI device
+ * @fdl: the corrected Function Dependency Link value
+ *
+ * For hardware presenting an incorrect Function Dependency Link in
+ * the SR-IOV Extended Capability, allow a driver to override it.
+ */
+void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl)
+{
+	dev->sriov->link = fdl;
+}
+EXPORT_SYMBOL_GPL(pci_sriov_fdl_override);
+
+/**
  * pci_disable_sriov - disable the SR-IOV capability
  * @dev: the PCI device
  */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2599a98..da8a5b3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1823,6 +1823,7 @@ int pci_num_vf(struct pci_dev *dev);
 int pci_vfs_assigned(struct pci_dev *dev);
 int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
 int pci_sriov_get_totalvfs(struct pci_dev *dev);
+void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl);
 resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
 #else
 static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
-- 
1.9.1

[PATCH 2/2] hwrng: thunderx: Add Cavium HWRNG driver for ThunderX SoC.

[Omer Khaliq]

The Cavium ThunderX SoC has a hardware random number generator.
This driver provides support using the HWRNG framework.

Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
Signed-off-by: Ananth Jasty <Ananth.Jasty@cavium.com>
Acked-by: David Daney <david.daney@cavium.com>
---
 drivers/char/hw_random/Kconfig         |  13 +++++
 drivers/char/hw_random/Makefile        |   1 +
 drivers/char/hw_random/cavium-rng-vf.c | 102 ++++++++++++++++++++++++++++++++
 drivers/char/hw_random/cavium-rng.c    | 103 +++++++++++++++++++++++++++++++++
 4 files changed, 219 insertions(+)
 create mode 100644 drivers/char/hw_random/cavium-rng-vf.c
 create mode 100644 drivers/char/hw_random/cavium-rng.c

diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
index 56ad5a5..fb9c7ad 100644
--- a/drivers/char/hw_random/Kconfig
+++ b/drivers/char/hw_random/Kconfig
@@ -410,6 +410,19 @@ config HW_RANDOM_MESON
 
 	  If unsure, say Y.
 
+config HW_RANDOM_CAVIUM
+       tristate "Cavium ThunderX Random Number Generator support"
+       depends on HW_RANDOM && PCI && (ARM64 || (COMPILE_TEST && 64BIT))
+       default HW_RANDOM
+       ---help---
+         This driver provides kernel-side support for the Random Number
+         Generator hardware found on Cavium SoCs.
+
+         To compile this driver as a module, choose M here: the
+         module will be called cavium_rng.
+
+         If unsure, say Y.
+
 endif # HW_RANDOM
 
 config UML_RANDOM
diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
index 04bb0b0..5f52b1e 100644
--- a/drivers/char/hw_random/Makefile
+++ b/drivers/char/hw_random/Makefile
@@ -35,3 +35,4 @@ obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
 obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
 obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
 obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
+obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
diff --git a/drivers/char/hw_random/cavium-rng-vf.c b/drivers/char/hw_random/cavium-rng-vf.c
new file mode 100644
index 0000000..8e80bce
--- /dev/null
+++ b/drivers/char/hw_random/cavium-rng-vf.c
@@ -0,0 +1,102 @@
+/*
+ * Hardware Random Number Generator support for Cavium, Inc.
+ * Thunder processor family.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2016 Cavium, Inc.
+ */
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/pci_ids.h>
+#include <linux/gfp.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+
+struct cavium_rng {
+	struct hwrng ops;
+	void __iomem *result;
+};
+
+/* Read data from the RNG unit */
+static int cavium_rng_read(struct hwrng *rng, void *dat, size_t max, bool wait)
+{
+	struct cavium_rng *p = container_of(rng, struct cavium_rng, ops);
+	unsigned int size = max;
+
+	while (size >= 8) {
+		*((u64 *)dat) = readq(p->result);
+		size -= 8;
+		dat += 8;
+	}
+	while (size > 0) {
+		*((u8 *)dat) = readb(p->result);
+		size--;
+		dat++;
+	}
+	return max;
+}
+
+/* Map Cavium RNG to an HWRNG object */
+static int cavium_rng_probe_vf(struct	pci_dev		*pdev,
+			 const struct	pci_device_id	*id)
+{
+	struct	cavium_rng *rng;
+	int	ret;
+
+	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
+	if (!rng)
+		return -ENOMEM;
+
+	/* Map the RNG result */
+	rng->result = pcim_iomap(pdev, 0, 0);
+	if (!rng->result) {
+		dev_err(&pdev->dev, "Error iomap failed retrieving result.\n");
+		return -ENOMEM;
+	}
+
+	rng->ops.name    = "cavium rng";
+	rng->ops.read    = cavium_rng_read;
+	rng->ops.quality = 1000;
+
+	pci_set_drvdata(pdev, rng);
+
+	ret = hwrng_register(&rng->ops);
+	if (ret) {
+		dev_err(&pdev->dev, "Error registering device as HWRNG.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Remove the VF */
+void  cavium_rng_remove_vf(struct pci_dev *pdev)
+{
+	struct cavium_rng *rng;
+
+	rng = pci_get_drvdata(pdev);
+	hwrng_unregister(&rng->ops);
+}
+
+static const struct pci_device_id cavium_rng_vf_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa033), 0, 0, 0},
+	{0,},
+};
+MODULE_DEVICE_TABLE(pci, cavium_rng_vf_id_table);
+
+static struct pci_driver cavium_rng_vf_driver = {
+	.name		= "cavium_rng_vf",
+	.id_table	= cavium_rng_vf_id_table,
+	.probe		= cavium_rng_probe_vf,
+	.remove		= cavium_rng_remove_vf,
+};
+module_pci_driver(cavium_rng_vf_driver);
+
+MODULE_AUTHOR("Omer Khaliq");
+MODULE_LICENSE("GPL");
diff --git a/drivers/char/hw_random/cavium-rng.c b/drivers/char/hw_random/cavium-rng.c
new file mode 100644
index 0000000..7f09ee4
--- /dev/null
+++ b/drivers/char/hw_random/cavium-rng.c
@@ -0,0 +1,103 @@
+/*
+ * Hardware Random Number Generator support for Cavium Inc.
+ * Thunder processor family.
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License.  See the file "COPYING" in the main directory of this archive
+ * for more details.
+ *
+ * Copyright (C) 2016 Cavium, Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/hw_random.h>
+#include <linux/io.h>
+#include <linux/pci_ids.h>
+#include <linux/gfp.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/pci-ats.h>
+
+#define THUNDERX_RNM_ENT_EN     0x1
+#define THUNDERX_RNM_RNG_EN     0x2
+
+struct cavium_rng_pf {
+	void __iomem *control_status;
+};
+
+
+/* Enable the RNG hardware and activate the VF */
+static int cavium_rng_probe(struct pci_dev *pdev,
+				 const struct pci_device_id *id)
+{
+	struct	cavium_rng_pf *rng;
+	int	iov_err;
+
+
+	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
+	if (!rng)
+		return -ENOMEM;
+
+	/*Map the RNG control */
+	rng->control_status = pcim_iomap(pdev, 0, 0);
+	if (!rng->control_status) {
+		dev_err(&pdev->dev,
+			"Error iomap failed retrieving control_status.\n");
+		return -ENOMEM;
+	}
+
+	/* Enable the RNG hardware and entropy source */
+	writeq(THUNDERX_RNM_RNG_EN | THUNDERX_RNM_ENT_EN,
+		rng->control_status);
+
+	pci_set_drvdata(pdev, rng);
+
+	/* Fix for improper link id reported for cn88XX */
+	if (pdev->subsystem_device == 0xa118)
+		pci_sriov_fdl_override(pdev, pdev->devfn);
+
+	/* Enable the Cavium RNG as a VF */
+	iov_err = pci_enable_sriov(pdev, 1);
+	if (iov_err != 0) {
+		dev_err(&pdev->dev,
+			"Error initializing RNG virtual function,(%i).\n",
+			iov_err);
+		return iov_err;
+	}
+
+	return 0;
+}
+
+/* Disable VF and RNG Hardware */
+void  cavium_rng_remove(struct pci_dev *pdev)
+{
+	struct cavium_rng_pf *rng;
+
+	rng = pci_get_drvdata(pdev);
+
+	/* Remove the VF */
+	pci_disable_sriov(pdev);
+
+	/* Disable the RNG hardware and entropy source */
+	writeq(0, rng->control_status);
+}
+
+static const struct pci_device_id cavium_rng_pf_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa018), 0, 0, 0}, /* Thunder RNM */
+	{0,},
+};
+
+MODULE_DEVICE_TABLE(pci, cavium_rng_pf_id_table);
+
+static struct pci_driver cavium_rng_pf_driver = {
+	.name		= "cavium_rng_pf",
+	.id_table	= cavium_rng_pf_id_table,
+	.probe		= cavium_rng_probe,
+	.remove		= cavium_rng_remove,
+};
+
+module_pci_driver(cavium_rng_pf_driver);
+MODULE_AUTHOR("Omer Khaliq");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH 2/2] hwrng: thunderx: Add Cavium HWRNG driver for ThunderX SoC.

[Corentin LABBE]

Hello

I have some minor comments below

On 20/08/2016 00:32, Omer Khaliq wrote:
> The Cavium ThunderX SoC has a hardware random number generator.
> This driver provides support using the HWRNG framework.
> 
> Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
> Signed-off-by: Ananth Jasty <Ananth.Jasty@cavium.com>
> Acked-by: David Daney <david.daney@cavium.com>
> ---
>  drivers/char/hw_random/Kconfig         |  13 +++++
>  drivers/char/hw_random/Makefile        |   1 +
>  drivers/char/hw_random/cavium-rng-vf.c | 102 ++++++++++++++++++++++++++++++++
>  drivers/char/hw_random/cavium-rng.c    | 103 +++++++++++++++++++++++++++++++++
>  4 files changed, 219 insertions(+)
>  create mode 100644 drivers/char/hw_random/cavium-rng-vf.c
>  create mode 100644 drivers/char/hw_random/cavium-rng.c
> 
> diff --git a/drivers/char/hw_random/Kconfig b/drivers/char/hw_random/Kconfig
> index 56ad5a5..fb9c7ad 100644
> --- a/drivers/char/hw_random/Kconfig
> +++ b/drivers/char/hw_random/Kconfig
> @@ -410,6 +410,19 @@ config HW_RANDOM_MESON
>  
>  	  If unsure, say Y.
>  
> +config HW_RANDOM_CAVIUM
> +       tristate "Cavium ThunderX Random Number Generator support"
> +       depends on HW_RANDOM && PCI && (ARM64 || (COMPILE_TEST && 64BIT))
> +       default HW_RANDOM
> +       ---help---
> +         This driver provides kernel-side support for the Random Number
> +         Generator hardware found on Cavium SoCs.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called cavium_rng.
> +
> +         If unsure, say Y.
> +
>  endif # HW_RANDOM
>  
>  config UML_RANDOM
> diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> index 04bb0b0..5f52b1e 100644
> --- a/drivers/char/hw_random/Makefile
> +++ b/drivers/char/hw_random/Makefile
> @@ -35,3 +35,4 @@ obj-$(CONFIG_HW_RANDOM_XGENE) += xgene-rng.o
>  obj-$(CONFIG_HW_RANDOM_STM32) += stm32-rng.o
>  obj-$(CONFIG_HW_RANDOM_PIC32) += pic32-rng.o
>  obj-$(CONFIG_HW_RANDOM_MESON) += meson-rng.o
> +obj-$(CONFIG_HW_RANDOM_CAVIUM) += cavium-rng.o cavium-rng-vf.o
> diff --git a/drivers/char/hw_random/cavium-rng-vf.c b/drivers/char/hw_random/cavium-rng-vf.c
> new file mode 100644
> index 0000000..8e80bce
> --- /dev/null
> +++ b/drivers/char/hw_random/cavium-rng-vf.c
> @@ -0,0 +1,102 @@
> +/*
> + * Hardware Random Number Generator support for Cavium, Inc.
> + * Thunder processor family.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2016 Cavium, Inc.
> + */
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/pci_ids.h>
> +#include <linux/gfp.h>
> +#include <linux/pci.h>
> +#include <linux/init.h>

Please alphabetize headers, and check if there are necessary, clearly platform_device.h is unnecessary.

> +
> +struct cavium_rng {
> +	struct hwrng ops;
> +	void __iomem *result;
> +};
> +
> +/* Read data from the RNG unit */
> +static int cavium_rng_read(struct hwrng *rng, void *dat, size_t max, bool wait)
> +{
> +	struct cavium_rng *p = container_of(rng, struct cavium_rng, ops);
> +	unsigned int size = max;
> +
> +	while (size >= 8) {
> +		*((u64 *)dat) = readq(p->result);
> +		size -= 8;
> +		dat += 8;
> +	}
> +	while (size > 0) {
> +		*((u8 *)dat) = readb(p->result);
> +		size--;
> +		dat++;
> +	}
> +	return max;
> +}
> +
> +/* Map Cavium RNG to an HWRNG object */
> +static int cavium_rng_probe_vf(struct	pci_dev		*pdev,
> +			 const struct	pci_device_id	*id)
> +{
> +	struct	cavium_rng *rng;
> +	int	ret;
> +
> +	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
> +	if (!rng)
> +		return -ENOMEM;
> +
> +	/* Map the RNG result */
> +	rng->result = pcim_iomap(pdev, 0, 0);
> +	if (!rng->result) {
> +		dev_err(&pdev->dev, "Error iomap failed retrieving result.\n");
> +		return -ENOMEM;
> +	}
> +
> +	rng->ops.name    = "cavium rng";
> +	rng->ops.read    = cavium_rng_read;
> +	rng->ops.quality = 1000;
> +
> +	pci_set_drvdata(pdev, rng);
> +
> +	ret = hwrng_register(&rng->ops);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Error registering device as HWRNG.\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Remove the VF */
> +void  cavium_rng_remove_vf(struct pci_dev *pdev)
> +{
> +	struct cavium_rng *rng;
> +
> +	rng = pci_get_drvdata(pdev);
> +	hwrng_unregister(&rng->ops);
> +}
> +
> +static const struct pci_device_id cavium_rng_vf_id_table[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa033), 0, 0, 0},
> +	{0,},
> +};
> +MODULE_DEVICE_TABLE(pci, cavium_rng_vf_id_table);
> +
> +static struct pci_driver cavium_rng_vf_driver = {
> +	.name		= "cavium_rng_vf",
> +	.id_table	= cavium_rng_vf_id_table,
> +	.probe		= cavium_rng_probe_vf,
> +	.remove		= cavium_rng_remove_vf,
> +};
> +module_pci_driver(cavium_rng_vf_driver);
> +
> +MODULE_AUTHOR("Omer Khaliq");

You could add your email address.

> +MODULE_LICENSE("GPL");
> diff --git a/drivers/char/hw_random/cavium-rng.c b/drivers/char/hw_random/cavium-rng.c
> new file mode 100644
> index 0000000..7f09ee4
> --- /dev/null
> +++ b/drivers/char/hw_random/cavium-rng.c
> @@ -0,0 +1,103 @@
> +/*
> + * Hardware Random Number Generator support for Cavium Inc.
> + * Thunder processor family.
> + *
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2016 Cavium, Inc.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/device.h>
> +#include <linux/hw_random.h>
> +#include <linux/io.h>
> +#include <linux/pci_ids.h>
> +#include <linux/gfp.h>
> +#include <linux/pci.h>
> +#include <linux/init.h>
> +#include <linux/pci-ats.h>

Same comment for headers

> +
> +#define THUNDERX_RNM_ENT_EN     0x1
> +#define THUNDERX_RNM_RNG_EN     0x2
> +
> +struct cavium_rng_pf {
> +	void __iomem *control_status;
> +};
> +
> +

Do you have run checkpatch.pl ?
No more than one blank line.

> +/* Enable the RNG hardware and activate the VF */
> +static int cavium_rng_probe(struct pci_dev *pdev,
> +				 const struct pci_device_id *id)
> +{
> +	struct	cavium_rng_pf *rng;
> +	int	iov_err;
> +
> +

Same problem

> +	rng = devm_kzalloc(&pdev->dev, sizeof(*rng), GFP_KERNEL);
> +	if (!rng)
> +		return -ENOMEM;
> +
> +	/*Map the RNG control */
> +	rng->control_status = pcim_iomap(pdev, 0, 0);
> +	if (!rng->control_status) {
> +		dev_err(&pdev->dev,
> +			"Error iomap failed retrieving control_status.\n");
> +		return -ENOMEM;
> +	}
> +
> +	/* Enable the RNG hardware and entropy source */
> +	writeq(THUNDERX_RNM_RNG_EN | THUNDERX_RNM_ENT_EN,
> +		rng->control_status);
> +
> +	pci_set_drvdata(pdev, rng);
> +
> +	/* Fix for improper link id reported for cn88XX */
> +	if (pdev->subsystem_device == 0xa118)
> +		pci_sriov_fdl_override(pdev, pdev->devfn);
> +
> +	/* Enable the Cavium RNG as a VF */
> +	iov_err = pci_enable_sriov(pdev, 1);
> +	if (iov_err != 0) {
> +		dev_err(&pdev->dev,
> +			"Error initializing RNG virtual function,(%i).\n",
> +			iov_err);
> +		return iov_err;

You return without disabling the RNG

> +	}
> +
> +	return 0;
> +}
> +
> +/* Disable VF and RNG Hardware */
> +void  cavium_rng_remove(struct pci_dev *pdev)
> +{
> +	struct cavium_rng_pf *rng;
> +
> +	rng = pci_get_drvdata(pdev);
> +
> +	/* Remove the VF */
> +	pci_disable_sriov(pdev);
> +
> +	/* Disable the RNG hardware and entropy source */
> +	writeq(0, rng->control_status);
> +}
> +
> +static const struct pci_device_id cavium_rng_pf_id_table[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, 0xa018), 0, 0, 0}, /* Thunder RNM */
> +	{0,},
> +};
> +
> +MODULE_DEVICE_TABLE(pci, cavium_rng_pf_id_table);
> +
> +static struct pci_driver cavium_rng_pf_driver = {
> +	.name		= "cavium_rng_pf",
> +	.id_table	= cavium_rng_pf_id_table,
> +	.probe		= cavium_rng_probe,
> +	.remove		= cavium_rng_remove,
> +};
> +
> +module_pci_driver(cavium_rng_pf_driver);
> +MODULE_AUTHOR("Omer Khaliq");
> +MODULE_LICENSE("GPL");
> 

Regards

Re: [PATCH 1/2] PCI/IOV: Add function to allow Function Dependency Link override.

[Bjorn Helgaas]

Hi David & Omer,

On Fri, Aug 19, 2016 at 03:32:12PM -0700, Omer Khaliq wrote:
> From: David Daney <david.daney@cavium.com>
> 
> Some hardware presents an incorrect SR-IOV Function Dependency Link,
> add a function to allow this to be overridden in the PF driver for
> such devices.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
> ---
>  drivers/pci/iov.c   | 14 ++++++++++++++
>  include/linux/pci.h |  1 +
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 2194b44..81f0672 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -640,6 +640,20 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
>  EXPORT_SYMBOL_GPL(pci_enable_sriov);
>  
>  /**
> + * pci_sriov_fdl_override - fix incorrect Function Dependency Link
> + * @dev: the PCI device
> + * @fdl: the corrected Function Dependency Link value
> + *
> + * For hardware presenting an incorrect Function Dependency Link in
> + * the SR-IOV Extended Capability, allow a driver to override it.
> + */
> +void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl)
> +{
> +	dev->sriov->link = fdl;
> +}
> +EXPORT_SYMBOL_GPL(pci_sriov_fdl_override);

We usually use quirks to work around problems in config space.  That's
a nice mechanism because we don't have to add new PCI core interfaces
and it makes it clear that we're working around a hardware problem.

Can you use a quirk here?  We allocate dev->sriov in the
pci_init_capabilities() path, so it looks like a pci_fixup_final quirk
should work.

> +
> +/**
>   * pci_disable_sriov - disable the SR-IOV capability
>   * @dev: the PCI device
>   */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2599a98..da8a5b3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1823,6 +1823,7 @@ int pci_num_vf(struct pci_dev *dev);
>  int pci_vfs_assigned(struct pci_dev *dev);
>  int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
>  int pci_sriov_get_totalvfs(struct pci_dev *dev);
> +void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl);
>  resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
>  #else
>  static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] PCI/IOV: Add function to allow Function Dependency Link override.

[David Daney]

On 08/22/2016 07:36 AM, Bjorn Helgaas wrote:
> Hi David & Omer,
>
> On Fri, Aug 19, 2016 at 03:32:12PM -0700, Omer Khaliq wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> Some hardware presents an incorrect SR-IOV Function Dependency Link,
>> add a function to allow this to be overridden in the PF driver for
>> such devices.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
>> ---
>>   drivers/pci/iov.c   | 14 ++++++++++++++
>>   include/linux/pci.h |  1 +
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index 2194b44..81f0672 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -640,6 +640,20 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
>>   EXPORT_SYMBOL_GPL(pci_enable_sriov);
>>
>>   /**
>> + * pci_sriov_fdl_override - fix incorrect Function Dependency Link
>> + * @dev: the PCI device
>> + * @fdl: the corrected Function Dependency Link value
>> + *
>> + * For hardware presenting an incorrect Function Dependency Link in
>> + * the SR-IOV Extended Capability, allow a driver to override it.
>> + */
>> +void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl)
>> +{
>> +	dev->sriov->link = fdl;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_sriov_fdl_override);
>
> We usually use quirks to work around problems in config space.  That's
> a nice mechanism because we don't have to add new PCI core interfaces
> and it makes it clear that we're working around a hardware problem.
>
> Can you use a quirk here?  We allocate dev->sriov in the
> pci_init_capabilities() path, so it looks like a pci_fixup_final quirk
> should work.
>

The struct pci_sriov definition is private to drivers/pci, so in order 
to use a quirk to fix this, we would have to put it in 
drivers/pci/quirks.c.  I was trying to keep this very device specific 
code in the driver, which requires an accessor to be able to manipulate 
the dev->sriov->link field.

If you prefer a quirk in drivers/pci/quirks.c, we can certainly do that 
instead.

Thanks for taking the time to look at this,
David Daney




>> +
>> +/**
>>    * pci_disable_sriov - disable the SR-IOV capability
>>    * @dev: the PCI device
>>    */
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 2599a98..da8a5b3 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1823,6 +1823,7 @@ int pci_num_vf(struct pci_dev *dev);
>>   int pci_vfs_assigned(struct pci_dev *dev);
>>   int pci_sriov_set_totalvfs(struct pci_dev *dev, u16 numvfs);
>>   int pci_sriov_get_totalvfs(struct pci_dev *dev);
>> +void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl);
>>   resource_size_t pci_iov_resource_size(struct pci_dev *dev, int resno);
>>   #else
>>   static inline int pci_iov_virtfn_bus(struct pci_dev *dev, int id)
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] PCI/IOV: Add function to allow Function Dependency Link override.

[Bjorn Helgaas]

On Mon, Aug 22, 2016 at 07:49:09AM -0700, David Daney wrote:
> On 08/22/2016 07:36 AM, Bjorn Helgaas wrote:
> >Hi David & Omer,
> >
> >On Fri, Aug 19, 2016 at 03:32:12PM -0700, Omer Khaliq wrote:
> >>From: David Daney <david.daney@cavium.com>
> >>
> >>Some hardware presents an incorrect SR-IOV Function Dependency Link,
> >>add a function to allow this to be overridden in the PF driver for
> >>such devices.
> >>
> >>Signed-off-by: David Daney <david.daney@cavium.com>
> >>Signed-off-by: Omer Khaliq <okhaliq@caviumnetworks.com>
> >>---
> >>  drivers/pci/iov.c   | 14 ++++++++++++++
> >>  include/linux/pci.h |  1 +
> >>  2 files changed, 15 insertions(+)
> >>
> >>diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> >>index 2194b44..81f0672 100644
> >>--- a/drivers/pci/iov.c
> >>+++ b/drivers/pci/iov.c
> >>@@ -640,6 +640,20 @@ int pci_enable_sriov(struct pci_dev *dev, int nr_virtfn)
> >>  EXPORT_SYMBOL_GPL(pci_enable_sriov);
> >>
> >>  /**
> >>+ * pci_sriov_fdl_override - fix incorrect Function Dependency Link
> >>+ * @dev: the PCI device
> >>+ * @fdl: the corrected Function Dependency Link value
> >>+ *
> >>+ * For hardware presenting an incorrect Function Dependency Link in
> >>+ * the SR-IOV Extended Capability, allow a driver to override it.
> >>+ */
> >>+void pci_sriov_fdl_override(struct pci_dev *dev, u8 fdl)
> >>+{
> >>+	dev->sriov->link = fdl;
> >>+}
> >>+EXPORT_SYMBOL_GPL(pci_sriov_fdl_override);
> >
> >We usually use quirks to work around problems in config space.  That's
> >a nice mechanism because we don't have to add new PCI core interfaces
> >and it makes it clear that we're working around a hardware problem.
> >
> >Can you use a quirk here?  We allocate dev->sriov in the
> >pci_init_capabilities() path, so it looks like a pci_fixup_final quirk
> >should work.
> >
> 
> The struct pci_sriov definition is private to drivers/pci, so in
> order to use a quirk to fix this, we would have to put it in
> drivers/pci/quirks.c.  I was trying to keep this very device
> specific code in the driver, which requires an accessor to be able
> to manipulate the dev->sriov->link field.
> 
> If you prefer a quirk in drivers/pci/quirks.c, we can certainly do
> that instead.

Oh, I didn't notice that pci_sriov was declared in drivers/pci/pci.h
instead of linux/pci.h.  I do think I would prefer a quirk, and I
think it's fine to put it in drivers/pci/quirks.c.

Bjorn