* [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device
@ 2020-08-26 9:33 Diana Craciun
2020-08-26 9:33 ` [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
` (10 more replies)
0 siblings, 11 replies; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun
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/msg3639226.html
v3 --> v4
- use bus provided functions to tear down the DPRC
- added reset support
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 (9):
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
vfio/fsl-mc: Add support for device reset
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 | 684 ++++++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 221 +++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 56 ++
include/uapi/linux/vfio.h | 1 +
9 files changed, 983 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
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 14:06 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
` (9 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan,
Diana Craciun
From: Bharat Bhushan <Bharat.Bhushan@nxp.com>
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 <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
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 3b186ade3597..f3f9ea108588 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18229,6 +18229,12 @@ F: drivers/vfio/
F: include/linux/vfio.h
F: include/uapi/linux/vfio.h
+VFIO FSL-MC DRIVER
+M: Diana Craciun <diana.craciun@oss.nxp.com>
+L: kvm@vger.kernel.org
+S: Maintained
+F: drivers/vfio/fsl-mc/
+
VFIO MEDIATED DEVICE DRIVERS
M: Kirti Wankhede <kwankhede@nvidia.com>
L: kvm@vger.kernel.org
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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
2020-08-26 9:33 ` [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 14:06 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
` (8 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
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 <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 84 +++++++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
2 files changed, 85 insertions(+)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 8b53c2a25b32..85e007be3a5d 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,72 @@ 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);
+ if (!mc_dev->driver_override)
+ dev_warn(dev, "Setting driver override for device in dprc %s failed\n",
+ dev_name(&mc_cont->dev));
+ 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);
+ dprc_cleanup(mc_dev);
+ bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+ }
+
+ return ret;
+}
+
static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
{
struct iommu_group *group;
@@ -112,6 +180,12 @@ 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;
}
@@ -124,6 +198,16 @@ 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)) {
+ dprc_remove_devices(mc_dev, NULL, 0);
+ dprc_cleanup(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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
2020-08-26 9:33 ` [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
2020-08-26 9:33 ` [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 14:13 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
` (7 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
Allow userspace to get fsl-mc device info (number of regions
and irqs).
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
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 85e007be3a5d..5a5460d01f00 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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (2 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 15:16 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
` (6 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
Expose to userspace information about the memory regions.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 79 ++++++++++++++++++++++-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 19 ++++++
2 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 5a5460d01f00..093b8d68496c 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:
{
@@ -204,6 +278,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;
}
@@ -227,6 +302,8 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
mc_dev->mc_io = NULL;
+ mutex_destroy(&vdev->driver_lock);
+
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 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
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (3 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 16:05 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
` (5 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
Allow userspace to mmap device regions for direct access of
fsl-mc devices.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 60 +++++++++++++++++++++++++++++--
1 file changed, 58 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 093b8d68496c..64d5c1fff51f 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -33,7 +33,8 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
vdev->regions[i].addr = res->start;
vdev->regions[i].size = resource_size(res);
- vdev->regions[i].flags = 0;
+ vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
+ vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
}
vdev->num_regions = mc_dev->obj_desc.region_count;
@@ -164,9 +165,64 @@ static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
return -EINVAL;
}
+static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
+ struct vm_area_struct *vma)
+{
+ u64 size = vma->vm_end - vma->vm_start;
+ u64 pgoff, base;
+ u8 region_cacheable;
+
+ pgoff = vma->vm_pgoff &
+ ((1U << (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
+ base = pgoff << PAGE_SHIFT;
+
+ if (region.size < PAGE_SIZE || base + size > region.size)
+ return -EINVAL;
+
+ region_cacheable = (region.type & FSL_MC_REGION_CACHEABLE) &&
+ (region.type & FSL_MC_REGION_SHAREABLE);
+ if (!region_cacheable)
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+ vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
+
+ return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+ size, vma->vm_page_prot);
+}
+
static int vfio_fsl_mc_mmap(void *device_data, struct vm_area_struct *vma)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int index;
+
+ index = vma->vm_pgoff >> (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT);
+
+ if (vma->vm_end < vma->vm_start)
+ return -EINVAL;
+ if (vma->vm_start & ~PAGE_MASK)
+ return -EINVAL;
+ if (vma->vm_end & ~PAGE_MASK)
+ return -EINVAL;
+ if (!(vma->vm_flags & VM_SHARED))
+ return -EINVAL;
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ)
+ && (vma->vm_flags & VM_READ))
+ return -EINVAL;
+
+ if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)
+ && (vma->vm_flags & VM_WRITE))
+ return -EINVAL;
+
+ vma->vm_private_data = mc_dev;
+
+ return vfio_fsl_mc_mmap_mmio(vdev->regions[index], vma);
}
static const struct vfio_device_ops vfio_fsl_mc_ops = {
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (4 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 19:55 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
` (4 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun
Only the DPRC object allocates interrupts from the MSI
interrupt domain. The interrupts are managed by the DPRC in
a pool of interrupts. The access to this pool of interrupts
has to be protected with a lock.
This patch extends the current lock implementation to have a
lock per DPRC.
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 91 +++++++++++++++++++++--
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 8 +-
2 files changed, 91 insertions(+), 8 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 64d5c1fff51f..bbd3365e877e 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -17,6 +17,77 @@
static struct fsl_mc_driver vfio_fsl_mc_driver;
+static DEFINE_MUTEX(reflck_lock);
+
+static void vfio_fsl_mc_reflck_get(struct vfio_fsl_mc_reflck *reflck)
+{
+ kref_get(&reflck->kref);
+}
+
+static void vfio_fsl_mc_reflck_release(struct kref *kref)
+{
+ struct vfio_fsl_mc_reflck *reflck = container_of(kref,
+ struct vfio_fsl_mc_reflck,
+ kref);
+
+ mutex_destroy(&reflck->lock);
+ kfree(reflck);
+ mutex_unlock(&reflck_lock);
+}
+
+static void vfio_fsl_mc_reflck_put(struct vfio_fsl_mc_reflck *reflck)
+{
+ kref_put_mutex(&reflck->kref, vfio_fsl_mc_reflck_release, &reflck_lock);
+}
+
+static struct vfio_fsl_mc_reflck *vfio_fsl_mc_reflck_alloc(void)
+{
+ struct vfio_fsl_mc_reflck *reflck;
+
+ reflck = kzalloc(sizeof(*reflck), GFP_KERNEL);
+ if (!reflck)
+ return ERR_PTR(-ENOMEM);
+
+ kref_init(&reflck->kref);
+ mutex_init(&reflck->lock);
+
+ return reflck;
+}
+
+static int vfio_fsl_mc_reflck_attach(struct vfio_fsl_mc_device *vdev)
+{
+ int ret = 0;
+
+ mutex_lock(&reflck_lock);
+ if (is_fsl_mc_bus_dprc(vdev->mc_dev)) {
+ vdev->reflck = vfio_fsl_mc_reflck_alloc();
+ } else {
+ struct device *mc_cont_dev = vdev->mc_dev->dev.parent;
+ struct vfio_device *device;
+ struct vfio_fsl_mc_device *cont_vdev;
+
+ device = vfio_device_get_from_dev(mc_cont_dev);
+ if (!device) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+
+ cont_vdev = vfio_device_data(device);
+ if (!cont_vdev->reflck) {
+ vfio_device_put(device);
+ ret = -ENODEV;
+ goto unlock;
+ }
+ vfio_fsl_mc_reflck_get(cont_vdev->reflck);
+ vdev->reflck = cont_vdev->reflck;
+ vfio_device_put(device);
+ }
+
+unlock:
+ mutex_unlock(&reflck_lock);
+ return ret;
+}
+
static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
{
struct fsl_mc_device *mc_dev = vdev->mc_dev;
@@ -55,7 +126,7 @@ static int vfio_fsl_mc_open(void *device_data)
if (!try_module_get(THIS_MODULE))
return -ENODEV;
- mutex_lock(&vdev->driver_lock);
+ mutex_lock(&vdev->reflck->lock);
if (!vdev->refcnt) {
ret = vfio_fsl_mc_regions_init(vdev);
if (ret)
@@ -63,12 +134,12 @@ static int vfio_fsl_mc_open(void *device_data)
}
vdev->refcnt++;
- mutex_unlock(&vdev->driver_lock);
+ mutex_unlock(&vdev->reflck->lock);
return 0;
err_reg_init:
- mutex_unlock(&vdev->driver_lock);
+ mutex_unlock(&vdev->reflck->lock);
module_put(THIS_MODULE);
return ret;
}
@@ -77,12 +148,12 @@ static void vfio_fsl_mc_release(void *device_data)
{
struct vfio_fsl_mc_device *vdev = device_data;
- mutex_lock(&vdev->driver_lock);
+ mutex_lock(&vdev->reflck->lock);
if (!(--vdev->refcnt))
vfio_fsl_mc_regions_cleanup(vdev);
- mutex_unlock(&vdev->driver_lock);
+ mutex_unlock(&vdev->reflck->lock);
module_put(THIS_MODULE);
}
@@ -329,12 +400,18 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
return ret;
}
+ ret = vfio_fsl_mc_reflck_attach(vdev);
+ if (ret) {
+ vfio_iommu_group_put(group, dev);
+ return ret;
+ }
+
ret = vfio_fsl_mc_init_device(vdev);
if (ret < 0) {
+ vfio_fsl_mc_reflck_put(vdev->reflck);
vfio_iommu_group_put(group, dev);
return ret;
}
- mutex_init(&vdev->driver_lock);
return ret;
}
@@ -358,7 +435,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
mc_dev->mc_io = NULL;
- mutex_destroy(&vdev->driver_lock);
+ vfio_fsl_mc_reflck_put(vdev->reflck);
vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index 818dfd3df4db..3b85d930e060 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -15,6 +15,11 @@
#define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
+struct vfio_fsl_mc_reflck {
+ struct kref kref;
+ struct mutex lock;
+};
+
struct vfio_fsl_mc_region {
u32 flags;
u32 type;
@@ -28,7 +33,8 @@ struct vfio_fsl_mc_device {
int refcnt;
u32 num_regions;
struct vfio_fsl_mc_region *regions;
- struct mutex driver_lock;
+ struct vfio_fsl_mc_reflck *reflck;
+
};
#endif /* VFIO_FSL_MC_PRIVATE_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (5 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-03 20:15 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
` (3 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
This patch adds the skeleton for interrupt support
for fsl-mc devices. The interrupts are not yet functional,
the functionality will be added by subsequent patches.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/Makefile | 2 +-
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 75 ++++++++++++++++++++++-
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 63 +++++++++++++++++++
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 7 ++-
4 files changed, 143 insertions(+), 4 deletions(-)
create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
diff --git a/drivers/vfio/fsl-mc/Makefile b/drivers/vfio/fsl-mc/Makefile
index 0c6e5d2ddaae..cad6dbf0b735 100644
--- a/drivers/vfio/fsl-mc/Makefile
+++ b/drivers/vfio/fsl-mc/Makefile
@@ -1,4 +1,4 @@
# SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
-vfio-fsl-mc-y := vfio_fsl_mc.o
+vfio-fsl-mc-y := vfio_fsl_mc.o vfio_fsl_mc_intr.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
index bbd3365e877e..42014297b484 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -209,11 +209,79 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
}
case VFIO_DEVICE_GET_IRQ_INFO:
{
- return -ENOTTY;
+ struct vfio_irq_info info;
+
+ minsz = offsetofend(struct vfio_irq_info, count);
+ if (copy_from_user(&info, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (info.argsz < minsz)
+ return -EINVAL;
+
+ if (info.index >= mc_dev->obj_desc.irq_count)
+ return -EINVAL;
+
+ info.flags = VFIO_IRQ_INFO_EVENTFD;
+ info.count = 1;
+
+ return copy_to_user((void __user *)arg, &info, minsz);
}
case VFIO_DEVICE_SET_IRQS:
{
- return -ENOTTY;
+ struct vfio_irq_set hdr;
+ u8 *data = NULL;
+ int ret = 0;
+
+ minsz = offsetofend(struct vfio_irq_set, count);
+
+ if (copy_from_user(&hdr, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (hdr.argsz < minsz)
+ return -EINVAL;
+
+ if (hdr.index >= mc_dev->obj_desc.irq_count)
+ return -EINVAL;
+
+ if (hdr.start != 0 || hdr.count > 1)
+ return -EINVAL;
+
+ if (hdr.count == 0 &&
+ (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE) ||
+ !(hdr.flags & VFIO_IRQ_SET_ACTION_TRIGGER)))
+ return -EINVAL;
+
+ if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
+ VFIO_IRQ_SET_ACTION_TYPE_MASK))
+ return -EINVAL;
+
+ if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
+ size_t size;
+
+ if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
+ size = sizeof(uint8_t);
+ else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
+ size = sizeof(int32_t);
+ else
+ return -EINVAL;
+
+ if (hdr.argsz - minsz < hdr.count * size)
+ return -EINVAL;
+
+ data = memdup_user((void __user *)(arg + minsz),
+ hdr.count * size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+ }
+
+ mutex_lock(&vdev->igate);
+ ret = vfio_fsl_mc_set_irqs_ioctl(vdev, hdr.flags,
+ hdr.index, hdr.start,
+ hdr.count, data);
+ mutex_unlock(&vdev->igate);
+ kfree(data);
+
+ return ret;
}
case VFIO_DEVICE_RESET:
{
@@ -413,6 +481,8 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
return ret;
}
+ mutex_init(&vdev->igate);
+
return ret;
}
@@ -436,6 +506,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
mc_dev->mc_io = NULL;
vfio_fsl_mc_reflck_put(vdev->reflck);
+ mutex_destroy(&vdev->igate);
vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
new file mode 100644
index 000000000000..058aa97aa54a
--- /dev/null
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/*
+ * Copyright 2013-2016 Freescale Semiconductor Inc.
+ * Copyright 2019 NXP
+ */
+
+#include <linux/vfio.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/eventfd.h>
+#include <linux/msi.h>
+
+#include "linux/fsl/mc.h"
+#include "vfio_fsl_mc_private.h"
+
+static int vfio_fsl_mc_irq_mask(struct vfio_fsl_mc_device *vdev,
+ unsigned int index, unsigned int start,
+ unsigned int count, u32 flags,
+ void *data)
+{
+ return -EINVAL;
+}
+
+static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
+ unsigned int index, unsigned int start,
+ unsigned int count, u32 flags,
+ void *data)
+{
+ return -EINVAL;
+}
+
+static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
+ unsigned int index, unsigned int start,
+ unsigned int count, u32 flags,
+ void *data)
+{
+ return -EINVAL;
+}
+
+int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
+ u32 flags, unsigned int index,
+ unsigned int start, unsigned int count,
+ void *data)
+{
+ int ret = -ENOTTY;
+
+ switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
+ case VFIO_IRQ_SET_ACTION_MASK:
+ ret = vfio_fsl_mc_irq_mask(vdev, index, start, count,
+ flags, data);
+ break;
+ case VFIO_IRQ_SET_ACTION_UNMASK:
+ ret = vfio_fsl_mc_irq_unmask(vdev, index, start, count,
+ flags, data);
+ break;
+ case VFIO_IRQ_SET_ACTION_TRIGGER:
+ ret = vfio_fsl_mc_set_irq_trigger(vdev, index, start,
+ count, flags, data);
+ break;
+ }
+
+ 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 3b85d930e060..d5b6fe891a48 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -34,7 +34,12 @@ struct vfio_fsl_mc_device {
u32 num_regions;
struct vfio_fsl_mc_region *regions;
struct vfio_fsl_mc_reflck *reflck;
-
+ struct mutex igate;
};
+extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
+ u32 flags, unsigned int index,
+ unsigned int start, unsigned int count,
+ void *data);
+
#endif /* VFIO_FSL_MC_PRIVATE_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (6 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-04 8:02 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
` (2 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
This patch allows to set an eventfd for fsl-mc device interrupts
and also to trigger the interrupt eventfd from userspace for testing.
All fsl-mc device interrupts are MSIs. The MSIs are allocated from
the MSI domain only once per DPRC and used by all the DPAA2 objects.
The interrupts are managed by the DPRC in a pool of interrupts. Each
device requests interrupts from this pool. The pool is allocated
when the first virtual device is setting the interrupts.
The pool of interrupts is protected by a lock.
The DPRC has an interrupt of its own which indicates if the DPRC
contents have changed. However, currently, the contents of a DPRC
assigned to the guest cannot be changed at runtime, so this interrupt
is not configured.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 18 ++-
drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 160 +++++++++++++++++++++-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 10 ++
3 files changed, 186 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 42014297b484..73834f488a94 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -147,12 +147,28 @@ static int vfio_fsl_mc_open(void *device_data)
static void vfio_fsl_mc_release(void *device_data)
{
struct vfio_fsl_mc_device *vdev = device_data;
+ int ret;
mutex_lock(&vdev->reflck->lock);
- if (!(--vdev->refcnt))
+ if (!(--vdev->refcnt)) {
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
+ struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
+
vfio_fsl_mc_regions_cleanup(vdev);
+ /* reset the device before cleaning up the interrupts */
+ ret = dprc_reset_container(mc_cont->mc_io, 0,
+ mc_cont->mc_handle,
+ mc_cont->obj_desc.id,
+ DPRC_RESET_OPTION_NON_RECURSIVE);
+
+ vfio_fsl_mc_irqs_cleanup(vdev);
+
+ fsl_mc_cleanup_irq_pool(mc_cont);
+ }
+
mutex_unlock(&vdev->reflck->lock);
module_put(THIS_MODULE);
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
index 058aa97aa54a..409f3507fcf3 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
@@ -29,12 +29,149 @@ static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
return -EINVAL;
}
+int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
+{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ struct vfio_fsl_mc_irq *mc_irq;
+ int irq_count;
+ int ret, i;
+
+ /* Device does not support any interrupt */
+ if (mc_dev->obj_desc.irq_count == 0)
+ return 0;
+
+ /* interrupts were already allocated for this device */
+ if (vdev->mc_irqs)
+ return 0;
+
+ irq_count = mc_dev->obj_desc.irq_count;
+
+ mc_irq = kcalloc(irq_count, sizeof(*mc_irq), GFP_KERNEL);
+ if (!mc_irq)
+ return -ENOMEM;
+
+ /* Allocate IRQs */
+ ret = fsl_mc_allocate_irqs(mc_dev);
+ if (ret) {
+ kfree(mc_irq);
+ return ret;
+ }
+
+ for (i = 0; i < irq_count; i++) {
+ mc_irq[i].count = 1;
+ mc_irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
+ }
+
+ vdev->mc_irqs = mc_irq;
+
+ return 0;
+}
+
+static irqreturn_t vfio_fsl_mc_irq_handler(int irq_num, void *arg)
+{
+ struct vfio_fsl_mc_irq *mc_irq = (struct vfio_fsl_mc_irq *)arg;
+
+ eventfd_signal(mc_irq->trigger, 1);
+ return IRQ_HANDLED;
+}
+
+static int vfio_set_trigger(struct vfio_fsl_mc_device *vdev,
+ int index, int fd)
+{
+ struct vfio_fsl_mc_irq *irq = &vdev->mc_irqs[index];
+ struct eventfd_ctx *trigger;
+ int hwirq;
+ int ret;
+
+ hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
+ if (irq->trigger) {
+ free_irq(hwirq, irq);
+ kfree(irq->name);
+ eventfd_ctx_put(irq->trigger);
+ irq->trigger = NULL;
+ }
+
+ if (fd < 0) /* Disable only */
+ return 0;
+
+ irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)",
+ hwirq, dev_name(&vdev->mc_dev->dev));
+ if (!irq->name)
+ return -ENOMEM;
+
+ trigger = eventfd_ctx_fdget(fd);
+ if (IS_ERR(trigger)) {
+ kfree(irq->name);
+ return PTR_ERR(trigger);
+ }
+
+ irq->trigger = trigger;
+
+ ret = request_irq(hwirq, vfio_fsl_mc_irq_handler, 0,
+ irq->name, irq);
+ if (ret) {
+ kfree(irq->name);
+ eventfd_ctx_put(trigger);
+ irq->trigger = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
unsigned int index, unsigned int start,
unsigned int count, u32 flags,
void *data)
{
- return -EINVAL;
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int ret, hwirq;
+ struct vfio_fsl_mc_irq *irq;
+ struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
+ struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
+
+ if (start != 0 || count != 1)
+ return -EINVAL;
+
+ mutex_lock(&vdev->reflck->lock);
+ ret = fsl_mc_populate_irq_pool(mc_cont,
+ FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
+ if (ret)
+ goto unlock;
+
+ ret = vfio_fsl_mc_irqs_allocate(vdev);
+ if (ret)
+ goto unlock;
+ mutex_unlock(&vdev->reflck->lock);
+
+ if (!count && (flags & VFIO_IRQ_SET_DATA_NONE))
+ return vfio_set_trigger(vdev, index, -1);
+
+ if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
+ s32 fd = *(s32 *)data;
+
+ return vfio_set_trigger(vdev, index, fd);
+ }
+
+ hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
+
+ irq = &vdev->mc_irqs[index];
+
+ if (flags & VFIO_IRQ_SET_DATA_NONE) {
+ vfio_fsl_mc_irq_handler(hwirq, irq);
+
+ } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
+ u8 trigger = *(u8 *)data;
+
+ if (trigger)
+ vfio_fsl_mc_irq_handler(hwirq, irq);
+ }
+
+ return 0;
+
+unlock:
+ mutex_unlock(&vdev->reflck->lock);
+ return ret;
}
int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
@@ -61,3 +198,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
return ret;
}
+
+/* Free All IRQs for the given MC object */
+void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev)
+{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int irq_count = mc_dev->obj_desc.irq_count;
+ int i;
+
+ /* Device does not support any interrupt or the interrupts
+ * were not configured
+ */
+ if (mc_dev->obj_desc.irq_count == 0 || !vdev->mc_irqs)
+ return;
+
+ for (i = 0; i < irq_count; i++)
+ vfio_set_trigger(vdev, i, -1);
+
+ fsl_mc_free_irqs(mc_dev);
+ kfree(vdev->mc_irqs);
+ vdev->mc_irqs = NULL;
+}
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index d5b6fe891a48..bbfca8b55f8a 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -15,6 +15,13 @@
#define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
+struct vfio_fsl_mc_irq {
+ u32 flags;
+ u32 count;
+ struct eventfd_ctx *trigger;
+ char *name;
+};
+
struct vfio_fsl_mc_reflck {
struct kref kref;
struct mutex lock;
@@ -35,6 +42,7 @@ struct vfio_fsl_mc_device {
struct vfio_fsl_mc_region *regions;
struct vfio_fsl_mc_reflck *reflck;
struct mutex igate;
+ struct vfio_fsl_mc_irq *mc_irqs;
};
extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
@@ -42,4 +50,6 @@ extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
unsigned int start, unsigned int count,
void *data);
+void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev);
+
#endif /* VFIO_FSL_MC_PRIVATE_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (7 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-04 8:18 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
2020-09-03 13:40 ` [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Auger Eric
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun,
Bharat Bhushan
The software uses a memory-mapped I/O command interface (MC portals) to
communicate with the MC hardware. This command interface is used to
discover, enumerate, configure and remove DPAA2 objects. The DPAA2
objects use MSIs, so the command interface needs to be emulated
such that the correct MSI is configured in the hardware (the guest
has the virtual MSIs).
This patch is adding read/write support for fsl-mc devices. The mc
commands are emulated by the userspace. The host is just passing
the correct command to the hardware.
Also the current patch limits userspace to write complete
64byte command once and read 64byte response by one ioctl.
Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 115 +++++++++++++++++++++-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
2 files changed, 114 insertions(+), 2 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 73834f488a94..27713aa86878 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -12,6 +12,7 @@
#include <linux/types.h>
#include <linux/vfio.h>
#include <linux/fsl/mc.h>
+#include <linux/delay.h>
#include "vfio_fsl_mc_private.h"
@@ -106,6 +107,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
vdev->regions[i].size = resource_size(res);
vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
+ if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
}
vdev->num_regions = mc_dev->obj_desc.region_count;
@@ -114,6 +118,11 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
{
+ int i;
+
+ for (i = 0; i < vdev->num_regions; i++)
+ iounmap(vdev->regions[i].ioaddr);
+
vdev->num_regions = 0;
kfree(vdev->regions);
}
@@ -311,13 +320,115 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
size_t count, loff_t *ppos)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
+ struct vfio_fsl_mc_region *region;
+ u64 data[8];
+ int i;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ region = &vdev->regions[index];
+
+ if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
+ return -EINVAL;
+
+ if (!region->ioaddr) {
+ region->ioaddr = ioremap(region->addr, region->size);
+ if (!region->ioaddr)
+ return -ENOMEM;
+ }
+
+ if (count != 64 || off != 0)
+ return -EINVAL;
+
+ for (i = 7; i >= 0; i--)
+ data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
+
+ if (copy_to_user(buf, data, 64))
+ return -EFAULT;
+
+ return count;
+}
+
+#define MC_CMD_COMPLETION_TIMEOUT_MS 5000
+#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS 500
+
+static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
+{
+ int i;
+ enum mc_cmd_status status;
+ unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
+
+ /* Write at command parameter into portal */
+ for (i = 7; i >= 1; i--)
+ writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
+
+ /* Write command header in the end */
+ writeq(cmd_data[0], ioaddr);
+
+ /* Wait for response before returning to user-space
+ * This can be optimized in future to even prepare response
+ * before returning to user-space and avoid read ioctl.
+ */
+ for (;;) {
+ u64 header;
+ struct mc_cmd_header *resp_hdr;
+
+ header = cpu_to_le64(readq_relaxed(ioaddr));
+
+ resp_hdr = (struct mc_cmd_header *)&header;
+ status = (enum mc_cmd_status)resp_hdr->status;
+ if (status != MC_CMD_STATUS_READY)
+ break;
+
+ udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
+ if (timeout_usecs == 0)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
}
static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
size_t count, loff_t *ppos)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
+ struct vfio_fsl_mc_region *region;
+ u64 data[8];
+ int ret;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ region = &vdev->regions[index];
+
+ if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE))
+ return -EINVAL;
+
+ if (!region->ioaddr) {
+ region->ioaddr = ioremap(region->addr, region->size);
+ if (!region->ioaddr)
+ return -ENOMEM;
+ }
+
+ if (count != 64 || off != 0)
+ return -EINVAL;
+
+ if (copy_from_user(&data, buf, 64))
+ return -EFAULT;
+
+ ret = vfio_fsl_mc_send_command(region->ioaddr, data);
+ if (ret)
+ return ret;
+
+ return count;
+
}
static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index bbfca8b55f8a..e6804e516c4a 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -32,6 +32,7 @@ struct vfio_fsl_mc_region {
u32 type;
u64 addr;
resource_size_t size;
+ void __iomem *ioaddr;
};
struct vfio_fsl_mc_device {
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (8 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
@ 2020-08-26 9:33 ` Diana Craciun
2020-09-04 8:21 ` Auger Eric
2020-09-03 13:40 ` [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Auger Eric
10 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun @ 2020-08-26 9:33 UTC (permalink / raw)
To: alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Diana Craciun
Currently only resetting the DPRC container is supported which
will reset all the objects inside it. Resetting individual
objects is possible from the userspace by issueing commands
towards MC firmware.
Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 27713aa86878..d17c5b3148ad 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -310,7 +310,20 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
}
case VFIO_DEVICE_RESET:
{
- return -ENOTTY;
+ int ret = 0;
+
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+
+ /* reset is supported only for the DPRC */
+ if (!is_fsl_mc_bus_dprc(mc_dev))
+ return -ENOTTY;
+
+ ret = dprc_reset_container(mc_dev->mc_io, 0,
+ mc_dev->mc_handle,
+ mc_dev->obj_desc.id,
+ DPRC_RESET_OPTION_NON_RECURSIVE);
+ return ret;
+
}
default:
return -ENOTTY;
--
2.17.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
` (9 preceding siblings ...)
2020-08-26 9:33 ` [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
@ 2020-09-03 13:40 ` Auger Eric
2020-09-04 8:03 ` Diana Craciun OSS
10 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 13:40 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> 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/msg3639226.html
Could you share a branch with both series? This would help the review.
Thanks
Eric
>
> v3 --> v4
> - use bus provided functions to tear down the DPRC
> - added reset support
>
> 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 (9):
> 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
> vfio/fsl-mc: Add support for device reset
>
> 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 | 684 ++++++++++++++++++++++
> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 221 +++++++
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 56 ++
> include/uapi/linux/vfio.h | 1 +
> 9 files changed, 983 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
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
2020-08-26 9:33 ` [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
@ 2020-09-03 14:06 ` Auger Eric
2020-09-07 14:38 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 14:06 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun 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 <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 84 +++++++++++++++++++++++
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
> 2 files changed, 85 insertions(+)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 8b53c2a25b32..85e007be3a5d 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,72 @@ 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);
> + if (!mc_dev->driver_override)
> + dev_warn(dev, "Setting driver override for device in dprc %s failed\n",
> + dev_name(&mc_cont->dev));
> + dev_info(dev, "Setting driver override for device in dprc %s\n",
> + dev_name(&mc_cont->dev));
Don't you miss an else here?
> + } 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) {
if (ret) here and in other places? or are there any > returned values
> + dev_err(&mc_dev->dev, "Failed to setup DPRC (error = %d)\n", ret);
nit: maybe align your error messages. Before you were using __func__,
here you don't. Maybe don't? also you may consider using strerror(-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);
> + dprc_cleanup(mc_dev);
I see dprc_cleanup is likely to fail. Generally cleanup shouldn't.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2283433.html
> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
nit: here also you can factorize code doing goto unregister;
shouldn't you reset vdev->nb.notifier_call to NULL as well. I see it is
tested in other places.
> + }
> +
> + return ret;
> +}
> +
> static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> {
> struct iommu_group *group;
> @@ -112,6 +180,12 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> return ret;
> }
>
> + ret = vfio_fsl_mc_init_device(vdev);
> + if (ret < 0) {
I think you also need to call vfio_del_group_dev(&pdev->dev)
> + vfio_iommu_group_put(group, dev);
> + return ret;
nit: goto put_group;
> + }
> +
> return ret;
> }
>
> @@ -124,6 +198,16 @@ 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)) {
> + dprc_remove_devices(mc_dev, NULL, 0);
> + dprc_cleanup(mc_dev);
> + }
you may consider doing the tear down in opposite order than
vfio_fsl_mc_init_device, ie. bus_unregister_notifier after the
dprc_cleanup? That's also what is done in vfio_fsl_mc_init_device error
path handling.
> +
> + 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 */>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices
2020-08-26 9:33 ` [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
@ 2020-09-03 14:06 ` Auger Eric
2020-09-04 13:59 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 14:06 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> From: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>
> 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 <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> 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 3b186ade3597..f3f9ea108588 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18229,6 +18229,12 @@ F: drivers/vfio/
> F: include/linux/vfio.h
> F: include/uapi/linux/vfio.h
>
> +VFIO FSL-MC DRIVER
> +M: Diana Craciun <diana.craciun@oss.nxp.com>
> +L: kvm@vger.kernel.org
> +S: Maintained
> +F: drivers/vfio/fsl-mc/
> +
> VFIO MEDIATED DEVICE DRIVERS
> M: Kirti Wankhede <kwankhede@nvidia.com>
> L: kvm@vger.kernel.org
> 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;
nit: introduce out_group_put: as in other files
This will be usable also in subsequent patches
> +}
> +
> +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,
not needed?
> + .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");
Don't you need MODULE_LICENSE("Dual BSD/GPL"); ?
> +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 */
> };
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl
2020-08-26 9:33 ` [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
@ 2020-09-03 14:13 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-03 14:13 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> Allow userspace to get fsl-mc device info (number of regions
> and irqs).
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Thanks
Eric
> ---
> 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 85e007be3a5d..5a5460d01f00 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:
> {
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call
2020-08-26 9:33 ` [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
@ 2020-09-03 15:16 ` Auger Eric
2020-09-03 15:22 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 15:16 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> Expose to userspace information about the memory regions.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 79 ++++++++++++++++++++++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 19 ++++++
> 2 files changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 5a5460d01f00..093b8d68496c 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;
why 0? I see in
> + }
> +
> + vdev->num_regions = mc_dev->obj_desc.region_count;
nit: you can use count directly fsl-mc-bus.c that flags can take
meaningful values
> + 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:
> {
> @@ -204,6 +278,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;
> }
> @@ -227,6 +302,8 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>
> mc_dev->mc_io = NULL;
>
> + mutex_destroy(&vdev->driver_lock);
> +
> 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 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 */
>
Otherwise looks good to me
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call
2020-09-03 15:16 ` Auger Eric
@ 2020-09-03 15:22 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-03 15:22 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 9/3/20 5:16 PM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> Expose to userspace information about the memory regions.
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 79 ++++++++++++++++++++++-
>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 19 ++++++
>> 2 files changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 5a5460d01f00..093b8d68496c 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;
> why 0? I see in
>> + }
>> +
>> + vdev->num_regions = mc_dev->obj_desc.region_count;
> nit: you can use count directly fsl-mc-bus.c that flags can take
> meaningful values
Sorry I missed flags and types. So you may set the type in this patch
instead of in next patch?
vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
Eric
>> + 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:
>> {
>> @@ -204,6 +278,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;
>> }
>> @@ -227,6 +302,8 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>>
>> mc_dev->mc_io = NULL;
>>
>> + mutex_destroy(&vdev->driver_lock);
>> +
>> 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 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 */
>>
> Otherwise looks good to me
>
> Thanks
>
> Eric
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions
2020-08-26 9:33 ` [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
@ 2020-09-03 16:05 ` Auger Eric
2020-09-07 12:49 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 16:05 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> Allow userspace to mmap device regions for direct access of
> fsl-mc devices.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 60 +++++++++++++++++++++++++++++--
> 1 file changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 093b8d68496c..64d5c1fff51f 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -33,7 +33,8 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>
> vdev->regions[i].addr = res->start;
> vdev->regions[i].size = resource_size(res);
> - vdev->regions[i].flags = 0;
> + vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
Is the region always mmappable or does it depend on the
mc_dev->regions[i].flags. Also on VFIO platform we checked some
alignment addr/size constraints.
> + vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
> }
>
> vdev->num_regions = mc_dev->obj_desc.region_count;
> @@ -164,9 +165,64 @@ static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
> return -EINVAL;
> }
>
> +static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
> + struct vm_area_struct *vma)
> +{
> + u64 size = vma->vm_end - vma->vm_start;
> + u64 pgoff, base;
> + u8 region_cacheable;
> +
> + pgoff = vma->vm_pgoff &
> + ((1U << (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
> + base = pgoff << PAGE_SHIFT;
> +
> + if (region.size < PAGE_SIZE || base + size > region.size)
> + return -EINVAL;
> +
> + region_cacheable = (region.type & FSL_MC_REGION_CACHEABLE) &&
> + (region.type & FSL_MC_REGION_SHAREABLE);
I see in fsl-mc-bus.c that IORESOURCE_CACHEABLE and IORESOURCE_MEM are
set on the regions flag?
> + if (!region_cacheable)
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
> +
> + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> + size, vma->vm_page_prot);
> +}
> +
> static int vfio_fsl_mc_mmap(void *device_data, struct vm_area_struct *vma)
> {
> - return -EINVAL;
> + struct vfio_fsl_mc_device *vdev = device_data;
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int index;
> +
> + index = vma->vm_pgoff >> (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT);
> +
> + if (vma->vm_end < vma->vm_start)
> + return -EINVAL;
> + if (vma->vm_start & ~PAGE_MASK)
> + return -EINVAL;
> + if (vma->vm_end & ~PAGE_MASK)
> + return -EINVAL;
> + if (!(vma->vm_flags & VM_SHARED))
> + return -EINVAL;
> + if (index >= vdev->num_regions)
> + return -EINVAL;
> +
> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
> + return -EINVAL;
> +
> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ)
> + && (vma->vm_flags & VM_READ))
> + return -EINVAL;
> +
> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)
> + && (vma->vm_flags & VM_WRITE))
> + return -EINVAL;
> +
> + vma->vm_private_data = mc_dev;
> +
> + return vfio_fsl_mc_mmap_mmio(vdev->regions[index], vma);
> }
>
> static const struct vfio_device_ops vfio_fsl_mc_ops = {
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling
2020-08-26 9:33 ` [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
@ 2020-09-03 19:55 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-03 19:55 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Diana
On 8/26/20 11:33 AM, Diana Craciun wrote:
> Only the DPRC object allocates interrupts from the MSI
> interrupt domain. The interrupts are managed by the DPRC in
> a pool of interrupts. The access to this pool of interrupts
> has to be protected with a lock.
> This patch extends the current lock implementation to have a
> lock per DPRC.
>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 91 +++++++++++++++++++++--
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 8 +-
> 2 files changed, 91 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 64d5c1fff51f..bbd3365e877e 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -17,6 +17,77 @@
>
> static struct fsl_mc_driver vfio_fsl_mc_driver;
>
> +static DEFINE_MUTEX(reflck_lock);
> +
> +static void vfio_fsl_mc_reflck_get(struct vfio_fsl_mc_reflck *reflck)
> +{
> + kref_get(&reflck->kref);
> +}
> +
> +static void vfio_fsl_mc_reflck_release(struct kref *kref)
> +{
> + struct vfio_fsl_mc_reflck *reflck = container_of(kref,
> + struct vfio_fsl_mc_reflck,
> + kref);
> +
> + mutex_destroy(&reflck->lock);
> + kfree(reflck);
> + mutex_unlock(&reflck_lock);
> +}
> +
> +static void vfio_fsl_mc_reflck_put(struct vfio_fsl_mc_reflck *reflck)
> +{
> + kref_put_mutex(&reflck->kref, vfio_fsl_mc_reflck_release, &reflck_lock);
> +}
> +
> +static struct vfio_fsl_mc_reflck *vfio_fsl_mc_reflck_alloc(void)
> +{
> + struct vfio_fsl_mc_reflck *reflck;
> +
> + reflck = kzalloc(sizeof(*reflck), GFP_KERNEL);
> + if (!reflck)
> + return ERR_PTR(-ENOMEM);
> +
> + kref_init(&reflck->kref);
> + mutex_init(&reflck->lock);
> +
> + return reflck;
> +}
> +
> +static int vfio_fsl_mc_reflck_attach(struct vfio_fsl_mc_device *vdev)
> +{
> + int ret = 0;
> +
> + mutex_lock(&reflck_lock);
> + if (is_fsl_mc_bus_dprc(vdev->mc_dev)) {
> + vdev->reflck = vfio_fsl_mc_reflck_alloc();
this can fail and if this happens I guess you shouldn't return 0.
> + } else {
> + struct device *mc_cont_dev = vdev->mc_dev->dev.parent;
> + struct vfio_device *device;
> + struct vfio_fsl_mc_device *cont_vdev;
> +
> + device = vfio_device_get_from_dev(mc_cont_dev);
> + if (!device) {
> + ret = -ENODEV;
> + goto unlock;
> + }
> +
> + cont_vdev = vfio_device_data(device);
are we sure cont_mdev always is != NULL?
> + if (!cont_vdev->reflck) {
> + vfio_device_put(device);
> + ret = -ENODEV;
> + goto unlock;
> + }
> + vfio_fsl_mc_reflck_get(cont_vdev->reflck);
> + vdev->reflck = cont_vdev->reflck;
> + vfio_device_put(device);
> + }
> +
> +unlock:
> + mutex_unlock(&reflck_lock);
> + return ret;
> +}
> +
> static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
> {
> struct fsl_mc_device *mc_dev = vdev->mc_dev;
> @@ -55,7 +126,7 @@ static int vfio_fsl_mc_open(void *device_data)
> if (!try_module_get(THIS_MODULE))
> return -ENODEV;
>
> - mutex_lock(&vdev->driver_lock);
> + mutex_lock(&vdev->reflck->lock);
> if (!vdev->refcnt) {
> ret = vfio_fsl_mc_regions_init(vdev);
> if (ret)
> @@ -63,12 +134,12 @@ static int vfio_fsl_mc_open(void *device_data)
> }
> vdev->refcnt++;
>
> - mutex_unlock(&vdev->driver_lock);
> + mutex_unlock(&vdev->reflck->lock);
>
> return 0;
>
> err_reg_init:
> - mutex_unlock(&vdev->driver_lock);
> + mutex_unlock(&vdev->reflck->lock);
> module_put(THIS_MODULE);
> return ret;
> }
> @@ -77,12 +148,12 @@ static void vfio_fsl_mc_release(void *device_data)
> {
> struct vfio_fsl_mc_device *vdev = device_data;
>
> - mutex_lock(&vdev->driver_lock);
> + mutex_lock(&vdev->reflck->lock);
>
> if (!(--vdev->refcnt))
> vfio_fsl_mc_regions_cleanup(vdev);
>
> - mutex_unlock(&vdev->driver_lock);
> + mutex_unlock(&vdev->reflck->lock);
>
> module_put(THIS_MODULE);
> }
> @@ -329,12 +400,18 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> return ret;
> }
>
> + ret = vfio_fsl_mc_reflck_attach(vdev);
> + if (ret) {
> + vfio_iommu_group_put(group, dev);
> + return ret;
> + }
> +
> ret = vfio_fsl_mc_init_device(vdev);
> if (ret < 0) {
> + vfio_fsl_mc_reflck_put(vdev->reflck);
> vfio_iommu_group_put(group, dev);
> return ret;
> }
> - mutex_init(&vdev->driver_lock);
>
> return ret;
> }
> @@ -358,7 +435,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>
> mc_dev->mc_io = NULL;
>
> - mutex_destroy(&vdev->driver_lock);
> + vfio_fsl_mc_reflck_put(vdev->reflck);
>
> vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> index 818dfd3df4db..3b85d930e060 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -15,6 +15,11 @@
> #define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
> ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
>
> +struct vfio_fsl_mc_reflck {
> + struct kref kref;
> + struct mutex lock;
> +};
> +
> struct vfio_fsl_mc_region {
> u32 flags;
> u32 type;
> @@ -28,7 +33,8 @@ struct vfio_fsl_mc_device {
> int refcnt;
> u32 num_regions;
> struct vfio_fsl_mc_region *regions;
> - struct mutex driver_lock;
> + struct vfio_fsl_mc_reflck *reflck;
> +
> };
>
> #endif /* VFIO_FSL_MC_PRIVATE_H */
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices
2020-08-26 9:33 ` [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
@ 2020-09-03 20:15 ` Auger Eric
2020-09-07 13:09 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-03 20:15 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> This patch adds the skeleton for interrupt support
> for fsl-mc devices. The interrupts are not yet functional,
> the functionality will be added by subsequent patches.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/Makefile | 2 +-
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 75 ++++++++++++++++++++++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 63 +++++++++++++++++++
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 7 ++-
> 4 files changed, 143 insertions(+), 4 deletions(-)
> create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>
> diff --git a/drivers/vfio/fsl-mc/Makefile b/drivers/vfio/fsl-mc/Makefile
> index 0c6e5d2ddaae..cad6dbf0b735 100644
> --- a/drivers/vfio/fsl-mc/Makefile
> +++ b/drivers/vfio/fsl-mc/Makefile
> @@ -1,4 +1,4 @@
> # SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>
> -vfio-fsl-mc-y := vfio_fsl_mc.o
> +vfio-fsl-mc-y := vfio_fsl_mc.o vfio_fsl_mc_intr.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
> index bbd3365e877e..42014297b484 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -209,11 +209,79 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
> }
> case VFIO_DEVICE_GET_IRQ_INFO:
> {
> - return -ENOTTY;
> + struct vfio_irq_info info;
> +
> + minsz = offsetofend(struct vfio_irq_info, count);
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + if (info.index >= mc_dev->obj_desc.irq_count)
> + return -EINVAL;
> +
> + info.flags = VFIO_IRQ_INFO_EVENTFD;
shouldn't it be MASKABLE as well? I see skeletons for MASK.
> + info.count = 1;
> +
> + return copy_to_user((void __user *)arg, &info, minsz);
> }
> case VFIO_DEVICE_SET_IRQS:
> {
> - return -ENOTTY;
> + struct vfio_irq_set hdr;
> + u8 *data = NULL;
> + int ret = 0;
> +
> + minsz = offsetofend(struct vfio_irq_set, count);
> +
> + if (copy_from_user(&hdr, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (hdr.argsz < minsz)
> + return -EINVAL;
> +
> + if (hdr.index >= mc_dev->obj_desc.irq_count)
> + return -EINVAL;
> +
> + if (hdr.start != 0 || hdr.count > 1)
> + return -EINVAL;
> +
> + if (hdr.count == 0 &&
> + (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE) ||
> + !(hdr.flags & VFIO_IRQ_SET_ACTION_TRIGGER)))
> + return -EINVAL;
> +
> + if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
> + VFIO_IRQ_SET_ACTION_TYPE_MASK))
> + return -EINVAL;
> +
> + if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
> + size_t size;
> +
> + if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
> + size = sizeof(uint8_t);
> + else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
> + size = sizeof(int32_t);
> + else
> + return -EINVAL;
> +
> + if (hdr.argsz - minsz < hdr.count * size)
> + return -EINVAL;
> +
> + data = memdup_user((void __user *)(arg + minsz),
> + hdr.count * size);
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> + }
can't you reuse vfio_set_irqs_validate_and_prepare()?
> +
> + mutex_lock(&vdev->igate);
> + ret = vfio_fsl_mc_set_irqs_ioctl(vdev, hdr.flags,
> + hdr.index, hdr.start,
> + hdr.count, data);
> + mutex_unlock(&vdev->igate);
> + kfree(data);
> +
> + return ret;
> }
> case VFIO_DEVICE_RESET:
> {
> @@ -413,6 +481,8 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
> return ret;
> }
>
> + mutex_init(&vdev->igate);
> +
> return ret;
> }
>
> @@ -436,6 +506,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
> mc_dev->mc_io = NULL;
>
> vfio_fsl_mc_reflck_put(vdev->reflck);
> + mutex_destroy(&vdev->igate);
>
> vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> new file mode 100644
> index 000000000000..058aa97aa54a
> --- /dev/null
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
> +/*
> + * Copyright 2013-2016 Freescale Semiconductor Inc.
> + * Copyright 2019 NXP
> + */
> +
> +#include <linux/vfio.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/eventfd.h>
> +#include <linux/msi.h>
> +
> +#include "linux/fsl/mc.h"
> +#include "vfio_fsl_mc_private.h"
> +
> +static int vfio_fsl_mc_irq_mask(struct vfio_fsl_mc_device *vdev,
> + unsigned int index, unsigned int start,
> + unsigned int count, u32 flags,
> + void *data)
> +{
> + return -EINVAL;
> +}
> +
> +static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
> + unsigned int index, unsigned int start,
> + unsigned int count, u32 flags,
> + void *data)
> +{
> + return -EINVAL;
> +}
> +
> +static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
> + unsigned int index, unsigned int start,
> + unsigned int count, u32 flags,
> + void *data)
> +{
> + return -EINVAL;
> +}
> +
> +int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
> + u32 flags, unsigned int index,
> + unsigned int start, unsigned int count,
> + void *data)
> +{
> + int ret = -ENOTTY;
> +
> + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
> + case VFIO_IRQ_SET_ACTION_MASK:
> + ret = vfio_fsl_mc_irq_mask(vdev, index, start, count,
> + flags, data);
> + break;
> + case VFIO_IRQ_SET_ACTION_UNMASK:
> + ret = vfio_fsl_mc_irq_unmask(vdev, index, start, count,
> + flags, data);
> + break;
> + case VFIO_IRQ_SET_ACTION_TRIGGER:
> + ret = vfio_fsl_mc_set_irq_trigger(vdev, index, start,
> + count, flags, data);
> + break;
> + }
> +
> + 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 3b85d930e060..d5b6fe891a48 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -34,7 +34,12 @@ struct vfio_fsl_mc_device {
> u32 num_regions;
> struct vfio_fsl_mc_region *regions;
> struct vfio_fsl_mc_reflck *reflck;
> -
> + struct mutex igate;
> };
>
> +extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
> + u32 flags, unsigned int index,
> + unsigned int start, unsigned int count,
> + void *data);
> +
> #endif /* VFIO_FSL_MC_PRIVATE_H */
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
2020-08-26 9:33 ` [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
@ 2020-09-04 8:02 ` Auger Eric
2020-09-07 14:02 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-04 8:02 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> This patch allows to set an eventfd for fsl-mc device interrupts
> and also to trigger the interrupt eventfd from userspace for testing.
>
> All fsl-mc device interrupts are MSIs. The MSIs are allocated from
> the MSI domain only once per DPRC and used by all the DPAA2 objects.
> The interrupts are managed by the DPRC in a pool of interrupts. Each
> device requests interrupts from this pool. The pool is allocated
> when the first virtual device is setting the interrupts.
> The pool of interrupts is protected by a lock.
>
> The DPRC has an interrupt of its own which indicates if the DPRC
> contents have changed. However, currently, the contents of a DPRC
> assigned to the guest cannot be changed at runtime, so this interrupt
> is not configured.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 18 ++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 160 +++++++++++++++++++++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 10 ++
> 3 files changed, 186 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 42014297b484..73834f488a94 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -147,12 +147,28 @@ static int vfio_fsl_mc_open(void *device_data)
> static void vfio_fsl_mc_release(void *device_data)
> {
> struct vfio_fsl_mc_device *vdev = device_data;
> + int ret;
>
> mutex_lock(&vdev->reflck->lock);
>
> - if (!(--vdev->refcnt))
> + if (!(--vdev->refcnt)) {
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
> +
> vfio_fsl_mc_regions_cleanup(vdev);
>
> + /* reset the device before cleaning up the interrupts */
> + ret = dprc_reset_container(mc_cont->mc_io, 0,
> + mc_cont->mc_handle,
> + mc_cont->obj_desc.id,
> + DPRC_RESET_OPTION_NON_RECURSIVE);
shouldn't you test ret?
> +
> + vfio_fsl_mc_irqs_cleanup(vdev);
> +
> + fsl_mc_cleanup_irq_pool(mc_cont);
> + }
> +
> mutex_unlock(&vdev->reflck->lock);
>
> module_put(THIS_MODULE);
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> index 058aa97aa54a..409f3507fcf3 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> @@ -29,12 +29,149 @@ static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
> return -EINVAL;
> }
>
> +int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
> +{
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + struct vfio_fsl_mc_irq *mc_irq;
> + int irq_count;
> + int ret, i;
> +
> + /* Device does not support any interrupt */
indent needs to be fixed
> + if (mc_dev->obj_desc.irq_count == 0)
> + return 0;
> +
> + /* interrupts were already allocated for this device */
> + if (vdev->mc_irqs)
> + return 0;
> +
> + irq_count = mc_dev->obj_desc.irq_count;
> +
> + mc_irq = kcalloc(irq_count, sizeof(*mc_irq), GFP_KERNEL);
> + if (!mc_irq)
> + return -ENOMEM;
> +
> + /* Allocate IRQs */
> + ret = fsl_mc_allocate_irqs(mc_dev);
> + if (ret) {
> + kfree(mc_irq);
> + return ret;
> + }
> +
> + for (i = 0; i < irq_count; i++) {
> + mc_irq[i].count = 1;
> + mc_irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
> + }
> +
> + vdev->mc_irqs = mc_irq;
> +
> + return 0;
> +}
> +
> +static irqreturn_t vfio_fsl_mc_irq_handler(int irq_num, void *arg)
> +{
> + struct vfio_fsl_mc_irq *mc_irq = (struct vfio_fsl_mc_irq *)arg;
> +
> + eventfd_signal(mc_irq->trigger, 1);
> + return IRQ_HANDLED;
> +}
> +
> +static int vfio_set_trigger(struct vfio_fsl_mc_device *vdev,
> + int index, int fd)
> +{
> + struct vfio_fsl_mc_irq *irq = &vdev->mc_irqs[index];
> + struct eventfd_ctx *trigger;
> + int hwirq;
> + int ret;
> +
> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
> + if (irq->trigger) {
> + free_irq(hwirq, irq);
> + kfree(irq->name);
> + eventfd_ctx_put(irq->trigger);
> + irq->trigger = NULL;
> + }
> +
> + if (fd < 0) /* Disable only */
> + return 0;
> +
> + irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)",
> + hwirq, dev_name(&vdev->mc_dev->dev));
> + if (!irq->name)
> + return -ENOMEM;
> +
> + trigger = eventfd_ctx_fdget(fd);
> + if (IS_ERR(trigger)) {
> + kfree(irq->name);
> + return PTR_ERR(trigger);
> + }
> +
> + irq->trigger = trigger;
> +
> + ret = request_irq(hwirq, vfio_fsl_mc_irq_handler, 0,
> + irq->name, irq);
> + if (ret) {
> + kfree(irq->name);
> + eventfd_ctx_put(trigger);
> + irq->trigger = NULL;
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
> unsigned int index, unsigned int start,
> unsigned int count, u32 flags,
> void *data)
> {
> - return -EINVAL;
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int ret, hwirq;
> + struct vfio_fsl_mc_irq *irq;
> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
> +
> + if (start != 0 || count != 1)
> + return -EINVAL;
> +
> + mutex_lock(&vdev->reflck->lock);
> + ret = fsl_mc_populate_irq_pool(mc_cont,
> + FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
> + if (ret)
> + goto unlock;
> +
> + ret = vfio_fsl_mc_irqs_allocate(vdev);
any reason the init is done in the set_irq() and not in the open() if
!vdev->refcnt?
> + if (ret)
> + goto unlock;
> + mutex_unlock(&vdev->reflck->lock);
> +
> + if (!count && (flags & VFIO_IRQ_SET_DATA_NONE))
> + return vfio_set_trigger(vdev, index, -1);
> +
> + if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
> + s32 fd = *(s32 *)data;
> +
> + return vfio_set_trigger(vdev, index, fd);
> + }
> +
> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
> +
> + irq = &vdev->mc_irqs[index];
> +
> + if (flags & VFIO_IRQ_SET_DATA_NONE) {
> + vfio_fsl_mc_irq_handler(hwirq, irq);
> +
> + } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
> + u8 trigger = *(u8 *)data;
> +
> + if (trigger)
> + vfio_fsl_mc_irq_handler(hwirq, irq);
> + }
> +
> + return 0;
> +
> +unlock:
> + mutex_unlock(&vdev->reflck->lock);
> + return ret;
> }
>
> int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
> @@ -61,3 +198,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>
> return ret;
> }
> +
> +/* Free All IRQs for the given MC object */
> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev)
> +{
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int irq_count = mc_dev->obj_desc.irq_count;
> + int i;
> +
> + /* Device does not support any interrupt or the interrupts
> + * were not configured
> + */
> + if (mc_dev->obj_desc.irq_count == 0 || !vdev->mc_irqs)
> + return;
> +
> + for (i = 0; i < irq_count; i++)
> + vfio_set_trigger(vdev, i, -1);
> +
> + fsl_mc_free_irqs(mc_dev);
> + kfree(vdev->mc_irqs);
> + vdev->mc_irqs = NULL;
> +}
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> index d5b6fe891a48..bbfca8b55f8a 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -15,6 +15,13 @@
> #define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
> ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
>
> +struct vfio_fsl_mc_irq {
> + u32 flags;
> + u32 count;
> + struct eventfd_ctx *trigger;
> + char *name;
> +};
> +
> struct vfio_fsl_mc_reflck {
> struct kref kref;
> struct mutex lock;
> @@ -35,6 +42,7 @@ struct vfio_fsl_mc_device {
> struct vfio_fsl_mc_region *regions;
> struct vfio_fsl_mc_reflck *reflck;
> struct mutex igate;
> + struct vfio_fsl_mc_irq *mc_irqs;
> };
>
> extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
> @@ -42,4 +50,6 @@ extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
> unsigned int start, unsigned int count,
> void *data);
>
> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev);
> +
> #endif /* VFIO_FSL_MC_PRIVATE_H */
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device
2020-09-03 13:40 ` [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Auger Eric
@ 2020-09-04 8:03 ` Diana Craciun OSS
2020-09-04 8:25 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-04 8:03 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Eric,
On 9/3/2020 4:40 PM, Auger Eric wrote:
>> 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/msg3639226.html
> Could you share a branch with both series? This would help the review.
> Thanks Eric
I have pushed both the series here:
https://source.codeaurora.org/external/qoriq/qoriq-components/linux-extras/log/?h=dpaa2_direct_assignment
Regards,
Diana
PS: I apologize if you received the message twice, I have sent it by
mistake as html first.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
2020-08-26 9:33 ` [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
@ 2020-09-04 8:18 ` Auger Eric
2020-09-07 14:34 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-04 8:18 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> The software uses a memory-mapped I/O command interface (MC portals) to
> communicate with the MC hardware. This command interface is used to
> discover, enumerate, configure and remove DPAA2 objects. The DPAA2
> objects use MSIs, so the command interface needs to be emulated
> such that the correct MSI is configured in the hardware (the guest
> has the virtual MSIs).
What I don't get is all the regions are mmappable too.
And this patch does not seem to introduce special handling with respect
to MSIs. Please could you clarify?
>
> This patch is adding read/write support for fsl-mc devices. The mc
> commands are emulated by the userspace. The host is just passing
> the correct command to the hardware.
>
> Also the current patch limits userspace to write complete
> 64byte command once and read 64byte response by one ioctl.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 115 +++++++++++++++++++++-
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
> 2 files changed, 114 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 73834f488a94..27713aa86878 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -12,6 +12,7 @@
> #include <linux/types.h>
> #include <linux/vfio.h>
> #include <linux/fsl/mc.h>
> +#include <linux/delay.h>
>
> #include "vfio_fsl_mc_private.h"
>
> @@ -106,6 +107,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
> vdev->regions[i].size = resource_size(res);
> vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
> vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
> + if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
> }
>
> vdev->num_regions = mc_dev->obj_desc.region_count;
> @@ -114,6 +118,11 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>
> static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
> {
> + int i;
> +
> + for (i = 0; i < vdev->num_regions; i++)
> + iounmap(vdev->regions[i].ioaddr);
> +
> vdev->num_regions = 0;
> kfree(vdev->regions);
> }
> @@ -311,13 +320,115 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
> static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return -EINVAL;
> + struct vfio_fsl_mc_device *vdev = device_data;
> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
> + struct vfio_fsl_mc_region *region;
> + u64 data[8];
> + int i;
> +
> + if (index >= vdev->num_regions)
> + return -EINVAL;
> +
> + region = &vdev->regions[index];
> +
> + if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
> + return -EINVAL;
> +
> + if (!region->ioaddr) {
> + region->ioaddr = ioremap(region->addr, region->size);
> + if (!region->ioaddr)
> + return -ENOMEM;
> + }
> +
> + if (count != 64 || off != 0)
> + return -EINVAL;
> +
> + for (i = 7; i >= 0; i--)
> + data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
> +
> + if (copy_to_user(buf, data, 64))
> + return -EFAULT;
> +
> + return count;
> +}
> +
> +#define MC_CMD_COMPLETION_TIMEOUT_MS 5000
> +#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS 500
> +
> +static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
> +{
> + int i;
> + enum mc_cmd_status status;
> + unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
> +
> + /* Write at command parameter into portal */
> + for (i = 7; i >= 1; i--)
> + writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
> +
> + /* Write command header in the end */
> + writeq(cmd_data[0], ioaddr);
> +
> + /* Wait for response before returning to user-space
> + * This can be optimized in future to even prepare response
> + * before returning to user-space and avoid read ioctl.
> + */
> + for (;;) {
> + u64 header;
> + struct mc_cmd_header *resp_hdr;
> +
> + header = cpu_to_le64(readq_relaxed(ioaddr));
> +
> + resp_hdr = (struct mc_cmd_header *)&header;
> + status = (enum mc_cmd_status)resp_hdr->status;
> + if (status != MC_CMD_STATUS_READY)
> + break;
> +
> + udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
> + timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
> + if (timeout_usecs == 0)
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> }
>
> static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return -EINVAL;
> + struct vfio_fsl_mc_device *vdev = device_data;
> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
> + struct vfio_fsl_mc_region *region;
> + u64 data[8];
> + int ret;
> +
> + if (index >= vdev->num_regions)
> + return -EINVAL;
> +
> + region = &vdev->regions[index];
> +
> + if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE))
> + return -EINVAL;
> +
> + if (!region->ioaddr) {
> + region->ioaddr = ioremap(region->addr, region->size);
> + if (!region->ioaddr)
> + return -ENOMEM;
> + }
> +
> + if (count != 64 || off != 0)
> + return -EINVAL;
> +
> + if (copy_from_user(&data, buf, 64))
> + return -EFAULT;
> +
> + ret = vfio_fsl_mc_send_command(region->ioaddr, data);
> + if (ret)
> + return ret;
> +
> + return count;
> +
> }
>
> static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> index bbfca8b55f8a..e6804e516c4a 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
> @@ -32,6 +32,7 @@ struct vfio_fsl_mc_region {
> u32 type;
> u64 addr;
> resource_size_t size;
> + void __iomem *ioaddr;
> };
>
> struct vfio_fsl_mc_device {
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset
2020-08-26 9:33 ` [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
@ 2020-09-04 8:21 ` Auger Eric
2020-09-07 14:36 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-04 8:21 UTC (permalink / raw)
To: Diana Craciun, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Diana,
On 8/26/20 11:33 AM, Diana Craciun wrote:
> Currently only resetting the DPRC container is supported which
> will reset all the objects inside it. Resetting individual
> objects is possible from the userspace by issueing commands
> towards MC firmware.
>
> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 27713aa86878..d17c5b3148ad 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -310,7 +310,20 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
> }
> case VFIO_DEVICE_RESET:
> {
> - return -ENOTTY;
> + int ret = 0;
initialization not needed
> +
spare empty line
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> +
> + /* reset is supported only for the DPRC */
> + if (!is_fsl_mc_bus_dprc(mc_dev))
> + return -ENOTTY;
it is an error case or do we just don't care?
> +
> + ret = dprc_reset_container(mc_dev->mc_io, 0,
> + mc_dev->mc_handle,
> + mc_dev->obj_desc.id,
> + DPRC_RESET_OPTION_NON_RECURSIVE);
> + return ret;
> +
> }
> default:
> return -ENOTTY;
>
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device
2020-09-04 8:03 ` Diana Craciun OSS
@ 2020-09-04 8:25 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-04 8:25 UTC (permalink / raw)
To: Diana Craciun OSS, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Diana,
On 9/4/20 10:03 AM, Diana Craciun OSS wrote:
> Hi Eric,
>
> On 9/3/2020 4:40 PM, Auger Eric wrote:
>>> 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/msg3639226.html
>> Could you share a branch with both series? This would help the review.
>> Thanks Eric
>
> I have pushed both the series here:
> https://source.codeaurora.org/external/qoriq/qoriq-components/linux-extras/log/?h=dpaa2_direct_assignment
>
>
> Regards,
>
> Diana
>
> PS: I apologize if you received the message twice, I have sent it by
> mistake as html first.
>
No Problem. Thank you for the branch. I have completed a first review
pass. Hope this helps.
Thanks
Eric
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices
2020-09-03 14:06 ` Auger Eric
@ 2020-09-04 13:59 ` Diana Craciun OSS
2020-09-04 14:11 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-04 13:59 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/3/2020 5:06 PM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> From: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>>
>> 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 <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> 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 3b186ade3597..f3f9ea108588 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -18229,6 +18229,12 @@ F: drivers/vfio/
>> F: include/linux/vfio.h
>> F: include/uapi/linux/vfio.h
>>
>> +VFIO FSL-MC DRIVER
>> +M: Diana Craciun <diana.craciun@oss.nxp.com>
>> +L: kvm@vger.kernel.org
>> +S: Maintained
>> +F: drivers/vfio/fsl-mc/
>> +
>> VFIO MEDIATED DEVICE DRIVERS
>> M: Kirti Wankhede <kwankhede@nvidia.com>
>> L: kvm@vger.kernel.org
>> 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;
> nit: introduce out_group_put: as in other files
> This will be usable also in subsequent patches
>> +}
>> +
>> +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,
> not needed?
The driver will always match on driver_override, there is no need for
static ids.
>> + .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");
> Don't you need MODULE_LICENSE("Dual BSD/GPL"); ?
Yes, will change.
>> +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 */
>> };
>>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices
2020-09-04 13:59 ` Diana Craciun OSS
@ 2020-09-04 14:11 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-04 14:11 UTC (permalink / raw)
To: Diana Craciun OSS, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 9/4/20 3:59 PM, Diana Craciun OSS wrote:
> Hi Eric,
>
> On 9/3/2020 5:06 PM, Auger Eric wrote:
>> Hi Diana,
>>
>> On 8/26/20 11:33 AM, Diana Craciun wrote:
>>> From: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>>>
>>> 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 <Bharat.Bhushan@nxp.com>
>>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>>> ---
>>> 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 3b186ade3597..f3f9ea108588 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -18229,6 +18229,12 @@ F: drivers/vfio/
>>> F: include/linux/vfio.h
>>> F: include/uapi/linux/vfio.h
>>> +VFIO FSL-MC DRIVER
>>> +M: Diana Craciun <diana.craciun@oss.nxp.com>
>>> +L: kvm@vger.kernel.org
>>> +S: Maintained
>>> +F: drivers/vfio/fsl-mc/
>>> +
>>> VFIO MEDIATED DEVICE DRIVERS
>>> M: Kirti Wankhede <kwankhede@nvidia.com>
>>> L: kvm@vger.kernel.org
>>> 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;
>> nit: introduce out_group_put: as in other files
>> This will be usable also in subsequent patches
>>> +}
>>> +
>>> +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,
>> not needed?
>
> The driver will always match on driver_override, there is no need for
> static ids.
I rather meant do you really need to initialize it? I guess it is the
same in vfio_platform and there is no such init.
Thanks
Eric
>
>>> + .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");
>> Don't you need MODULE_LICENSE("Dual BSD/GPL"); ?
>
> Yes, will change.
>
>>> +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 */
>>> };
>>>
>> Thanks
>>
>> Eric
>>
>
> Thanks,
> Diana
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions
2020-09-03 16:05 ` Auger Eric
@ 2020-09-07 12:49 ` Diana Craciun OSS
0 siblings, 0 replies; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 12:49 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/3/2020 7:05 PM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> Allow userspace to mmap device regions for direct access of
>> fsl-mc devices.
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 60 +++++++++++++++++++++++++++++--
>> 1 file changed, 58 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 093b8d68496c..64d5c1fff51f 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> @@ -33,7 +33,8 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>>
>> vdev->regions[i].addr = res->start;
>> vdev->regions[i].size = resource_size(res);
>> - vdev->regions[i].flags = 0;
>> + vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
> Is the region always mmappable or does it depend on the
> mc_dev->regions[i].flags. Also on VFIO platform we checked some
> alignment addr/size constraints.
The region is mmappable regardless of the region flags. However, I see
that there are some questions regarding the fact that the regions are
always mmappable in the following patches, so I'll try to clarify here.
The way the userspace communicates with the hardware is through some
commands (special data written in the device region). The commands can
be issued using special channels (devices): dprc and dpmcp. Most of the
commands can be passthrough, the command that configures the interrupts
is the most important example. In order to reduce the complexity, the
command which configures the interrupts was restricted from the firmware
to be issued only on a single type of device (dprc). However, in the
current implementation the memory region corresponding to the dpcr can
be passthorugh as well. The reason is that it can be used (for example)
by a DPDK application in the userspace. The DPDK application configures
the interrupts using the VFIO_DEVICE_SET_IRQS system call. When it comes
to virtual machines the dprc and the dpmc will be emulated in QEMU.
Regarding the alignemnet/size constraints I agree, I will add some checks.
>> + vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
>> }
>>
>> vdev->num_regions = mc_dev->obj_desc.region_count;
>> @@ -164,9 +165,64 @@ static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
>> return -EINVAL;
>> }
>>
>> +static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
>> + struct vm_area_struct *vma)
>> +{
>> + u64 size = vma->vm_end - vma->vm_start;
>> + u64 pgoff, base;
>> + u8 region_cacheable;
>> +
>> + pgoff = vma->vm_pgoff &
>> + ((1U << (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
>> + base = pgoff << PAGE_SHIFT;
>> +
>> + if (region.size < PAGE_SIZE || base + size > region.size)
>> + return -EINVAL;
>> +
>> + region_cacheable = (region.type & FSL_MC_REGION_CACHEABLE) &&
>> + (region.type & FSL_MC_REGION_SHAREABLE);
> I see in fsl-mc-bus.c that IORESOURCE_CACHEABLE and IORESOURCE_MEM are
> set on the regions flag?
Yes, initially the two flags were set (IORESOURCE_CACHEABLE and
IORESOURCE_MEM). However, I have changed the behaviour a little bit (it
was wrong in the past) and IORESOURCE_MEM is still present, but also
FSL_MC_REGION_CACHEABLE and FSL_MC_REGION_SHAREABLE which are retrieved
from the firmware through commands.
>> + if (!region_cacheable)
>> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>> +
>> + vma->vm_pgoff = (region.addr >> PAGE_SHIFT) + pgoff;
>> +
>> + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
>> + size, vma->vm_page_prot);
>> +}
>> +
>> static int vfio_fsl_mc_mmap(void *device_data, struct vm_area_struct *vma)
>> {
>> - return -EINVAL;
>> + struct vfio_fsl_mc_device *vdev = device_data;
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> + int index;
>> +
>> + index = vma->vm_pgoff >> (VFIO_FSL_MC_OFFSET_SHIFT - PAGE_SHIFT);
>> +
>> + if (vma->vm_end < vma->vm_start)
>> + return -EINVAL;
>> + if (vma->vm_start & ~PAGE_MASK)
>> + return -EINVAL;
>> + if (vma->vm_end & ~PAGE_MASK)
>> + return -EINVAL;
>> + if (!(vma->vm_flags & VM_SHARED))
>> + return -EINVAL;
>> + if (index >= vdev->num_regions)
>> + return -EINVAL;
>> +
>> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_MMAP))
>> + return -EINVAL;
>> +
>> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_READ)
>> + && (vma->vm_flags & VM_READ))
>> + return -EINVAL;
>> +
>> + if (!(vdev->regions[index].flags & VFIO_REGION_INFO_FLAG_WRITE)
>> + && (vma->vm_flags & VM_WRITE))
>> + return -EINVAL;
>> +
>> + vma->vm_private_data = mc_dev;
>> +
>> + return vfio_fsl_mc_mmap_mmio(vdev->regions[index], vma);
>> }
>>
>> static const struct vfio_device_ops vfio_fsl_mc_ops = {
>>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices
2020-09-03 20:15 ` Auger Eric
@ 2020-09-07 13:09 ` Diana Craciun OSS
0 siblings, 0 replies; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 13:09 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/3/2020 11:15 PM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> This patch adds the skeleton for interrupt support
>> for fsl-mc devices. The interrupts are not yet functional,
>> the functionality will be added by subsequent patches.
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/Makefile | 2 +-
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 75 ++++++++++++++++++++++-
>> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 63 +++++++++++++++++++
>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 7 ++-
>> 4 files changed, 143 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>>
>> diff --git a/drivers/vfio/fsl-mc/Makefile b/drivers/vfio/fsl-mc/Makefile
>> index 0c6e5d2ddaae..cad6dbf0b735 100644
>> --- a/drivers/vfio/fsl-mc/Makefile
>> +++ b/drivers/vfio/fsl-mc/Makefile
>> @@ -1,4 +1,4 @@
>> # SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>>
>> -vfio-fsl-mc-y := vfio_fsl_mc.o
>> +vfio-fsl-mc-y := vfio_fsl_mc.o vfio_fsl_mc_intr.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
>> index bbd3365e877e..42014297b484 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> @@ -209,11 +209,79 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
>> }
>> case VFIO_DEVICE_GET_IRQ_INFO:
>> {
>> - return -ENOTTY;
>> + struct vfio_irq_info info;
>> +
>> + minsz = offsetofend(struct vfio_irq_info, count);
>> + if (copy_from_user(&info, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (info.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (info.index >= mc_dev->obj_desc.irq_count)
>> + return -EINVAL;
>> +
>> + info.flags = VFIO_IRQ_INFO_EVENTFD;
> shouldn't it be MASKABLE as well? I see skeletons for MASK.
The skeletons for mask are not implemented. Maybe I should just remove
the skeletons.
>> + info.count = 1;
>> +
>> + return copy_to_user((void __user *)arg, &info, minsz);
>> }
>> case VFIO_DEVICE_SET_IRQS:
>> {
>> - return -ENOTTY;
>> + struct vfio_irq_set hdr;
>> + u8 *data = NULL;
>> + int ret = 0;
>> +
>> + minsz = offsetofend(struct vfio_irq_set, count);
>> +
>> + if (copy_from_user(&hdr, (void __user *)arg, minsz))
>> + return -EFAULT;
>> +
>> + if (hdr.argsz < minsz)
>> + return -EINVAL;
>> +
>> + if (hdr.index >= mc_dev->obj_desc.irq_count)
>> + return -EINVAL;
>> +
>> + if (hdr.start != 0 || hdr.count > 1)
>> + return -EINVAL;
>> +
>> + if (hdr.count == 0 &&
>> + (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE) ||
>> + !(hdr.flags & VFIO_IRQ_SET_ACTION_TRIGGER)))
>> + return -EINVAL;
>> +
>> + if (hdr.flags & ~(VFIO_IRQ_SET_DATA_TYPE_MASK |
>> + VFIO_IRQ_SET_ACTION_TYPE_MASK))
>> + return -EINVAL;
>> +
>> + if (!(hdr.flags & VFIO_IRQ_SET_DATA_NONE)) {
>> + size_t size;
>> +
>> + if (hdr.flags & VFIO_IRQ_SET_DATA_BOOL)
>> + size = sizeof(uint8_t);
>> + else if (hdr.flags & VFIO_IRQ_SET_DATA_EVENTFD)
>> + size = sizeof(int32_t);
>> + else
>> + return -EINVAL;
>> +
>> + if (hdr.argsz - minsz < hdr.count * size)
>> + return -EINVAL;
>> +
>> + data = memdup_user((void __user *)(arg + minsz),
>> + hdr.count * size);
>> + if (IS_ERR(data))
>> + return PTR_ERR(data);
>> + }
> can't you reuse vfio_set_irqs_validate_and_prepare()?
Yes, I think I can reuse it.
>> +
>> + mutex_lock(&vdev->igate);
>> + ret = vfio_fsl_mc_set_irqs_ioctl(vdev, hdr.flags,
>> + hdr.index, hdr.start,
>> + hdr.count, data);
>> + mutex_unlock(&vdev->igate);
>> + kfree(data);
>> +
>> + return ret;
>> }
>> case VFIO_DEVICE_RESET:
>> {
>> @@ -413,6 +481,8 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>> return ret;
>> }
>>
>> + mutex_init(&vdev->igate);
>> +
>> return ret;
>> }
>>
>> @@ -436,6 +506,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>> mc_dev->mc_io = NULL;
>>
>> vfio_fsl_mc_reflck_put(vdev->reflck);
>> + mutex_destroy(&vdev->igate);
>>
>> vfio_iommu_group_put(mc_dev->dev.iommu_group, dev);
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>> new file mode 100644
>> index 000000000000..058aa97aa54a
>> --- /dev/null
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>> @@ -0,0 +1,63 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
>> +/*
>> + * Copyright 2013-2016 Freescale Semiconductor Inc.
>> + * Copyright 2019 NXP
>> + */
>> +
>> +#include <linux/vfio.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +#include <linux/eventfd.h>
>> +#include <linux/msi.h>
>> +
>> +#include "linux/fsl/mc.h"
>> +#include "vfio_fsl_mc_private.h"
>> +
>> +static int vfio_fsl_mc_irq_mask(struct vfio_fsl_mc_device *vdev,
>> + unsigned int index, unsigned int start,
>> + unsigned int count, u32 flags,
>> + void *data)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
>> + unsigned int index, unsigned int start,
>> + unsigned int count, u32 flags,
>> + void *data)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
>> + unsigned int index, unsigned int start,
>> + unsigned int count, u32 flags,
>> + void *data)
>> +{
>> + return -EINVAL;
>> +}
>> +
>> +int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>> + u32 flags, unsigned int index,
>> + unsigned int start, unsigned int count,
>> + void *data)
>> +{
>> + int ret = -ENOTTY;
>> +
>> + switch (flags & VFIO_IRQ_SET_ACTION_TYPE_MASK) {
>> + case VFIO_IRQ_SET_ACTION_MASK:
>> + ret = vfio_fsl_mc_irq_mask(vdev, index, start, count,
>> + flags, data);
>> + break;
>> + case VFIO_IRQ_SET_ACTION_UNMASK:
>> + ret = vfio_fsl_mc_irq_unmask(vdev, index, start, count,
>> + flags, data);
>> + break;
>> + case VFIO_IRQ_SET_ACTION_TRIGGER:
>> + ret = vfio_fsl_mc_set_irq_trigger(vdev, index, start,
>> + count, flags, data);
>> + break;
>> + }
>> +
>> + 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 3b85d930e060..d5b6fe891a48 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> @@ -34,7 +34,12 @@ struct vfio_fsl_mc_device {
>> u32 num_regions;
>> struct vfio_fsl_mc_region *regions;
>> struct vfio_fsl_mc_reflck *reflck;
>> -
>> + struct mutex igate;
>> };
>>
>> +extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>> + u32 flags, unsigned int index,
>> + unsigned int start, unsigned int count,
>> + void *data);
>> +
>> #endif /* VFIO_FSL_MC_PRIVATE_H */
>>
> Thanks
>
> Eric
>
Regards,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
2020-09-04 8:02 ` Auger Eric
@ 2020-09-07 14:02 ` Diana Craciun OSS
2020-09-10 8:16 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 14:02 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/4/2020 11:02 AM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> This patch allows to set an eventfd for fsl-mc device interrupts
>> and also to trigger the interrupt eventfd from userspace for testing.
>>
>> All fsl-mc device interrupts are MSIs. The MSIs are allocated from
>> the MSI domain only once per DPRC and used by all the DPAA2 objects.
>> The interrupts are managed by the DPRC in a pool of interrupts. Each
>> device requests interrupts from this pool. The pool is allocated
>> when the first virtual device is setting the interrupts.
>> The pool of interrupts is protected by a lock.
>>
>> The DPRC has an interrupt of its own which indicates if the DPRC
>> contents have changed. However, currently, the contents of a DPRC
>> assigned to the guest cannot be changed at runtime, so this interrupt
>> is not configured.
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 18 ++-
>> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 160 +++++++++++++++++++++-
>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 10 ++
>> 3 files changed, 186 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 42014297b484..73834f488a94 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> @@ -147,12 +147,28 @@ static int vfio_fsl_mc_open(void *device_data)
>> static void vfio_fsl_mc_release(void *device_data)
>> {
>> struct vfio_fsl_mc_device *vdev = device_data;
>> + int ret;
>>
>> mutex_lock(&vdev->reflck->lock);
>>
>> - if (!(--vdev->refcnt))
>> + if (!(--vdev->refcnt)) {
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
>> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
>> +
>> vfio_fsl_mc_regions_cleanup(vdev);
>>
>> + /* reset the device before cleaning up the interrupts */
>> + ret = dprc_reset_container(mc_cont->mc_io, 0,
>> + mc_cont->mc_handle,
>> + mc_cont->obj_desc.id,
>> + DPRC_RESET_OPTION_NON_RECURSIVE);
> shouldn't you test ret?
>> +
>> + vfio_fsl_mc_irqs_cleanup(vdev);
>> +
>> + fsl_mc_cleanup_irq_pool(mc_cont);
>> + }
>> +
>> mutex_unlock(&vdev->reflck->lock);
>>
>> module_put(THIS_MODULE);
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>> index 058aa97aa54a..409f3507fcf3 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>> @@ -29,12 +29,149 @@ static int vfio_fsl_mc_irq_unmask(struct vfio_fsl_mc_device *vdev,
>> return -EINVAL;
>> }
>>
>> +int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
>> +{
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> + struct vfio_fsl_mc_irq *mc_irq;
>> + int irq_count;
>> + int ret, i;
>> +
>> + /* Device does not support any interrupt */
> indent needs to be fixed
>> + if (mc_dev->obj_desc.irq_count == 0)
>> + return 0;
>> +
>> + /* interrupts were already allocated for this device */
>> + if (vdev->mc_irqs)
>> + return 0;
>> +
>> + irq_count = mc_dev->obj_desc.irq_count;
>> +
>> + mc_irq = kcalloc(irq_count, sizeof(*mc_irq), GFP_KERNEL);
>> + if (!mc_irq)
>> + return -ENOMEM;
>> +
>> + /* Allocate IRQs */
>> + ret = fsl_mc_allocate_irqs(mc_dev);
>> + if (ret) {
>> + kfree(mc_irq);
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < irq_count; i++) {
>> + mc_irq[i].count = 1;
>> + mc_irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
>> + }
>> +
>> + vdev->mc_irqs = mc_irq;
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t vfio_fsl_mc_irq_handler(int irq_num, void *arg)
>> +{
>> + struct vfio_fsl_mc_irq *mc_irq = (struct vfio_fsl_mc_irq *)arg;
>> +
>> + eventfd_signal(mc_irq->trigger, 1);
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int vfio_set_trigger(struct vfio_fsl_mc_device *vdev,
>> + int index, int fd)
>> +{
>> + struct vfio_fsl_mc_irq *irq = &vdev->mc_irqs[index];
>> + struct eventfd_ctx *trigger;
>> + int hwirq;
>> + int ret;
>> +
>> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
>> + if (irq->trigger) {
>> + free_irq(hwirq, irq);
>> + kfree(irq->name);
>> + eventfd_ctx_put(irq->trigger);
>> + irq->trigger = NULL;
>> + }
>> +
>> + if (fd < 0) /* Disable only */
>> + return 0;
>> +
>> + irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)",
>> + hwirq, dev_name(&vdev->mc_dev->dev));
>> + if (!irq->name)
>> + return -ENOMEM;
>> +
>> + trigger = eventfd_ctx_fdget(fd);
>> + if (IS_ERR(trigger)) {
>> + kfree(irq->name);
>> + return PTR_ERR(trigger);
>> + }
>> +
>> + irq->trigger = trigger;
>> +
>> + ret = request_irq(hwirq, vfio_fsl_mc_irq_handler, 0,
>> + irq->name, irq);
>> + if (ret) {
>> + kfree(irq->name);
>> + eventfd_ctx_put(trigger);
>> + irq->trigger = NULL;
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device *vdev,
>> unsigned int index, unsigned int start,
>> unsigned int count, u32 flags,
>> void *data)
>> {
>> - return -EINVAL;
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> + int ret, hwirq;
>> + struct vfio_fsl_mc_irq *irq;
>> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
>> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
>> +
>> + if (start != 0 || count != 1)
>> + return -EINVAL;
>> +
>> + mutex_lock(&vdev->reflck->lock);
>> + ret = fsl_mc_populate_irq_pool(mc_cont,
>> + FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
>> + if (ret)
>> + goto unlock;
>> +
>> + ret = vfio_fsl_mc_irqs_allocate(vdev);
> any reason the init is done in the set_irq() and not in the open() if
> !vdev->refcnt?
Actually yes, there is a reason. It comes from the way the MSIs are
handled.
The DPAA2 devices (the devices that can be assigned to the userspace)
are organized in a hierarchical way. The DPRC is some kind of container
(parent) for child devices. Only the DPRC is allocating interrupts (it
allocates MSIs from the MSI domain). The MSIs cannot be allocated in the
open() because the MSI setup needs an IOMMU cookie which is set when the
IOMMU is set with VFIO_SET_IOMMU. But iommu is set later, after open(),
so the MSIs should be allocated afterwards.
So, the DPRC is allocating a pool of MSIs and each child object will
request a number of interrupts from that pool. This is what
vfio_fsl_mc_irqs_allocate does: it requests a number of interrupts from
the pool.
>> + if (ret)
>> + goto unlock;
>> + mutex_unlock(&vdev->reflck->lock);
>> +
>> + if (!count && (flags & VFIO_IRQ_SET_DATA_NONE))
>> + return vfio_set_trigger(vdev, index, -1);
>> +
>> + if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
>> + s32 fd = *(s32 *)data;
>> +
>> + return vfio_set_trigger(vdev, index, fd);
>> + }
>> +
>> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
>> +
>> + irq = &vdev->mc_irqs[index];
>> +
>> + if (flags & VFIO_IRQ_SET_DATA_NONE) {
>> + vfio_fsl_mc_irq_handler(hwirq, irq);
>> +
>> + } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
>> + u8 trigger = *(u8 *)data;
>> +
>> + if (trigger)
>> + vfio_fsl_mc_irq_handler(hwirq, irq);
>> + }
>> +
>> + return 0;
>> +
>> +unlock:
>> + mutex_unlock(&vdev->reflck->lock);
>> + return ret;
>> }
>>
>> int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>> @@ -61,3 +198,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>>
>> return ret;
>> }
>> +
>> +/* Free All IRQs for the given MC object */
>> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev)
>> +{
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> + int irq_count = mc_dev->obj_desc.irq_count;
>> + int i;
>> +
>> + /* Device does not support any interrupt or the interrupts
>> + * were not configured
>> + */
>> + if (mc_dev->obj_desc.irq_count == 0 || !vdev->mc_irqs)
>> + return;
>> +
>> + for (i = 0; i < irq_count; i++)
>> + vfio_set_trigger(vdev, i, -1);
>> +
>> + fsl_mc_free_irqs(mc_dev);
>> + kfree(vdev->mc_irqs);
>> + vdev->mc_irqs = NULL;
>> +}
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> index d5b6fe891a48..bbfca8b55f8a 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> @@ -15,6 +15,13 @@
>> #define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
>> ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
>>
>> +struct vfio_fsl_mc_irq {
>> + u32 flags;
>> + u32 count;
>> + struct eventfd_ctx *trigger;
>> + char *name;
>> +};
>> +
>> struct vfio_fsl_mc_reflck {
>> struct kref kref;
>> struct mutex lock;
>> @@ -35,6 +42,7 @@ struct vfio_fsl_mc_device {
>> struct vfio_fsl_mc_region *regions;
>> struct vfio_fsl_mc_reflck *reflck;
>> struct mutex igate;
>> + struct vfio_fsl_mc_irq *mc_irqs;
>> };
>>
>> extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>> @@ -42,4 +50,6 @@ extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>> unsigned int start, unsigned int count,
>> void *data);
>>
>> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev);
>> +
>> #endif /* VFIO_FSL_MC_PRIVATE_H */
>>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
2020-09-04 8:18 ` Auger Eric
@ 2020-09-07 14:34 ` Diana Craciun OSS
2020-09-10 8:20 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 14:34 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/4/2020 11:18 AM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> The software uses a memory-mapped I/O command interface (MC portals) to
>> communicate with the MC hardware. This command interface is used to
>> discover, enumerate, configure and remove DPAA2 objects. The DPAA2
>> objects use MSIs, so the command interface needs to be emulated
>> such that the correct MSI is configured in the hardware (the guest
>> has the virtual MSIs).
> What I don't get is all the regions are mmappable too.
> And this patch does not seem to introduce special handling with respect
> to MSIs. Please could you clarify?
The device can be controlled using commands issued towards a firmware.
Most of the commands can be passthrough, among exceptions is the command
that configures the interrupts. In a guest the interrupts are emulated
and for the hardware the numbers in the guest mean nothing. So, in a
virtual machine scenario, the DPMCP and DPRC regions are emulated in
qemu such that the command which configures the interrupts will not go
to hardware with the information set by the guest.
However there are other scenarios apart from virtual machines like DPDK
in which the interrupt configuration command is not used. The problem
might be that the userspace could issue the command because there is no
restriction in the VFIO, but in that case the worst thing that may
happen is for the interrupts for the device not to work.
However it is possible to restrict the command for this scenario as well
if I change the code and not allow the DPRC region to be mmapable. In
practice it proved that it might not gain much by direct assigning that
area. Also the interrupt configuration command was restricted from the
firmware to be issued only from the DPRC device region to help such a
scenario.
>>
>> This patch is adding read/write support for fsl-mc devices. The mc
>> commands are emulated by the userspace. The host is just passing
>> the correct command to the hardware.
>>
>> Also the current patch limits userspace to write complete
>> 64byte command once and read 64byte response by one ioctl.
>>
>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 115 +++++++++++++++++++++-
>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
>> 2 files changed, 114 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 73834f488a94..27713aa86878 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> @@ -12,6 +12,7 @@
>> #include <linux/types.h>
>> #include <linux/vfio.h>
>> #include <linux/fsl/mc.h>
>> +#include <linux/delay.h>
>>
>> #include "vfio_fsl_mc_private.h"
>>
>> @@ -106,6 +107,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>> vdev->regions[i].size = resource_size(res);
>> vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
>> vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
>> + if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
>> }
>>
>> vdev->num_regions = mc_dev->obj_desc.region_count;
>> @@ -114,6 +118,11 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>>
>> static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
>> {
>> + int i;
>> +
>> + for (i = 0; i < vdev->num_regions; i++)
>> + iounmap(vdev->regions[i].ioaddr);
>> +
>> vdev->num_regions = 0;
>> kfree(vdev->regions);
>> }
>> @@ -311,13 +320,115 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
>> static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return -EINVAL;
>> + struct vfio_fsl_mc_device *vdev = device_data;
>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>> + struct vfio_fsl_mc_region *region;
>> + u64 data[8];
>> + int i;
>> +
>> + if (index >= vdev->num_regions)
>> + return -EINVAL;
>> +
>> + region = &vdev->regions[index];
>> +
>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
>> + return -EINVAL;
>> +
>> + if (!region->ioaddr) {
>> + region->ioaddr = ioremap(region->addr, region->size);
>> + if (!region->ioaddr)
>> + return -ENOMEM;
>> + }
>> +
>> + if (count != 64 || off != 0)
>> + return -EINVAL;
>> +
>> + for (i = 7; i >= 0; i--)
>> + data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
>> +
>> + if (copy_to_user(buf, data, 64))
>> + return -EFAULT;
>> +
>> + return count;
>> +}
>> +
>> +#define MC_CMD_COMPLETION_TIMEOUT_MS 5000
>> +#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS 500
>> +
>> +static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
>> +{
>> + int i;
>> + enum mc_cmd_status status;
>> + unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
>> +
>> + /* Write at command parameter into portal */
>> + for (i = 7; i >= 1; i--)
>> + writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
>> +
>> + /* Write command header in the end */
>> + writeq(cmd_data[0], ioaddr);
>> +
>> + /* Wait for response before returning to user-space
>> + * This can be optimized in future to even prepare response
>> + * before returning to user-space and avoid read ioctl.
>> + */
>> + for (;;) {
>> + u64 header;
>> + struct mc_cmd_header *resp_hdr;
>> +
>> + header = cpu_to_le64(readq_relaxed(ioaddr));
>> +
>> + resp_hdr = (struct mc_cmd_header *)&header;
>> + status = (enum mc_cmd_status)resp_hdr->status;
>> + if (status != MC_CMD_STATUS_READY)
>> + break;
>> +
>> + udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
>> + timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
>> + if (timeout_usecs == 0)
>> + return -ETIMEDOUT;
>> + }
>> +
>> + return 0;
>> }
>>
>> static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return -EINVAL;
>> + struct vfio_fsl_mc_device *vdev = device_data;
>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>> + struct vfio_fsl_mc_region *region;
>> + u64 data[8];
>> + int ret;
>> +
>> + if (index >= vdev->num_regions)
>> + return -EINVAL;
>> +
>> + region = &vdev->regions[index];
>> +
>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE))
>> + return -EINVAL;
>> +
>> + if (!region->ioaddr) {
>> + region->ioaddr = ioremap(region->addr, region->size);
>> + if (!region->ioaddr)
>> + return -ENOMEM;
>> + }
>> +
>> + if (count != 64 || off != 0)
>> + return -EINVAL;
>> +
>> + if (copy_from_user(&data, buf, 64))
>> + return -EFAULT;
>> +
>> + ret = vfio_fsl_mc_send_command(region->ioaddr, data);
>> + if (ret)
>> + return ret;
>> +
>> + return count;
>> +
>> }
>>
>> static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> index bbfca8b55f8a..e6804e516c4a 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>> @@ -32,6 +32,7 @@ struct vfio_fsl_mc_region {
>> u32 type;
>> u64 addr;
>> resource_size_t size;
>> + void __iomem *ioaddr;
>> };
>>
>> struct vfio_fsl_mc_device {
>>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset
2020-09-04 8:21 ` Auger Eric
@ 2020-09-07 14:36 ` Diana Craciun OSS
2020-09-10 8:21 ` Auger Eric
0 siblings, 1 reply; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 14:36 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Eric,
On 9/4/2020 11:21 AM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun wrote:
>> Currently only resetting the DPRC container is supported which
>> will reset all the objects inside it. Resetting individual
>> objects is possible from the userspace by issueing commands
>> towards MC firmware.
>>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 27713aa86878..d17c5b3148ad 100644
>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> @@ -310,7 +310,20 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
>> }
>> case VFIO_DEVICE_RESET:
>> {
>> - return -ENOTTY;
>> + int ret = 0;
> initialization not needed
>> +
> spare empty line
>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> +
>> + /* reset is supported only for the DPRC */
>> + if (!is_fsl_mc_bus_dprc(mc_dev))
>> + return -ENOTTY;
> it is an error case or do we just don't care?
I rather don't care, but shouldn't the userspace know that the reset for
that device failed?
>> +
>> + ret = dprc_reset_container(mc_dev->mc_io, 0,
>> + mc_dev->mc_handle,
>> + mc_dev->obj_desc.id,
>> + DPRC_RESET_OPTION_NON_RECURSIVE);
>> + return ret;
>> +
>> }
>> default:
>> return -ENOTTY;
>>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
2020-09-03 14:06 ` Auger Eric
@ 2020-09-07 14:38 ` Diana Craciun OSS
0 siblings, 0 replies; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-07 14:38 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Eric,
On 9/3/2020 5:06 PM, Auger Eric wrote:
> Hi Diana,
>
> On 8/26/20 11:33 AM, Diana Craciun 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 <Bharat.Bhushan@nxp.com>
>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>> ---
>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 84 +++++++++++++++++++++++
>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
>> 2 files changed, 85 insertions(+)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 8b53c2a25b32..85e007be3a5d 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,72 @@ 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);
>> + if (!mc_dev->driver_override)
>> + dev_warn(dev, "Setting driver override for device in dprc %s failed\n",
>> + dev_name(&mc_cont->dev));
>> + dev_info(dev, "Setting driver override for device in dprc %s\n",
>> + dev_name(&mc_cont->dev));
> Don't you miss an else here?
>> + } 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) {
> if (ret) here and in other places? or are there any > returned values
>> + dev_err(&mc_dev->dev, "Failed to setup DPRC (error = %d)\n", ret);
> nit: maybe align your error messages. Before you were using __func__,
> here you don't. Maybe don't? also you may consider using strerror(-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);
>> + dprc_cleanup(mc_dev);
> I see dprc_cleanup is likely to fail. Generally cleanup shouldn't.
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2283433.html
Right, I will change the dprc_cleanup not to fail.
>> + bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> nit: here also you can factorize code doing goto unregister;
> shouldn't you reset vdev->nb.notifier_call to NULL as well. I see it is
> tested in other places.
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>> {
>> struct iommu_group *group;
>> @@ -112,6 +180,12 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>> return ret;
>> }
>>
>> + ret = vfio_fsl_mc_init_device(vdev);
>> + if (ret < 0) {
> I think you also need to call vfio_del_group_dev(&pdev->dev)
>> + vfio_iommu_group_put(group, dev);
>> + return ret;
> nit: goto put_group;
>> + }
>> +
>> return ret;
>> }
>>
>> @@ -124,6 +198,16 @@ 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)) {
>> + dprc_remove_devices(mc_dev, NULL, 0);
>> + dprc_cleanup(mc_dev);
>> + }
> you may consider doing the tear down in opposite order than
> vfio_fsl_mc_init_device, ie. bus_unregister_notifier after the
> dprc_cleanup? That's also what is done in vfio_fsl_mc_init_device error
> path handling.
>> +
>> + 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 */>
> Thanks
>
> Eric
>
Thanks,
Diana
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
2020-09-07 14:02 ` Diana Craciun OSS
@ 2020-09-10 8:16 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-10 8:16 UTC (permalink / raw)
To: Diana Craciun OSS, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 9/7/20 4:02 PM, Diana Craciun OSS wrote:
> Hi Eric,
>
> On 9/4/2020 11:02 AM, Auger Eric wrote:
>> Hi Diana,
>>
>> On 8/26/20 11:33 AM, Diana Craciun wrote:
>>> This patch allows to set an eventfd for fsl-mc device interrupts
>>> and also to trigger the interrupt eventfd from userspace for testing.
>>>
>>> All fsl-mc device interrupts are MSIs. The MSIs are allocated from
>>> the MSI domain only once per DPRC and used by all the DPAA2 objects.
>>> The interrupts are managed by the DPRC in a pool of interrupts. Each
>>> device requests interrupts from this pool. The pool is allocated
>>> when the first virtual device is setting the interrupts.
>>> The pool of interrupts is protected by a lock.
>>>
>>> The DPRC has an interrupt of its own which indicates if the DPRC
>>> contents have changed. However, currently, the contents of a DPRC
>>> assigned to the guest cannot be changed at runtime, so this interrupt
>>> is not configured.
>>>
>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>>> ---
>>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 18 ++-
>>> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c | 160 +++++++++++++++++++++-
>>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 10 ++
>>> 3 files changed, 186 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> index 42014297b484..73834f488a94 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> @@ -147,12 +147,28 @@ static int vfio_fsl_mc_open(void *device_data)
>>> static void vfio_fsl_mc_release(void *device_data)
>>> {
>>> struct vfio_fsl_mc_device *vdev = device_data;
>>> + int ret;
>>> mutex_lock(&vdev->reflck->lock);
>>> - if (!(--vdev->refcnt))
>>> + if (!(--vdev->refcnt)) {
>>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>>> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
>>> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
>>> +
>>> vfio_fsl_mc_regions_cleanup(vdev);
>>> + /* reset the device before cleaning up the interrupts */
>>> + ret = dprc_reset_container(mc_cont->mc_io, 0,
>>> + mc_cont->mc_handle,
>>> + mc_cont->obj_desc.id,
>>> + DPRC_RESET_OPTION_NON_RECURSIVE);
>> shouldn't you test ret?
>>> +
>>> + vfio_fsl_mc_irqs_cleanup(vdev);
>>> +
>>> + fsl_mc_cleanup_irq_pool(mc_cont);
>>> + }
>>> +
>>> mutex_unlock(&vdev->reflck->lock);
>>> module_put(THIS_MODULE);
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>>> index 058aa97aa54a..409f3507fcf3 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
>>> @@ -29,12 +29,149 @@ static int vfio_fsl_mc_irq_unmask(struct
>>> vfio_fsl_mc_device *vdev,
>>> return -EINVAL;
>>> }
>>> +int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
>>> +{
>>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>>> + struct vfio_fsl_mc_irq *mc_irq;
>>> + int irq_count;
>>> + int ret, i;
>>> +
>>> + /* Device does not support any interrupt */
>> indent needs to be fixed
>>> + if (mc_dev->obj_desc.irq_count == 0)
>>> + return 0;
>>> +
>>> + /* interrupts were already allocated for this device */
>>> + if (vdev->mc_irqs)
>>> + return 0;
>>> +
>>> + irq_count = mc_dev->obj_desc.irq_count;
>>> +
>>> + mc_irq = kcalloc(irq_count, sizeof(*mc_irq), GFP_KERNEL);
>>> + if (!mc_irq)
>>> + return -ENOMEM;
>>> +
>>> + /* Allocate IRQs */
>>> + ret = fsl_mc_allocate_irqs(mc_dev);
>>> + if (ret) {
>>> + kfree(mc_irq);
>>> + return ret;
>>> + }
>>> +
>>> + for (i = 0; i < irq_count; i++) {
>>> + mc_irq[i].count = 1;
>>> + mc_irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
>>> + }
>>> +
>>> + vdev->mc_irqs = mc_irq;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t vfio_fsl_mc_irq_handler(int irq_num, void *arg)
>>> +{
>>> + struct vfio_fsl_mc_irq *mc_irq = (struct vfio_fsl_mc_irq *)arg;
>>> +
>>> + eventfd_signal(mc_irq->trigger, 1);
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int vfio_set_trigger(struct vfio_fsl_mc_device *vdev,
>>> + int index, int fd)
>>> +{
>>> + struct vfio_fsl_mc_irq *irq = &vdev->mc_irqs[index];
>>> + struct eventfd_ctx *trigger;
>>> + int hwirq;
>>> + int ret;
>>> +
>>> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
>>> + if (irq->trigger) {
>>> + free_irq(hwirq, irq);
>>> + kfree(irq->name);
>>> + eventfd_ctx_put(irq->trigger);
>>> + irq->trigger = NULL;
>>> + }
>>> +
>>> + if (fd < 0) /* Disable only */
>>> + return 0;
>>> +
>>> + irq->name = kasprintf(GFP_KERNEL, "vfio-irq[%d](%s)",
>>> + hwirq, dev_name(&vdev->mc_dev->dev));
>>> + if (!irq->name)
>>> + return -ENOMEM;
>>> +
>>> + trigger = eventfd_ctx_fdget(fd);
>>> + if (IS_ERR(trigger)) {
>>> + kfree(irq->name);
>>> + return PTR_ERR(trigger);
>>> + }
>>> +
>>> + irq->trigger = trigger;
>>> +
>>> + ret = request_irq(hwirq, vfio_fsl_mc_irq_handler, 0,
>>> + irq->name, irq);
>>> + if (ret) {
>>> + kfree(irq->name);
>>> + eventfd_ctx_put(trigger);
>>> + irq->trigger = NULL;
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static int vfio_fsl_mc_set_irq_trigger(struct vfio_fsl_mc_device
>>> *vdev,
>>> unsigned int index, unsigned int start,
>>> unsigned int count, u32 flags,
>>> void *data)
>>> {
>>> - return -EINVAL;
>>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>>> + int ret, hwirq;
>>> + struct vfio_fsl_mc_irq *irq;
>>> + struct device *cont_dev = fsl_mc_cont_dev(&mc_dev->dev);
>>> + struct fsl_mc_device *mc_cont = to_fsl_mc_device(cont_dev);
>>> +
>>> + if (start != 0 || count != 1)
>>> + return -EINVAL;
>>> +
>>> + mutex_lock(&vdev->reflck->lock);
>>> + ret = fsl_mc_populate_irq_pool(mc_cont,
>>> + FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
>>> + if (ret)
>>> + goto unlock;
>>> +
>>> + ret = vfio_fsl_mc_irqs_allocate(vdev);
>> any reason the init is done in the set_irq() and not in the open() if
>> !vdev->refcnt?
>
> Actually yes, there is a reason. It comes from the way the MSIs are
> handled.
>
> The DPAA2 devices (the devices that can be assigned to the userspace)
> are organized in a hierarchical way. The DPRC is some kind of container
> (parent) for child devices. Only the DPRC is allocating interrupts (it
> allocates MSIs from the MSI domain). The MSIs cannot be allocated in the
> open() because the MSI setup needs an IOMMU cookie which is set when the
> IOMMU is set with VFIO_SET_IOMMU. But iommu is set later, after open(),
> so the MSIs should be allocated afterwards.
>
> So, the DPRC is allocating a pool of MSIs and each child object will
> request a number of interrupts from that pool. This is what
> vfio_fsl_mc_irqs_allocate does: it requests a number of interrupts from
> the pool.
OK, thank you for the clarifications.
Thanks
Eric
>
>
>>> + if (ret)
>>> + goto unlock;
>>> + mutex_unlock(&vdev->reflck->lock);
>>> +
>>> + if (!count && (flags & VFIO_IRQ_SET_DATA_NONE))
>>> + return vfio_set_trigger(vdev, index, -1);
>>> +
>>> + if (flags & VFIO_IRQ_SET_DATA_EVENTFD) {
>>> + s32 fd = *(s32 *)data;
>>> +
>>> + return vfio_set_trigger(vdev, index, fd);
>>> + }
>>> +
>>> + hwirq = vdev->mc_dev->irqs[index]->msi_desc->irq;
>>> +
>>> + irq = &vdev->mc_irqs[index];
>>> +
>>> + if (flags & VFIO_IRQ_SET_DATA_NONE) {
>>> + vfio_fsl_mc_irq_handler(hwirq, irq);
>>> +
>>> + } else if (flags & VFIO_IRQ_SET_DATA_BOOL) {
>>> + u8 trigger = *(u8 *)data;
>>> +
>>> + if (trigger)
>>> + vfio_fsl_mc_irq_handler(hwirq, irq);
>>> + }
>>> +
>>> + return 0;
>>> +
>>> +unlock:
>>> + mutex_unlock(&vdev->reflck->lock);
>>> + return ret;
>>> }
>>> int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>>> @@ -61,3 +198,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct
>>> vfio_fsl_mc_device *vdev,
>>> return ret;
>>> }
>>> +
>>> +/* Free All IRQs for the given MC object */
>>> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev)
>>> +{
>>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>>> + int irq_count = mc_dev->obj_desc.irq_count;
>>> + int i;
>>> +
>>> + /* Device does not support any interrupt or the interrupts
>>> + * were not configured
>>> + */
>>> + if (mc_dev->obj_desc.irq_count == 0 || !vdev->mc_irqs)
>>> + return;
>>> +
>>> + for (i = 0; i < irq_count; i++)
>>> + vfio_set_trigger(vdev, i, -1);
>>> +
>>> + fsl_mc_free_irqs(mc_dev);
>>> + kfree(vdev->mc_irqs);
>>> + vdev->mc_irqs = NULL;
>>> +}
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> index d5b6fe891a48..bbfca8b55f8a 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> @@ -15,6 +15,13 @@
>>> #define VFIO_FSL_MC_INDEX_TO_OFFSET(index) \
>>> ((u64)(index) << VFIO_FSL_MC_OFFSET_SHIFT)
>>> +struct vfio_fsl_mc_irq {
>>> + u32 flags;
>>> + u32 count;
>>> + struct eventfd_ctx *trigger;
>>> + char *name;
>>> +};
>>> +
>>> struct vfio_fsl_mc_reflck {
>>> struct kref kref;
>>> struct mutex lock;
>>> @@ -35,6 +42,7 @@ struct vfio_fsl_mc_device {
>>> struct vfio_fsl_mc_region *regions;
>>> struct vfio_fsl_mc_reflck *reflck;
>>> struct mutex igate;
>>> + struct vfio_fsl_mc_irq *mc_irqs;
>>> };
>>> extern int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device
>>> *vdev,
>>> @@ -42,4 +50,6 @@ extern int vfio_fsl_mc_set_irqs_ioctl(struct
>>> vfio_fsl_mc_device *vdev,
>>> unsigned int start, unsigned int count,
>>> void *data);
>>> +void vfio_fsl_mc_irqs_cleanup(struct vfio_fsl_mc_device *vdev);
>>> +
>>> #endif /* VFIO_FSL_MC_PRIVATE_H */
>>>
>> Thanks
>>
>> Eric
>>
>
> Thanks,
> Diana
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
2020-09-07 14:34 ` Diana Craciun OSS
@ 2020-09-10 8:20 ` Auger Eric
2020-09-11 9:15 ` Diana Craciun OSS
0 siblings, 1 reply; 37+ messages in thread
From: Auger Eric @ 2020-09-10 8:20 UTC (permalink / raw)
To: Diana Craciun OSS, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor, Bharat Bhushan
Hi Diana,
On 9/7/20 4:34 PM, Diana Craciun OSS wrote:
> Hi Eric,
>
> On 9/4/2020 11:18 AM, Auger Eric wrote:
>> Hi Diana,
>>
>> On 8/26/20 11:33 AM, Diana Craciun wrote:
>>> The software uses a memory-mapped I/O command interface (MC portals) to
>>> communicate with the MC hardware. This command interface is used to
>>> discover, enumerate, configure and remove DPAA2 objects. The DPAA2
>>> objects use MSIs, so the command interface needs to be emulated
>>> such that the correct MSI is configured in the hardware (the guest
>>> has the virtual MSIs).
>> What I don't get is all the regions are mmappable too.
>> And this patch does not seem to introduce special handling with respect
>> to MSIs. Please could you clarify?
>
> The device can be controlled using commands issued towards a firmware.
> Most of the commands can be passthrough, among exceptions is the command
> that configures the interrupts. In a guest the interrupts are emulated
> and for the hardware the numbers in the guest mean nothing. So, in a
> virtual machine scenario, the DPMCP and DPRC regions are emulated in
> qemu such that the command which configures the interrupts will not go
> to hardware with the information set by the guest.
> However there are other scenarios apart from virtual machines like DPDK
> in which the interrupt configuration command is not used. The problem
> might be that the userspace could issue the command because there is no
> restriction in the VFIO, but in that case the worst thing that may
> happen is for the interrupts for the device not to work.
> However it is possible to restrict the command for this scenario as well
> if I change the code and not allow the DPRC region to be mmapable. In
> practice it proved that it might not gain much by direct assigning that
> area. Also the interrupt configuration command was restricted from the
> firmware to be issued only from the DPRC device region to help such a
> scenario.
Yes actually I meant that the region used to configure MSIs should not
be mmappable then?
Thanks
Eric
>
>
>>>
>>> This patch is adding read/write support for fsl-mc devices. The mc
>>> commands are emulated by the userspace. The host is just passing
>>> the correct command to the hardware.
>>>
>>> Also the current patch limits userspace to write complete
>>> 64byte command once and read 64byte response by one ioctl.
>>>
>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>>> ---
>>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 115 +++++++++++++++++++++-
>>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
>>> 2 files changed, 114 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> index 73834f488a94..27713aa86878 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> @@ -12,6 +12,7 @@
>>> #include <linux/types.h>
>>> #include <linux/vfio.h>
>>> #include <linux/fsl/mc.h>
>>> +#include <linux/delay.h>
>>> #include "vfio_fsl_mc_private.h"
>>> @@ -106,6 +107,9 @@ static int vfio_fsl_mc_regions_init(struct
>>> vfio_fsl_mc_device *vdev)
>>> vdev->regions[i].size = resource_size(res);
>>> vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
>>> vdev->regions[i].type = mc_dev->regions[i].flags &
>>> IORESOURCE_BITS;
>>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
>>> + if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
>>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
>>> }
>>> vdev->num_regions = mc_dev->obj_desc.region_count;
>>> @@ -114,6 +118,11 @@ static int vfio_fsl_mc_regions_init(struct
>>> vfio_fsl_mc_device *vdev)
>>> static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device
>>> *vdev)
>>> {
>>> + int i;
>>> +
>>> + for (i = 0; i < vdev->num_regions; i++)
>>> + iounmap(vdev->regions[i].ioaddr);
>>> +
>>> vdev->num_regions = 0;
>>> kfree(vdev->regions);
>>> }
>>> @@ -311,13 +320,115 @@ static long vfio_fsl_mc_ioctl(void
>>> *device_data, unsigned int cmd,
>>> static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
>>> size_t count, loff_t *ppos)
>>> {
>>> - return -EINVAL;
>>> + struct vfio_fsl_mc_device *vdev = device_data;
>>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>>> + struct vfio_fsl_mc_region *region;
>>> + u64 data[8];
>>> + int i;
>>> +
>>> + if (index >= vdev->num_regions)
>>> + return -EINVAL;
>>> +
>>> + region = &vdev->regions[index];
>>> +
>>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
>>> + return -EINVAL;
>>> +
>>> + if (!region->ioaddr) {
>>> + region->ioaddr = ioremap(region->addr, region->size);
>>> + if (!region->ioaddr)
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + if (count != 64 || off != 0)
>>> + return -EINVAL;
>>> +
>>> + for (i = 7; i >= 0; i--)
>>> + data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
>>> +
>>> + if (copy_to_user(buf, data, 64))
>>> + return -EFAULT;
>>> +
>>> + return count;
>>> +}
>>> +
>>> +#define MC_CMD_COMPLETION_TIMEOUT_MS 5000
>>> +#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS 500
>>> +
>>> +static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t
>>> *cmd_data)
>>> +{
>>> + int i;
>>> + enum mc_cmd_status status;
>>> + unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
>>> +
>>> + /* Write at command parameter into portal */
>>> + for (i = 7; i >= 1; i--)
>>> + writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
>>> +
>>> + /* Write command header in the end */
>>> + writeq(cmd_data[0], ioaddr);
>>> +
>>> + /* Wait for response before returning to user-space
>>> + * This can be optimized in future to even prepare response
>>> + * before returning to user-space and avoid read ioctl.
>>> + */
>>> + for (;;) {
>>> + u64 header;
>>> + struct mc_cmd_header *resp_hdr;
>>> +
>>> + header = cpu_to_le64(readq_relaxed(ioaddr));
>>> +
>>> + resp_hdr = (struct mc_cmd_header *)&header;
>>> + status = (enum mc_cmd_status)resp_hdr->status;
>>> + if (status != MC_CMD_STATUS_READY)
>>> + break;
>>> +
>>> + udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
>>> + timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
>>> + if (timeout_usecs == 0)
>>> + return -ETIMEDOUT;
>>> + }
>>> +
>>> + return 0;
>>> }
>>> static ssize_t vfio_fsl_mc_write(void *device_data, const char
>>> __user *buf,
>>> size_t count, loff_t *ppos)
>>> {
>>> - return -EINVAL;
>>> + struct vfio_fsl_mc_device *vdev = device_data;
>>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>>> + struct vfio_fsl_mc_region *region;
>>> + u64 data[8];
>>> + int ret;
>>> +
>>> + if (index >= vdev->num_regions)
>>> + return -EINVAL;
>>> +
>>> + region = &vdev->regions[index];
>>> +
>>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE))
>>> + return -EINVAL;
>>> +
>>> + if (!region->ioaddr) {
>>> + region->ioaddr = ioremap(region->addr, region->size);
>>> + if (!region->ioaddr)
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + if (count != 64 || off != 0)
>>> + return -EINVAL;
>>> +
>>> + if (copy_from_user(&data, buf, 64))
>>> + return -EFAULT;
>>> +
>>> + ret = vfio_fsl_mc_send_command(region->ioaddr, data);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return count;
>>> +
>>> }
>>> static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> index bbfca8b55f8a..e6804e516c4a 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>> @@ -32,6 +32,7 @@ struct vfio_fsl_mc_region {
>>> u32 type;
>>> u64 addr;
>>> resource_size_t size;
>>> + void __iomem *ioaddr;
>>> };
>>> struct vfio_fsl_mc_device {
>>>
>> Thanks
>>
>> Eric
>>
>
> Thanks,
> Diana
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset
2020-09-07 14:36 ` Diana Craciun OSS
@ 2020-09-10 8:21 ` Auger Eric
0 siblings, 0 replies; 37+ messages in thread
From: Auger Eric @ 2020-09-10 8:21 UTC (permalink / raw)
To: Diana Craciun OSS, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Diana,
On 9/7/20 4:36 PM, Diana Craciun OSS wrote:
> Hi Eric,
>
> On 9/4/2020 11:21 AM, Auger Eric wrote:
>> Hi Diana,
>>
>> On 8/26/20 11:33 AM, Diana Craciun wrote:
>>> Currently only resetting the DPRC container is supported which
>>> will reset all the objects inside it. Resetting individual
>>> objects is possible from the userspace by issueing commands
>>> towards MC firmware.
>>>
>>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>>> ---
>>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 15 ++++++++++++++-
>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> index 27713aa86878..d17c5b3148ad 100644
>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>> @@ -310,7 +310,20 @@ static long vfio_fsl_mc_ioctl(void *device_data,
>>> unsigned int cmd,
>>> }
>>> case VFIO_DEVICE_RESET:
>>> {
>>> - return -ENOTTY;
>>> + int ret = 0;
>> initialization not needed
>>> +
>> spare empty line
>>> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
>>> +
>>> + /* reset is supported only for the DPRC */
>>> + if (!is_fsl_mc_bus_dprc(mc_dev))
>>> + return -ENOTTY;
>> it is an error case or do we just don't care?
>
>
> I rather don't care, but shouldn't the userspace know that the reset for
> that device failed?I don't know what makes more sense. It was more a question.
Thanks
Eric
>
>>> +
>>> + ret = dprc_reset_container(mc_dev->mc_io, 0,
>>> + mc_dev->mc_handle,
>>> + mc_dev->obj_desc.id,
>>> + DPRC_RESET_OPTION_NON_RECURSIVE);
>>> + return ret;
>>> +
>>> }
>>> default:
>>> return -ENOTTY;
>>>
>> Thanks
>>
>> Eric
>>
>
> Thanks,
> Diana
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
2020-09-10 8:20 ` Auger Eric
@ 2020-09-11 9:15 ` Diana Craciun OSS
0 siblings, 0 replies; 37+ messages in thread
From: Diana Craciun OSS @ 2020-09-11 9:15 UTC (permalink / raw)
To: Auger Eric, alex.williamson, kvm
Cc: linux-kernel, bharatb.linux, laurentiu.tudor
Hi Eric,
On 9/10/2020 11:20 AM, Auger Eric wrote:
> Hi Diana,
>
> On 9/7/20 4:34 PM, Diana Craciun OSS wrote:
>> Hi Eric,
>>
>> On 9/4/2020 11:18 AM, Auger Eric wrote:
>>> Hi Diana,
>>>
>>> On 8/26/20 11:33 AM, Diana Craciun wrote:
>>>> The software uses a memory-mapped I/O command interface (MC portals) to
>>>> communicate with the MC hardware. This command interface is used to
>>>> discover, enumerate, configure and remove DPAA2 objects. The DPAA2
>>>> objects use MSIs, so the command interface needs to be emulated
>>>> such that the correct MSI is configured in the hardware (the guest
>>>> has the virtual MSIs).
>>> What I don't get is all the regions are mmappable too.
>>> And this patch does not seem to introduce special handling with respect
>>> to MSIs. Please could you clarify?
>>
>> The device can be controlled using commands issued towards a firmware.
>> Most of the commands can be passthrough, among exceptions is the command
>> that configures the interrupts. In a guest the interrupts are emulated
>> and for the hardware the numbers in the guest mean nothing. So, in a
>> virtual machine scenario, the DPMCP and DPRC regions are emulated in
>> qemu such that the command which configures the interrupts will not go
>> to hardware with the information set by the guest.
>> However there are other scenarios apart from virtual machines like DPDK
>> in which the interrupt configuration command is not used. The problem
>> might be that the userspace could issue the command because there is no
>> restriction in the VFIO, but in that case the worst thing that may
>> happen is for the interrupts for the device not to work.
>> However it is possible to restrict the command for this scenario as well
>> if I change the code and not allow the DPRC region to be mmapable. In
>> practice it proved that it might not gain much by direct assigning that
>> area. Also the interrupt configuration command was restricted from the
>> firmware to be issued only from the DPRC device region to help such a
>> scenario.
> Yes actually I meant that the region used to configure MSIs should not
> be mmappable then?
>
>
That region is not used only for the MSI configuration. The way it works
is through commands that are written at a certain memory address. And
the commands can be different. Applications like DPDK do not use the
command to configure interrupts, so that is the reason that the region
is mmapable. But at a second thought, I think that I can restrict the
DPRC region not to be mmapable. And that is the only region that can be
used to configure interrupts.
Thanks,
Diana
> Thanks
>
> Eric
>>
>>
>>>>
>>>> This patch is adding read/write support for fsl-mc devices. The mc
>>>> commands are emulated by the userspace. The host is just passing
>>>> the correct command to the hardware.
>>>>
>>>> Also the current patch limits userspace to write complete
>>>> 64byte command once and read 64byte response by one ioctl.
>>>>
>>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@nxp.com>
>>>> Signed-off-by: Diana Craciun <diana.craciun@oss.nxp.com>
>>>> ---
>>>> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 115 +++++++++++++++++++++-
>>>> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
>>>> 2 files changed, 114 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>>> index 73834f488a94..27713aa86878 100644
>>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>>>> @@ -12,6 +12,7 @@
>>>> #include <linux/types.h>
>>>> #include <linux/vfio.h>
>>>> #include <linux/fsl/mc.h>
>>>> +#include <linux/delay.h>
>>>> #include "vfio_fsl_mc_private.h"
>>>> @@ -106,6 +107,9 @@ static int vfio_fsl_mc_regions_init(struct
>>>> vfio_fsl_mc_device *vdev)
>>>> vdev->regions[i].size = resource_size(res);
>>>> vdev->regions[i].flags = VFIO_REGION_INFO_FLAG_MMAP;
>>>> vdev->regions[i].type = mc_dev->regions[i].flags &
>>>> IORESOURCE_BITS;
>>>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
>>>> + if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
>>>> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
>>>> }
>>>> vdev->num_regions = mc_dev->obj_desc.region_count;
>>>> @@ -114,6 +118,11 @@ static int vfio_fsl_mc_regions_init(struct
>>>> vfio_fsl_mc_device *vdev)
>>>> static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device
>>>> *vdev)
>>>> {
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < vdev->num_regions; i++)
>>>> + iounmap(vdev->regions[i].ioaddr);
>>>> +
>>>> vdev->num_regions = 0;
>>>> kfree(vdev->regions);
>>>> }
>>>> @@ -311,13 +320,115 @@ static long vfio_fsl_mc_ioctl(void
>>>> *device_data, unsigned int cmd,
>>>> static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
>>>> size_t count, loff_t *ppos)
>>>> {
>>>> - return -EINVAL;
>>>> + struct vfio_fsl_mc_device *vdev = device_data;
>>>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>>>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>>>> + struct vfio_fsl_mc_region *region;
>>>> + u64 data[8];
>>>> + int i;
>>>> +
>>>> + if (index >= vdev->num_regions)
>>>> + return -EINVAL;
>>>> +
>>>> + region = &vdev->regions[index];
>>>> +
>>>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
>>>> + return -EINVAL;
>>>> +
>>>> + if (!region->ioaddr) {
>>>> + region->ioaddr = ioremap(region->addr, region->size);
>>>> + if (!region->ioaddr)
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + if (count != 64 || off != 0)
>>>> + return -EINVAL;
>>>> +
>>>> + for (i = 7; i >= 0; i--)
>>>> + data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
>>>> +
>>>> + if (copy_to_user(buf, data, 64))
>>>> + return -EFAULT;
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +#define MC_CMD_COMPLETION_TIMEOUT_MS 5000
>>>> +#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS 500
>>>> +
>>>> +static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t
>>>> *cmd_data)
>>>> +{
>>>> + int i;
>>>> + enum mc_cmd_status status;
>>>> + unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
>>>> +
>>>> + /* Write at command parameter into portal */
>>>> + for (i = 7; i >= 1; i--)
>>>> + writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
>>>> +
>>>> + /* Write command header in the end */
>>>> + writeq(cmd_data[0], ioaddr);
>>>> +
>>>> + /* Wait for response before returning to user-space
>>>> + * This can be optimized in future to even prepare response
>>>> + * before returning to user-space and avoid read ioctl.
>>>> + */
>>>> + for (;;) {
>>>> + u64 header;
>>>> + struct mc_cmd_header *resp_hdr;
>>>> +
>>>> + header = cpu_to_le64(readq_relaxed(ioaddr));
>>>> +
>>>> + resp_hdr = (struct mc_cmd_header *)&header;
>>>> + status = (enum mc_cmd_status)resp_hdr->status;
>>>> + if (status != MC_CMD_STATUS_READY)
>>>> + break;
>>>> +
>>>> + udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
>>>> + timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
>>>> + if (timeout_usecs == 0)
>>>> + return -ETIMEDOUT;
>>>> + }
>>>> +
>>>> + return 0;
>>>> }
>>>> static ssize_t vfio_fsl_mc_write(void *device_data, const char
>>>> __user *buf,
>>>> size_t count, loff_t *ppos)
>>>> {
>>>> - return -EINVAL;
>>>> + struct vfio_fsl_mc_device *vdev = device_data;
>>>> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
>>>> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
>>>> + struct vfio_fsl_mc_region *region;
>>>> + u64 data[8];
>>>> + int ret;
>>>> +
>>>> + if (index >= vdev->num_regions)
>>>> + return -EINVAL;
>>>> +
>>>> + region = &vdev->regions[index];
>>>> +
>>>> + if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE))
>>>> + return -EINVAL;
>>>> +
>>>> + if (!region->ioaddr) {
>>>> + region->ioaddr = ioremap(region->addr, region->size);
>>>> + if (!region->ioaddr)
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + if (count != 64 || off != 0)
>>>> + return -EINVAL;
>>>> +
>>>> + if (copy_from_user(&data, buf, 64))
>>>> + return -EFAULT;
>>>> +
>>>> + ret = vfio_fsl_mc_send_command(region->ioaddr, data);
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + return count;
>>>> +
>>>> }
>>>> static int vfio_fsl_mc_mmap_mmio(struct vfio_fsl_mc_region region,
>>>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>>> b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>>> index bbfca8b55f8a..e6804e516c4a 100644
>>>> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>>> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
>>>> @@ -32,6 +32,7 @@ struct vfio_fsl_mc_region {
>>>> u32 type;
>>>> u64 addr;
>>>> resource_size_t size;
>>>> + void __iomem *ioaddr;
>>>> };
>>>> struct vfio_fsl_mc_device {
>>>>
>>> Thanks
>>>
>>> Eric
>>>
>>
>> Thanks,
>> Diana
>>
>
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2020-09-11 9:15 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-26 9:33 [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
2020-08-26 9:33 ` [PATCH v4 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
2020-09-03 14:06 ` Auger Eric
2020-09-04 13:59 ` Diana Craciun OSS
2020-09-04 14:11 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
2020-09-03 14:06 ` Auger Eric
2020-09-07 14:38 ` Diana Craciun OSS
2020-08-26 9:33 ` [PATCH v4 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
2020-09-03 14:13 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
2020-09-03 15:16 ` Auger Eric
2020-09-03 15:22 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
2020-09-03 16:05 ` Auger Eric
2020-09-07 12:49 ` Diana Craciun OSS
2020-08-26 9:33 ` [PATCH v4 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
2020-09-03 19:55 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
2020-09-03 20:15 ` Auger Eric
2020-09-07 13:09 ` Diana Craciun OSS
2020-08-26 9:33 ` [PATCH v4 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
2020-09-04 8:02 ` Auger Eric
2020-09-07 14:02 ` Diana Craciun OSS
2020-09-10 8:16 ` Auger Eric
2020-08-26 9:33 ` [PATCH v4 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
2020-09-04 8:18 ` Auger Eric
2020-09-07 14:34 ` Diana Craciun OSS
2020-09-10 8:20 ` Auger Eric
2020-09-11 9:15 ` Diana Craciun OSS
2020-08-26 9:33 ` [PATCH v4 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
2020-09-04 8:21 ` Auger Eric
2020-09-07 14:36 ` Diana Craciun OSS
2020-09-10 8:21 ` Auger Eric
2020-09-03 13:40 ` [PATCH v4 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Auger Eric
2020-09-04 8:03 ` Diana Craciun OSS
2020-09-04 8:25 ` Auger Eric
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).