2020-07-06 15:45:05

by Diana Madalina Craciun

[permalink] [raw]
Subject: [PATCH v3 0/9] vfio/fsl-mc: VFIO support for FSL-MC devices

DPAA2 (Data Path Acceleration Architecture) consists in
mechanisms for processing Ethernet packets, queue management,
accelerators, etc.

The Management Complex (mc) is a hardware entity that manages the DPAA2
hardware resources. It provides an object-based abstraction for software
drivers to use the DPAA2 hardware. The MC mediates operations such as
create, discover, destroy of DPAA2 objects.
The MC provides memory-mapped I/O command interfaces (MC portals) which
DPAA2 software drivers use to operate on DPAA2 objects.

A DPRC is a container object that holds other types of DPAA2 objects.
Each object in the DPRC is a Linux device and bound to a driver.
The MC-bus driver is a platform driver (different from PCI or platform
bus). The DPRC driver does runtime management of a bus instance. It
performs the initial scan of the DPRC and handles changes in the DPRC
configuration (adding/removing objects).

All objects inside a container share the same hardware isolation
context, meaning that only an entire DPRC can be assigned to
a virtual machine.
When a container is assigned to a virtual machine, all the objects
within that container are assigned to that virtual machine.
The DPRC container assigned to the virtual machine is not allowed
to change contents (add/remove objects) by the guest. The restriction
is set by the host and enforced by the mc hardware.

The DPAA2 objects can be directly assigned to the guest. However
the MC portals (the memory mapped command interface to the MC) need
to be emulated because there are commands that configure the
interrupts and the isolation IDs which are virtual in the guest.

Example:
echo vfio-fsl-mc > /sys/bus/fsl-mc/devices/dprc.2/driver_override
echo dprc.2 > /sys/bus/fsl-mc/drivers/vfio-fsl-mc/bind

The dprc.2 is bound to the VFIO driver and all the objects within
dprc.2 are going to be bound to the VFIO driver.

More details about the DPAA2 objects can be found here:
Documentation/networking/device_drivers/freescale/dpaa2/overview.rst

The patches are dependent on some changes in the mc-bus (bus/fsl-mc)
driver. The changes were needed in order to re-use code and to export
some more functions that are needed by the VFIO driver.
Currenlty the mc-bus patches are under review:
https://www.spinics.net/lists/kernel/msg3578910.html

v2 --> v3
- There is no need to align region size to page size
- read/write implemented for all DPAA2 objects
- review fixes

v1 --> v2
- Fixed the container reset, a new flag added to the firmware command
- Implement a bus notifier for setting driver_override

Bharat Bhushan (1):
vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices

Diana Craciun (8):
vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl
vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call
vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions
vfio/fsl-mc: Added lock support in preparation for interrupt handling
vfio/fsl-mc: Add irq infrastructure for fsl-mc devices
vfio/fsl-mc: trigger an interrupt via eventfd
vfio/fsl-mc: Add read/write support for fsl-mc devices

MAINTAINERS | 6 +
drivers/vfio/Kconfig | 1 +
drivers/vfio/Makefile | 1 +
drivers/vfio/fsl-mc/Kconfig | 9 +
drivers/vfio/fsl-mc/Makefile | 4 +
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 687 ++++++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 221 +++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 55 ++
include/uapi/linux/vfio.h | 1 +
9 files changed, 985 insertions(+)
create mode 100644 drivers/vfio/fsl-mc/Kconfig
create mode 100644 drivers/vfio/fsl-mc/Makefile
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc.c
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h

--
2.17.1


2020-07-06 15:45:11

by Diana Madalina Craciun

[permalink] [raw]
Subject: [PATCH v3 1/9] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices

From: Bharat Bhushan <[email protected]>

DPAA2 (Data Path Acceleration Architecture) consists in
mechanisms for processing Ethernet packets, queue management,
accelerators, etc.

The Management Complex (mc) is a hardware entity that manages the DPAA2
hardware resources. It provides an object-based abstraction for software
drivers to use the DPAA2 hardware. The MC mediates operations such as
create, discover, destroy of DPAA2 objects.
The MC provides memory-mapped I/O command interfaces (MC portals) which
DPAA2 software drivers use to operate on DPAA2 objects.

A DPRC is a container object that holds other types of DPAA2 objects.
Each object in the DPRC is a Linux device and bound to a driver.
The MC-bus driver is a platform driver (different from PCI or platform
bus). The DPRC driver does runtime management of a bus instance. It
performs the initial scan of the DPRC and handles changes in the DPRC
configuration (adding/removing objects).

All objects inside a container share the same hardware isolation
context, meaning that only an entire DPRC can be assigned to
a virtual machine.
When a container is assigned to a virtual machine, all the objects
within that container are assigned to that virtual machine.
The DPRC container assigned to the virtual machine is not allowed
to change contents (add/remove objects) by the guest. The restriction
is set by the host and enforced by the mc hardware.

The DPAA2 objects can be directly assigned to the guest. However
the MC portals (the memory mapped command interface to the MC) need
to be emulated because there are commands that configure the
interrupts and the isolation IDs which are virtual in the guest.

Example:
echo vfio-fsl-mc > /sys/bus/fsl-mc/devices/dprc.2/driver_override
echo dprc.2 > /sys/bus/fsl-mc/drivers/vfio-fsl-mc/bind

The dprc.2 is bound to the VFIO driver and all the objects within
dprc.2 are going to be bound to the VFIO driver.

This patch adds the infrastructure for VFIO support for fsl-mc
devices. Subsequent patches will add support for binding and secure
assigning these devices using VFIO.

More details about the DPAA2 objects can be found here:
Documentation/networking/device_drivers/freescale/dpaa2/overview.rst

Signed-off-by: Bharat Bhushan <[email protected]>
Signed-off-by: Diana Craciun <[email protected]>
---
MAINTAINERS | 6 +
drivers/vfio/Kconfig | 1 +
drivers/vfio/Makefile | 1 +
drivers/vfio/fsl-mc/Kconfig | 9 ++
drivers/vfio/fsl-mc/Makefile | 4 +
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 160 ++++++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 14 ++
include/uapi/linux/vfio.h | 1 +
8 files changed, 196 insertions(+)
create mode 100644 drivers/vfio/fsl-mc/Kconfig
create mode 100644 drivers/vfio/fsl-mc/Makefile
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc.c
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 1d4aa7f942de..18af3e530f8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18015,6 +18015,12 @@ F: drivers/vfio/
F: include/linux/vfio.h
F: include/uapi/linux/vfio.h

+VFIO FSL-MC DRIVER
+M: Diana Craciun <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/vfio/fsl-mc/
+
VFIO MEDIATED DEVICE DRIVERS
M: Kirti Wankhede <[email protected]>
L: [email protected]
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index fd17db9b432f..5533df91b257 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -47,4 +47,5 @@ menuconfig VFIO_NOIOMMU
source "drivers/vfio/pci/Kconfig"
source "drivers/vfio/platform/Kconfig"
source "drivers/vfio/mdev/Kconfig"
+source "drivers/vfio/fsl-mc/Kconfig"
source "virt/lib/Kconfig"
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index de67c4725cce..fee73f3d9480 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VFIO_SPAPR_EEH) += vfio_spapr_eeh.o
obj-$(CONFIG_VFIO_PCI) += pci/
obj-$(CONFIG_VFIO_PLATFORM) += platform/
obj-$(CONFIG_VFIO_MDEV) += mdev/
+obj-$(CONFIG_VFIO_FSL_MC) += fsl-mc/
diff --git a/drivers/vfio/fsl-mc/Kconfig b/drivers/vfio/fsl-mc/Kconfig
new file mode 100644
index 000000000000..b1a527d6b6f2
--- /dev/null
+++ b/drivers/vfio/fsl-mc/Kconfig
@@ -0,0 +1,9 @@
+config VFIO_FSL_MC
+ tristate "VFIO support for QorIQ DPAA2 fsl-mc bus devices"
+ depends on VFIO && FSL_MC_BUS && EVENTFD
+ help
+ Driver to enable support for the VFIO QorIQ DPAA2 fsl-mc
+ (Management Complex) devices. This is required to passthrough
+ fsl-mc bus devices using the VFIO framework.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/fsl-mc/Makefile b/drivers/vfio/fsl-mc/Makefile
new file mode 100644
index 000000000000..0c6e5d2ddaae
--- /dev/null
+++ b/drivers/vfio/fsl-mc/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+
+vfio-fsl-mc-y := vfio_fsl_mc.o
+obj-$(CONFIG_VFIO_FSL_MC) += vfio-fsl-mc.o
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
new file mode 100644
index 000000000000..8b53c2a25b32
--- /dev/null
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/*
+ * Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2016-2017,2019-2020 NXP
+ */
+
+#include <linux/device.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/vfio.h>
+#include <linux/fsl/mc.h>
+
+#include "vfio_fsl_mc_private.h"
+
+static int vfio_fsl_mc_open(void *device_data)
+{
+ if (!try_module_get(THIS_MODULE))
+ return -ENODEV;
+
+ return 0;
+}
+
+static void vfio_fsl_mc_release(void *device_data)
+{
+ module_put(THIS_MODULE);
+}
+
+static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
+ unsigned long arg)
+{
+ switch (cmd) {
+ case VFIO_DEVICE_GET_INFO:
+ {
+ return -ENOTTY;
+ }
+ case VFIO_DEVICE_GET_REGION_INFO:
+ {
+ return -ENOTTY;
+ }
+ case VFIO_DEVICE_GET_IRQ_INFO:
+ {
+ return -ENOTTY;
+ }
+ case VFIO_DEVICE_SET_IRQS:
+ {
+ return -ENOTTY;
+ }
+ case VFIO_DEVICE_RESET:
+ {
+ return -ENOTTY;
+ }
+ default:
+ return -ENOTTY;
+ }
+}
+
+static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ return -EINVAL;
+}
+
+static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ return -EINVAL;
+}
+
+static int vfio_fsl_mc_mmap(void *device_data, struct vm_area_struct *vma)
+{
+ return -EINVAL;
+}
+
+static const struct vfio_device_ops vfio_fsl_mc_ops = {
+ .name = "vfio-fsl-mc",
+ .open = vfio_fsl_mc_open,
+ .release = vfio_fsl_mc_release,
+ .ioctl = vfio_fsl_mc_ioctl,
+ .read = vfio_fsl_mc_read,
+ .write = vfio_fsl_mc_write,
+ .mmap = vfio_fsl_mc_mmap,
+};
+
+static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
+{
+ struct iommu_group *group;
+ struct vfio_fsl_mc_device *vdev;
+ struct device *dev = &mc_dev->dev;
+ int ret;
+
+ group = vfio_iommu_group_get(dev);
+ if (!group) {
+ dev_err(dev, "%s: VFIO: No IOMMU group\n", __func__);
+ return -EINVAL;
+ }
+
+ vdev = devm_kzalloc(dev, sizeof(*vdev), GFP_KERNEL);
+ if (!vdev) {
+ vfio_iommu_group_put(group, dev);
+ return -ENOMEM;
+ }
+
+ vdev->mc_dev = mc_dev;
+
+ ret = vfio_add_group_dev(dev, &vfio_fsl_mc_ops, vdev);
+ if (ret) {
+ dev_err(dev, "%s: Failed to add to vfio group\n", __func__);
+ vfio_iommu_group_put(group, dev);
+ return ret;
+ }
+
+ return ret;
+}
+
+static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
+{
+ struct vfio_fsl_mc_device *vdev;
+ struct device *dev = &mc_dev->dev;
+
+ vdev = vfio_del_group_dev(dev);
+ if (!vdev)
+ return -EINVAL;
+
+ vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
+
+ return 0;
+}
+
+/*
+ * vfio-fsl_mc is a meta-driver, so use driver_override interface to
+ * bind a fsl_mc container with this driver and match_id_table is NULL.
+ */
+static struct fsl_mc_driver vfio_fsl_mc_driver = {
+ .probe = vfio_fsl_mc_probe,
+ .remove = vfio_fsl_mc_remove,
+ .match_id_table = NULL,
+ .driver = {
+ .name = "vfio-fsl-mc",
+ .owner = THIS_MODULE,
+ },
+};
+
+static int __init vfio_fsl_mc_driver_init(void)
+{
+ return fsl_mc_driver_register(&vfio_fsl_mc_driver);
+}
+
+static void __exit vfio_fsl_mc_driver_exit(void)
+{
+ fsl_mc_driver_unregister(&vfio_fsl_mc_driver);
+}
+
+module_init(vfio_fsl_mc_driver_init);
+module_exit(vfio_fsl_mc_driver_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("VFIO for FSL-MC devices - User Level meta-driver");
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
new file mode 100644
index 000000000000..e79cc116f6b8
--- /dev/null
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/*
+ * Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2016,2019-2020 NXP
+ */
+
+#ifndef VFIO_FSL_MC_PRIVATE_H
+#define VFIO_FSL_MC_PRIVATE_H
+
+struct vfio_fsl_mc_device {
+ struct fsl_mc_device *mc_dev;
+};
+
+#endif /* VFIO_FSL_MC_PRIVATE_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 920470502329..95deac891378 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -201,6 +201,7 @@ struct vfio_device_info {
#define VFIO_DEVICE_FLAGS_AMBA (1 << 3) /* vfio-amba device */
#define VFIO_DEVICE_FLAGS_CCW (1 << 4) /* vfio-ccw device */
#define VFIO_DEVICE_FLAGS_AP (1 << 5) /* vfio-ap device */
+#define VFIO_DEVICE_FLAGS_FSL_MC (1 << 6) /* vfio-fsl-mc device */
__u32 num_regions; /* Max region index + 1 */
__u32 num_irqs; /* Max IRQ index + 1 */
};
--
2.17.1

2020-07-06 15:45:46

by Diana Madalina Craciun

[permalink] [raw]
Subject: [PATCH v3 4/9] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call

Expose to userspace information about the memory regions.

Signed-off-by: Bharat Bhushan <[email protected]>
Signed-off-by: Diana Craciun <[email protected]>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 77 ++++++++++++++++++++++-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 19 ++++++
2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 937b6eddc71a..10bd9f78b8de 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -17,16 +17,72 @@

static struct fsl_mc_driver vfio_fsl_mc_driver;

+static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
+{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int count = mc_dev->obj_desc.region_count;
+ int i;
+
+ vdev->regions = kcalloc(count, sizeof(struct vfio_fsl_mc_region),
+ GFP_KERNEL);
+ if (!vdev->regions)
+ return -ENOMEM;
+
+ for (i = 0; i < count; i++) {
+ struct resource *res = &mc_dev->regions[i];
+
+ vdev->regions[i].addr = res->start;
+ vdev->regions[i].size = resource_size(res);
+ vdev->regions[i].flags = 0;
+ }
+
+ vdev->num_regions = mc_dev->obj_desc.region_count;
+ return 0;
+}
+
+static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
+{
+ vdev->num_regions = 0;
+ kfree(vdev->regions);
+}
+
static int vfio_fsl_mc_open(void *device_data)
{
+ struct vfio_fsl_mc_device *vdev = device_data;
+ int ret;
+
if (!try_module_get(THIS_MODULE))
return -ENODEV;

+ mutex_lock(&vdev->driver_lock);
+ if (!vdev->refcnt) {
+ ret = vfio_fsl_mc_regions_init(vdev);
+ if (ret)
+ goto err_reg_init;
+ }
+ vdev->refcnt++;
+
+ mutex_unlock(&vdev->driver_lock);
+
return 0;
+
+err_reg_init:
+ mutex_unlock(&vdev->driver_lock);
+ module_put(THIS_MODULE);
+ return ret;
}

static void vfio_fsl_mc_release(void *device_data)
{
+ struct vfio_fsl_mc_device *vdev = device_data;
+
+ mutex_lock(&vdev->driver_lock);
+
+ if (!(--vdev->refcnt))
+ vfio_fsl_mc_regions_cleanup(vdev);
+
+ mutex_unlock(&vdev->driver_lock);
+
module_put(THIS_MODULE);
}

@@ -59,7 +115,25 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
}
case VFIO_DEVICE_GET_REGION_INFO:
{
- return -ENOTTY;
+ struct vfio_region_info info;
+
+ minsz = offsetofend(struct vfio_region_info, offset);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ if (info.index >= vdev->num_regions)
+ return -EINVAL;
+
+ /* map offset to the physical address */
+ info.offset = VFIO_FSL_MC_INDEX_TO_OFFSET(info.index);
+ info.size = vdev->regions[info.index].size;
+ info.flags = vdev->regions[info.index].flags;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
}
case VFIO_DEVICE_GET_IRQ_INFO:
{
@@ -201,6 +275,7 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
vfio_iommu_group_put(group, dev);
return ret;
}
+ mutex_init(&vdev->driver_lock);

return ret;
}
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index 37d61eaa58c8..818dfd3df4db 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -7,9 +7,28 @@
#ifndef VFIO_FSL_MC_PRIVATE_H
#define VFIO_FSL_MC_PRIVATE_H

+#define VFIO_FSL_MC_OFFSET_SHIFT 40
+#define VFIO_FSL_MC_OFFSET_MASK (((u64)(1) << VFIO_FSL_MC_OFFSET_SHIFT) - 1)
+
+#define VFIO_FSL_MC_OFFSET_TO_INDEX(off) ((off) >> VFIO_FSL_MC_OFFSET_SHIFT)
+
+#define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
+ ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
+
+struct vfio_fsl_mc_region {
+ u32 flags;
+ u32 type;
+ u64 addr;
+ resource_size_t size;
+};
+
struct vfio_fsl_mc_device {
struct fsl_mc_device *mc_dev;
struct notifier_block nb;
+ int refcnt;
+ u32 num_regions;
+ struct vfio_fsl_mc_region *regions;
+ struct mutex driver_lock;
};

#endif /* VFIO_FSL_MC_PRIVATE_H */
--
2.17.1

2020-07-06 15:46:10

by Diana Madalina Craciun

[permalink] [raw]
Subject: [PATCH v3 3/9] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl

Allow userspace to get fsl-mc device info (number of regions
and irqs).

Signed-off-by: Bharat Bhushan <[email protected]>
Signed-off-by: Diana Craciun <[email protected]>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index ad8d06cceb71..937b6eddc71a 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -33,10 +33,29 @@ static void vfio_fsl_mc_release(void *device_data)
static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
unsigned long arg)
{
+ unsigned long minsz;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+
switch (cmd) {
case VFIO_DEVICE_GET_INFO:
{
- return -ENOTTY;
+ struct vfio_device_info info;
+
+ minsz = offsetofend(struct vfio_device_info, num_irqs);
+
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ info.flags = VFIO_DEVICE_FLAGS_FSL_MC;
+ info.num_regions = mc_dev->obj_desc.region_count;
+ info.num_irqs = mc_dev->obj_desc.irq_count;
+
+ return copy_to_user((void __user *)arg, &info, minsz) ?
+ -EFAULT : 0;
}
case VFIO_DEVICE_GET_REGION_INFO:
{
--
2.17.1

2020-07-06 15:46:30

by Diana Madalina Craciun

[permalink] [raw]
Subject: [PATCH v3 2/9] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind

The DPRC (Data Path Resource Container) device is a bus device and has
child devices attached to it. When the vfio-fsl-mc driver is probed
the DPRC is scanned and the child devices discovered and initialized.

Signed-off-by: Bharat Bhushan <[email protected]>
Signed-off-by: Diana Craciun <[email protected]>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 106 ++++++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
2 files changed, 107 insertions(+)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 8b53c2a25b32..ad8d06cceb71 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -15,6 +15,8 @@

#include "vfio_fsl_mc_private.h"

+static struct fsl_mc_driver vfio_fsl_mc_driver;
+
static int vfio_fsl_mc_open(void *device_data)
{
if (!try_module_get(THIS_MODULE))
@@ -84,6 +86,69 @@ static const struct vfio_device_ops vfio_fsl_mc_ops = {
.mmap = vfio_fsl_mc_mmap,
};

+static int vfio_fsl_mc_bus_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct vfio_fsl_mc_device *vdev = container_of(nb,
+ struct vfio_fsl_mc_device, nb);
+ struct device *dev = data;
+ struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+ struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent);
+
+ if (action == BUS_NOTIFY_ADD_DEVICE &&
+ vdev->mc_dev == mc_cont) {
+ mc_dev->driver_override = kasprintf(GFP_KERNEL, "%s",
+ vfio_fsl_mc_ops.name);
+ dev_info(dev, "Setting driver override for device in dprc %s\n",
+ dev_name(&mc_cont->dev));
+ } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
+ vdev->mc_dev == mc_cont) {
+ struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
+
+ if (mc_drv && mc_drv != &vfio_fsl_mc_driver)
+ dev_warn(dev, "Object %s bound to driver %s while DPRC bound to vfio-fsl-mc\n",
+ dev_name(dev), mc_drv->driver.name);
+ }
+
+ return 0;
+}
+
+static int vfio_fsl_mc_init_device(struct vfio_fsl_mc_device *vdev)
+{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int ret;
+
+ /* Non-dprc devices share mc_io from parent */
+ if (!is_fsl_mc_bus_dprc(mc_dev)) {
+ struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent);
+
+ mc_dev->mc_io = mc_cont->mc_io;
+ return 0;
+ }
+
+ vdev->nb.notifier_call = vfio_fsl_mc_bus_notifier;
+ ret = bus_register_notifier(&fsl_mc_bus_type, &vdev->nb);
+ if (ret)
+ return ret;
+
+ /* open DPRC, allocate a MC portal */
+ ret = dprc_setup(mc_dev);
+ if (ret < 0) {
+ dev_err(&mc_dev->dev, "Failed to setup DPRC (error = %d)\n", ret);
+ bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+ return ret;
+ }
+
+ ret = dprc_scan_container(mc_dev, false);
+ if (ret < 0) {
+ dev_err(&mc_dev->dev, "Container scanning failed: %d\n", ret);
+ bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+ dprc_cleanup(mc_dev);
+ }
+
+ return ret;
+}
+
static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
{
struct iommu_group *group;
@@ -112,9 +177,42 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
return ret;
}

+ ret = vfio_fsl_mc_init_device(vdev);
+ if (ret < 0) {
+ vfio_iommu_group_put(group, dev);
+ return ret;
+ }
+
return ret;
}

+static int vfio_fsl_mc_device_remove(struct device *dev, void *data)
+{
+ struct fsl_mc_device *mc_dev;
+
+ WARN_ON(!dev);
+ mc_dev = to_fsl_mc_device(dev);
+ if (WARN_ON(!mc_dev))
+ return -ENODEV;
+
+ kfree(mc_dev->driver_override);
+ mc_dev->driver_override = NULL;
+
+ /*
+ * The device-specific remove callback will get invoked by device_del()
+ */
+ device_del(&mc_dev->dev);
+ put_device(&mc_dev->dev);
+
+ return 0;
+}
+
+static void vfio_fsl_mc_cleanup_dprc(struct fsl_mc_device *mc_dev)
+{
+ device_for_each_child(&mc_dev->dev, NULL, vfio_fsl_mc_device_remove);
+ dprc_cleanup(mc_dev);
+}
+
static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
{
struct vfio_fsl_mc_device *vdev;
@@ -124,6 +222,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
if (!vdev)
return -EINVAL;

+ if (vdev->nb.notifier_call)
+ bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+
+ if (is_fsl_mc_bus_dprc(mc_dev))
+ vfio_fsl_mc_cleanup_dprc(vdev->mc_dev);
+
+ mc_dev->mc_io = NULL;
+
vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);

return 0;
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index e79cc116f6b8..37d61eaa58c8 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -9,6 +9,7 @@

struct vfio_fsl_mc_device {
struct fsl_mc_device *mc_dev;
+ struct notifier_block nb;
};

#endif /* VFIO_FSL_MC_PRIVATE_H */
--
2.17.1

2020-07-09 22:45:54

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v3 2/9] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind

On Mon, 6 Jul 2020 18:41:46 +0300
Diana Craciun <[email protected]> wrote:

> The DPRC (Data Path Resource Container) device is a bus device and has
> child devices attached to it. When the vfio-fsl-mc driver is probed
> the DPRC is scanned and the child devices discovered and initialized.
>
> Signed-off-by: Bharat Bhushan <[email protected]>
> Signed-off-by: Diana Craciun <[email protected]>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 106 ++++++++++++++++++++++
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
> 2 files changed, 107 insertions(+)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 8b53c2a25b32..ad8d06cceb71 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -15,6 +15,8 @@
>
> #include "vfio_fsl_mc_private.h"
>
> +static struct fsl_mc_driver vfio_fsl_mc_driver;
> +
> static int vfio_fsl_mc_open(void *device_data)
> {
> if (!try_module_get(THIS_MODULE))
> @@ -84,6 +86,69 @@ static const struct vfio_device_ops vfio_fsl_mc_ops = {
> .mmap = vfio_fsl_mc_mmap,
> };
>
> +static int vfio_fsl_mc_bus_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct vfio_fsl_mc_device *vdev = container_of(nb,
> + struct vfio_fsl_mc_device, nb);
> + struct device *dev = data;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent);
> +
> + if (action == BUS_NOTIFY_ADD_DEVICE &&
> + vdev->mc_dev == mc_cont) {
> + mc_dev->driver_override = kasprintf(GFP_KERNEL, "%s",
> + vfio_fsl_mc_ops.name);

I notice the vfio-pci code that this is modeled from also doesn't check
this allocation for NULL. Maybe both should print a dev_warn on the
ultra slim chance it would fail.

> + dev_info(dev, "Setting driver override for device in dprc %s\n",
> + dev_name(&mc_cont->dev));
> + } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> + vdev->mc_dev == mc_cont) {
> + struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
> +
> + if (mc_drv && mc_drv != &vfio_fsl_mc_driver)
> + dev_warn(dev, "Object %s bound to driver %s while DPRC bound to vfio-fsl-mc\n",
> + dev_name(dev), mc_drv->driver.name);
> + }

Nit, } is over-indented, should be aligned to the previous 'else if'.

> +
> + return 0;
> +}
> +
> +static int vfio_fsl_mc_init_device(struct vfio_fsl_mc_device *vdev)
> +{
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int ret;
> +
> + /* Non-dprc devices share mc_io from parent */
> + if (!is_fsl_mc_bus_dprc(mc_dev)) {
> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(mc_dev->dev.parent);
> +
> + mc_dev->mc_io = mc_cont->mc_io;
> + return 0;
> + }
> +
> + vdev->nb.notifier_call = vfio_fsl_mc_bus_notifier;
> + ret = bus_register_notifier(&fsl_mc_bus_type, &vdev->nb);
> + if (ret)
> + return ret;
> +
> + /* open DPRC, allocate a MC portal */
> + ret = dprc_setup(mc_dev);
> + if (ret < 0) {
> + dev_err(&mc_dev->dev, "Failed to setup DPRC (error = %d)\n", ret);
> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> + return ret;
> + }
> +
> + ret = dprc_scan_container(mc_dev, false);
> + if (ret < 0) {
> + dev_err(&mc_dev->dev, "Container scanning failed: %d\n", ret);
> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> + dprc_cleanup(mc_dev);

All else being equal, should these be reversed to mirror the setup?

> + }
> +
> + return ret;
> +}
> +
> static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> {
> struct iommu_group *group;
> @@ -112,9 +177,42 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> return ret;
> }
>
> + ret = vfio_fsl_mc_init_device(vdev);
> + if (ret < 0) {
> + vfio_iommu_group_put(group, dev);
> + return ret;
> + }
> +
> return ret;
> }
>
> +static int vfio_fsl_mc_device_remove(struct device *dev, void *data)
> +{
> + struct fsl_mc_device *mc_dev;
> +
> + WARN_ON(!dev);
> + mc_dev = to_fsl_mc_device(dev);
> + if (WARN_ON(!mc_dev))
> + return -ENODEV;
> +
> + kfree(mc_dev->driver_override);
> + mc_dev->driver_override = NULL;

This is out of scope, all other buses that support a driver_override
free this is the bus driver code. Why isn't it sufficient that it's
done in fsl_mc_device_remove()?

> +
> + /*
> + * The device-specific remove callback will get invoked by device_del()
> + */
> + device_del(&mc_dev->dev);
> + put_device(&mc_dev->dev);

In fact, why are we doing any of this? I think these devices were
created via dprc_scan_container(), so shouldn't there be a dprc
callback to remove them? What happens if one of them did get bound to
another driver, haven't we just deleted the device out from under them?
In vfio-pci for instance, we call pci_disable_sriov() to remove any
vfs. Thanks,

Alex

> +
> + return 0;
> +}
> +
> +static void vfio_fsl_mc_cleanup_dprc(struct fsl_mc_device *mc_dev)
> +{
> + device_for_each_child(&mc_dev->dev, NULL, vfio_fsl_mc_device_remove);
> + dprc_cleanup(mc_dev);
> +}
> +
> static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
> {
> struct vfio_fsl_mc_device *vdev;
> @@ -124,6 +222,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
> if (!vdev)
> return -EINVAL;
>
> + if (vdev->nb.notifier_call)
> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> +
> + if (is_fsl_mc_bus_dprc(mc_dev))
> + vfio_fsl_mc_cleanup_dprc(vdev->mc_dev);
> +
> + mc_dev->mc_io = NULL;
> +
> vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
>
> return 0;
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> index e79cc116f6b8..37d61eaa58c8 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -9,6 +9,7 @@
>
> struct vfio_fsl_mc_device {
> struct fsl_mc_device *mc_dev;
> + struct notifier_block nb;
> };
>
> #endif /* VFIO_FSL_MC_PRIVATE_H */

2020-07-09 22:54:57

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH v3 4/9] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call

On Mon, 6 Jul 2020 18:41:48 +0300
Diana Craciun <[email protected]> wrote:

> Expose to userspace information about the memory regions.
>
> Signed-off-by: Bharat Bhushan <[email protected]>
> Signed-off-by: Diana Craciun <[email protected]>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 77 ++++++++++++++++++++++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 19 ++++++
> 2 files changed, 95 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 937b6eddc71a..10bd9f78b8de 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -17,16 +17,72 @@
>
> static struct fsl_mc_driver vfio_fsl_mc_driver;
>
> +static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
> +{
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int count = mc_dev->obj_desc.region_count;
> + int i;
> +
> + vdev->regions = kcalloc(count, sizeof(struct vfio_fsl_mc_region),
> + GFP_KERNEL);
> + if (!vdev->regions)
> + return -ENOMEM;
> +
> + for (i = 0; i < count; i++) {
> + struct resource *res = &mc_dev->regions[i];
> +
> + vdev->regions[i].addr = res->start;
> + vdev->regions[i].size = resource_size(res);
> + vdev->regions[i].flags = 0;
> + }
> +
> + vdev->num_regions = mc_dev->obj_desc.region_count;
> + return 0;
> +}
> +
> +static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
> +{
> + vdev->num_regions = 0;
> + kfree(vdev->regions);
> +}
> +
> static int vfio_fsl_mc_open(void *device_data)
> {
> + struct vfio_fsl_mc_device *vdev = device_data;
> + int ret;
> +
> if (!try_module_get(THIS_MODULE))
> return -ENODEV;
>
> + mutex_lock(&vdev->driver_lock);
> + if (!vdev->refcnt) {
> + ret = vfio_fsl_mc_regions_init(vdev);
> + if (ret)
> + goto err_reg_init;
> + }
> + vdev->refcnt++;
> +
> + mutex_unlock(&vdev->driver_lock);
> +
> return 0;
> +
> +err_reg_init:
> + mutex_unlock(&vdev->driver_lock);
> + module_put(THIS_MODULE);
> + return ret;
> }
>
> static void vfio_fsl_mc_release(void *device_data)
> {
> + struct vfio_fsl_mc_device *vdev = device_data;
> +
> + mutex_lock(&vdev->driver_lock);
> +
> + if (!(--vdev->refcnt))
> + vfio_fsl_mc_regions_cleanup(vdev);
> +
> + mutex_unlock(&vdev->driver_lock);
> +
> module_put(THIS_MODULE);
> }
>
> @@ -59,7 +115,25 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
> }
> case VFIO_DEVICE_GET_REGION_INFO:
> {
> - return -ENOTTY;
> + struct vfio_region_info info;
> +
> + minsz = offsetofend(struct vfio_region_info, offset);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + if (info.index >= vdev->num_regions)
> + return -EINVAL;
> +
> + /* map offset to the physical address */
> + info.offset = VFIO_FSL_MC_INDEX_TO_OFFSET(info.index);
> + info.size = vdev->regions[info.index].size;
> + info.flags = vdev->regions[info.index].flags;
> +
> + return copy_to_user((void __user *)arg, &info, minsz);
> }
> case VFIO_DEVICE_GET_IRQ_INFO:
> {
> @@ -201,6 +275,7 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> vfio_iommu_group_put(group, dev);
> return ret;
> }
> + mutex_init(&vdev->driver_lock);


Consider all calling mutex_destory() in the remove callback, it's only
used for lock debugging, so we're only partially successful in calling
it. Thanks,

Alex

>
> return ret;
> }
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> index 37d61eaa58c8..818dfd3df4db 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -7,9 +7,28 @@
> #ifndef VFIO_FSL_MC_PRIVATE_H
> #define VFIO_FSL_MC_PRIVATE_H
>
> +#define VFIO_FSL_MC_OFFSET_SHIFT 40
> +#define VFIO_FSL_MC_OFFSET_MASK (((u64)(1) << VFIO_FSL_MC_OFFSET_SHIFT) - 1)
> +
> +#define VFIO_FSL_MC_OFFSET_TO_INDEX(off) ((off) >> VFIO_FSL_MC_OFFSET_SHIFT)
> +
> +#define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
> + ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
> +
> +struct vfio_fsl_mc_region {
> + u32 flags;
> + u32 type;
> + u64 addr;
> + resource_size_t size;
> +};
> +
> struct vfio_fsl_mc_device {
> struct fsl_mc_device *mc_dev;
> struct notifier_block nb;
> + int refcnt;
> + u32 num_regions;
> + struct vfio_fsl_mc_region *regions;
> + struct mutex driver_lock;
> };
>
> #endif /* VFIO_FSL_MC_PRIVATE_H */