In situations where the userspace driver is stopped abnormally and the
VFIO platform device is released, the assigned HW device currently is
left running. As a consequence the HW device might continue issuing IRQs
and performing DMA accesses.
On release, no physical IRQ handler is setup anymore. Also the DMA buffers
are unmapped leading to IOMMU aborts. So there is no serious consequence.
However when assigning that HW device again to another userspace driver,
this latter might face some unexpected IRQs and DMA accesses, which are
the result of the previous assignment.
In virtualization use-case, a VM newly granted with that HW device may be
impacted by the assignment of that device to a previous VM:
- IRQs may be injected very early when booting the new guest, even before
the guest driver has initialized leading to possible driver state
inconsistency.
- DMA accesses may hit the newly mapped VM address space at addresses that
may jeopardize the integrity of the newly installed VM.
Obviously the criticity depends on the assigned HW devcie.
As opposed to PCI, there is no standard mechanism to reset the platform
device.
This series adds VFIO platform reset support by specializing the vfio
platform driver for a given device, adding its HW reset modality. Generally
that code can be reused from the native driver. It basically deals with
IRQ disabling and DMA transfert stop.
It is still possible to use the generic VFIO platform driver, without
the reset modality. Here we introduce a new driver for Calxeda xgmac,
illustrating the mechanism. That code was tested on Calxeda Midway where
smmu aborts do not seem to be observed anymore.
Obviously the drawback of this approach is possible multiplication of
VFIO platform drivers. Other solution I envisionned was to put that vfio
reset function in the native driver and implement some enumeration
mechanism in the driver core but this looks very weird at the end, with the
a device somehow bound to 2 different platform drivers.
Any feedback welcome!
Best Regards
Eric
The series can be found at
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/4.O_forward_and_reset
Also some user guidelines are available at
https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway
Eric Auger (3):
VFIO: platform: add reset support
VFIO: platform: export platform callbacks, probe and remove
VFIO: platform: add vfio-platform-calxedaxgmac driver
drivers/vfio/platform/Kconfig | 2 +
drivers/vfio/platform/Makefile | 2 +
drivers/vfio/platform/reset/Kconfig | 7 ++
drivers/vfio/platform/reset/Makefile | 5 +
.../platform/reset/vfio_platform_calxedaxgmac.c | 109 +++++++++++++++++++++
drivers/vfio/platform/vfio_platform.c | 14 ++-
drivers/vfio/platform/vfio_platform_common.c | 12 ++-
drivers/vfio/platform/vfio_platform_private.h | 8 ++
8 files changed, 152 insertions(+), 7 deletions(-)
create mode 100644 drivers/vfio/platform/reset/Kconfig
create mode 100644 drivers/vfio/platform/reset/Makefile
create mode 100644 drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
--
1.9.1
This patch adds support for vfio platform reset. The modality only is
exposed when a specialized VFIO driver populates the reset callback.
For the generic vfio platform/amba driver, the modality is not supported
since the driver is by essence a metadriver and does not know how to
reset the underlying device.
Signed-off-by: Eric Auger <[email protected]>
---
drivers/vfio/platform/vfio_platform_common.c | 12 ++++++++++--
drivers/vfio/platform/vfio_platform_private.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 4113d46..b989ce5 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -100,6 +100,8 @@ static void vfio_platform_release(void *device_data)
mutex_lock(&driver_lock);
if (!(--vdev->refcnt)) {
+ if (vdev->reset)
+ vdev->reset(vdev);
vfio_platform_regions_cleanup(vdev);
vfio_platform_irq_cleanup(vdev);
}
@@ -159,6 +161,8 @@ static long vfio_platform_ioctl(void *device_data,
if (info.argsz < minsz)
return -EINVAL;
+ if (vdev->reset)
+ vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
info.flags = vdev->flags;
info.num_regions = vdev->num_regions;
info.num_irqs = vdev->num_irqs;
@@ -252,8 +256,12 @@ static long vfio_platform_ioctl(void *device_data,
return ret;
- } else if (cmd == VFIO_DEVICE_RESET)
- return -EINVAL;
+ } else if (cmd == VFIO_DEVICE_RESET) {
+ if (vdev->reset)
+ return vdev->reset(vdev);
+ else
+ return -ENOTTY;
+ }
return -ENOTTY;
}
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 253caa3..0a20028 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -68,6 +68,7 @@ struct vfio_platform_device {
struct resource*
(*get_resource)(struct vfio_platform_device *vdev, int i);
int (*get_irq)(struct vfio_platform_device *vdev, int i);
+ int (*reset)(struct vfio_platform_device *vdev);
};
extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
--
1.9.1
We intend to derive the platform driver into specialized ones, featuring
reset modality. In order to avoid duplication, let's export the functions
that can be reused in all vfio_platform drivers:
- get_platform_resource
- get_platform_irq
- vfio_platform_probe
- vfio_platform_remove
Practically, only the vfio_platform_probe method should need to be
overridden.
Signed-off-by: Eric Auger <[email protected]>
---
drivers/vfio/platform/vfio_platform.c | 14 +++++++++-----
drivers/vfio/platform/vfio_platform_private.h | 7 +++++++
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index cef645c..81adb03 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -25,8 +25,8 @@
/* probing devices from the linux platform bus */
-static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
- int num)
+struct resource *get_platform_resource(struct vfio_platform_device *vdev,
+ int num)
{
struct platform_device *dev = (struct platform_device *) vdev->opaque;
int i;
@@ -43,15 +43,17 @@ static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
}
return NULL;
}
+EXPORT_SYMBOL_GPL(get_platform_resource);
-static int get_platform_irq(struct vfio_platform_device *vdev, int i)
+int get_platform_irq(struct vfio_platform_device *vdev, int i)
{
struct platform_device *pdev = (struct platform_device *) vdev->opaque;
return platform_get_irq(pdev, i);
}
+EXPORT_SYMBOL_GPL(get_platform_irq);
-static int vfio_platform_probe(struct platform_device *pdev)
+int vfio_platform_probe(struct platform_device *pdev)
{
struct vfio_platform_device *vdev;
int ret;
@@ -72,8 +74,9 @@ static int vfio_platform_probe(struct platform_device *pdev)
return ret;
}
+EXPORT_SYMBOL_GPL(vfio_platform_probe);
-static int vfio_platform_remove(struct platform_device *pdev)
+int vfio_platform_remove(struct platform_device *pdev)
{
struct vfio_platform_device *vdev;
@@ -85,6 +88,7 @@ static int vfio_platform_remove(struct platform_device *pdev)
return -EINVAL;
}
+EXPORT_SYMBOL_GPL(vfio_platform_remove);
static struct platform_driver vfio_platform_driver = {
.probe = vfio_platform_probe,
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index 0a20028..e2b4f0b 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -95,4 +95,11 @@ extern int vfio_platform_external_set_automasked(void *device_data,
unsigned count,
bool automasked);
+struct platform_device;
+struct resource *get_platform_resource(struct vfio_platform_device *vdev,
+ int num);
+int get_platform_irq(struct vfio_platform_device *vdev, int i);
+int vfio_platform_probe(struct platform_device *pdev);
+int vfio_platform_remove(struct platform_device *pdev);
+
#endif /* VFIO_PLATFORM_PRIVATE_H */
--
1.9.1
This patch introduces a specialized vfio platform driver for the
calxeda xgmac. On top of the generic vfio platform driver functionalities,
it implements the reset modality. This latter basically disables interrupts
and stops DMA transfers.
Code is inherited from calxeda xgmac native driver
Signed-off-by: Eric Auger <[email protected]>
---
drivers/vfio/platform/Kconfig | 2 +
drivers/vfio/platform/Makefile | 2 +
drivers/vfio/platform/reset/Kconfig | 7 ++
drivers/vfio/platform/reset/Makefile | 5 +
.../platform/reset/vfio_platform_calxedaxgmac.c | 109 +++++++++++++++++++++
5 files changed, 125 insertions(+)
create mode 100644 drivers/vfio/platform/reset/Kconfig
create mode 100644 drivers/vfio/platform/reset/Makefile
create mode 100644 drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
index 9a4403e..1df7477 100644
--- a/drivers/vfio/platform/Kconfig
+++ b/drivers/vfio/platform/Kconfig
@@ -18,3 +18,5 @@ config VFIO_AMBA
framework.
If you don't know what to do here, say N.
+
+source "drivers/vfio/platform/reset/Kconfig"
diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 81de144..9ce8afe 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -2,7 +2,9 @@
vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
+obj-$(CONFIG_VFIO_PLATFORM) += reset/
vfio-amba-y := vfio_amba.o
obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o
+obj-$(CONFIG_VFIO_AMBA) += reset/
diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
new file mode 100644
index 0000000..2c09cea
--- /dev/null
+++ b/drivers/vfio/platform/reset/Kconfig
@@ -0,0 +1,7 @@
+config VFIO_PLATFORM_CALXEDAXGMAC
+ tristate "VFIO support for calxeda xgmac"
+ depends on VFIO_PLATFORM
+ help
+ Support for VFIO platform driver specialized for Calxeda xgmac reset.
+
+ 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
new file mode 100644
index 0000000..8977721
--- /dev/null
+++ b/drivers/vfio/platform/reset/Makefile
@@ -0,0 +1,5 @@
+vfio-platform-calxedaxgmac-y := vfio_platform_calxedaxgmac.o
+
+ccflags-y += -Idrivers/vfio/platform
+
+obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC) += vfio-platform-calxedaxgmac.o
diff --git a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
new file mode 100644
index 0000000..729d0cd
--- /dev/null
+++ b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
@@ -0,0 +1,109 @@
+/*
+ * VFIO platform driver specialized for Calxeda xgmac reset
+ * reset code is inherited from calxeda xgmac native driver
+ *
+ * Copyright 2010-2011 Calxeda, Inc.
+ * Copyright (c) 2015 Linaro Ltd.
+ * http://www.linaro.org
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/>.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/vfio.h>
+#include "vfio_platform_private.h"
+#include <linux/io.h>
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "Eric Auger <[email protected]>"
+#define DRIVER_DESC "VFIO - Calxeda xgmac vfio platform driver"
+
+/* XGMAC Register definitions */
+#define XGMAC_CONTROL 0x00000000 /* MAC Configuration */
+
+/* DMA Control and Status Registers */
+#define XGMAC_DMA_CONTROL 0x00000f18 /* Ctrl (Operational Mode) */
+#define XGMAC_DMA_INTR_ENA 0x00000f1c /* Interrupt Enable */
+
+/* DMA Control registe defines */
+#define DMA_CONTROL_ST 0x00002000 /* Start/Stop Transmission */
+#define DMA_CONTROL_SR 0x00000002 /* Start/Stop Receive */
+
+/* Common MAC defines */
+#define MAC_ENABLE_TX 0x00000008 /* Transmitter Enable */
+#define MAC_ENABLE_RX 0x00000004 /* Receiver Enable */
+
+static inline void xgmac_mac_disable(void __iomem *ioaddr)
+{
+ u32 value = readl(ioaddr + XGMAC_DMA_CONTROL);
+
+ value &= ~(DMA_CONTROL_ST | DMA_CONTROL_SR);
+ writel(value, ioaddr + XGMAC_DMA_CONTROL);
+
+ value = readl(ioaddr + XGMAC_CONTROL);
+ value &= ~(MAC_ENABLE_TX | MAC_ENABLE_RX);
+ writel(value, ioaddr + XGMAC_CONTROL);
+}
+
+static int vfio_platform_calxedaxgmac_reset(struct vfio_platform_device *vdev)
+{
+ struct resource *res = get_platform_resource(vdev, 0);
+ void __iomem *base = phys_to_virt(res->start);
+
+ if (!base)
+ return -ENOMEM;
+
+ /* disable IRQ */
+ writel(0, base + XGMAC_DMA_INTR_ENA);
+
+ /* Disable the MAC core */
+ xgmac_mac_disable(base);
+
+ return 0;
+}
+
+static int vfio_platform_calxedaxgmac_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct vfio_device *vfio_dev;
+ struct vfio_platform_device *vpdev;
+
+ ret = vfio_platform_probe(pdev);
+ if (ret)
+ return ret;
+
+ vfio_dev = vfio_device_get_from_dev(&pdev->dev),
+ vpdev = (struct vfio_platform_device *) vfio_device_data(vfio_dev);
+ vpdev->reset = vfio_platform_calxedaxgmac_reset;
+
+ return ret;
+}
+
+static struct platform_driver vfio_platform_calxedaxgmac_driver = {
+ .driver = {
+ .name = "vfio-platform-calxedaxgmac",
+ },
+ .probe = vfio_platform_calxedaxgmac_probe,
+ .remove = vfio_platform_remove,
+};
+
+module_platform_driver(vfio_platform_calxedaxgmac_driver);
+
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
--
1.9.1
On Fri, 2015-04-17 at 15:37 +0200, Eric Auger wrote:
> This patch adds support for vfio platform reset. The modality only is
> exposed when a specialized VFIO driver populates the reset callback.
> For the generic vfio platform/amba driver, the modality is not supported
> since the driver is by essence a metadriver and does not know how to
> reset the underlying device.
>
> Signed-off-by: Eric Auger <[email protected]>
> ---
> drivers/vfio/platform/vfio_platform_common.c | 12 ++++++++++--
> drivers/vfio/platform/vfio_platform_private.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index 4113d46..b989ce5 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -100,6 +100,8 @@ static void vfio_platform_release(void *device_data)
> mutex_lock(&driver_lock);
>
> if (!(--vdev->refcnt)) {
> + if (vdev->reset)
> + vdev->reset(vdev);
> vfio_platform_regions_cleanup(vdev);
> vfio_platform_irq_cleanup(vdev);
> }
> @@ -159,6 +161,8 @@ static long vfio_platform_ioctl(void *device_data,
> if (info.argsz < minsz)
> return -EINVAL;
>
> + if (vdev->reset)
> + vdev->flags |= VFIO_DEVICE_FLAGS_RESET;
> info.flags = vdev->flags;
> info.num_regions = vdev->num_regions;
> info.num_irqs = vdev->num_irqs;
> @@ -252,8 +256,12 @@ static long vfio_platform_ioctl(void *device_data,
>
> return ret;
>
> - } else if (cmd == VFIO_DEVICE_RESET)
> - return -EINVAL;
> + } else if (cmd == VFIO_DEVICE_RESET) {
> + if (vdev->reset)
> + return vdev->reset(vdev);
> + else
> + return -ENOTTY;
nit, this should probably still return -EINVAL. The ioctl is there, but
it's unsupported for this device as the user should have learned from
the above flags. It's therefore not lack of an ioctl, but invalid use
of an ioctl.
> + }
>
> return -ENOTTY;
> }
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 253caa3..0a20028 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -68,6 +68,7 @@ struct vfio_platform_device {
> struct resource*
> (*get_resource)(struct vfio_platform_device *vdev, int i);
> int (*get_irq)(struct vfio_platform_device *vdev, int i);
> + int (*reset)(struct vfio_platform_device *vdev);
> };
>
> extern int vfio_platform_probe_common(struct vfio_platform_device *vdev,
On Fri, 2015-04-17 at 15:37 +0200, Eric Auger wrote:
> We intend to derive the platform driver into specialized ones, featuring
> reset modality. In order to avoid duplication, let's export the functions
> that can be reused in all vfio_platform drivers:
> - get_platform_resource
> - get_platform_irq
> - vfio_platform_probe
> - vfio_platform_remove
>
> Practically, only the vfio_platform_probe method should need to be
> overridden.
>
> Signed-off-by: Eric Auger <[email protected]>
> ---
> drivers/vfio/platform/vfio_platform.c | 14 +++++++++-----
> drivers/vfio/platform/vfio_platform_private.h | 7 +++++++
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> index cef645c..81adb03 100644
> --- a/drivers/vfio/platform/vfio_platform.c
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -25,8 +25,8 @@
>
> /* probing devices from the linux platform bus */
>
> -static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> - int num)
> +struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> + int num)
> {
> struct platform_device *dev = (struct platform_device *) vdev->opaque;
> int i;
> @@ -43,15 +43,17 @@ static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> }
> return NULL;
> }
> +EXPORT_SYMBOL_GPL(get_platform_resource);
Consider the global namespace if you're going to export these. They
should be localized, preferably using the vfio_platform_ prefix.
>
> -static int get_platform_irq(struct vfio_platform_device *vdev, int i)
> +int get_platform_irq(struct vfio_platform_device *vdev, int i)
> {
> struct platform_device *pdev = (struct platform_device *) vdev->opaque;
>
> return platform_get_irq(pdev, i);
> }
> +EXPORT_SYMBOL_GPL(get_platform_irq);
>
> -static int vfio_platform_probe(struct platform_device *pdev)
> +int vfio_platform_probe(struct platform_device *pdev)
> {
> struct vfio_platform_device *vdev;
> int ret;
> @@ -72,8 +74,9 @@ static int vfio_platform_probe(struct platform_device *pdev)
>
> return ret;
> }
> +EXPORT_SYMBOL_GPL(vfio_platform_probe);
>
> -static int vfio_platform_remove(struct platform_device *pdev)
> +int vfio_platform_remove(struct platform_device *pdev)
> {
> struct vfio_platform_device *vdev;
>
> @@ -85,6 +88,7 @@ static int vfio_platform_remove(struct platform_device *pdev)
>
> return -EINVAL;
> }
> +EXPORT_SYMBOL_GPL(vfio_platform_remove);
>
> static struct platform_driver vfio_platform_driver = {
> .probe = vfio_platform_probe,
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index 0a20028..e2b4f0b 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -95,4 +95,11 @@ extern int vfio_platform_external_set_automasked(void *device_data,
> unsigned count,
> bool automasked);
>
> +struct platform_device;
> +struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> + int num);
> +int get_platform_irq(struct vfio_platform_device *vdev, int i);
> +int vfio_platform_probe(struct platform_device *pdev);
> +int vfio_platform_remove(struct platform_device *pdev);
> +
> #endif /* VFIO_PLATFORM_PRIVATE_H */
On Fri, 2015-04-17 at 15:37 +0200, Eric Auger wrote:
> This patch introduces a specialized vfio platform driver for the
> calxeda xgmac. On top of the generic vfio platform driver functionalities,
> it implements the reset modality. This latter basically disables interrupts
> and stops DMA transfers.
>
> Code is inherited from calxeda xgmac native driver
>
> Signed-off-by: Eric Auger <[email protected]>
> ---
> drivers/vfio/platform/Kconfig | 2 +
> drivers/vfio/platform/Makefile | 2 +
> drivers/vfio/platform/reset/Kconfig | 7 ++
> drivers/vfio/platform/reset/Makefile | 5 +
> .../platform/reset/vfio_platform_calxedaxgmac.c | 109 +++++++++++++++++++++
> 5 files changed, 125 insertions(+)
> create mode 100644 drivers/vfio/platform/reset/Kconfig
> create mode 100644 drivers/vfio/platform/reset/Makefile
> create mode 100644 drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
>
> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
> index 9a4403e..1df7477 100644
> --- a/drivers/vfio/platform/Kconfig
> +++ b/drivers/vfio/platform/Kconfig
> @@ -18,3 +18,5 @@ config VFIO_AMBA
> framework.
>
> If you don't know what to do here, say N.
> +
> +source "drivers/vfio/platform/reset/Kconfig"
> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
> index 81de144..9ce8afe 100644
> --- a/drivers/vfio/platform/Makefile
> +++ b/drivers/vfio/platform/Makefile
> @@ -2,7 +2,9 @@
> vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
>
> obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
> +obj-$(CONFIG_VFIO_PLATFORM) += reset/
>
> vfio-amba-y := vfio_amba.o
>
> obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o
> +obj-$(CONFIG_VFIO_AMBA) += reset/
> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
> new file mode 100644
> index 0000000..2c09cea
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/Kconfig
> @@ -0,0 +1,7 @@
> +config VFIO_PLATFORM_CALXEDAXGMAC
> + tristate "VFIO support for calxeda xgmac"
> + depends on VFIO_PLATFORM
> + help
> + Support for VFIO platform driver specialized for Calxeda xgmac reset.
> +
> + 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
> new file mode 100644
> index 0000000..8977721
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/Makefile
> @@ -0,0 +1,5 @@
> +vfio-platform-calxedaxgmac-y := vfio_platform_calxedaxgmac.o
> +
> +ccflags-y += -Idrivers/vfio/platform
> +
> +obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC) += vfio-platform-calxedaxgmac.o
> diff --git a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
> new file mode 100644
> index 0000000..729d0cd
> --- /dev/null
> +++ b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
> @@ -0,0 +1,109 @@
> +/*
> + * VFIO platform driver specialized for Calxeda xgmac reset
> + * reset code is inherited from calxeda xgmac native driver
> + *
> + * Copyright 2010-2011 Calxeda, Inc.
> + * Copyright (c) 2015 Linaro Ltd.
> + * http://www.linaro.org
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/>.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/vfio.h>
> +#include "vfio_platform_private.h"
> +#include <linux/io.h>
> +
> +#define DRIVER_VERSION "0.1"
> +#define DRIVER_AUTHOR "Eric Auger <[email protected]>"
> +#define DRIVER_DESC "VFIO - Calxeda xgmac vfio platform driver"
> +
> +/* XGMAC Register definitions */
> +#define XGMAC_CONTROL 0x00000000 /* MAC Configuration */
> +
> +/* DMA Control and Status Registers */
> +#define XGMAC_DMA_CONTROL 0x00000f18 /* Ctrl (Operational Mode) */
> +#define XGMAC_DMA_INTR_ENA 0x00000f1c /* Interrupt Enable */
> +
> +/* DMA Control registe defines */
> +#define DMA_CONTROL_ST 0x00002000 /* Start/Stop Transmission */
> +#define DMA_CONTROL_SR 0x00000002 /* Start/Stop Receive */
> +
> +/* Common MAC defines */
> +#define MAC_ENABLE_TX 0x00000008 /* Transmitter Enable */
> +#define MAC_ENABLE_RX 0x00000004 /* Receiver Enable */
> +
> +static inline void xgmac_mac_disable(void __iomem *ioaddr)
> +{
> + u32 value = readl(ioaddr + XGMAC_DMA_CONTROL);
> +
> + value &= ~(DMA_CONTROL_ST | DMA_CONTROL_SR);
> + writel(value, ioaddr + XGMAC_DMA_CONTROL);
> +
> + value = readl(ioaddr + XGMAC_CONTROL);
> + value &= ~(MAC_ENABLE_TX | MAC_ENABLE_RX);
> + writel(value, ioaddr + XGMAC_CONTROL);
> +}
> +
> +static int vfio_platform_calxedaxgmac_reset(struct vfio_platform_device *vdev)
> +{
> + struct resource *res = get_platform_resource(vdev, 0);
> + void __iomem *base = phys_to_virt(res->start);
> +
> + if (!base)
> + return -ENOMEM;
> +
> + /* disable IRQ */
> + writel(0, base + XGMAC_DMA_INTR_ENA);
> +
> + /* Disable the MAC core */
> + xgmac_mac_disable(base);
> +
> + return 0;
> +}
> +
> +static int vfio_platform_calxedaxgmac_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct vfio_device *vfio_dev;
> + struct vfio_platform_device *vpdev;
> +
> + ret = vfio_platform_probe(pdev);
> + if (ret)
> + return ret;
> +
> + vfio_dev = vfio_device_get_from_dev(&pdev->dev),
> + vpdev = (struct vfio_platform_device *) vfio_device_data(vfio_dev);
> + vpdev->reset = vfio_platform_calxedaxgmac_reset;
> +
> + return ret;
> +}
> +
> +static struct platform_driver vfio_platform_calxedaxgmac_driver = {
> + .driver = {
> + .name = "vfio-platform-calxedaxgmac",
> + },
> + .probe = vfio_platform_calxedaxgmac_probe,
> + .remove = vfio_platform_remove,
> +};
> +
> +module_platform_driver(vfio_platform_calxedaxgmac_driver);
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
I don't really understand why this needs to be a new driver that wraps
around the vfio platform driver rather than just an entry in a table of
available device specific reset functions, setup through the normal
probe path. Do we really have no clue what the device is to be able to
attach a reset function to it from the base vfio platform code? This
also seems like a pain for users, who now need to figure out which
vfio_platform driver to bind to a given device. If the user can figure
that out, why can't the kernel just pick the right reset callback for
them? Thanks,
Alex
Hi Alex,
On 04/17/2015 04:29 PM, Alex Williamson wrote:
> On Fri, 2015-04-17 at 15:37 +0200, Eric Auger wrote:
>> This patch introduces a specialized vfio platform driver for the
>> calxeda xgmac. On top of the generic vfio platform driver functionalities,
>> it implements the reset modality. This latter basically disables interrupts
>> and stops DMA transfers.
>>
>> Code is inherited from calxeda xgmac native driver
>>
>> Signed-off-by: Eric Auger <[email protected]>
>> ---
>> drivers/vfio/platform/Kconfig | 2 +
>> drivers/vfio/platform/Makefile | 2 +
>> drivers/vfio/platform/reset/Kconfig | 7 ++
>> drivers/vfio/platform/reset/Makefile | 5 +
>> .../platform/reset/vfio_platform_calxedaxgmac.c | 109 +++++++++++++++++++++
>> 5 files changed, 125 insertions(+)
>> create mode 100644 drivers/vfio/platform/reset/Kconfig
>> create mode 100644 drivers/vfio/platform/reset/Makefile
>> create mode 100644 drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
>>
>> diff --git a/drivers/vfio/platform/Kconfig b/drivers/vfio/platform/Kconfig
>> index 9a4403e..1df7477 100644
>> --- a/drivers/vfio/platform/Kconfig
>> +++ b/drivers/vfio/platform/Kconfig
>> @@ -18,3 +18,5 @@ config VFIO_AMBA
>> framework.
>>
>> If you don't know what to do here, say N.
>> +
>> +source "drivers/vfio/platform/reset/Kconfig"
>> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
>> index 81de144..9ce8afe 100644
>> --- a/drivers/vfio/platform/Makefile
>> +++ b/drivers/vfio/platform/Makefile
>> @@ -2,7 +2,9 @@
>> vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
>>
>> obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
>> +obj-$(CONFIG_VFIO_PLATFORM) += reset/
>>
>> vfio-amba-y := vfio_amba.o
>>
>> obj-$(CONFIG_VFIO_AMBA) += vfio-amba.o
>> +obj-$(CONFIG_VFIO_AMBA) += reset/
>> diff --git a/drivers/vfio/platform/reset/Kconfig b/drivers/vfio/platform/reset/Kconfig
>> new file mode 100644
>> index 0000000..2c09cea
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/Kconfig
>> @@ -0,0 +1,7 @@
>> +config VFIO_PLATFORM_CALXEDAXGMAC
>> + tristate "VFIO support for calxeda xgmac"
>> + depends on VFIO_PLATFORM
>> + help
>> + Support for VFIO platform driver specialized for Calxeda xgmac reset.
>> +
>> + 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
>> new file mode 100644
>> index 0000000..8977721
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/Makefile
>> @@ -0,0 +1,5 @@
>> +vfio-platform-calxedaxgmac-y := vfio_platform_calxedaxgmac.o
>> +
>> +ccflags-y += -Idrivers/vfio/platform
>> +
>> +obj-$(CONFIG_VFIO_PLATFORM_CALXEDAXGMAC) += vfio-platform-calxedaxgmac.o
>> diff --git a/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
>> new file mode 100644
>> index 0000000..729d0cd
>> --- /dev/null
>> +++ b/drivers/vfio/platform/reset/vfio_platform_calxedaxgmac.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * VFIO platform driver specialized for Calxeda xgmac reset
>> + * reset code is inherited from calxeda xgmac native driver
>> + *
>> + * Copyright 2010-2011 Calxeda, Inc.
>> + * Copyright (c) 2015 Linaro Ltd.
>> + * http://www.linaro.org
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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/>.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/vfio.h>
>> +#include "vfio_platform_private.h"
>> +#include <linux/io.h>
>> +
>> +#define DRIVER_VERSION "0.1"
>> +#define DRIVER_AUTHOR "Eric Auger <[email protected]>"
>> +#define DRIVER_DESC "VFIO - Calxeda xgmac vfio platform driver"
>> +
>> +/* XGMAC Register definitions */
>> +#define XGMAC_CONTROL 0x00000000 /* MAC Configuration */
>> +
>> +/* DMA Control and Status Registers */
>> +#define XGMAC_DMA_CONTROL 0x00000f18 /* Ctrl (Operational Mode) */
>> +#define XGMAC_DMA_INTR_ENA 0x00000f1c /* Interrupt Enable */
>> +
>> +/* DMA Control registe defines */
>> +#define DMA_CONTROL_ST 0x00002000 /* Start/Stop Transmission */
>> +#define DMA_CONTROL_SR 0x00000002 /* Start/Stop Receive */
>> +
>> +/* Common MAC defines */
>> +#define MAC_ENABLE_TX 0x00000008 /* Transmitter Enable */
>> +#define MAC_ENABLE_RX 0x00000004 /* Receiver Enable */
>> +
>> +static inline void xgmac_mac_disable(void __iomem *ioaddr)
>> +{
>> + u32 value = readl(ioaddr + XGMAC_DMA_CONTROL);
>> +
>> + value &= ~(DMA_CONTROL_ST | DMA_CONTROL_SR);
>> + writel(value, ioaddr + XGMAC_DMA_CONTROL);
>> +
>> + value = readl(ioaddr + XGMAC_CONTROL);
>> + value &= ~(MAC_ENABLE_TX | MAC_ENABLE_RX);
>> + writel(value, ioaddr + XGMAC_CONTROL);
>> +}
>> +
>> +static int vfio_platform_calxedaxgmac_reset(struct vfio_platform_device *vdev)
>> +{
>> + struct resource *res = get_platform_resource(vdev, 0);
>> + void __iomem *base = phys_to_virt(res->start);
>> +
>> + if (!base)
>> + return -ENOMEM;
>> +
>> + /* disable IRQ */
>> + writel(0, base + XGMAC_DMA_INTR_ENA);
>> +
>> + /* Disable the MAC core */
>> + xgmac_mac_disable(base);
>> +
>> + return 0;
>> +}
>> +
>> +static int vfio_platform_calxedaxgmac_probe(struct platform_device *pdev)
>> +{
>> + int ret;
>> + struct vfio_device *vfio_dev;
>> + struct vfio_platform_device *vpdev;
>> +
>> + ret = vfio_platform_probe(pdev);
>> + if (ret)
>> + return ret;
>> +
>> + vfio_dev = vfio_device_get_from_dev(&pdev->dev),
>> + vpdev = (struct vfio_platform_device *) vfio_device_data(vfio_dev);
>> + vpdev->reset = vfio_platform_calxedaxgmac_reset;
>> +
>> + return ret;
>> +}
>> +
>> +static struct platform_driver vfio_platform_calxedaxgmac_driver = {
>> + .driver = {
>> + .name = "vfio-platform-calxedaxgmac",
>> + },
>> + .probe = vfio_platform_calxedaxgmac_probe,
>> + .remove = vfio_platform_remove,
>> +};
>> +
>> +module_platform_driver(vfio_platform_calxedaxgmac_driver);
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>
> I don't really understand why this needs to be a new driver that wraps
> around the vfio platform driver rather than just an entry in a table of
> available device specific reset functions, setup through the normal
> probe path. Do we really have no clue what the device is to be able to
> attach a reset function to it from the base vfio platform code? This
> also seems like a pain for users, who now need to figure out which
> vfio_platform driver to bind to a given device. If the user can figure
> that out, why can't the kernel just pick the right reset callback for
> them? Thanks,
Yes I can do the proposed way. I can get access to the compat string of
the device and according to that info I will choose some proper reset
function. I will respin shortly taking into account the other comments.
Thanks!
Eric
>
> Alex
>