Discussion:
[PATCH v2 0/2] FlexRM support in VFIO platform
Anup Patel
2017-07-20 04:32:32 UTC
Permalink
This patchset primarily adds Broadcom FlexRM reset module for
VFIO platform driver. We also have minor improvments in IOMMU
and VFIO driver to allow VFIO no-IOMMU mode access to FlexRM.

The patches are based on Linux-4.13-rc1 and can also be
found at flexrm-vfio-v2 branch of
https://github.com/Broadcom/arm64-linux.git

Changes since v1:
- Remove iommu_present() check in vfio_iommu_group_get()
- Drop PATCH1-to-PATCH3 because IOMMU_CAP_BYPASS is not
required
- Move additional comments out of license header in
vfio_platform_bcmflexrm.c

Anup Patel (2):
vfio: Allow No-IOMMU mode without checking iommu_present()
vfio: platform: reset: Add Broadcom FlexRM reset module

drivers/vfio/platform/reset/Kconfig | 9 +++
drivers/vfio/platform/reset/Makefile | 1 +
.../vfio/platform/reset/vfio_platform_bcmflexrm.c | 93 ++++++++++++++++++++++
drivers/vfio/vfio.c | 7 +-
4 files changed, 106 insertions(+), 4 deletions(-)
create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
--
2.7.4
Anup Patel
2017-07-20 04:32:33 UTC
Permalink
Not allowing No-IOMMU mode for devices already having
iommu_ops on their bus is very conservative.

We now have IOMMU (such as ARM SMMU) which can bypass
transcations when IOMMU is not configured for a given
device. In addition, it is not necessary to have all
devices on bus to be upstream to an IOMMU on that bus.

Due above reasons, having iommu_present() check for
VFIO No-IOMMU mode is not appropriate.

Signed-off-by: Anup Patel <***@broadcom.com>
---
drivers/vfio/vfio.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 330d505..9d90de7 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -124,11 +124,10 @@ struct iommu_group *vfio_iommu_group_get(struct device *dev)
#ifdef CONFIG_VFIO_NOIOMMU
/*
* With noiommu enabled, an IOMMU group will be created for a device
- * that doesn't already have one and doesn't have an iommu_ops on their
- * bus. We set iommudata simply to be able to identify these groups
- * as special use and for reclamation later.
+ * that doesn't already have one. We set iommudata simply to be able
+ * to identify these groups as special use and for reclamation later.
*/
- if (group || !noiommu || iommu_present(dev->bus))
+ if (group || !noiommu)
return group;

group = iommu_group_alloc();
--
2.7.4
Will Deacon
2017-07-20 09:19:10 UTC
Permalink
Post by Anup Patel
Not allowing No-IOMMU mode for devices already having
iommu_ops on their bus is very conservative.
We now have IOMMU (such as ARM SMMU) which can bypass
transcations when IOMMU is not configured for a given
device. In addition, it is not necessary to have all
devices on bus to be upstream to an IOMMU on that bus.
How does the SMMU know to bypass in these cases? As I explained before, the
driver-specific command line option is the wrong way to go about arranging
this.

Will
Alex Williamson
2017-07-24 17:24:06 UTC
Permalink
On Thu, 20 Jul 2017 10:02:33 +0530
Post by Anup Patel
Not allowing No-IOMMU mode for devices already having
iommu_ops on their bus is very conservative.
We now have IOMMU (such as ARM SMMU) which can bypass
transcations when IOMMU is not configured for a given
device. In addition, it is not necessary to have all
devices on bus to be upstream to an IOMMU on that bus.
Due above reasons, having iommu_present() check for
VFIO No-IOMMU mode is not appropriate.
---
drivers/vfio/vfio.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 330d505..9d90de7 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -124,11 +124,10 @@ struct iommu_group *vfio_iommu_group_get(struct device *dev)
#ifdef CONFIG_VFIO_NOIOMMU
/*
* With noiommu enabled, an IOMMU group will be created for a device
- * that doesn't already have one and doesn't have an iommu_ops on their
- * bus. We set iommudata simply to be able to identify these groups
- * as special use and for reclamation later.
+ * that doesn't already have one. We set iommudata simply to be able
+ * to identify these groups as special use and for reclamation later.
*/
- if (group || !noiommu || iommu_present(dev->bus))
+ if (group || !noiommu)
return group;
group = iommu_group_alloc();
Nak, no-iommu is intentionally conservative. How do we tell the
difference between the iommu_ops available to us with a fake group vs
a proper group created by the iommu if we ignore iommu_present()? If
an iommu exists, use it. Thanks,

Alex

Anup Patel
2017-07-20 04:32:34 UTC
Permalink
This patch adds Broadcom FlexRM low-level reset for
VFIO platform.

It will do the following:
1. Disable/Deactivate each FlexRM ring
2. Flush each FlexRM ring

The cleanup sequence for FlexRM rings is adapted from
Broadcom FlexRM mailbox driver.

Signed-off-by: Anup Patel <***@broadcom.com>
Reviewed-by: Oza Oza <***@broadcom.com>
Reviewed-by: Scott Branden <***@broadcom.com>
---
drivers/vfio/platform/reset/Kconfig | 9 +++
drivers/vfio/platform/reset/Makefile | 1 +
.../vfio/platform/reset/vfio_platform_bcmflexrm.c | 93 ++++++++++++++++++++++
3 files changed, 103 insertions(+)
create mode 100644 drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c

diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
index 70cccc5..8d8d226 100644
--- a/drivers/vfio/platform/reset/Kconfig
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -13,3 +13,12 @@ config VFIO_PLATFORM_AMDXGBE_RESET
Enables the VFIO platform driver to handle reset for AMD XGBE

If you don't know what to do here, say N.
+
+config VFIO_PLATFORM_BCMFLEXRM_RESET
+ tristate "VFIO support for Broadcom FlexRM reset"
+ depends on VFIO_PLATFORM
+ depends on ARCH_BCM_IPROC || COMPILE_TEST
+ help
+ Enables the VFIO platform driver to handle reset for Broadcom FlexRM
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/platform/reset/Makefile b/drivers/vfio/platform/reset/Makefile
index 93f4e23..8d9874b 100644
--- a/drivers/vfio/platform/reset/Makefile
+++ b/drivers/vfio/platform/reset/Makefile
@@ -5,3 +5,4 @@ ccflags-y += -Idrivers/vfio/platform

obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC_RESET) += vfio-platform-calxedaxgmac.o
obj-$(CONFIG_VFIO_PLATFORM_AMDXGBE_RESET) += vfio-platform-amdxgbe.o
+obj-$(CONFIG_VFIO_PLATFORM_BCMFLEXRM_RESET) += vfio_platform_bcmflexrm.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
new file mode 100644
index 0000000..a198196
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_bcmflexrm.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * This driver provides reset support for Broadcom FlexRM ring manager
+ * to VFIO platform.
+ */
+
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "vfio_platform_private.h"
+
+/* FlexRM configuration */
+#define RING_REGS_SIZE 0x10000
+#define RING_VER_MAGIC 0x76303031
+
+/* Per-Ring register offsets */
+#define RING_VER 0x000
+#define RING_CONTROL 0x034
+#define RING_FLUSH_DONE 0x038
+
+/* Register RING_CONTROL fields */
+#define CONTROL_FLUSH_SHIFT 5
+#define CONTROL_ACTIVE_SHIFT 4
+
+/* Register RING_FLUSH_DONE fields */
+#define FLUSH_DONE_MASK 0x1
+
+static void vfio_platform_bcmflexrm_shutdown(void __iomem *ring)
+{
+ unsigned int timeout;
+
+ /* Disable/inactivate ring */
+ writel_relaxed(0x0, ring + RING_CONTROL);
+
+ /* Flush ring with timeout of 1s */
+ timeout = 1000;
+ writel_relaxed(BIT(CONTROL_FLUSH_SHIFT), ring + RING_CONTROL);
+ do {
+ if (readl_relaxed(ring + RING_FLUSH_DONE) & FLUSH_DONE_MASK)
+ break;
+ mdelay(1);
+ } while (timeout--);
+
+ if (!timeout)
+ pr_warn("VFIO FlexRM shutdown timedout\n");
+}
+
+static int vfio_platform_bcmflexrm_reset(struct vfio_platform_device *vdev)
+{
+ void __iomem *ring;
+ struct vfio_platform_region *reg = &vdev->regions[0];
+
+ /* Map FlexRM ring registers if not mapped */
+ if (!reg->ioaddr) {
+ reg->ioaddr = ioremap_nocache(reg->addr, reg->size);
+ if (!reg->ioaddr)
+ return -ENOMEM;
+ }
+
+ /* Discover and shutdown each FlexRM ring */
+ for (ring = reg->ioaddr;
+ ring < (reg->ioaddr + reg->size); ring += RING_REGS_SIZE) {
+ if (readl_relaxed(ring + RING_VER) == RING_VER_MAGIC)
+ vfio_platform_bcmflexrm_shutdown(ring);
+ }
+
+ return 0;
+}
+
+module_vfio_reset_handler("brcm,iproc-flexrm-mbox",
+ vfio_platform_bcmflexrm_reset);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anup Patel <***@broadcom.com>");
+MODULE_DESCRIPTION("Reset support for Broadcom FlexRM VFIO platform device");
--
2.7.4
Loading...