linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device
@ 2020-09-29  9:03 Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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/msg3680670.html

v4 --> v5
- do not allow mmap for DPRCs
- style fixes

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         | 677 ++++++++++++++++++++++
 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c    | 193 ++++++
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  55 ++
 include/uapi/linux/vfio.h                 |   1 +
 9 files changed, 947 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] 18+ messages in thread

* [PATCH v5 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         | 157 ++++++++++++++++++++++
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  14 ++
 include/uapi/linux/vfio.h                 |   1 +
 8 files changed, 193 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 9350506a1127..cf2c7b21fefd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18257,6 +18257,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..a7a483a1e90b
--- /dev/null
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -0,0 +1,157 @@
+// 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, "VFIO_FSL_MC: No IOMMU group\n");
+		return -EINVAL;
+	}
+
+	vdev = devm_kzalloc(dev, sizeof(*vdev), GFP_KERNEL);
+	if (!vdev) {
+		ret = -ENOMEM;
+		goto out_group_put;
+	}
+
+	vdev->mc_dev = mc_dev;
+
+	ret = vfio_add_group_dev(dev, &vfio_fsl_mc_ops, vdev);
+	if (ret) {
+		dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n");
+		goto out_group_put;
+	}
+	return 0;
+
+out_group_put:
+	vfio_iommu_group_put(group, dev);
+	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;
+}
+
+static struct fsl_mc_driver vfio_fsl_mc_driver = {
+	.probe		= vfio_fsl_mc_probe,
+	.remove		= vfio_fsl_mc_remove,
+	.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("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 */
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-10-02 17:24   ` Alex Williamson
  2020-09-29  9:03 ` [PATCH v5 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         | 90 +++++++++++++++++++++++
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  1 +
 2 files changed, 91 insertions(+)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index a7a483a1e90b..ba44d6d01cc9 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,79 @@ 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, "VFIO_FSL_MC: Setting driver override for device in dprc %s failed\n",
+			     dev_name(&mc_cont->dev));
+		else
+			dev_info(dev, "VFIO_FSL_MC: 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, "VFIO_FSL_MC: 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) {
+		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Failed to setup DPRC (%d)\n", ret);
+		goto out_nc_unreg;
+	}
+
+	ret = dprc_scan_container(mc_dev, false);
+	if (ret) {
+		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Container scanning failed (%d)\n", ret);
+		goto out_dprc_cleanup;
+	}
+
+	return 0;
+
+out_dprc_cleanup:
+	dprc_cleanup(mc_dev);
+out_nc_unreg:
+	bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+	vdev->nb.notifier_call = NULL;
+
+	return ret;
+}
+
 static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
 {
 	struct iommu_group *group;
@@ -110,8 +185,15 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
 		dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n");
 		goto out_group_put;
 	}
+
+	ret = vfio_fsl_mc_init_device(vdev);
+	if (ret)
+		goto out_group_dev;
+
 	return 0;
 
+out_group_dev:
+	vfio_del_group_dev(dev);
 out_group_put:
 	vfio_iommu_group_put(group, dev);
 	return ret;
@@ -126,6 +208,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
 	if (!vdev)
 		return -EINVAL;
 
+	if (is_fsl_mc_bus_dprc(mc_dev)) {
+		dprc_remove_devices(mc_dev, NULL, 0);
+		dprc_cleanup(mc_dev);
+	}
+
+	if (vdev->nb.notifier_call)
+		bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
+
 	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] 18+ messages in thread

* [PATCH v5 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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>
Reviewed-by: Eric Auger <eric.auger@redhat.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 ba44d6d01cc9..fa46676c735b 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] 18+ messages in thread

* [PATCH v5 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (2 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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 | 18 ++++++
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index fa46676c735b..802f3a00337f 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -17,16 +17,71 @@
 
 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->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
+	}
+
+	return 0;
+}
+
+static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
+{
+	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 +114,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 >= mc_dev->obj_desc.region_count)
+			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:
 	{
@@ -209,6 +282,8 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
 	if (ret)
 		goto out_group_dev;
 
+	mutex_init(&vdev->driver_lock);
+
 	return 0;
 
 out_group_dev:
@@ -227,6 +302,8 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
 	if (!vdev)
 		return -EINVAL;
 
+	mutex_destroy(&vdev->driver_lock);
+
 	if (is_fsl_mc_bus_dprc(mc_dev)) {
 		dprc_remove_devices(mc_dev, NULL, 0);
 		dprc_cleanup(mc_dev);
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index 37d61eaa58c8..be60f41af30f 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -7,9 +7,27 @@
 #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;
+	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] 18+ messages in thread

* [PATCH v5 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (3 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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 | 68 ++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 802f3a00337f..7ceadb801082 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -30,11 +30,20 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
 
 	for (i = 0; i < count; i++) {
 		struct resource *res = &mc_dev->regions[i];
+		int no_mmap = is_fsl_mc_bus_dprc(mc_dev);
 
 		vdev->regions[i].addr = res->start;
 		vdev->regions[i].size = resource_size(res);
-		vdev->regions[i].flags = 0;
 		vdev->regions[i].type = mc_dev->regions[i].flags & IORESOURCE_BITS;
+		/*
+		 * Only regions addressed with PAGE granularity may be
+		 * MMAPed securely.
+		 */
+		if (!no_mmap && !(vdev->regions[i].addr & ~PAGE_MASK) &&
+				!(vdev->regions[i].size & ~PAGE_MASK))
+			vdev->regions[i].flags |=
+					VFIO_REGION_INFO_FLAG_MMAP;
+
 	}
 
 	return 0;
@@ -163,9 +172,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 >= mc_dev->obj_desc.region_count)
+		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] 18+ messages in thread

* [PATCH v5 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (4 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         | 92 +++++++++++++++++++++--
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  7 +-
 2 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 7ceadb801082..548fb4d80f3f 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -17,6 +17,78 @@
 
 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;
+
+	mutex_lock(&reflck_lock);
+	if (is_fsl_mc_bus_dprc(vdev->mc_dev)) {
+		vdev->reflck = vfio_fsl_mc_reflck_alloc();
+		ret = PTR_ERR_OR_ZERO(vdev->reflck);
+	} 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 || !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;
@@ -62,7 +134,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)
@@ -70,12 +142,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;
 }
@@ -84,12 +156,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);
 }
@@ -342,14 +414,18 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
 		goto out_group_put;
 	}
 
-	ret = vfio_fsl_mc_init_device(vdev);
+	ret = vfio_fsl_mc_reflck_attach(vdev);
 	if (ret)
 		goto out_group_dev;
 
-	mutex_init(&vdev->driver_lock);
+	ret = vfio_fsl_mc_init_device(vdev);
+	if (ret)
+		goto out_reflck;
 
 	return 0;
 
+out_reflck:
+	vfio_fsl_mc_reflck_put(vdev->reflck);
 out_group_dev:
 	vfio_del_group_dev(dev);
 out_group_put:
@@ -366,7 +442,7 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
 	if (!vdev)
 		return -EINVAL;
 
-	mutex_destroy(&vdev->driver_lock);
+	vfio_fsl_mc_reflck_put(vdev->reflck);
 
 	if (is_fsl_mc_bus_dprc(mc_dev)) {
 		dprc_remove_devices(mc_dev, NULL, 0);
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index be60f41af30f..d47ef6215429 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;
@@ -27,7 +32,7 @@ struct vfio_fsl_mc_device {
 	struct notifier_block        nb;
 	int				refcnt;
 	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] 18+ messages in thread

* [PATCH v5 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (5 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         | 52 ++++++++++++++++++++++-
 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c    | 34 +++++++++++++++
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  6 +++
 4 files changed, 91 insertions(+), 3 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 548fb4d80f3f..2919e2d0041b 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -217,11 +217,55 @@ 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;
+		size_t data_size = 0;
+
+		minsz = offsetofend(struct vfio_irq_set, count);
+
+		if (copy_from_user(&hdr, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		ret = vfio_set_irqs_validate_and_prepare(&hdr, mc_dev->obj_desc.irq_count,
+					mc_dev->obj_desc.irq_count, &data_size);
+		if (ret)
+			return ret;
+
+		if (data_size) {
+			data = memdup_user((void __user *)(arg + minsz),
+				   data_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:
 	{
@@ -422,6 +466,8 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
 	if (ret)
 		goto out_reflck;
 
+	mutex_init(&vdev->igate);
+
 	return 0;
 
 out_reflck:
@@ -442,6 +488,8 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
 	if (!vdev)
 		return -EINVAL;
 
+	mutex_destroy(&vdev->igate);
+
 	vfio_fsl_mc_reflck_put(vdev->reflck);
 
 	if (is_fsl_mc_bus_dprc(mc_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..5232f208e361
--- /dev/null
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
@@ -0,0 +1,34 @@
+// 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_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)
+{
+	if (flags & VFIO_IRQ_SET_ACTION_TRIGGER)
+		return  vfio_fsl_mc_set_irq_trigger(vdev, index, start,
+			  count, flags, data);
+	else
+		return -EINVAL;
+}
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
index d47ef6215429..2c3f625a3240 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_private.h
@@ -33,6 +33,12 @@ struct vfio_fsl_mc_device {
 	int				refcnt;
 	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] 18+ messages in thread

* [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (6 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-09-29 11:27   ` kernel test robot
  2020-10-02 20:29   ` Alex Williamson
  2020-09-29  9:03 ` [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
  2020-09-29  9:03 ` [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
  9 siblings, 2 replies; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         |  24 +++-
 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c    | 161 +++++++++++++++++++++-
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  10 ++
 3 files changed, 193 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 2919e2d0041b..82157837f37a 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -155,12 +155,34 @@ 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);
+
+		if (ret) {
+			dev_warn(&mc_cont->dev, "VFIO_FLS_MC: reset device has failed (%d)\n",
+				 ret);
+			WARN_ON(1);
+		}
+
+		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 5232f208e361..992ee18f1f6f 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
@@ -13,12 +13,150 @@
 #include "linux/fsl/mc.h"
 #include "vfio_fsl_mc_private.h"
 
+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,
@@ -32,3 +170,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
 	else
 		return -EINVAL;
 }
+
+/* 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 2c3f625a3240..7aa49b9ba60d 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;
@@ -34,6 +41,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,
@@ -41,4 +49,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] 18+ messages in thread

* [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (7 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-10-02 20:50   ` Alex Williamson
  2020-09-29  9:03 ` [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
  9 siblings, 1 reply; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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         | 118 +++++++++++++++++++++-
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |   1 +
 2 files changed, 116 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 82157837f37a..0aff99cdf722 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"
 
@@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
 				!(vdev->regions[i].size & ~PAGE_MASK))
 			vdev->regions[i].flags |=
 					VFIO_REGION_INFO_FLAG_MMAP;
-
+		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;
 	}
 
 	return 0;
@@ -123,6 +126,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)
 {
+	struct fsl_mc_device *mc_dev = vdev->mc_dev;
+	int i;
+
+	for (i = 0; i < mc_dev->obj_desc.region_count; i++)
+		iounmap(vdev->regions[i].ioaddr);
 	kfree(vdev->regions);
 }
 
@@ -301,13 +309,117 @@ 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 fsl_mc_device *mc_dev = vdev->mc_dev;
+	struct vfio_fsl_mc_region *region;
+	u64 data[8];
+	int i;
+
+	if (index >= mc_dev->obj_desc.region_count)
+		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 fsl_mc_device *mc_dev = vdev->mc_dev;
+	struct vfio_fsl_mc_region *region;
+	u64 data[8];
+	int ret;
+
+	if (index >= mc_dev->obj_desc.region_count)
+		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 7aa49b9ba60d..a97ee691ed47 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] 18+ messages in thread

* [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset
  2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
                   ` (8 preceding siblings ...)
  2020-09-29  9:03 ` [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
@ 2020-09-29  9:03 ` Diana Craciun
  2020-10-02 20:50   ` Alex Williamson
  9 siblings, 1 reply; 18+ messages in thread
From: Diana Craciun @ 2020-09-29  9:03 UTC (permalink / raw)
  To: alex.williamson, kvm
  Cc: bharatb.linux, linux-kernel, eric.auger, 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 | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 0aff99cdf722..e1b2dea8a5fe 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -299,7 +299,19 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
 	}
 	case VFIO_DEVICE_RESET:
 	{
-		return -ENOTTY;
+		int ret;
+		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] 18+ messages in thread

* Re: [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
  2020-09-29  9:03 ` [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
@ 2020-09-29 11:27   ` kernel test robot
  2020-10-02 20:29   ` Alex Williamson
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2020-09-29 11:27 UTC (permalink / raw)
  To: Diana Craciun, alex.williamson, kvm
  Cc: kbuild-all, bharatb.linux, linux-kernel, eric.auger,
	Diana Craciun, Bharat Bhushan

[-- Attachment #1: Type: text/plain, Size: 3636 bytes --]

Hi Diana,

I love your patch! Perhaps something to improve:

[auto build test WARNING on vfio/next]
[also build test WARNING on linux/master linus/master v5.9-rc7 next-20200929]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Diana-Craciun/vfio-fsl-mc-VFIO-support-for-FSL-MC-device/20200929-170752
base:   https://github.com/awilliam/linux-vfio.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/2efbc8ba7e0dcd6dc33365f917d29c15ae0fc29a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Diana-Craciun/vfio-fsl-mc-VFIO-support-for-FSL-MC-device/20200929-170752
        git checkout 2efbc8ba7e0dcd6dc33365f917d29c15ae0fc29a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c:16:5: warning: no previous prototype for 'vfio_fsl_mc_irqs_allocate' [-Wmissing-prototypes]
      16 | int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c: In function 'vfio_fsl_mc_set_irq_trigger':
   drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c:121:8: error: implicit declaration of function 'fsl_mc_populate_irq_pool' [-Werror=implicit-function-declaration]
     121 |  ret = fsl_mc_populate_irq_pool(mc_cont,
         |        ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c:122:4: error: 'FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS' undeclared (first use in this function)
     122 |    FSL_MC_IRQ_POOL_MAX_TOTAL_IRQS);
         |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c:122:4: note: each undeclared identifier is reported only once for each function it appears in
   cc1: some warnings being treated as errors

vim +/vfio_fsl_mc_irqs_allocate +16 drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c

    15	
  > 16	int vfio_fsl_mc_irqs_allocate(struct vfio_fsl_mc_device *vdev)
    17	{
    18		struct fsl_mc_device *mc_dev = vdev->mc_dev;
    19		struct vfio_fsl_mc_irq *mc_irq;
    20		int irq_count;
    21		int ret, i;
    22	
    23		/* Device does not support any interrupt */
    24		if (mc_dev->obj_desc.irq_count == 0)
    25			return 0;
    26	
    27		/* interrupts were already allocated for this device */
    28		if (vdev->mc_irqs)
    29			return 0;
    30	
    31		irq_count = mc_dev->obj_desc.irq_count;
    32	
    33		mc_irq = kcalloc(irq_count, sizeof(*mc_irq), GFP_KERNEL);
    34		if (!mc_irq)
    35			return -ENOMEM;
    36	
    37		/* Allocate IRQs */
    38		ret = fsl_mc_allocate_irqs(mc_dev);
    39		if (ret) {
    40			kfree(mc_irq);
    41			return ret;
    42		}
    43	
    44		for (i = 0; i < irq_count; i++) {
    45			mc_irq[i].count = 1;
    46			mc_irq[i].flags = VFIO_IRQ_INFO_EVENTFD;
    47		}
    48	
    49		vdev->mc_irqs = mc_irq;
    50	
    51		return 0;
    52	}
    53	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 70408 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
  2020-09-29  9:03 ` [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
@ 2020-10-02 17:24   ` Alex Williamson
  2020-10-05 17:24     ` Diana Craciun OSS
  0 siblings, 1 reply; 18+ messages in thread
From: Alex Williamson @ 2020-10-02 17:24 UTC (permalink / raw)
  To: Diana Craciun
  Cc: kvm, bharatb.linux, linux-kernel, eric.auger, Bharat Bhushan

On Tue, 29 Sep 2020 12:03:31 +0300
Diana Craciun <diana.craciun@oss.nxp.com> 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         | 90 +++++++++++++++++++++++
>  drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  1 +
>  2 files changed, 91 insertions(+)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index a7a483a1e90b..ba44d6d01cc9 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,79 @@ 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, "VFIO_FSL_MC: Setting driver override for device in dprc %s failed\n",
> +			     dev_name(&mc_cont->dev));
> +		else
> +			dev_info(dev, "VFIO_FSL_MC: Setting driver override for device in dprc %s\n",
> +			 dev_name(&mc_cont->dev));

Nit, some whitespace inconsistencies on the second line of each of
these.  I can fixup on commit if we don't find anything else worth a
respin.

> +	} 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, "VFIO_FSL_MC: 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) {
> +		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Failed to setup DPRC (%d)\n", ret);
> +		goto out_nc_unreg;
> +	}
> +
> +	ret = dprc_scan_container(mc_dev, false);
> +	if (ret) {
> +		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Container scanning failed (%d)\n", ret);
> +		goto out_dprc_cleanup;
> +	}

If I understand this correctly, we've setup the bus notifier to write
the driver override as each sub-devices appear on the bus from this
scan.  When non-dprc devices are removed below, it appears we remove all
their sub-devices.  Is there a chance here that an error from the scan
leaves residual sub-devices, ie. should we proceed the below
dprc_cleanup() with a call to dprc_remove_devices() to be certain none
remain?  Thanks,

Alex


> +
> +	return 0;
> +
> +out_dprc_cleanup:
> +	dprc_cleanup(mc_dev);
> +out_nc_unreg:
> +	bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> +	vdev->nb.notifier_call = NULL;
> +
> +	return ret;
> +}
> +
>  static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>  {
>  	struct iommu_group *group;
> @@ -110,8 +185,15 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>  		dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n");
>  		goto out_group_put;
>  	}
> +
> +	ret = vfio_fsl_mc_init_device(vdev);
> +	if (ret)
> +		goto out_group_dev;
> +
>  	return 0;
>  
> +out_group_dev:
> +	vfio_del_group_dev(dev);
>  out_group_put:
>  	vfio_iommu_group_put(group, dev);
>  	return ret;
> @@ -126,6 +208,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>  	if (!vdev)
>  		return -EINVAL;
>  
> +	if (is_fsl_mc_bus_dprc(mc_dev)) {
> +		dprc_remove_devices(mc_dev, NULL, 0);
> +		dprc_cleanup(mc_dev);
> +	}
> +
> +	if (vdev->nb.notifier_call)
> +		bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
> +
>  	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 */


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd
  2020-09-29  9:03 ` [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
  2020-09-29 11:27   ` kernel test robot
@ 2020-10-02 20:29   ` Alex Williamson
  1 sibling, 0 replies; 18+ messages in thread
From: Alex Williamson @ 2020-10-02 20:29 UTC (permalink / raw)
  To: Diana Craciun
  Cc: kvm, bharatb.linux, linux-kernel, eric.auger, Bharat Bhushan

On Tue, 29 Sep 2020 12:03:37 +0300
Diana Craciun <diana.craciun@oss.nxp.com> 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         |  24 +++-
>  drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c    | 161 +++++++++++++++++++++-
>  drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  10 ++
>  3 files changed, 193 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 2919e2d0041b..82157837f37a 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -155,12 +155,34 @@ 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);
> +
> +		if (ret) {
> +			dev_warn(&mc_cont->dev, "VFIO_FLS_MC: reset device has failed (%d)\n",
> +				 ret);
> +			WARN_ON(1);
> +		}
> +
> +		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 5232f208e361..992ee18f1f6f 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc_intr.c
> @@ -13,12 +13,150 @@
>  #include "linux/fsl/mc.h"
>  #include "vfio_fsl_mc_private.h"
>  
> +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,
> @@ -32,3 +170,24 @@ int vfio_fsl_mc_set_irqs_ioctl(struct vfio_fsl_mc_device *vdev,
>  	else
>  		return -EINVAL;
>  }
> +
> +/* 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)

Nit, the former test seems unnecessary, the latter test covers both
cases.  Comment style also switched here.  Thanks,

Alex

> +		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 2c3f625a3240..7aa49b9ba60d 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;
> @@ -34,6 +41,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,
> @@ -41,4 +49,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 */


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset
  2020-09-29  9:03 ` [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
@ 2020-10-02 20:50   ` Alex Williamson
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Williamson @ 2020-10-02 20:50 UTC (permalink / raw)
  To: Diana Craciun; +Cc: kvm, bharatb.linux, linux-kernel, eric.auger

On Tue, 29 Sep 2020 12:03:39 +0300
Diana Craciun <diana.craciun@oss.nxp.com> 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 | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 0aff99cdf722..e1b2dea8a5fe 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -299,7 +299,19 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
>  	}
>  	case VFIO_DEVICE_RESET:
>  	{
> -		return -ENOTTY;
> +		int ret;
> +		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;

This should also result in setting the VFIO_DEVICE_FLAGS_RESET bit in
vfio_device_info.flags for the appropriate devices.  Thanks,

Alex


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
  2020-09-29  9:03 ` [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
@ 2020-10-02 20:50   ` Alex Williamson
  2020-10-05 17:29     ` Diana Craciun OSS
  0 siblings, 1 reply; 18+ messages in thread
From: Alex Williamson @ 2020-10-02 20:50 UTC (permalink / raw)
  To: Diana Craciun
  Cc: kvm, bharatb.linux, linux-kernel, eric.auger, Bharat Bhushan

On Tue, 29 Sep 2020 12:03:38 +0300
Diana Craciun <diana.craciun@oss.nxp.com> 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).
> 
> 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         | 118 +++++++++++++++++++++-
>  drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |   1 +
>  2 files changed, 116 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 82157837f37a..0aff99cdf722 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"
>  
> @@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>  				!(vdev->regions[i].size & ~PAGE_MASK))
>  			vdev->regions[i].flags |=
>  					VFIO_REGION_INFO_FLAG_MMAP;
> -
> +		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;
>  	}
>  
>  	return 0;
> @@ -123,6 +126,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)
>  {
> +	struct fsl_mc_device *mc_dev = vdev->mc_dev;
> +	int i;
> +
> +	for (i = 0; i < mc_dev->obj_desc.region_count; i++)
> +		iounmap(vdev->regions[i].ioaddr);
>  	kfree(vdev->regions);
>  }
>  
> @@ -301,13 +309,117 @@ 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 fsl_mc_device *mc_dev = vdev->mc_dev;
> +	struct vfio_fsl_mc_region *region;
> +	u64 data[8];
> +	int i;
> +
> +	if (index >= mc_dev->obj_desc.region_count)
> +		return -EINVAL;
> +
> +	region = &vdev->regions[index];
> +
> +	if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
> +		return -EINVAL;

Nit, there are no regions w/o read access according to the regions_init
code above.  Maybe this is just for symmetry with write?  Keep it if
you prefer.  Thanks,

Alex

> +
> +	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 fsl_mc_device *mc_dev = vdev->mc_dev;
> +	struct vfio_fsl_mc_region *region;
> +	u64 data[8];
> +	int ret;
> +
> +	if (index >= mc_dev->obj_desc.region_count)
> +		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 7aa49b9ba60d..a97ee691ed47 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 {


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind
  2020-10-02 17:24   ` Alex Williamson
@ 2020-10-05 17:24     ` Diana Craciun OSS
  0 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun OSS @ 2020-10-05 17:24 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, bharatb.linux, linux-kernel, eric.auger, Bharat Bhushan

On 10/2/2020 8:24 PM, Alex Williamson wrote:
> On Tue, 29 Sep 2020 12:03:31 +0300
> Diana Craciun <diana.craciun@oss.nxp.com> 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         | 90 +++++++++++++++++++++++
>>   drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |  1 +
>>   2 files changed, 91 insertions(+)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index a7a483a1e90b..ba44d6d01cc9 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,79 @@ 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, "VFIO_FSL_MC: Setting driver override for device in dprc %s failed\n",
>> +			     dev_name(&mc_cont->dev));
>> +		else
>> +			dev_info(dev, "VFIO_FSL_MC: Setting driver override for device in dprc %s\n",
>> +			 dev_name(&mc_cont->dev));
> 
> Nit, some whitespace inconsistencies on the second line of each of
> these.  I can fixup on commit if we don't find anything else worth a
> respin.
> 
>> +	} 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, "VFIO_FSL_MC: 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) {
>> +		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Failed to setup DPRC (%d)\n", ret);
>> +		goto out_nc_unreg;
>> +	}
>> +
>> +	ret = dprc_scan_container(mc_dev, false);
>> +	if (ret) {
>> +		dev_err(&mc_dev->dev, "VFIO_FSL_MC: Container scanning failed (%d)\n", ret);
>> +		goto out_dprc_cleanup;
>> +	}
> 
> If I understand this correctly, we've setup the bus notifier to write
> the driver override as each sub-devices appear on the bus from this
> scan.  When non-dprc devices are removed below, it appears we remove all
> their sub-devices.  Is there a chance here that an error from the scan
> leaves residual sub-devices, ie. should we proceed the below
> dprc_cleanup() with a call to dprc_remove_devices() to be certain none
> remain?  Thanks,
> 
> Alex

Right, we should call dprc_remove_devices as well. I will respin another 
version.

> 
> 
>> +
>> +	return 0;
>> +
>> +out_dprc_cleanup:
>> +	dprc_cleanup(mc_dev);
>> +out_nc_unreg:
>> +	bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
>> +	vdev->nb.notifier_call = NULL;
>> +
>> +	return ret;
>> +}
>> +
>>   static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>>   {
>>   	struct iommu_group *group;
>> @@ -110,8 +185,15 @@ static int vfio_fsl_mc_probe(struct fsl_mc_device *mc_dev)
>>   		dev_err(dev, "VFIO_FSL_MC: Failed to add to vfio group\n");
>>   		goto out_group_put;
>>   	}
>> +
>> +	ret = vfio_fsl_mc_init_device(vdev);
>> +	if (ret)
>> +		goto out_group_dev;
>> +
>>   	return 0;
>>   
>> +out_group_dev:
>> +	vfio_del_group_dev(dev);
>>   out_group_put:
>>   	vfio_iommu_group_put(group, dev);
>>   	return ret;
>> @@ -126,6 +208,14 @@ static int vfio_fsl_mc_remove(struct fsl_mc_device *mc_dev)
>>   	if (!vdev)
>>   		return -EINVAL;
>>   
>> +	if (is_fsl_mc_bus_dprc(mc_dev)) {
>> +		dprc_remove_devices(mc_dev, NULL, 0);
>> +		dprc_cleanup(mc_dev);
>> +	}
>> +
>> +	if (vdev->nb.notifier_call)
>> +		bus_unregister_notifier(&fsl_mc_bus_type, &vdev->nb);
>> +
>>   	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 */
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
  2020-10-02 20:50   ` Alex Williamson
@ 2020-10-05 17:29     ` Diana Craciun OSS
  0 siblings, 0 replies; 18+ messages in thread
From: Diana Craciun OSS @ 2020-10-05 17:29 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, bharatb.linux, linux-kernel, eric.auger, Bharat Bhushan

On 10/2/2020 11:50 PM, Alex Williamson wrote:
> On Tue, 29 Sep 2020 12:03:38 +0300
> Diana Craciun <diana.craciun@oss.nxp.com> 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).
>>
>> 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         | 118 +++++++++++++++++++++-
>>   drivers/vfio/fsl-mc/vfio_fsl_mc_private.h |   1 +
>>   2 files changed, 116 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
>> index 82157837f37a..0aff99cdf722 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"
>>   
>> @@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
>>   				!(vdev->regions[i].size & ~PAGE_MASK))
>>   			vdev->regions[i].flags |=
>>   					VFIO_REGION_INFO_FLAG_MMAP;
>> -
>> +		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;
>>   	}
>>   
>>   	return 0;
>> @@ -123,6 +126,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)
>>   {
>> +	struct fsl_mc_device *mc_dev = vdev->mc_dev;
>> +	int i;
>> +
>> +	for (i = 0; i < mc_dev->obj_desc.region_count; i++)
>> +		iounmap(vdev->regions[i].ioaddr);
>>   	kfree(vdev->regions);
>>   }
>>   
>> @@ -301,13 +309,117 @@ 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 fsl_mc_device *mc_dev = vdev->mc_dev;
>> +	struct vfio_fsl_mc_region *region;
>> +	u64 data[8];
>> +	int i;
>> +
>> +	if (index >= mc_dev->obj_desc.region_count)
>> +		return -EINVAL;
>> +
>> +	region = &vdev->regions[index];
>> +
>> +	if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
>> +		return -EINVAL;
> 
> Nit, there are no regions w/o read access according to the regions_init
> code above.  Maybe this is just for symmetry with write?  Keep it if
> you prefer.  Thanks,

I would prefer to keep it like this for symmetry with write.

Thanks,
Diana

> 
> Alex
> 
>> +
>> +	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 fsl_mc_device *mc_dev = vdev->mc_dev;
>> +	struct vfio_fsl_mc_region *region;
>> +	u64 data[8];
>> +	int ret;
>> +
>> +	if (index >= mc_dev->obj_desc.region_count)
>> +		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 7aa49b9ba60d..a97ee691ed47 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 {
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-10-05 17:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29  9:03 [PATCH v5 00/10] vfio/fsl-mc: VFIO support for FSL-MC device Diana Craciun
2020-09-29  9:03 ` [PATCH v5 01/10] vfio/fsl-mc: Add VFIO framework skeleton for fsl-mc devices Diana Craciun
2020-09-29  9:03 ` [PATCH v5 02/10] vfio/fsl-mc: Scan DPRC objects on vfio-fsl-mc driver bind Diana Craciun
2020-10-02 17:24   ` Alex Williamson
2020-10-05 17:24     ` Diana Craciun OSS
2020-09-29  9:03 ` [PATCH v5 03/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_INFO ioctl Diana Craciun
2020-09-29  9:03 ` [PATCH v5 04/10] vfio/fsl-mc: Implement VFIO_DEVICE_GET_REGION_INFO ioctl call Diana Craciun
2020-09-29  9:03 ` [PATCH v5 05/10] vfio/fsl-mc: Allow userspace to MMAP fsl-mc device MMIO regions Diana Craciun
2020-09-29  9:03 ` [PATCH v5 06/10] vfio/fsl-mc: Added lock support in preparation for interrupt handling Diana Craciun
2020-09-29  9:03 ` [PATCH v5 07/10] vfio/fsl-mc: Add irq infrastructure for fsl-mc devices Diana Craciun
2020-09-29  9:03 ` [PATCH v5 08/10] vfio/fsl-mc: trigger an interrupt via eventfd Diana Craciun
2020-09-29 11:27   ` kernel test robot
2020-10-02 20:29   ` Alex Williamson
2020-09-29  9:03 ` [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices Diana Craciun
2020-10-02 20:50   ` Alex Williamson
2020-10-05 17:29     ` Diana Craciun OSS
2020-09-29  9:03 ` [PATCH v5 10/10] vfio/fsl-mc: Add support for device reset Diana Craciun
2020-10-02 20:50   ` Alex Williamson

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).