[RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Antonios Motakis]

This RFC's intention is to show what an interface to access device
properties for the VFIO platform driver can look like. These properties
can be from the device tree node describing the device, or ACPI properties
in the future.

If a device property corresponding to a platform device bound by VFIO PLATFORM
or VFIO AMBA is available, this patch series will allow the user to query
for this information. This can be useful for userspace drivers to automatically
query parameters related to the device.

Specifically for QEMU, reading the "compatible" property of the device tree
node could be of use to find out what device is being assigned to the guest and
handle appropriately a wider range of devices in the future, and to generate an
appropriate device tree for the guest.

Older versions of this series where specifically targeted at device tree
properties. This version has been reworked on top of Rafael J. Wysocki's
uniform device properties API for device tree and ACPI devices. This will allow
us to use the API in the future with devices described via ACPI.

This also means a kernel including the uniform device properties API is needed
to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/

The API to get the list of available properties and the device tree full_name
have been removed. These probably don't serve an useful purpose, as the user
of this API need to know anyway what properties are specific to the device he
wants to access with VFIO. If we decide to reintroduce the list of properties
in the future, the generic device properties API in the kernel will have to
be extended accordingly.

A kernel with this series and all the dependencies applied can be pulled from
branch vfio-device-properties-v3 from the repository:
https://github.com/virtualopensystems/linux-kvm-arm.git

Changes since v2:
 - Reworked on top of Rafael J. Wysocki's uniform device properties API for
   device tree and ACPI
 - Support for u64 array properties
 - Removed API to get list of available properties and device tree full_name
Changes since v1:
 - Updated for VFIO platform patch series v8:
   VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
 - Refactored and cleaned up the code

Antonios Motakis (3):
  vfio: platform: add device properties skeleton and user API
  vfio: platform: access device property as a list of strings
  vfio: platform: return device properties as arrays of unsigned
    integers

 drivers/vfio/platform/Makefile                |   3 +-
 drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
 drivers/vfio/platform/vfio_platform_private.h |   7 ++
 include/uapi/linux/vfio.h                     |  26 +++++
 5 files changed, 232 insertions(+), 1 deletion(-)
 create mode 100644 drivers/vfio/platform/properties.c

-- 
2.1.3

[RFC PATCH v3 1/3] vfio: platform: add device properties skeleton and user API

[Antonios Motakis]

This patch introduced the API to return device properties about
a PLATFORM device (if described by a device tree or ACPI) and the
skeleton of the implementation for VFIO_PLATFORM. Information about any
device node bound by VFIO_PLATFORM should be queried via the introduced
ioctl VFIO_DEVICE_GET_DEV_PROPERTY.

The user needs to know the name and the data type of the property he is
accessing.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/platform/Makefile                |  3 +-
 drivers/vfio/platform/properties.c            | 61 +++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_common.c  | 35 +++++++++++++++
 drivers/vfio/platform/vfio_platform_private.h |  7 +++
 include/uapi/linux/vfio.h                     | 26 ++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/vfio/platform/properties.c

diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 81de144..99f3ba1 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -1,5 +1,6 @@
 
-vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
+vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o \
+		   devtree.o
 
 obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
 
diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
new file mode 100644
index 0000000..8b90465
--- /dev/null
+++ b/drivers/vfio/platform/properties.c
@@ -0,0 +1,61 @@
+#include <linux/slab.h>
+#include <linux/vfio.h>
+#include <linux/property.h>
+#include "vfio_platform_private.h"
+
+static int dev_property_get_strings(struct device *dev,
+				    char *name, unsigned *lenp,
+				    void __user *datap, unsigned long datasz)
+{
+	return -EINVAL;
+}
+
+static int dev_property_get_uint(struct device *dev, char *name,
+				 uint32_t type, unsigned *lenp,
+				 void __user *datap, unsigned long datasz)
+{
+	return -EINVAL;
+}
+
+int vfio_platform_dev_properties(struct device *dev,
+				 uint32_t type, unsigned *lenp,
+				 void __user *datap, unsigned long datasz)
+{
+	char *name;
+	long namesz;
+	int ret;
+
+	namesz = strnlen_user(datap, datasz);
+	if (!namesz)
+		return -EFAULT;
+	if (namesz > datasz)
+		return -EINVAL;
+
+	name = kzalloc(namesz, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+	if (strncpy_from_user(name, datap, namesz) <= 0) {
+		kfree(name);
+		return -EFAULT;
+	}
+
+	switch (type) {
+	case VFIO_DEV_PROPERTY_TYPE_STRINGS:
+		ret = dev_property_get_strings(dev, name, lenp, datap, datasz);
+		break;
+
+	case VFIO_DEV_PROPERTY_TYPE_U64:
+	case VFIO_DEV_PROPERTY_TYPE_U32:
+	case VFIO_DEV_PROPERTY_TYPE_U16:
+	case VFIO_DEV_PROPERTY_TYPE_U8:
+		ret = dev_property_get_uint(dev, name, type, lenp,
+					    datap, datasz);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	kfree(name);
+	return ret;
+}
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index a532a25..83ad4b74 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/vfio.h>
+#include <linux/property.h>
 
 #include "vfio_platform_private.h"
 
@@ -251,6 +252,34 @@ static long vfio_platform_ioctl(void *device_data,
 
 		return ret;
 
+	} else if (cmd == VFIO_DEVICE_GET_DEV_PROPERTY) {
+		struct vfio_dev_property info;
+		void __user *datap;
+		unsigned long datasz;
+		int ret;
+
+		if (!vdev->dev)
+			return -EINVAL;
+
+		minsz = offsetofend(struct vfio_dev_property, length);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		datap = (void __user *) arg + minsz;
+		datasz = info.argsz - minsz;
+
+		ret = vfio_platform_dev_properties(vdev->dev, info.type,
+						   &info.length, datap, datasz);
+
+		if (copy_to_user((void __user *)arg, &info, minsz))
+			ret = -EFAULT;
+
+		return ret;
+
 	} else if (cmd == VFIO_DEVICE_RESET)
 		return -EINVAL;
 
@@ -501,6 +530,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
 		return ret;
 	}
 
+	/* add device properties flag */
+	if (device_property_present(dev, "name")) {
+		vdev->dev = dev;
+		vdev->flags |= VFIO_DEVICE_FLAGS_DEV_PROPERTIES;
+	}
+
 	mutex_init(&vdev->igate);
 
 	return 0;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index c3d5b4b..fc6b1fb 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -53,6 +53,7 @@ struct vfio_platform_device {
 	u32				num_irqs;
 	int				refcnt;
 	struct mutex			igate;
+	struct device			*dev;
 
 	/*
 	 * These fields should be filled by the bus specific binder
@@ -79,4 +80,10 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
 					unsigned start, unsigned count,
 					void *data);
 
+/* device properties support in devtree.c */
+extern int vfio_platform_dev_properties(struct device *dev,
+					uint32_t type, unsigned *lenp,
+					void __user *datap,
+					unsigned long datasz);
+
 #endif /* VFIO_PLATFORM_PRIVATE_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 544d3d8..ac50ab3f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -161,12 +161,38 @@ struct vfio_device_info {
 #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
 #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)	/* vfio-platform device */
 #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)	/* vfio-amba device */
+#define VFIO_DEVICE_FLAGS_DEV_PROPERTIES (1 << 4) /* Device properties */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
 #define VFIO_DEVICE_GET_INFO		_IO(VFIO_TYPE, VFIO_BASE + 7)
 
 /**
+ * VFIO_DEVICE_GET_DEV_PROPERTY - _IOR(VFIO_TYPE, VFIO_BASE + 16,
+ *						struct vfio_devtree_info)
+ *
+ * Retrive a device property, e.g. from a device tree if available.
+ * Caller will initialize data[] with a single string with the requested
+ * devicetree property name, and type depending on whether a array of strings
+ * or an array of u32 values is expected. On success, data[] will be extended
+ * with the requested information, either as an array of u32, or with a list
+ * of strings sepparated by the NULL terminating character.
+ * Return: 0 on success, -errno on failure.
+ */
+struct vfio_dev_property {
+	__u32	argsz;
+	__u32	type;
+#define VFIO_DEV_PROPERTY_TYPE_STRINGS	0
+#define VFIO_DEV_PROPERTY_TYPE_U8	1
+#define VFIO_DEV_PROPERTY_TYPE_U16	2
+#define VFIO_DEV_PROPERTY_TYPE_U32	3
+#define VFIO_DEV_PROPERTY_TYPE_U64	4
+	__u32	length;
+	__u8	data[];
+};
+#define VFIO_DEVICE_GET_DEV_PROPERTY	_IO(VFIO_TYPE, VFIO_BASE + 17)
+
+/**
  * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
  *				       struct vfio_region_info)
  *
-- 
2.1.3

[RFC PATCH v3 1/3] vfio: platform: add device properties skeleton and user API

[Antonios Motakis]

This patch introduced the API to return device properties about
a PLATFORM device (if described by a device tree or ACPI) and the
skeleton of the implementation for VFIO_PLATFORM. Information about any
device node bound by VFIO_PLATFORM should be queried via the introduced
ioctl VFIO_DEVICE_GET_DEV_PROPERTY.

The user needs to know the name and the data type of the property he is
accessing.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/platform/Makefile                |  3 +-
 drivers/vfio/platform/properties.c            | 61 +++++++++++++++++++++++++++
 drivers/vfio/platform/vfio_platform_common.c  | 35 +++++++++++++++
 drivers/vfio/platform/vfio_platform_private.h |  7 +++
 include/uapi/linux/vfio.h                     | 26 ++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/vfio/platform/properties.c

diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
index 81de144..99f3ba1 100644
--- a/drivers/vfio/platform/Makefile
+++ b/drivers/vfio/platform/Makefile
@@ -1,5 +1,6 @@
 
-vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
+vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o \
+		   devtree.o
 
 obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
 
diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
new file mode 100644
index 0000000..8b90465
--- /dev/null
+++ b/drivers/vfio/platform/properties.c
@@ -0,0 +1,61 @@
+#include <linux/slab.h>
+#include <linux/vfio.h>
+#include <linux/property.h>
+#include "vfio_platform_private.h"
+
+static int dev_property_get_strings(struct device *dev,
+				    char *name, unsigned *lenp,
+				    void __user *datap, unsigned long datasz)
+{
+	return -EINVAL;
+}
+
+static int dev_property_get_uint(struct device *dev, char *name,
+				 uint32_t type, unsigned *lenp,
+				 void __user *datap, unsigned long datasz)
+{
+	return -EINVAL;
+}
+
+int vfio_platform_dev_properties(struct device *dev,
+				 uint32_t type, unsigned *lenp,
+				 void __user *datap, unsigned long datasz)
+{
+	char *name;
+	long namesz;
+	int ret;
+
+	namesz = strnlen_user(datap, datasz);
+	if (!namesz)
+		return -EFAULT;
+	if (namesz > datasz)
+		return -EINVAL;
+
+	name = kzalloc(namesz, GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+	if (strncpy_from_user(name, datap, namesz) <= 0) {
+		kfree(name);
+		return -EFAULT;
+	}
+
+	switch (type) {
+	case VFIO_DEV_PROPERTY_TYPE_STRINGS:
+		ret = dev_property_get_strings(dev, name, lenp, datap, datasz);
+		break;
+
+	case VFIO_DEV_PROPERTY_TYPE_U64:
+	case VFIO_DEV_PROPERTY_TYPE_U32:
+	case VFIO_DEV_PROPERTY_TYPE_U16:
+	case VFIO_DEV_PROPERTY_TYPE_U8:
+		ret = dev_property_get_uint(dev, name, type, lenp,
+					    datap, datasz);
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+
+	kfree(name);
+	return ret;
+}
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index a532a25..83ad4b74 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/vfio.h>
+#include <linux/property.h>
 
 #include "vfio_platform_private.h"
 
@@ -251,6 +252,34 @@ static long vfio_platform_ioctl(void *device_data,
 
 		return ret;
 
+	} else if (cmd == VFIO_DEVICE_GET_DEV_PROPERTY) {
+		struct vfio_dev_property info;
+		void __user *datap;
+		unsigned long datasz;
+		int ret;
+
+		if (!vdev->dev)
+			return -EINVAL;
+
+		minsz = offsetofend(struct vfio_dev_property, length);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		datap = (void __user *) arg + minsz;
+		datasz = info.argsz - minsz;
+
+		ret = vfio_platform_dev_properties(vdev->dev, info.type,
+						   &info.length, datap, datasz);
+
+		if (copy_to_user((void __user *)arg, &info, minsz))
+			ret = -EFAULT;
+
+		return ret;
+
 	} else if (cmd == VFIO_DEVICE_RESET)
 		return -EINVAL;
 
@@ -501,6 +530,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
 		return ret;
 	}
 
+	/* add device properties flag */
+	if (device_property_present(dev, "name")) {
+		vdev->dev = dev;
+		vdev->flags |= VFIO_DEVICE_FLAGS_DEV_PROPERTIES;
+	}
+
 	mutex_init(&vdev->igate);
 
 	return 0;
diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
index c3d5b4b..fc6b1fb 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -53,6 +53,7 @@ struct vfio_platform_device {
 	u32				num_irqs;
 	int				refcnt;
 	struct mutex			igate;
+	struct device			*dev;
 
 	/*
 	 * These fields should be filled by the bus specific binder
@@ -79,4 +80,10 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
 					unsigned start, unsigned count,
 					void *data);
 
+/* device properties support in devtree.c */
+extern int vfio_platform_dev_properties(struct device *dev,
+					uint32_t type, unsigned *lenp,
+					void __user *datap,
+					unsigned long datasz);
+
 #endif /* VFIO_PLATFORM_PRIVATE_H */
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 544d3d8..ac50ab3f 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -161,12 +161,38 @@ struct vfio_device_info {
 #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
 #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)	/* vfio-platform device */
 #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)	/* vfio-amba device */
+#define VFIO_DEVICE_FLAGS_DEV_PROPERTIES (1 << 4) /* Device properties */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
 #define VFIO_DEVICE_GET_INFO		_IO(VFIO_TYPE, VFIO_BASE + 7)
 
 /**
+ * VFIO_DEVICE_GET_DEV_PROPERTY - _IOR(VFIO_TYPE, VFIO_BASE + 16,
+ *						struct vfio_devtree_info)
+ *
+ * Retrive a device property, e.g. from a device tree if available.
+ * Caller will initialize data[] with a single string with the requested
+ * devicetree property name, and type depending on whether a array of strings
+ * or an array of u32 values is expected. On success, data[] will be extended
+ * with the requested information, either as an array of u32, or with a list
+ * of strings sepparated by the NULL terminating character.
+ * Return: 0 on success, -errno on failure.
+ */
+struct vfio_dev_property {
+	__u32	argsz;
+	__u32	type;
+#define VFIO_DEV_PROPERTY_TYPE_STRINGS	0
+#define VFIO_DEV_PROPERTY_TYPE_U8	1
+#define VFIO_DEV_PROPERTY_TYPE_U16	2
+#define VFIO_DEV_PROPERTY_TYPE_U32	3
+#define VFIO_DEV_PROPERTY_TYPE_U64	4
+	__u32	length;
+	__u8	data[];
+};
+#define VFIO_DEVICE_GET_DEV_PROPERTY	_IO(VFIO_TYPE, VFIO_BASE + 17)
+
+/**
  * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
  *				       struct vfio_region_info)
  *
-- 
2.1.3

[RFC PATCH v3 2/3] vfio: platform: access device property as a list of strings

[Antonios Motakis]

Certain device properties (e.g. the device node name, the compatible
string), are available as a list of strings (separated by the null
terminating character). Let the VFIO user query this type of properties.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/platform/properties.c | 43 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
index 8b90465..39c6342 100644
--- a/drivers/vfio/platform/properties.c
+++ b/drivers/vfio/platform/properties.c
@@ -7,7 +7,48 @@ static int dev_property_get_strings(struct device *dev,
 				    char *name, unsigned *lenp,
 				    void __user *datap, unsigned long datasz)
 {
-	return -EINVAL;
+	const char **val;
+	int n, i, ret;
+
+	*lenp = 0;
+
+	n = device_property_read_string_array(dev, name, NULL, 0);
+	if (n < 0)
+		return n;
+
+	val = kcalloc(n, sizeof(char*), GFP_KERNEL);
+	if (!val)
+		return -ENOMEM;
+
+	ret = device_property_read_string_array(dev, name, val, n);
+	if (ret < 0)
+		goto out;
+
+	ret = 0;
+
+	for (i = 0; i < n; i++) {
+		size_t len = strlen(val[i]) + 1;
+
+		if (datasz < len) {
+			ret = -EOVERFLOW;
+			while (i < n)
+				*lenp += strlen(val[i++]) + 1;
+			goto out;
+		}
+
+		if (copy_to_user(datap, val[i], len)) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		*lenp += len;
+		datap += len;
+		datasz -= len;
+	}
+
+out:
+	kfree(val);
+	return ret;
 }
 
 static int dev_property_get_uint(struct device *dev, char *name,
-- 
2.1.3

[RFC PATCH v3 2/3] vfio: platform: access device property as a list of strings

[Antonios Motakis]

Certain device properties (e.g. the device node name, the compatible
string), are available as a list of strings (separated by the null
terminating character). Let the VFIO user query this type of properties.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/platform/properties.c | 43 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
index 8b90465..39c6342 100644
--- a/drivers/vfio/platform/properties.c
+++ b/drivers/vfio/platform/properties.c
@@ -7,7 +7,48 @@ static int dev_property_get_strings(struct device *dev,
 				    char *name, unsigned *lenp,
 				    void __user *datap, unsigned long datasz)
 {
-	return -EINVAL;
+	const char **val;
+	int n, i, ret;
+
+	*lenp = 0;
+
+	n = device_property_read_string_array(dev, name, NULL, 0);
+	if (n < 0)
+		return n;
+
+	val = kcalloc(n, sizeof(char*), GFP_KERNEL);
+	if (!val)
+		return -ENOMEM;
+
+	ret = device_property_read_string_array(dev, name, val, n);
+	if (ret < 0)
+		goto out;
+
+	ret = 0;
+
+	for (i = 0; i < n; i++) {
+		size_t len = strlen(val[i]) + 1;
+
+		if (datasz < len) {
+			ret = -EOVERFLOW;
+			while (i < n)
+				*lenp += strlen(val[i++]) + 1;
+			goto out;
+		}
+
+		if (copy_to_user(datap, val[i], len)) {
+			ret = -EFAULT;
+			goto out;
+		}
+
+		*lenp += len;
+		datap += len;
+		datasz -= len;
+	}
+
+out:
+	kfree(val);
+	return ret;
 }
 
 static int dev_property_get_uint(struct device *dev, char *name,
-- 
2.1.3

[RFC PATCH v3 3/3] vfio: platform: return device properties as arrays of unsigned integers

[Antonios Motakis]

Certain properties of a device are accessible as an array of unsigned
integers, either u64, u32, u16, or u8. Let the VFIO user query this
type of device properties.

Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
---
 drivers/vfio/platform/properties.c | 62 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
index 39c6342..645f6e5c 100644
--- a/drivers/vfio/platform/properties.c
+++ b/drivers/vfio/platform/properties.c
@@ -55,7 +55,67 @@ static int dev_property_get_uint(struct device *dev, char *name,
 				 uint32_t type, unsigned *lenp,
 				 void __user *datap, unsigned long datasz)
 {
-	return -EINVAL;
+	int ret, n;
+	u8 *out;
+	size_t sz;
+	int (*func)(const struct device *, const char *, void *, size_t)
+		= NULL;
+
+	switch (type) {
+	case VFIO_DEV_PROPERTY_TYPE_U64:
+		sz = sizeof(u64);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u64_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U32:
+		sz = sizeof(u32);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u32_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U16:
+		sz = sizeof(u16);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u16_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U8:
+		sz = sizeof(u8);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u8_array;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	/* get size of array */
+	n = func(dev, name, NULL, 0);
+	if (n < 0)
+		return n;
+
+	if (lenp)
+		*lenp = n * sz;
+
+	if (n * sz > datasz)
+		return -EOVERFLOW;
+
+	out = kcalloc(n, sz, GFP_KERNEL);
+	if (!out)
+		return -EFAULT;
+
+	ret = func(dev, name, out, n);
+	if (ret)
+		goto out;
+
+	if (copy_to_user(datap, out, n * sz))
+		ret = -EFAULT;
+
+out:
+	kfree(out);
+	return ret;
 }
 
 int vfio_platform_dev_properties(struct device *dev,
-- 
2.1.3

[RFC PATCH v3 3/3] vfio: platform: return device properties as arrays of unsigned integers

[Antonios Motakis]

Certain properties of a device are accessible as an array of unsigned
integers, either u64, u32, u16, or u8. Let the VFIO user query this
type of device properties.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/platform/properties.c | 62 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
index 39c6342..645f6e5c 100644
--- a/drivers/vfio/platform/properties.c
+++ b/drivers/vfio/platform/properties.c
@@ -55,7 +55,67 @@ static int dev_property_get_uint(struct device *dev, char *name,
 				 uint32_t type, unsigned *lenp,
 				 void __user *datap, unsigned long datasz)
 {
-	return -EINVAL;
+	int ret, n;
+	u8 *out;
+	size_t sz;
+	int (*func)(const struct device *, const char *, void *, size_t)
+		= NULL;
+
+	switch (type) {
+	case VFIO_DEV_PROPERTY_TYPE_U64:
+		sz = sizeof(u64);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u64_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U32:
+		sz = sizeof(u32);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u32_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U16:
+		sz = sizeof(u16);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u16_array;
+		break;
+	case VFIO_DEV_PROPERTY_TYPE_U8:
+		sz = sizeof(u8);
+		func = (int (*)(const struct device *,
+				const char *, void *, size_t))
+			device_property_read_u8_array;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	/* get size of array */
+	n = func(dev, name, NULL, 0);
+	if (n < 0)
+		return n;
+
+	if (lenp)
+		*lenp = n * sz;
+
+	if (n * sz > datasz)
+		return -EOVERFLOW;
+
+	out = kcalloc(n, sz, GFP_KERNEL);
+	if (!out)
+		return -EFAULT;
+
+	ret = func(dev, name, out, n);
+	if (ret)
+		goto out;
+
+	if (copy_to_user(datap, out, n * sz))
+		ret = -EFAULT;
+
+out:
+	kfree(out);
+	return ret;
 }
 
 int vfio_platform_dev_properties(struct device *dev,
-- 
2.1.3

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

Hi Baptiste, Antonios,

What are the plans wrt this series? I am currently integrating another
QEMU VFIO platform devices where this series could be useful I think
(Feb 2015). Do you intend to follow up and bring it upstream?

Alex, do you see some show-stoppers in this series or do you advise to
simply follow-up?

Thank you in advance

Best Regards

Eric


On 12/19/2014 10:20 PM, Antonios Motakis wrote:
> This RFC's intention is to show what an interface to access device
> properties for the VFIO platform driver can look like. These properties
> can be from the device tree node describing the device, or ACPI properties
> in the future.
> 
> If a device property corresponding to a platform device bound by VFIO PLATFORM
> or VFIO AMBA is available, this patch series will allow the user to query
> for this information. This can be useful for userspace drivers to automatically
> query parameters related to the device.
> 
> Specifically for QEMU, reading the "compatible" property of the device tree
> node could be of use to find out what device is being assigned to the guest and
> handle appropriately a wider range of devices in the future, and to generate an
> appropriate device tree for the guest.
> 
> Older versions of this series where specifically targeted at device tree
> properties. This version has been reworked on top of Rafael J. Wysocki's
> uniform device properties API for device tree and ACPI devices. This will allow
> us to use the API in the future with devices described via ACPI.
> 
> This also means a kernel including the uniform device properties API is needed
> to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
> 
> The API to get the list of available properties and the device tree full_name
> have been removed. These probably don't serve an useful purpose, as the user
> of this API need to know anyway what properties are specific to the device he
> wants to access with VFIO. If we decide to reintroduce the list of properties
> in the future, the generic device properties API in the kernel will have to
> be extended accordingly.
> 
> A kernel with this series and all the dependencies applied can be pulled from
> branch vfio-device-properties-v3 from the repository:
> https://github.com/virtualopensystems/linux-kvm-arm.git
> 
> Changes since v2:
>  - Reworked on top of Rafael J. Wysocki's uniform device properties API for
>    device tree and ACPI
>  - Support for u64 array properties
>  - Removed API to get list of available properties and device tree full_name
> Changes since v1:
>  - Updated for VFIO platform patch series v8:
>    VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>  - Refactored and cleaned up the code
> 
> Antonios Motakis (3):
>   vfio: platform: add device properties skeleton and user API
>   vfio: platform: access device property as a list of strings
>   vfio: platform: return device properties as arrays of unsigned
>     integers
> 
>  drivers/vfio/platform/Makefile                |   3 +-
>  drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
>  drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>  drivers/vfio/platform/vfio_platform_private.h |   7 ++
>  include/uapi/linux/vfio.h                     |  26 +++++
>  5 files changed, 232 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/vfio/platform/properties.c
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Antonios Motakis]

Hello Eric,

Myself I am not involved with anything VFIO related at the moment, so (alas) I don't have any plans to revisit this series.

If someone decides to pick it up, of course I can offer my feedback and support.

Cheers,
Antonios

On 27-Aug-15 14:52, Eric Auger wrote:
> Hi Baptiste, Antonios,
> 
> What are the plans wrt this series? I am currently integrating another
> QEMU VFIO platform devices where this series could be useful I think
> (Feb 2015). Do you intend to follow up and bring it upstream?
> 
> Alex, do you see some show-stoppers in this series or do you advise to
> simply follow-up?
> 
> Thank you in advance
> 
> Best Regards
> 
> Eric
> 
> 
> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
>> This RFC's intention is to show what an interface to access device
>> properties for the VFIO platform driver can look like. These properties
>> can be from the device tree node describing the device, or ACPI properties
>> in the future.
>>
>> If a device property corresponding to a platform device bound by VFIO PLATFORM
>> or VFIO AMBA is available, this patch series will allow the user to query
>> for this information. This can be useful for userspace drivers to automatically
>> query parameters related to the device.
>>
>> Specifically for QEMU, reading the "compatible" property of the device tree
>> node could be of use to find out what device is being assigned to the guest and
>> handle appropriately a wider range of devices in the future, and to generate an
>> appropriate device tree for the guest.
>>
>> Older versions of this series where specifically targeted at device tree
>> properties. This version has been reworked on top of Rafael J. Wysocki's
>> uniform device properties API for device tree and ACPI devices. This will allow
>> us to use the API in the future with devices described via ACPI.
>>
>> This also means a kernel including the uniform device properties API is needed
>> to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
>> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>>
>> The API to get the list of available properties and the device tree full_name
>> have been removed. These probably don't serve an useful purpose, as the user
>> of this API need to know anyway what properties are specific to the device he
>> wants to access with VFIO. If we decide to reintroduce the list of properties
>> in the future, the generic device properties API in the kernel will have to
>> be extended accordingly.
>>
>> A kernel with this series and all the dependencies applied can be pulled from
>> branch vfio-device-properties-v3 from the repository:
>> https://github.com/virtualopensystems/linux-kvm-arm.git
>>
>> Changes since v2:
>>  - Reworked on top of Rafael J. Wysocki's uniform device properties API for
>>    device tree and ACPI
>>  - Support for u64 array properties
>>  - Removed API to get list of available properties and device tree full_name
>> Changes since v1:
>>  - Updated for VFIO platform patch series v8:
>>    VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>>  - Refactored and cleaned up the code
>>
>> Antonios Motakis (3):
>>   vfio: platform: add device properties skeleton and user API
>>   vfio: platform: access device property as a list of strings
>>   vfio: platform: return device properties as arrays of unsigned
>>     integers
>>
>>  drivers/vfio/platform/Makefile                |   3 +-
>>  drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
>>  drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>>  drivers/vfio/platform/vfio_platform_private.h |   7 ++
>>  include/uapi/linux/vfio.h                     |  26 +++++
>>  5 files changed, 232 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/vfio/platform/properties.c
>>
> 

-- 
Antonios Motakis
Virtualization Engineer
Huawei Technologies Duesseldorf GmbH
European Research Center
Riesstrasse 25, 80992 München

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

Hi Antonios,

Thank you for your quick answer. Waiting for Baptiste feedback then.

Best Regards

Eric

On 08/27/2015 03:36 PM, Antonios Motakis wrote:
> Hello Eric,
> 
> Myself I am not involved with anything VFIO related at the moment, so (alas) I don't have any plans to revisit this series.
> 
> If someone decides to pick it up, of course I can offer my feedback and support.
> 
> Cheers,
> Antonios
> 
> On 27-Aug-15 14:52, Eric Auger wrote:
>> Hi Baptiste, Antonios,
>>
>> What are the plans wrt this series? I am currently integrating another
>> QEMU VFIO platform devices where this series could be useful I think
>> (Feb 2015). Do you intend to follow up and bring it upstream?
>>
>> Alex, do you see some show-stoppers in this series or do you advise to
>> simply follow-up?
>>
>> Thank you in advance
>>
>> Best Regards
>>
>> Eric
>>
>>
>> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
>>> This RFC's intention is to show what an interface to access device
>>> properties for the VFIO platform driver can look like. These properties
>>> can be from the device tree node describing the device, or ACPI properties
>>> in the future.
>>>
>>> If a device property corresponding to a platform device bound by VFIO PLATFORM
>>> or VFIO AMBA is available, this patch series will allow the user to query
>>> for this information. This can be useful for userspace drivers to automatically
>>> query parameters related to the device.
>>>
>>> Specifically for QEMU, reading the "compatible" property of the device tree
>>> node could be of use to find out what device is being assigned to the guest and
>>> handle appropriately a wider range of devices in the future, and to generate an
>>> appropriate device tree for the guest.
>>>
>>> Older versions of this series where specifically targeted at device tree
>>> properties. This version has been reworked on top of Rafael J. Wysocki's
>>> uniform device properties API for device tree and ACPI devices. This will allow
>>> us to use the API in the future with devices described via ACPI.
>>>
>>> This also means a kernel including the uniform device properties API is needed
>>> to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
>>> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>>>
>>> The API to get the list of available properties and the device tree full_name
>>> have been removed. These probably don't serve an useful purpose, as the user
>>> of this API need to know anyway what properties are specific to the device he
>>> wants to access with VFIO. If we decide to reintroduce the list of properties
>>> in the future, the generic device properties API in the kernel will have to
>>> be extended accordingly.
>>>
>>> A kernel with this series and all the dependencies applied can be pulled from
>>> branch vfio-device-properties-v3 from the repository:
>>> https://github.com/virtualopensystems/linux-kvm-arm.git
>>>
>>> Changes since v2:
>>>  - Reworked on top of Rafael J. Wysocki's uniform device properties API for
>>>    device tree and ACPI
>>>  - Support for u64 array properties
>>>  - Removed API to get list of available properties and device tree full_name
>>> Changes since v1:
>>>  - Updated for VFIO platform patch series v8:
>>>    VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>>>  - Refactored and cleaned up the code
>>>
>>> Antonios Motakis (3):
>>>   vfio: platform: add device properties skeleton and user API
>>>   vfio: platform: access device property as a list of strings
>>>   vfio: platform: return device properties as arrays of unsigned
>>>     integers
>>>
>>>  drivers/vfio/platform/Makefile                |   3 +-
>>>  drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
>>>  drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>>>  drivers/vfio/platform/vfio_platform_private.h |   7 ++
>>>  include/uapi/linux/vfio.h                     |  26 +++++
>>>  5 files changed, 232 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/vfio/platform/properties.c
>>>
>>
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christian Pinto]

Hello Eric,

We intend to follow-up on this patch series to have it upstream 
depending on the comments of the community.

Best,

Christian

On 27/08/2015 14:52, Eric Auger wrote:
> Hi Baptiste, Antonios,
>
> What are the plans wrt this series? I am currently integrating another
> QEMU VFIO platform devices where this series could be useful I think
> (Feb 2015). Do you intend to follow up and bring it upstream?
>
> Alex, do you see some show-stoppers in this series or do you advise to
> simply follow-up?
>
> Thank you in advance
>
> Best Regards
>
> Eric
>
>
> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
>> This RFC's intention is to show what an interface to access device
>> properties for the VFIO platform driver can look like. These properties
>> can be from the device tree node describing the device, or ACPI properties
>> in the future.
>>
>> If a device property corresponding to a platform device bound by VFIO PLATFORM
>> or VFIO AMBA is available, this patch series will allow the user to query
>> for this information. This can be useful for userspace drivers to automatically
>> query parameters related to the device.
>>
>> Specifically for QEMU, reading the "compatible" property of the device tree
>> node could be of use to find out what device is being assigned to the guest and
>> handle appropriately a wider range of devices in the future, and to generate an
>> appropriate device tree for the guest.
>>
>> Older versions of this series where specifically targeted at device tree
>> properties. This version has been reworked on top of Rafael J. Wysocki's
>> uniform device properties API for device tree and ACPI devices. This will allow
>> us to use the API in the future with devices described via ACPI.
>>
>> This also means a kernel including the uniform device properties API is needed
>> to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
>> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>>
>> The API to get the list of available properties and the device tree full_name
>> have been removed. These probably don't serve an useful purpose, as the user
>> of this API need to know anyway what properties are specific to the device he
>> wants to access with VFIO. If we decide to reintroduce the list of properties
>> in the future, the generic device properties API in the kernel will have to
>> be extended accordingly.
>>
>> A kernel with this series and all the dependencies applied can be pulled from
>> branch vfio-device-properties-v3 from the repository:
>> https://github.com/virtualopensystems/linux-kvm-arm.git
>>
>> Changes since v2:
>>   - Reworked on top of Rafael J. Wysocki's uniform device properties API for
>>     device tree and ACPI
>>   - Support for u64 array properties
>>   - Removed API to get list of available properties and device tree full_name
>> Changes since v1:
>>   - Updated for VFIO platform patch series v8:
>>     VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>>   - Refactored and cleaned up the code
>>
>> Antonios Motakis (3):
>>    vfio: platform: add device properties skeleton and user API
>>    vfio: platform: access device property as a list of strings
>>    vfio: platform: return device properties as arrays of unsigned
>>      integers
>>
>>   drivers/vfio/platform/Makefile                |   3 +-
>>   drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
>>   drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>>   drivers/vfio/platform/vfio_platform_private.h |   7 ++
>>   include/uapi/linux/vfio.h                     |  26 +++++
>>   5 files changed, 232 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/vfio/platform/properties.c
>>

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

OK Thanks!

I will duly review it then ;-)

Best Regards

Eric

On 08/27/2015 04:27 PM, Christian Pinto wrote:
> Hello Eric,
> 
> We intend to follow-up on this patch series to have it upstream
> depending on the comments of the community.
> 
> Best,
> 
> Christian
> 
> On 27/08/2015 14:52, Eric Auger wrote:
>> Hi Baptiste, Antonios,
>>
>> What are the plans wrt this series? I am currently integrating another
>> QEMU VFIO platform devices where this series could be useful I think
>> (Feb 2015). Do you intend to follow up and bring it upstream?
>>
>> Alex, do you see some show-stoppers in this series or do you advise to
>> simply follow-up?
>>
>> Thank you in advance
>>
>> Best Regards
>>
>> Eric
>>
>>
>> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
>>> This RFC's intention is to show what an interface to access device
>>> properties for the VFIO platform driver can look like. These properties
>>> can be from the device tree node describing the device, or ACPI
>>> properties
>>> in the future.
>>>
>>> If a device property corresponding to a platform device bound by VFIO
>>> PLATFORM
>>> or VFIO AMBA is available, this patch series will allow the user to
>>> query
>>> for this information. This can be useful for userspace drivers to
>>> automatically
>>> query parameters related to the device.
>>>
>>> Specifically for QEMU, reading the "compatible" property of the
>>> device tree
>>> node could be of use to find out what device is being assigned to the
>>> guest and
>>> handle appropriately a wider range of devices in the future, and to
>>> generate an
>>> appropriate device tree for the guest.
>>>
>>> Older versions of this series where specifically targeted at device tree
>>> properties. This version has been reworked on top of Rafael J. Wysocki's
>>> uniform device properties API for device tree and ACPI devices. This
>>> will allow
>>> us to use the API in the future with devices described via ACPI.
>>>
>>> This also means a kernel including the uniform device properties API
>>> is needed
>>> to apply these patches, in additon to the VFIO patches, e.g. branch
>>> pm+acpi-3.19-rc1
>>> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>>>
>>> The API to get the list of available properties and the device tree
>>> full_name
>>> have been removed. These probably don't serve an useful purpose, as
>>> the user
>>> of this API need to know anyway what properties are specific to the
>>> device he
>>> wants to access with VFIO. If we decide to reintroduce the list of
>>> properties
>>> in the future, the generic device properties API in the kernel will
>>> have to
>>> be extended accordingly.
>>>
>>> A kernel with this series and all the dependencies applied can be
>>> pulled from
>>> branch vfio-device-properties-v3 from the repository:
>>> https://github.com/virtualopensystems/linux-kvm-arm.git
>>>
>>> Changes since v2:
>>>   - Reworked on top of Rafael J. Wysocki's uniform device properties
>>> API for
>>>     device tree and ACPI
>>>   - Support for u64 array properties
>>>   - Removed API to get list of available properties and device tree
>>> full_name
>>> Changes since v1:
>>>   - Updated for VFIO platform patch series v8:
>>>     VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>>>   - Refactored and cleaned up the code
>>>
>>> Antonios Motakis (3):
>>>    vfio: platform: add device properties skeleton and user API
>>>    vfio: platform: access device property as a list of strings
>>>    vfio: platform: return device properties as arrays of unsigned
>>>      integers
>>>
>>>   drivers/vfio/platform/Makefile                |   3 +-
>>>   drivers/vfio/platform/properties.c            | 162
>>> ++++++++++++++++++++++++++
>>>   drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>>>   drivers/vfio/platform/vfio_platform_private.h |   7 ++
>>>   include/uapi/linux/vfio.h                     |  26 +++++
>>>   5 files changed, 232 insertions(+), 1 deletion(-)
>>>   create mode 100644 drivers/vfio/platform/properties.c
>>>
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Alex Williamson]

On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
> Hi Baptiste, Antonios,
> 
> What are the plans wrt this series? I am currently integrating another
> QEMU VFIO platform devices where this series could be useful I think
> (Feb 2015). Do you intend to follow up and bring it upstream?
> 
> Alex, do you see some show-stoppers in this series or do you advise to
> simply follow-up?

I'm at a bit of a disadvantage not really knowing what types of
properties a user will be able to retrieve here.  The interface itself
seems sort of like a game of Go Fish.  Should the properties we're
looking for be generally available via sysfs?  The ioctl proposed needs
some work to fit within the argsz/flags model that vfio typically uses.
It's unclear what argsz vs length represents and defining the flags
field as 'type' limits the future extensions of the ioctl.  Thanks,

Alex

> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
> > This RFC's intention is to show what an interface to access device
> > properties for the VFIO platform driver can look like. These properties
> > can be from the device tree node describing the device, or ACPI properties
> > in the future.
> > 
> > If a device property corresponding to a platform device bound by VFIO PLATFORM
> > or VFIO AMBA is available, this patch series will allow the user to query
> > for this information. This can be useful for userspace drivers to automatically
> > query parameters related to the device.
> > 
> > Specifically for QEMU, reading the "compatible" property of the device tree
> > node could be of use to find out what device is being assigned to the guest and
> > handle appropriately a wider range of devices in the future, and to generate an
> > appropriate device tree for the guest.
> > 
> > Older versions of this series where specifically targeted at device tree
> > properties. This version has been reworked on top of Rafael J. Wysocki's
> > uniform device properties API for device tree and ACPI devices. This will allow
> > us to use the API in the future with devices described via ACPI.
> > 
> > This also means a kernel including the uniform device properties API is needed
> > to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
> > from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
> > 
> > The API to get the list of available properties and the device tree full_name
> > have been removed. These probably don't serve an useful purpose, as the user
> > of this API need to know anyway what properties are specific to the device he
> > wants to access with VFIO. If we decide to reintroduce the list of properties
> > in the future, the generic device properties API in the kernel will have to
> > be extended accordingly.
> > 
> > A kernel with this series and all the dependencies applied can be pulled from
> > branch vfio-device-properties-v3 from the repository:
> > https://github.com/virtualopensystems/linux-kvm-arm.git
> > 
> > Changes since v2:
> >  - Reworked on top of Rafael J. Wysocki's uniform device properties API for
> >    device tree and ACPI
> >  - Support for u64 array properties
> >  - Removed API to get list of available properties and device tree full_name
> > Changes since v1:
> >  - Updated for VFIO platform patch series v8:
> >    VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
> >  - Refactored and cleaned up the code
> > 
> > Antonios Motakis (3):
> >   vfio: platform: add device properties skeleton and user API
> >   vfio: platform: access device property as a list of strings
> >   vfio: platform: return device properties as arrays of unsigned
> >     integers
> > 
> >  drivers/vfio/platform/Makefile                |   3 +-
> >  drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
> >  drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
> >  drivers/vfio/platform/vfio_platform_private.h |   7 ++
> >  include/uapi/linux/vfio.h                     |  26 +++++
> >  5 files changed, 232 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/vfio/platform/properties.c
> > 
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

Hi Alex,
On 08/27/2015 06:11 PM, Alex Williamson wrote:
> On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
>> Hi Baptiste, Antonios,
>>
>> What are the plans wrt this series? I am currently integrating another
>> QEMU VFIO platform devices where this series could be useful I think
>> (Feb 2015). Do you intend to follow up and bring it upstream?
>>
>> Alex, do you see some show-stoppers in this series or do you advise to
>> simply follow-up?
> 
> I'm at a bit of a disadvantage not really knowing what types of
> properties a user will be able to retrieve here.  The interface itself
> seems sort of like a game of Go Fish.  Should the properties we're
> looking for be generally available via sysfs?  The ioctl proposed needs
> some work to fit within the argsz/flags model that vfio typically uses.
> It's unclear what argsz vs length represents and defining the flags
> field as 'type' limits the future extensions of the ioctl.  Thanks,

The properties that I need can be found in /proc/device-tree too. They
are of different types, with void cell value, with string cell
values(single & multiple), with integer cell value(s), array ...

The aim is to be able to assign a highly configurable platform device to
a guest, read some property values on host and write the same values in
guest dt node.

Do you advise to read the fs directly instead of using such API?

Thanks

Eric

> 
> Alex
> 
>> On 12/19/2014 10:20 PM, Antonios Motakis wrote:
>>> This RFC's intention is to show what an interface to access device
>>> properties for the VFIO platform driver can look like. These properties
>>> can be from the device tree node describing the device, or ACPI properties
>>> in the future.
>>>
>>> If a device property corresponding to a platform device bound by VFIO PLATFORM
>>> or VFIO AMBA is available, this patch series will allow the user to query
>>> for this information. This can be useful for userspace drivers to automatically
>>> query parameters related to the device.
>>>
>>> Specifically for QEMU, reading the "compatible" property of the device tree
>>> node could be of use to find out what device is being assigned to the guest and
>>> handle appropriately a wider range of devices in the future, and to generate an
>>> appropriate device tree for the guest.
>>>
>>> Older versions of this series where specifically targeted at device tree
>>> properties. This version has been reworked on top of Rafael J. Wysocki's
>>> uniform device properties API for device tree and ACPI devices. This will allow
>>> us to use the API in the future with devices described via ACPI.
>>>
>>> This also means a kernel including the uniform device properties API is needed
>>> to apply these patches, in additon to the VFIO patches, e.g. branch pm+acpi-3.19-rc1
>>> from https://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/
>>>
>>> The API to get the list of available properties and the device tree full_name
>>> have been removed. These probably don't serve an useful purpose, as the user
>>> of this API need to know anyway what properties are specific to the device he
>>> wants to access with VFIO. If we decide to reintroduce the list of properties
>>> in the future, the generic device properties API in the kernel will have to
>>> be extended accordingly.
>>>
>>> A kernel with this series and all the dependencies applied can be pulled from
>>> branch vfio-device-properties-v3 from the repository:
>>> https://github.com/virtualopensystems/linux-kvm-arm.git
>>>
>>> Changes since v2:
>>>  - Reworked on top of Rafael J. Wysocki's uniform device properties API for
>>>    device tree and ACPI
>>>  - Support for u64 array properties
>>>  - Removed API to get list of available properties and device tree full_name
>>> Changes since v1:
>>>  - Updated for VFIO platform patch series v8:
>>>    VFIO AMBA devices now supported in addition to VFIO PLATFORM devices
>>>  - Refactored and cleaned up the code
>>>
>>> Antonios Motakis (3):
>>>   vfio: platform: add device properties skeleton and user API
>>>   vfio: platform: access device property as a list of strings
>>>   vfio: platform: return device properties as arrays of unsigned
>>>     integers
>>>
>>>  drivers/vfio/platform/Makefile                |   3 +-
>>>  drivers/vfio/platform/properties.c            | 162 ++++++++++++++++++++++++++
>>>  drivers/vfio/platform/vfio_platform_common.c  |  35 ++++++
>>>  drivers/vfio/platform/vfio_platform_private.h |   7 ++
>>>  include/uapi/linux/vfio.h                     |  26 +++++
>>>  5 files changed, 232 insertions(+), 1 deletion(-)
>>>  create mode 100644 drivers/vfio/platform/properties.c
>>>
>>
> 
> 
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

Hi Eric,

On Thu, Aug 27, 2015 at 07:16:00PM +0200, Eric Auger wrote:
> Hi Alex,
> On 08/27/2015 06:11 PM, Alex Williamson wrote:
> > On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
> >> Hi Baptiste, Antonios,
> >>
> >> What are the plans wrt this series? I am currently integrating another
> >> QEMU VFIO platform devices where this series could be useful I think
> >> (Feb 2015). Do you intend to follow up and bring it upstream?
> >>
> >> Alex, do you see some show-stoppers in this series or do you advise to
> >> simply follow-up?
> > 
> > I'm at a bit of a disadvantage not really knowing what types of
> > properties a user will be able to retrieve here.  The interface itself
> > seems sort of like a game of Go Fish.  Should the properties we're
> > looking for be generally available via sysfs?  The ioctl proposed needs
> > some work to fit within the argsz/flags model that vfio typically uses.
> > It's unclear what argsz vs length represents and defining the flags
> > field as 'type' limits the future extensions of the ioctl.  Thanks,
> 
> The properties that I need can be found in /proc/device-tree too. They
> are of different types, with void cell value, with string cell
> values(single & multiple), with integer cell value(s), array ...
> 
> The aim is to be able to assign a highly configurable platform device to
> a guest, read some property values on host and write the same values in
> guest dt node.
> 
Can you reiterate why QEMU and VFIO don't already have the information
necessary to setup resources and present a DT to the guest that the
guest can use?

In any case, using /proc/device-tree seems brittle to me.

-Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

Hi Christoffer,
On 08/30/2015 04:06 PM, Christoffer Dall wrote:
> Hi Eric,
> 
> On Thu, Aug 27, 2015 at 07:16:00PM +0200, Eric Auger wrote:
>> Hi Alex,
>> On 08/27/2015 06:11 PM, Alex Williamson wrote:
>>> On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
>>>> Hi Baptiste, Antonios,
>>>>
>>>> What are the plans wrt this series? I am currently integrating another
>>>> QEMU VFIO platform devices where this series could be useful I think
>>>> (Feb 2015). Do you intend to follow up and bring it upstream?
>>>>
>>>> Alex, do you see some show-stoppers in this series or do you advise to
>>>> simply follow-up?
>>>
>>> I'm at a bit of a disadvantage not really knowing what types of
>>> properties a user will be able to retrieve here.  The interface itself
>>> seems sort of like a game of Go Fish.  Should the properties we're
>>> looking for be generally available via sysfs?  The ioctl proposed needs
>>> some work to fit within the argsz/flags model that vfio typically uses.
>>> It's unclear what argsz vs length represents and defining the flags
>>> field as 'type' limits the future extensions of the ioctl.  Thanks,
>>
>> The properties that I need can be found in /proc/device-tree too. They
>> are of different types, with void cell value, with string cell
>> values(single & multiple), with integer cell value(s), array ...
>>
>> The aim is to be able to assign a highly configurable platform device to
>> a guest, read some property values on host and write the same values in
>> guest dt node.
>>
> Can you reiterate why QEMU and VFIO don't already have the information
> necessary to setup resources and present a DT to the guest that the
> guest can use?
A vfio-platform driver was bound to the passthrough'ed device. QEMU
current knows the compat string of the device and the node's name and
that's it.

The VFIO platform driver currently does not allow to return device
specific information. It just returns generic info such as resource
info. The driver is HW agnostic.


The QEMU VFIO device should be able to check some characteristics of the
host device tree. Typically if the host node does not comply with some
constraints it may not be possible to assign the device.

We do not want the QEMU end-user to have in-depth knowledge of the HW so
passing the info in the QEMU command line does not sound to be the good
solution.


As you mentioned /proc/device-tree depends on kernel option. I am able
to find the properties in sysfs too but can we systematically rely on
sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
readable but they are not. Currently investigating open/read from qemu
but is better than an ioctl API? ...

Please let me know if I answered to your question.

Best Regards

Eric
> 
> In any case, using /proc/device-tree seems brittle to me.
> 
> -Christoffer
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Mon, Aug 31, 2015 at 07:02:15PM +0200, Eric Auger wrote:
> Hi Christoffer,
> On 08/30/2015 04:06 PM, Christoffer Dall wrote:
> > Hi Eric,
> > 
> > On Thu, Aug 27, 2015 at 07:16:00PM +0200, Eric Auger wrote:
> >> Hi Alex,
> >> On 08/27/2015 06:11 PM, Alex Williamson wrote:
> >>> On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
> >>>> Hi Baptiste, Antonios,
> >>>>
> >>>> What are the plans wrt this series? I am currently integrating another
> >>>> QEMU VFIO platform devices where this series could be useful I think
> >>>> (Feb 2015). Do you intend to follow up and bring it upstream?
> >>>>
> >>>> Alex, do you see some show-stoppers in this series or do you advise to
> >>>> simply follow-up?
> >>>
> >>> I'm at a bit of a disadvantage not really knowing what types of
> >>> properties a user will be able to retrieve here.  The interface itself
> >>> seems sort of like a game of Go Fish.  Should the properties we're
> >>> looking for be generally available via sysfs?  The ioctl proposed needs
> >>> some work to fit within the argsz/flags model that vfio typically uses.
> >>> It's unclear what argsz vs length represents and defining the flags
> >>> field as 'type' limits the future extensions of the ioctl.  Thanks,
> >>
> >> The properties that I need can be found in /proc/device-tree too. They
> >> are of different types, with void cell value, with string cell
> >> values(single & multiple), with integer cell value(s), array ...
> >>
> >> The aim is to be able to assign a highly configurable platform device to
> >> a guest, read some property values on host and write the same values in
> >> guest dt node.
> >>
> > Can you reiterate why QEMU and VFIO don't already have the information
> > necessary to setup resources and present a DT to the guest that the
> > guest can use?
> A vfio-platform driver was bound to the passthrough'ed device. QEMU
> current knows the compat string of the device and the node's name and
> that's it.
> 
> The VFIO platform driver currently does not allow to return device
> specific information. It just returns generic info such as resource
> info. The driver is HW agnostic.
> 
> 
> The QEMU VFIO device should be able to check some characteristics of the
> host device tree. Typically if the host node does not comply with some
> constraints it may not be possible to assign the device.
> 
> We do not want the QEMU end-user to have in-depth knowledge of the HW so
> passing the info in the QEMU command line does not sound to be the good
> solution.
> 
> 
> As you mentioned /proc/device-tree depends on kernel option. I am able
> to find the properties in sysfs too but can we systematically rely on
> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
> readable but they are not. Currently investigating open/read from qemu
> but is better than an ioctl API? ...
> 
Yeah it does, but I thought we originally planned that the driver for a
specific platform device in QEMU should know these details, and that you
simply have to add code for each supported device, instead of trying to
solve this generically.  (It was never the user who should supply it on
the command line).  But perhaps we moved on from our original idea?

I think relying on sysfs is probably reasonable, but I'm really not an
expert in the right ways here.

Thanks,
-Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

On 08/31/2015 07:33 PM, Christoffer Dall wrote:
> On Mon, Aug 31, 2015 at 07:02:15PM +0200, Eric Auger wrote:
>> Hi Christoffer,
>> On 08/30/2015 04:06 PM, Christoffer Dall wrote:
>>> Hi Eric,
>>>
>>> On Thu, Aug 27, 2015 at 07:16:00PM +0200, Eric Auger wrote:
>>>> Hi Alex,
>>>> On 08/27/2015 06:11 PM, Alex Williamson wrote:
>>>>> On Thu, 2015-08-27 at 14:52 +0200, Eric Auger wrote:
>>>>>> Hi Baptiste, Antonios,
>>>>>>
>>>>>> What are the plans wrt this series? I am currently integrating another
>>>>>> QEMU VFIO platform devices where this series could be useful I think
>>>>>> (Feb 2015). Do you intend to follow up and bring it upstream?
>>>>>>
>>>>>> Alex, do you see some show-stoppers in this series or do you advise to
>>>>>> simply follow-up?
>>>>>
>>>>> I'm at a bit of a disadvantage not really knowing what types of
>>>>> properties a user will be able to retrieve here.  The interface itself
>>>>> seems sort of like a game of Go Fish.  Should the properties we're
>>>>> looking for be generally available via sysfs?  The ioctl proposed needs
>>>>> some work to fit within the argsz/flags model that vfio typically uses.
>>>>> It's unclear what argsz vs length represents and defining the flags
>>>>> field as 'type' limits the future extensions of the ioctl.  Thanks,
>>>>
>>>> The properties that I need can be found in /proc/device-tree too. They
>>>> are of different types, with void cell value, with string cell
>>>> values(single & multiple), with integer cell value(s), array ...
>>>>
>>>> The aim is to be able to assign a highly configurable platform device to
>>>> a guest, read some property values on host and write the same values in
>>>> guest dt node.
>>>>
>>> Can you reiterate why QEMU and VFIO don't already have the information
>>> necessary to setup resources and present a DT to the guest that the
>>> guest can use?
>> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>> current knows the compat string of the device and the node's name and
>> that's it.
>>
>> The VFIO platform driver currently does not allow to return device
>> specific information. It just returns generic info such as resource
>> info. The driver is HW agnostic.
>>
>>
>> The QEMU VFIO device should be able to check some characteristics of the
>> host device tree. Typically if the host node does not comply with some
>> constraints it may not be possible to assign the device.
>>
>> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>> passing the info in the QEMU command line does not sound to be the good
>> solution.
>>
>>
>> As you mentioned /proc/device-tree depends on kernel option. I am able
>> to find the properties in sysfs too but can we systematically rely on
>> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>> readable but they are not. Currently investigating open/read from qemu
>> but is better than an ioctl API? ...
>>
> Yeah it does, but I thought we originally planned that the driver for a
> specific platform device in QEMU should know these details,
Yes that's the objective to put this intelligence in the QEMU VFIO
device or more precisely in the associated function that builds its
guest dt node.

Let's take an example. Assuming an xgmac supports different speeds, you
need to set those on guest. Either you put arbitrary values or you reuse
the values that were set on host. Typically some values may not be
supported by the HW.

Idea of genericity was ruled out indeed meaning each device needs to
have a specialized QEMU VFIO device and an associated dt node creation
function. Now this does not prevent from exploiting host dt information.
Does that make sense?

Eric
 and that you
> simply have to add code for each supported device, instead of trying to
> solve this generically.  (It was never the user who should supply it on
> the command line).  But perhaps we moved on from our original idea?
> 
> I think relying on sysfs is probably reasonable, but I'm really not an
> expert in the right ways here.
> 
> Thanks,
> -Christoffer
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

Hi Eric,

On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:

[...]

> >>> Can you reiterate why QEMU and VFIO don't already have the information
> >>> necessary to setup resources and present a DT to the guest that the
> >>> guest can use?
> >> A vfio-platform driver was bound to the passthrough'ed device. QEMU
> >> current knows the compat string of the device and the node's name and
> >> that's it.
> >>
> >> The VFIO platform driver currently does not allow to return device
> >> specific information. It just returns generic info such as resource
> >> info. The driver is HW agnostic.
> >>
> >>
> >> The QEMU VFIO device should be able to check some characteristics of the
> >> host device tree. Typically if the host node does not comply with some
> >> constraints it may not be possible to assign the device.
> >>
> >> We do not want the QEMU end-user to have in-depth knowledge of the HW so
> >> passing the info in the QEMU command line does not sound to be the good
> >> solution.
> >>
> >>
> >> As you mentioned /proc/device-tree depends on kernel option. I am able
> >> to find the properties in sysfs too but can we systematically rely on
> >> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
> >> readable but they are not. Currently investigating open/read from qemu
> >> but is better than an ioctl API? ...
> >>
> > Yeah it does, but I thought we originally planned that the driver for a
> > specific platform device in QEMU should know these details,
> Yes that's the objective to put this intelligence in the QEMU VFIO
> device or more precisely in the associated function that builds its
> guest dt node.
> 
> Let's take an example. Assuming an xgmac supports different speeds, you
> need to set those on guest. Either you put arbitrary values or you reuse
> the values that were set on host. Typically some values may not be
> supported by the HW.
> 

I see.  I'm no expert here, but I could imagine that drivers could
overwrite some things in the DT or do some advanced probing of the
hardware which is then only exported in the sysfs and not the DT, so I
would go the sysfs approach myself.

> Idea of genericity was ruled out indeed meaning each device needs to
> have a specialized QEMU VFIO device and an associated dt node creation
> function. Now this does not prevent from exploiting host dt information.
> Does that make sense?
> 
Yes, thanks for the explanation.

-Christoffer

Building assigned device guest dt node from host device tree ...

[Eric Auger]

Dear all,

I am currently investigating the usage of sysfs to retrieve info from
the host device tree to build guest dt node for assigned devices (just
to explain a bit of context for Peter & Alex added in cc). For more
complex devices we could typically copy host settings on guest side (mac
@, various speeds, functional modes, ...).

This follows the "Re: [RFC PATCH v3 0/3] vfio: platform: return device
properties for a platform device" thread.

I have some questions:

- the device directory can be somewhere in /sys/devices/platform, ie.
can be in sub-directories. The first difficulty is to locate it. Do you
know any C routing doing find-file matching a file name pattern? Didn't
find any in qemu, nor in glib.

- I currently prototype this functionality using popen + find. That
works but relies on "find" executable availability. Is it a valid
assumption?

- I would need to build various utility functions such as
read_u32_array_prop(char *path, const char *prop_name,
                               uint32_t *props, uint32 num)
path being the of_node directory path, prop_name being the name of the
property to get the value; this for each type of property I need to
fetch and assign to the guest ... Is it the right direction?

Putting things on kernel - VOSYS approach - and exposing new VFIO IOTCL
enable to use std kernel APIs and remove the problem of finding data on
the fs ...

Please advise on the direction to follow, continue exploring the sysfs
method or revert to VOSYS approach?

Thank you in advance

Best Regards

Eric



On 09/01/2015 11:28 AM, Christoffer Dall wrote:
> Hi Eric,
> 
> On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
> 
> [...]
> 
>>>>> Can you reiterate why QEMU and VFIO don't already have the information
>>>>> necessary to setup resources and present a DT to the guest that the
>>>>> guest can use?
>>>> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>>>> current knows the compat string of the device and the node's name and
>>>> that's it.
>>>>
>>>> The VFIO platform driver currently does not allow to return device
>>>> specific information. It just returns generic info such as resource
>>>> info. The driver is HW agnostic.
>>>>
>>>>
>>>> The QEMU VFIO device should be able to check some characteristics of the
>>>> host device tree. Typically if the host node does not comply with some
>>>> constraints it may not be possible to assign the device.
>>>>
>>>> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>>>> passing the info in the QEMU command line does not sound to be the good
>>>> solution.
>>>>
>>>>
>>>> As you mentioned /proc/device-tree depends on kernel option. I am able
>>>> to find the properties in sysfs too but can we systematically rely on
>>>> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>>>> readable but they are not. Currently investigating open/read from qemu
>>>> but is better than an ioctl API? ...
>>>>
>>> Yeah it does, but I thought we originally planned that the driver for a
>>> specific platform device in QEMU should know these details,
>> Yes that's the objective to put this intelligence in the QEMU VFIO
>> device or more precisely in the associated function that builds its
>> guest dt node.
>>
>> Let's take an example. Assuming an xgmac supports different speeds, you
>> need to set those on guest. Either you put arbitrary values or you reuse
>> the values that were set on host. Typically some values may not be
>> supported by the HW.
>>
> 
> I see.  I'm no expert here, but I could imagine that drivers could
> overwrite some things in the DT or do some advanced probing of the
> hardware which is then only exported in the sysfs and not the DT, so I
> would go the sysfs approach myself.
> 
>> Idea of genericity was ruled out indeed meaning each device needs to
>> have a specialized QEMU VFIO device and an associated dt node creation
>> function. Now this does not prevent from exploiting host dt information.
>> Does that make sense?
>>
> Yes, thanks for the explanation.
> 
> -Christoffer
> 

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

Hi everyone,

The usefullness of this patch has already been discussed during the
first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
I underline the fact that it avoids implementing the logic on the
userspace program, as VFIO can be used for many usage (userspace
drivers and device assignment).

If you're interested in the implementation on the userspace side, an
RFC has been suggested for QEMU:
http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html

We are willing to put some efforts on this patch serie, so a new
version will be released according to your comments by the end of the
week.

Best regards,
Baptiste

On Tue, Sep 1, 2015 at 11:28 AM, Christoffer Dall
<christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Hi Eric,
>
> On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
>
> [...]
>
>> >>> Can you reiterate why QEMU and VFIO don't already have the information
>> >>> necessary to setup resources and present a DT to the guest that the
>> >>> guest can use?
>> >> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>> >> current knows the compat string of the device and the node's name and
>> >> that's it.
>> >>
>> >> The VFIO platform driver currently does not allow to return device
>> >> specific information. It just returns generic info such as resource
>> >> info. The driver is HW agnostic.
>> >>
>> >>
>> >> The QEMU VFIO device should be able to check some characteristics of the
>> >> host device tree. Typically if the host node does not comply with some
>> >> constraints it may not be possible to assign the device.
>> >>
>> >> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>> >> passing the info in the QEMU command line does not sound to be the good
>> >> solution.
>> >>
>> >>
>> >> As you mentioned /proc/device-tree depends on kernel option. I am able
>> >> to find the properties in sysfs too but can we systematically rely on
>> >> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>> >> readable but they are not. Currently investigating open/read from qemu
>> >> but is better than an ioctl API? ...
>> >>
>> > Yeah it does, but I thought we originally planned that the driver for a
>> > specific platform device in QEMU should know these details,
>> Yes that's the objective to put this intelligence in the QEMU VFIO
>> device or more precisely in the associated function that builds its
>> guest dt node.
>>
>> Let's take an example. Assuming an xgmac supports different speeds, you
>> need to set those on guest. Either you put arbitrary values or you reuse
>> the values that were set on host. Typically some values may not be
>> supported by the HW.
>>
>
> I see.  I'm no expert here, but I could imagine that drivers could
> overwrite some things in the DT or do some advanced probing of the
> hardware which is then only exported in the sysfs and not the DT, so I
> would go the sysfs approach myself.
>
>> Idea of genericity was ruled out indeed meaning each device needs to
>> have a specialized QEMU VFIO device and an associated dt node creation
>> function. Now this does not prevent from exploiting host dt information.
>> Does that make sense?
>>
> Yes, thanks for the explanation.
>
> -Christoffer

Re: Building assigned device guest dt node from host device tree ...

[Peter Maydell]

On 1 September 2015 at 16:29, Eric Auger <eric.auger@linaro.org> wrote:
> - the device directory can be somewhere in /sys/devices/platform, ie.
> can be in sub-directories. The first difficulty is to locate it. Do you
> know any C routing doing find-file matching a file name pattern? Didn't
> find any in qemu, nor in glib.
>
> - I currently prototype this functionality using popen + find. That
> works but relies on "find" executable availability. Is it a valid
> assumption?

This is definitely not an OK way to do it for a non-prototype
solution.

thanks
-- PMM

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> Hi everyone,
> 
> The usefullness of this patch has already been discussed during the
> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> I underline the fact that it avoids implementing the logic on the
> userspace program, as VFIO can be used for many usage (userspace
> drivers and device assignment).
> 
> If you're interested in the implementation on the userspace side, an
> RFC has been suggested for QEMU:
> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html

This one-year-old discussion is hardly exhaustive.

I think you missed the gist of the question for Eric a bit as well.

One important question for me is whether seeing the host DT is always
sufficient or if the kernel and physical device driver can have more
information about the device and its configuration which userspace may
need, which cannot be directly read in the DT (for example because the
driver has initialized the device in a specific way).  My point is, it's
really not about DT-specific things (what if you used ACPI?), but it's
about retrieving properties of a device and its configuration from
userspace.

Have we thought about the possible ways to achieve this and weight one
option against the other?

-Christoffer

> 
> On Tue, Sep 1, 2015 at 11:28 AM, Christoffer Dall
> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Hi Eric,
> >
> > On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
> >
> > [...]
> >
> >> >>> Can you reiterate why QEMU and VFIO don't already have the information
> >> >>> necessary to setup resources and present a DT to the guest that the
> >> >>> guest can use?
> >> >> A vfio-platform driver was bound to the passthrough'ed device. QEMU
> >> >> current knows the compat string of the device and the node's name and
> >> >> that's it.
> >> >>
> >> >> The VFIO platform driver currently does not allow to return device
> >> >> specific information. It just returns generic info such as resource
> >> >> info. The driver is HW agnostic.
> >> >>
> >> >>
> >> >> The QEMU VFIO device should be able to check some characteristics of the
> >> >> host device tree. Typically if the host node does not comply with some
> >> >> constraints it may not be possible to assign the device.
> >> >>
> >> >> We do not want the QEMU end-user to have in-depth knowledge of the HW so
> >> >> passing the info in the QEMU command line does not sound to be the good
> >> >> solution.
> >> >>
> >> >>
> >> >> As you mentioned /proc/device-tree depends on kernel option. I am able
> >> >> to find the properties in sysfs too but can we systematically rely on
> >> >> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
> >> >> readable but they are not. Currently investigating open/read from qemu
> >> >> but is better than an ioctl API? ...
> >> >>
> >> > Yeah it does, but I thought we originally planned that the driver for a
> >> > specific platform device in QEMU should know these details,
> >> Yes that's the objective to put this intelligence in the QEMU VFIO
> >> device or more precisely in the associated function that builds its
> >> guest dt node.
> >>
> >> Let's take an example. Assuming an xgmac supports different speeds, you
> >> need to set those on guest. Either you put arbitrary values or you reuse
> >> the values that were set on host. Typically some values may not be
> >> supported by the HW.
> >>
> >
> > I see.  I'm no expert here, but I could imagine that drivers could
> > overwrite some things in the DT or do some advanced probing of the
> > hardware which is then only exported in the sysfs and not the DT, so I
> > would go the sysfs approach myself.
> >
> >> Idea of genericity was ruled out indeed meaning each device needs to
> >> have a specialized QEMU VFIO device and an associated dt node creation
> >> function. Now this does not prevent from exploiting host dt information.
> >> Does that make sense?
> >>
> > Yes, thanks for the explanation.
> >
> > -Christoffer

Re: Building assigned device guest dt node from host device tree ...

[Alexander Graf]

> Am 01.09.2015 um 17:29 schrieb Eric Auger <eric.auger@linaro.org>:
> 
> Dear all,
> 
> I am currently investigating the usage of sysfs to retrieve info from
> the host device tree to build guest dt node for assigned devices (just
> to explain a bit of context for Peter & Alex added in cc). For more
> complex devices we could typically copy host settings on guest side (mac
> @, various speeds, functional modes, ...).
> 
> This follows the "Re: [RFC PATCH v3 0/3] vfio: platform: return device
> properties for a platform device" thread.
> 
> I have some questions:
> 
> - the device directory can be somewhere in /sys/devices/platform, ie.
> can be in sub-directories. The first difficulty is to locate it. Do you
> know any C routing doing find-file matching a file name pattern? Didn't
> find any in qemu, nor in glib.

Best path imho is to have the vfio fd expose its path via an ioctl. Didn't we have that?

> 
> - I currently prototype this functionality using popen + find. That
> works but relies on "find" executable availability. Is it a valid
> assumption?

No :)

> 
> - I would need to build various utility functions such as
> read_u32_array_prop(char *path, const char *prop_name,
>                               uint32_t *props, uint32 num)
> path being the of_node directory path, prop_name being the name of the
> property to get the value; this for each type of property I need to
> fetch and assign to the guest ... Is it the right direction?

I don't know yet. Start with very specific needs for very specific devices first, create a list for your use case. Then go through each property and see whether you really need to get it from the host and how many there really are.

Alex

> 
> Putting things on kernel - VOSYS approach - and exposing new VFIO IOTCL
> enable to use std kernel APIs and remove the problem of finding data on
> the fs ...
> 
> Please advise on the direction to follow, continue exploring the sysfs
> method or revert to VOSYS approach?
> 
> Thank you in advance
> 
> Best Regards
> 
> Eric
> 
> 
> 
>> On 09/01/2015 11:28 AM, Christoffer Dall wrote:
>> Hi Eric,
>> 
>> On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
>> 
>> [...]
>> 
>>>>>> Can you reiterate why QEMU and VFIO don't already have the information
>>>>>> necessary to setup resources and present a DT to the guest that the
>>>>>> guest can use?
>>>>> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>>>>> current knows the compat string of the device and the node's name and
>>>>> that's it.
>>>>> 
>>>>> The VFIO platform driver currently does not allow to return device
>>>>> specific information. It just returns generic info such as resource
>>>>> info. The driver is HW agnostic.
>>>>> 
>>>>> 
>>>>> The QEMU VFIO device should be able to check some characteristics of the
>>>>> host device tree. Typically if the host node does not comply with some
>>>>> constraints it may not be possible to assign the device.
>>>>> 
>>>>> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>>>>> passing the info in the QEMU command line does not sound to be the good
>>>>> solution.
>>>>> 
>>>>> 
>>>>> As you mentioned /proc/device-tree depends on kernel option. I am able
>>>>> to find the properties in sysfs too but can we systematically rely on
>>>>> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>>>>> readable but they are not. Currently investigating open/read from qemu
>>>>> but is better than an ioctl API? ...
>>>> Yeah it does, but I thought we originally planned that the driver for a
>>>> specific platform device in QEMU should know these details,
>>> Yes that's the objective to put this intelligence in the QEMU VFIO
>>> device or more precisely in the associated function that builds its
>>> guest dt node.
>>> 
>>> Let's take an example. Assuming an xgmac supports different speeds, you
>>> need to set those on guest. Either you put arbitrary values or you reuse
>>> the values that were set on host. Typically some values may not be
>>> supported by the HW.
>> 
>> I see.  I'm no expert here, but I could imagine that drivers could
>> overwrite some things in the DT or do some advanced probing of the
>> hardware which is then only exported in the sysfs and not the DT, so I
>> would go the sysfs approach myself.
>> 
>>> Idea of genericity was ruled out indeed meaning each device needs to
>>> have a specialized QEMU VFIO device and an associated dt node creation
>>> function. Now this does not prevent from exploiting host dt information.
>>> Does that make sense?
>> Yes, thanks for the explanation.
>> 
>> -Christoffer
> 

Re: Building assigned device guest dt node from host device tree ...

[Eric Auger]

Hi Alex,
On 09/01/2015 07:00 PM, Alexander Graf wrote:
> 
> 
>> Am 01.09.2015 um 17:29 schrieb Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:
>>
>> Dear all,
>>
>> I am currently investigating the usage of sysfs to retrieve info from
>> the host device tree to build guest dt node for assigned devices (just
>> to explain a bit of context for Peter & Alex added in cc). For more
>> complex devices we could typically copy host settings on guest side (mac
>> @, various speeds, functional modes, ...).
>>
>> This follows the "Re: [RFC PATCH v3 0/3] vfio: platform: return device
>> properties for a platform device" thread.
>>
>> I have some questions:
>>
>> - the device directory can be somewhere in /sys/devices/platform, ie.
>> can be in sub-directories. The first difficulty is to locate it. Do you
>> know any C routing doing find-file matching a file name pattern? Didn't
>> find any in qemu, nor in glib.
> 
> Best path imho is to have the vfio fd expose its path via an ioctl. Didn't we have that?
no we don't. We may extend VFIO_DEVICE_GET_INFO ioctl though?
> 
>>
>> - I currently prototype this functionality using popen + find. That
>> works but relies on "find" executable availability. Is it a valid
>> assumption?
> 
> No :)
You can't win if you don't play ;-)
> 
>>
>> - I would need to build various utility functions such as
>> read_u32_array_prop(char *path, const char *prop_name,
>>                               uint32_t *props, uint32 num)
>> path being the of_node directory path, prop_name being the name of the
>> property to get the value; this for each type of property I need to
>> fetch and assign to the guest ... Is it the right direction?
> 
> I don't know yet. Start with very specific needs for very specific devices first, create a list for your use case. Then go through each property and see whether you really need to get it from the host and how many there really are.
OK. I will do that shortly.

Many thanks for your reply

Eric
> 
> Alex
> 
>>
>> Putting things on kernel - VOSYS approach - and exposing new VFIO IOTCL
>> enable to use std kernel APIs and remove the problem of finding data on
>> the fs ...
>>
>> Please advise on the direction to follow, continue exploring the sysfs
>> method or revert to VOSYS approach?
>>
>> Thank you in advance
>>
>> Best Regards
>>
>> Eric
>>
>>
>>
>>> On 09/01/2015 11:28 AM, Christoffer Dall wrote:
>>> Hi Eric,
>>>
>>> On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
>>>
>>> [...]
>>>
>>>>>>> Can you reiterate why QEMU and VFIO don't already have the information
>>>>>>> necessary to setup resources and present a DT to the guest that the
>>>>>>> guest can use?
>>>>>> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>>>>>> current knows the compat string of the device and the node's name and
>>>>>> that's it.
>>>>>>
>>>>>> The VFIO platform driver currently does not allow to return device
>>>>>> specific information. It just returns generic info such as resource
>>>>>> info. The driver is HW agnostic.
>>>>>>
>>>>>>
>>>>>> The QEMU VFIO device should be able to check some characteristics of the
>>>>>> host device tree. Typically if the host node does not comply with some
>>>>>> constraints it may not be possible to assign the device.
>>>>>>
>>>>>> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>>>>>> passing the info in the QEMU command line does not sound to be the good
>>>>>> solution.
>>>>>>
>>>>>>
>>>>>> As you mentioned /proc/device-tree depends on kernel option. I am able
>>>>>> to find the properties in sysfs too but can we systematically rely on
>>>>>> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>>>>>> readable but they are not. Currently investigating open/read from qemu
>>>>>> but is better than an ioctl API? ...
>>>>> Yeah it does, but I thought we originally planned that the driver for a
>>>>> specific platform device in QEMU should know these details,
>>>> Yes that's the objective to put this intelligence in the QEMU VFIO
>>>> device or more precisely in the associated function that builds its
>>>> guest dt node.
>>>>
>>>> Let's take an example. Assuming an xgmac supports different speeds, you
>>>> need to set those on guest. Either you put arbitrary values or you reuse
>>>> the values that were set on host. Typically some values may not be
>>>> supported by the HW.
>>>
>>> I see.  I'm no expert here, but I could imagine that drivers could
>>> overwrite some things in the DT or do some advanced probing of the
>>> hardware which is then only exported in the sysfs and not the DT, so I
>>> would go the sysfs approach myself.
>>>
>>>> Idea of genericity was ruled out indeed meaning each device needs to
>>>> have a specialized QEMU VFIO device and an associated dt node creation
>>>> function. Now this does not prevent from exploiting host dt information.
>>>> Does that make sense?
>>> Yes, thanks for the explanation.
>>>
>>> -Christoffer
>>

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
<christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
>> Hi everyone,
>>
>> The usefullness of this patch has already been discussed during the
>> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
>> I underline the fact that it avoids implementing the logic on the
>> userspace program, as VFIO can be used for many usage (userspace
>> drivers and device assignment).
>>
>> If you're interested in the implementation on the userspace side, an
>> RFC has been suggested for QEMU:
>> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
>
> This one-year-old discussion is hardly exhaustive.
>
> I think you missed the gist of the question for Eric a bit as well.
>
> One important question for me is whether seeing the host DT is always
> sufficient or if the kernel and physical device driver can have more
> information about the device and its configuration which userspace may
> need, which cannot be directly read in the DT (for example because the
> driver has initialized the device in a specific way).  My point is, it's
> really not about DT-specific things (what if you used ACPI?), but it's
> about retrieving properties of a device and its configuration from
> userspace.
>
> Have we thought about the possible ways to achieve this and weight one
> option against the other?

Problem is that now we only have a very few platform devices behind an
IOMMU, so we don't have the visibility to know if such a case will
occur. With the current use cases, the interface seems to be
sufficient. By using IOCTL, we can always change the implementation
later without any change on the userspace.

Do you think we can valid the API, then if such a case occurs (when
the device informations changes between the device tree and the
running state) we can change the kernel implementation ?

>
> -Christoffer
>
>>
>> On Tue, Sep 1, 2015 at 11:28 AM, Christoffer Dall
>> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > Hi Eric,
>> >
>> > On Tue, Sep 01, 2015 at 09:31:56AM +0200, Eric Auger wrote:
>> >
>> > [...]
>> >
>> >> >>> Can you reiterate why QEMU and VFIO don't already have the information
>> >> >>> necessary to setup resources and present a DT to the guest that the
>> >> >>> guest can use?
>> >> >> A vfio-platform driver was bound to the passthrough'ed device. QEMU
>> >> >> current knows the compat string of the device and the node's name and
>> >> >> that's it.
>> >> >>
>> >> >> The VFIO platform driver currently does not allow to return device
>> >> >> specific information. It just returns generic info such as resource
>> >> >> info. The driver is HW agnostic.
>> >> >>
>> >> >>
>> >> >> The QEMU VFIO device should be able to check some characteristics of the
>> >> >> host device tree. Typically if the host node does not comply with some
>> >> >> constraints it may not be possible to assign the device.
>> >> >>
>> >> >> We do not want the QEMU end-user to have in-depth knowledge of the HW so
>> >> >> passing the info in the QEMU command line does not sound to be the good
>> >> >> solution.
>> >> >>
>> >> >>
>> >> >> As you mentioned /proc/device-tree depends on kernel option. I am able
>> >> >> to find the properties in sysfs too but can we systematically rely on
>> >> >> sysfs (CONFIG_SYSFS)? Also I would have expected the values to be human
>> >> >> readable but they are not. Currently investigating open/read from qemu
>> >> >> but is better than an ioctl API? ...
>> >> >>
>> >> > Yeah it does, but I thought we originally planned that the driver for a
>> >> > specific platform device in QEMU should know these details,
>> >> Yes that's the objective to put this intelligence in the QEMU VFIO
>> >> device or more precisely in the associated function that builds its
>> >> guest dt node.
>> >>
>> >> Let's take an example. Assuming an xgmac supports different speeds, you
>> >> need to set those on guest. Either you put arbitrary values or you reuse
>> >> the values that were set on host. Typically some values may not be
>> >> supported by the HW.
>> >>
>> >
>> > I see.  I'm no expert here, but I could imagine that drivers could
>> > overwrite some things in the DT or do some advanced probing of the
>> > hardware which is then only exported in the sysfs and not the DT, so I
>> > would go the sysfs approach myself.
>> >
>> >> Idea of genericity was ruled out indeed meaning each device needs to
>> >> have a specialized QEMU VFIO device and an associated dt node creation
>> >> function. Now this does not prevent from exploiting host dt information.
>> >> Does that make sense?
>> >>
>> > Yes, thanks for the explanation.
>> >
>> > -Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> >> Hi everyone,
> >>
> >> The usefullness of this patch has already been discussed during the
> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> >> I underline the fact that it avoids implementing the logic on the
> >> userspace program, as VFIO can be used for many usage (userspace
> >> drivers and device assignment).
> >>
> >> If you're interested in the implementation on the userspace side, an
> >> RFC has been suggested for QEMU:
> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
> >
> > This one-year-old discussion is hardly exhaustive.
> >
> > I think you missed the gist of the question for Eric a bit as well.
> >
> > One important question for me is whether seeing the host DT is always
> > sufficient or if the kernel and physical device driver can have more
> > information about the device and its configuration which userspace may
> > need, which cannot be directly read in the DT (for example because the
> > driver has initialized the device in a specific way).  My point is, it's
> > really not about DT-specific things (what if you used ACPI?), but it's
> > about retrieving properties of a device and its configuration from
> > userspace.
> >
> > Have we thought about the possible ways to achieve this and weight one
> > option against the other?
> 
> Problem is that now we only have a very few platform devices behind an
> IOMMU, so we don't have the visibility to know if such a case will
> occur. With the current use cases, the interface seems to be
> sufficient. 

Ideally we can think about future use cases based on the experience of
people in the community and come up with a solution considering future
use cases.

> By using IOCTL, we can always change the implementation
> later without any change on the userspace.

Can you be more concrete with what you mean here?

> 
> Do you think we can valid the API, then if such a case occurs (when
> the device informations changes between the device tree and the
> running state) we can change the kernel implementation ?
> 

Sorry, I don't understand this paragraph.

-Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
<christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
>> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
>> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
>> >> Hi everyone,
>> >>
>> >> The usefullness of this patch has already been discussed during the
>> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
>> >> I underline the fact that it avoids implementing the logic on the
>> >> userspace program, as VFIO can be used for many usage (userspace
>> >> drivers and device assignment).
>> >>
>> >> If you're interested in the implementation on the userspace side, an
>> >> RFC has been suggested for QEMU:
>> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
>> >
>> > This one-year-old discussion is hardly exhaustive.
>> >
>> > I think you missed the gist of the question for Eric a bit as well.
>> >
>> > One important question for me is whether seeing the host DT is always
>> > sufficient or if the kernel and physical device driver can have more
>> > information about the device and its configuration which userspace may
>> > need, which cannot be directly read in the DT (for example because the
>> > driver has initialized the device in a specific way).  My point is, it's
>> > really not about DT-specific things (what if you used ACPI?), but it's
>> > about retrieving properties of a device and its configuration from
>> > userspace.
>> >
>> > Have we thought about the possible ways to achieve this and weight one
>> > option against the other?
>>
>> Problem is that now we only have a very few platform devices behind an
>> IOMMU, so we don't have the visibility to know if such a case will
>> occur. With the current use cases, the interface seems to be
>> sufficient.
>
> Ideally we can think about future use cases based on the experience of
> people in the community and come up with a solution considering future
> use cases.
>
>> By using IOCTL, we can always change the implementation
>> later without any change on the userspace.
>
> Can you be more concrete with what you mean here?
>

By using an IOCTL, we define an API that allows to retrieve device
properties from userspace. The way it is retrieved is handled by the
kernel (For example for now if OF is unavailable, the kernel will
retrieve the property using ACPI) and is totally transparent from the
userspace point of view.

My point is that we can go with the current naive implementation for
now, and we might extend it later according to the needs of future
devices, without changing anything from the userspace point of view.

>>
>> Do you think we can valid the API, then if such a case occurs (when
>> the device informations changes between the device tree and the
>> running state) we can change the kernel implementation ?
>>
>
> Sorry, I don't understand this paragraph.
>
> -Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Wed, Sep 02, 2015 at 11:49:12AM +0200, Baptiste Reynal wrote:
> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
> >> <christoffer.dall@linaro.org> wrote:
> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> >> >> Hi everyone,
> >> >>
> >> >> The usefullness of this patch has already been discussed during the
> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> >> >> I underline the fact that it avoids implementing the logic on the
> >> >> userspace program, as VFIO can be used for many usage (userspace
> >> >> drivers and device assignment).
> >> >>
> >> >> If you're interested in the implementation on the userspace side, an
> >> >> RFC has been suggested for QEMU:
> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
> >> >
> >> > This one-year-old discussion is hardly exhaustive.
> >> >
> >> > I think you missed the gist of the question for Eric a bit as well.
> >> >
> >> > One important question for me is whether seeing the host DT is always
> >> > sufficient or if the kernel and physical device driver can have more
> >> > information about the device and its configuration which userspace may
> >> > need, which cannot be directly read in the DT (for example because the
> >> > driver has initialized the device in a specific way).  My point is, it's
> >> > really not about DT-specific things (what if you used ACPI?), but it's
> >> > about retrieving properties of a device and its configuration from
> >> > userspace.
> >> >
> >> > Have we thought about the possible ways to achieve this and weight one
> >> > option against the other?
> >>
> >> Problem is that now we only have a very few platform devices behind an
> >> IOMMU, so we don't have the visibility to know if such a case will
> >> occur. With the current use cases, the interface seems to be
> >> sufficient.
> >
> > Ideally we can think about future use cases based on the experience of
> > people in the community and come up with a solution considering future
> > use cases.
> >
> >> By using IOCTL, we can always change the implementation
> >> later without any change on the userspace.
> >
> > Can you be more concrete with what you mean here?
> >
> 
> By using an IOCTL, we define an API that allows to retrieve device
> properties from userspace. The way it is retrieved is handled by the
> kernel (For example for now if OF is unavailable, the kernel will
> retrieve the property using ACPI) and is totally transparent from the
> userspace point of view.

ok, I thought that this series was targeting device tree specifically,
but I see that you changed this approach in v3.

> 
> My point is that we can go with the current naive implementation for
> now, and we might extend it later according to the needs of future
> devices, without changing anything from the userspace point of view.
> 
fair enough.

Is this series exporting properties not already exported through sysfs?

-Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

On Wed, Sep 2, 2015 at 12:32 PM, Christoffer Dall
<christoffer.dall@linaro.org> wrote:
> On Wed, Sep 02, 2015 at 11:49:12AM +0200, Baptiste Reynal wrote:
>> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
>> <christoffer.dall@linaro.org> wrote:
>> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
>> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
>> >> <christoffer.dall@linaro.org> wrote:
>> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
>> >> >> Hi everyone,
>> >> >>
>> >> >> The usefullness of this patch has already been discussed during the
>> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
>> >> >> I underline the fact that it avoids implementing the logic on the
>> >> >> userspace program, as VFIO can be used for many usage (userspace
>> >> >> drivers and device assignment).
>> >> >>
>> >> >> If you're interested in the implementation on the userspace side, an
>> >> >> RFC has been suggested for QEMU:
>> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
>> >> >
>> >> > This one-year-old discussion is hardly exhaustive.
>> >> >
>> >> > I think you missed the gist of the question for Eric a bit as well.
>> >> >
>> >> > One important question for me is whether seeing the host DT is always
>> >> > sufficient or if the kernel and physical device driver can have more
>> >> > information about the device and its configuration which userspace may
>> >> > need, which cannot be directly read in the DT (for example because the
>> >> > driver has initialized the device in a specific way).  My point is, it's
>> >> > really not about DT-specific things (what if you used ACPI?), but it's
>> >> > about retrieving properties of a device and its configuration from
>> >> > userspace.
>> >> >
>> >> > Have we thought about the possible ways to achieve this and weight one
>> >> > option against the other?
>> >>
>> >> Problem is that now we only have a very few platform devices behind an
>> >> IOMMU, so we don't have the visibility to know if such a case will
>> >> occur. With the current use cases, the interface seems to be
>> >> sufficient.
>> >
>> > Ideally we can think about future use cases based on the experience of
>> > people in the community and come up with a solution considering future
>> > use cases.
>> >
>> >> By using IOCTL, we can always change the implementation
>> >> later without any change on the userspace.
>> >
>> > Can you be more concrete with what you mean here?
>> >
>>
>> By using an IOCTL, we define an API that allows to retrieve device
>> properties from userspace. The way it is retrieved is handled by the
>> kernel (For example for now if OF is unavailable, the kernel will
>> retrieve the property using ACPI) and is totally transparent from the
>> userspace point of view.
>
> ok, I thought that this series was targeting device tree specifically,
> but I see that you changed this approach in v3.
>
>>
>> My point is that we can go with the current naive implementation for
>> now, and we might extend it later according to the needs of future
>> devices, without changing anything from the userspace point of view.
>>
> fair enough.
>
> Is this series exporting properties not already exported through sysfs?

Currently not, only OF and ACPI properties are read.

>
> -Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Alex Williamson]

On Wed, 2015-09-02 at 11:49 +0200, Baptiste Reynal wrote:
> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
> <christoffer.dall@linaro.org> wrote:
> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
> >> <christoffer.dall@linaro.org> wrote:
> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> >> >> Hi everyone,
> >> >>
> >> >> The usefullness of this patch has already been discussed during the
> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> >> >> I underline the fact that it avoids implementing the logic on the
> >> >> userspace program, as VFIO can be used for many usage (userspace
> >> >> drivers and device assignment).
> >> >>
> >> >> If you're interested in the implementation on the userspace side, an
> >> >> RFC has been suggested for QEMU:
> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
> >> >
> >> > This one-year-old discussion is hardly exhaustive.
> >> >
> >> > I think you missed the gist of the question for Eric a bit as well.
> >> >
> >> > One important question for me is whether seeing the host DT is always
> >> > sufficient or if the kernel and physical device driver can have more
> >> > information about the device and its configuration which userspace may
> >> > need, which cannot be directly read in the DT (for example because the
> >> > driver has initialized the device in a specific way).  My point is, it's
> >> > really not about DT-specific things (what if you used ACPI?), but it's
> >> > about retrieving properties of a device and its configuration from
> >> > userspace.
> >> >
> >> > Have we thought about the possible ways to achieve this and weight one
> >> > option against the other?
> >>
> >> Problem is that now we only have a very few platform devices behind an
> >> IOMMU, so we don't have the visibility to know if such a case will
> >> occur. With the current use cases, the interface seems to be
> >> sufficient.
> >
> > Ideally we can think about future use cases based on the experience of
> > people in the community and come up with a solution considering future
> > use cases.
> >
> >> By using IOCTL, we can always change the implementation
> >> later without any change on the userspace.
> >
> > Can you be more concrete with what you mean here?
> >
> 
> By using an IOCTL, we define an API that allows to retrieve device
> properties from userspace. The way it is retrieved is handled by the
> kernel (For example for now if OF is unavailable, the kernel will
> retrieve the property using ACPI) and is totally transparent from the
> userspace point of view.
> 
> My point is that we can go with the current naive implementation for
> now, and we might extend it later according to the needs of future
> devices, without changing anything from the userspace point of view.

I don't really like "ioctl" and "naive implementation" in the same
thought.  The ioctl itself needs to be well thought out for current and
future needs, including independence from the source data type
(OF/DT/ACPI/etc), proven that it's necessary, it's not redundant to
existing interfaces, and vfio should be the one hosting it.  Thanks,

Alex

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Baptiste Reynal]

On Wed, Sep 2, 2015 at 6:52 PM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 2015-09-02 at 11:49 +0200, Baptiste Reynal wrote:
>> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
>> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
>> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
>> >> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
>> >> >> Hi everyone,
>> >> >>
>> >> >> The usefullness of this patch has already been discussed during the
>> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
>> >> >> I underline the fact that it avoids implementing the logic on the
>> >> >> userspace program, as VFIO can be used for many usage (userspace
>> >> >> drivers and device assignment).
>> >> >>
>> >> >> If you're interested in the implementation on the userspace side, an
>> >> >> RFC has been suggested for QEMU:
>> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
>> >> >
>> >> > This one-year-old discussion is hardly exhaustive.
>> >> >
>> >> > I think you missed the gist of the question for Eric a bit as well.
>> >> >
>> >> > One important question for me is whether seeing the host DT is always
>> >> > sufficient or if the kernel and physical device driver can have more
>> >> > information about the device and its configuration which userspace may
>> >> > need, which cannot be directly read in the DT (for example because the
>> >> > driver has initialized the device in a specific way).  My point is, it's
>> >> > really not about DT-specific things (what if you used ACPI?), but it's
>> >> > about retrieving properties of a device and its configuration from
>> >> > userspace.
>> >> >
>> >> > Have we thought about the possible ways to achieve this and weight one
>> >> > option against the other?
>> >>
>> >> Problem is that now we only have a very few platform devices behind an
>> >> IOMMU, so we don't have the visibility to know if such a case will
>> >> occur. With the current use cases, the interface seems to be
>> >> sufficient.
>> >
>> > Ideally we can think about future use cases based on the experience of
>> > people in the community and come up with a solution considering future
>> > use cases.
>> >
>> >> By using IOCTL, we can always change the implementation
>> >> later without any change on the userspace.
>> >
>> > Can you be more concrete with what you mean here?
>> >
>>
>> By using an IOCTL, we define an API that allows to retrieve device
>> properties from userspace. The way it is retrieved is handled by the
>> kernel (For example for now if OF is unavailable, the kernel will
>> retrieve the property using ACPI) and is totally transparent from the
>> userspace point of view.
>>
>> My point is that we can go with the current naive implementation for
>> now, and we might extend it later according to the needs of future
>> devices, without changing anything from the userspace point of view.
>
> I don't really like "ioctl" and "naive implementation" in the same
> thought.  The ioctl itself needs to be well thought out for current and
> future needs, including independence from the source data type
> (OF/DT/ACPI/etc), proven that it's necessary, it's not redundant to
> existing interfaces, and vfio should be the one hosting it.  Thanks,
>

The word we used, naive, does not mean a bad implementation of the
IOCTL interface but rather it was referring to the fact that at this
moment we export 1-to-1 the information available in the device tree
via sys or proc file system. So one could obtain the same result
without such interface, however our implementation is aready
independent from the source of such information, since OF and ACPI are
supported already, and there will be no problem in extending the
interface to support more.

The next release will take into account your comments about argsz and flags.

Regards,
Baptiste

> Alex
>

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Christoffer Dall]

On Thu, Sep 03, 2015 at 09:42:32AM +0200, Baptiste Reynal wrote:
> On Wed, Sep 2, 2015 at 6:52 PM, Alex Williamson
> <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Wed, 2015-09-02 at 11:49 +0200, Baptiste Reynal wrote:
> >> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
> >> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
> >> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
> >> >> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> >> >> >> Hi everyone,
> >> >> >>
> >> >> >> The usefullness of this patch has already been discussed during the
> >> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> >> >> >> I underline the fact that it avoids implementing the logic on the
> >> >> >> userspace program, as VFIO can be used for many usage (userspace
> >> >> >> drivers and device assignment).
> >> >> >>
> >> >> >> If you're interested in the implementation on the userspace side, an
> >> >> >> RFC has been suggested for QEMU:
> >> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
> >> >> >
> >> >> > This one-year-old discussion is hardly exhaustive.
> >> >> >
> >> >> > I think you missed the gist of the question for Eric a bit as well.
> >> >> >
> >> >> > One important question for me is whether seeing the host DT is always
> >> >> > sufficient or if the kernel and physical device driver can have more
> >> >> > information about the device and its configuration which userspace may
> >> >> > need, which cannot be directly read in the DT (for example because the
> >> >> > driver has initialized the device in a specific way).  My point is, it's
> >> >> > really not about DT-specific things (what if you used ACPI?), but it's
> >> >> > about retrieving properties of a device and its configuration from
> >> >> > userspace.
> >> >> >
> >> >> > Have we thought about the possible ways to achieve this and weight one
> >> >> > option against the other?
> >> >>
> >> >> Problem is that now we only have a very few platform devices behind an
> >> >> IOMMU, so we don't have the visibility to know if such a case will
> >> >> occur. With the current use cases, the interface seems to be
> >> >> sufficient.
> >> >
> >> > Ideally we can think about future use cases based on the experience of
> >> > people in the community and come up with a solution considering future
> >> > use cases.
> >> >
> >> >> By using IOCTL, we can always change the implementation
> >> >> later without any change on the userspace.
> >> >
> >> > Can you be more concrete with what you mean here?
> >> >
> >>
> >> By using an IOCTL, we define an API that allows to retrieve device
> >> properties from userspace. The way it is retrieved is handled by the
> >> kernel (For example for now if OF is unavailable, the kernel will
> >> retrieve the property using ACPI) and is totally transparent from the
> >> userspace point of view.
> >>
> >> My point is that we can go with the current naive implementation for
> >> now, and we might extend it later according to the needs of future
> >> devices, without changing anything from the userspace point of view.
> >
> > I don't really like "ioctl" and "naive implementation" in the same
> > thought.  The ioctl itself needs to be well thought out for current and
> > future needs, including independence from the source data type
> > (OF/DT/ACPI/etc), proven that it's necessary, it's not redundant to
> > existing interfaces, and vfio should be the one hosting it.  Thanks,
> >
> 
> The word we used, naive, does not mean a bad implementation of the
> IOCTL interface but rather it was referring to the fact that at this
> moment we export 1-to-1 the information available in the device tree
> via sys or proc file system. So one could obtain the same result
> without such interface, however our implementation is aready
> independent from the source of such information, since OF and ACPI are
> supported already, and there will be no problem in extending the
> interface to support more.
> 
I'm going to have to ask again then.  Did you mean to say that these
patches export the same information as you can obtain through sysfs?

Then what again is the rationale for adding this?

Is it not common practice for userspace tools to look in sysfs for
device information?

-Christoffer

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Alex Williamson]

On Thu, 2015-09-03 at 10:49 +0200, Christoffer Dall wrote:
> On Thu, Sep 03, 2015 at 09:42:32AM +0200, Baptiste Reynal wrote:
> > On Wed, Sep 2, 2015 at 6:52 PM, Alex Williamson
> > <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > > On Wed, 2015-09-02 at 11:49 +0200, Baptiste Reynal wrote:
> > >> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
> > >> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> > On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
> > >> >> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
> > >> >> <christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> >> > On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
> > >> >> >> Hi everyone,
> > >> >> >>
> > >> >> >> The usefullness of this patch has already been discussed during the
> > >> >> >> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
> > >> >> >> I underline the fact that it avoids implementing the logic on the
> > >> >> >> userspace program, as VFIO can be used for many usage (userspace
> > >> >> >> drivers and device assignment).
> > >> >> >>
> > >> >> >> If you're interested in the implementation on the userspace side, an
> > >> >> >> RFC has been suggested for QEMU:
> > >> >> >> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
> > >> >> >
> > >> >> > This one-year-old discussion is hardly exhaustive.
> > >> >> >
> > >> >> > I think you missed the gist of the question for Eric a bit as well.
> > >> >> >
> > >> >> > One important question for me is whether seeing the host DT is always
> > >> >> > sufficient or if the kernel and physical device driver can have more
> > >> >> > information about the device and its configuration which userspace may
> > >> >> > need, which cannot be directly read in the DT (for example because the
> > >> >> > driver has initialized the device in a specific way).  My point is, it's
> > >> >> > really not about DT-specific things (what if you used ACPI?), but it's
> > >> >> > about retrieving properties of a device and its configuration from
> > >> >> > userspace.
> > >> >> >
> > >> >> > Have we thought about the possible ways to achieve this and weight one
> > >> >> > option against the other?
> > >> >>
> > >> >> Problem is that now we only have a very few platform devices behind an
> > >> >> IOMMU, so we don't have the visibility to know if such a case will
> > >> >> occur. With the current use cases, the interface seems to be
> > >> >> sufficient.
> > >> >
> > >> > Ideally we can think about future use cases based on the experience of
> > >> > people in the community and come up with a solution considering future
> > >> > use cases.
> > >> >
> > >> >> By using IOCTL, we can always change the implementation
> > >> >> later without any change on the userspace.
> > >> >
> > >> > Can you be more concrete with what you mean here?
> > >> >
> > >>
> > >> By using an IOCTL, we define an API that allows to retrieve device
> > >> properties from userspace. The way it is retrieved is handled by the
> > >> kernel (For example for now if OF is unavailable, the kernel will
> > >> retrieve the property using ACPI) and is totally transparent from the
> > >> userspace point of view.
> > >>
> > >> My point is that we can go with the current naive implementation for
> > >> now, and we might extend it later according to the needs of future
> > >> devices, without changing anything from the userspace point of view.
> > >
> > > I don't really like "ioctl" and "naive implementation" in the same
> > > thought.  The ioctl itself needs to be well thought out for current and
> > > future needs, including independence from the source data type
> > > (OF/DT/ACPI/etc), proven that it's necessary, it's not redundant to
> > > existing interfaces, and vfio should be the one hosting it.  Thanks,
> > >
> > 
> > The word we used, naive, does not mean a bad implementation of the
> > IOCTL interface but rather it was referring to the fact that at this
> > moment we export 1-to-1 the information available in the device tree
> > via sys or proc file system. So one could obtain the same result
> > without such interface, however our implementation is aready
> > independent from the source of such information, since OF and ACPI are
> > supported already, and there will be no problem in extending the
> > interface to support more.
> > 
> I'm going to have to ask again then.  Did you mean to say that these
> patches export the same information as you can obtain through sysfs?
> 
> Then what again is the rationale for adding this?
> 
> Is it not common practice for userspace tools to look in sysfs for
> device information?

Note that vfio-platform in QEMU, like vfio-pci, relies on sysfs to
determine the device to IOMMU group mapping.  So if there's some notion
of creating this ioctl so as not to rely on sysfs, the dependency
already exists.  Thanks,

Alex

Re: [RFC PATCH v3 0/3] vfio: platform: return device properties for a platform device

[Eric Auger]

Dear all,
On 09/03/2015 09:42 AM, Baptiste Reynal wrote:
> On Wed, Sep 2, 2015 at 6:52 PM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
>> On Wed, 2015-09-02 at 11:49 +0200, Baptiste Reynal wrote:
>>> On Wed, Sep 2, 2015 at 11:21 AM, Christoffer Dall
>>> <christoffer.dall@linaro.org> wrote:
>>>> On Wed, Sep 02, 2015 at 09:21:26AM +0200, Baptiste Reynal wrote:
>>>>> On Tue, Sep 1, 2015 at 5:52 PM, Christoffer Dall
>>>>> <christoffer.dall@linaro.org> wrote:
>>>>>> On Tue, Sep 01, 2015 at 05:32:21PM +0200, Baptiste Reynal wrote:
>>>>>>> Hi everyone,
>>>>>>>
>>>>>>> The usefullness of this patch has already been discussed during the
>>>>>>> first releases (http://lists.linuxfoundation.org/pipermail/iommu/2014-August/009586.html).
>>>>>>> I underline the fact that it avoids implementing the logic on the
>>>>>>> userspace program, as VFIO can be used for many usage (userspace
>>>>>>> drivers and device assignment).
>>>>>>>
>>>>>>> If you're interested in the implementation on the userspace side, an
>>>>>>> RFC has been suggested for QEMU:
>>>>>>> http://lists.gnu.org/archive/html/qemu-devel/2015-01/msg01211.html
>>>>>>
>>>>>> This one-year-old discussion is hardly exhaustive.
>>>>>>
>>>>>> I think you missed the gist of the question for Eric a bit as well.
>>>>>>
>>>>>> One important question for me is whether seeing the host DT is always
>>>>>> sufficient or if the kernel and physical device driver can have more
>>>>>> information about the device and its configuration which userspace may
>>>>>> need, which cannot be directly read in the DT (for example because the
>>>>>> driver has initialized the device in a specific way).  My point is, it's
>>>>>> really not about DT-specific things (what if you used ACPI?), but it's
>>>>>> about retrieving properties of a device and its configuration from
>>>>>> userspace.
>>>>>>
>>>>>> Have we thought about the possible ways to achieve this and weight one
>>>>>> option against the other?
>>>>>
>>>>> Problem is that now we only have a very few platform devices behind an
>>>>> IOMMU, so we don't have the visibility to know if such a case will
>>>>> occur. With the current use cases, the interface seems to be
>>>>> sufficient.
>>>>
>>>> Ideally we can think about future use cases based on the experience of
>>>> people in the community and come up with a solution considering future
>>>> use cases.
>>>>
>>>>> By using IOCTL, we can always change the implementation
>>>>> later without any change on the userspace.
>>>>
>>>> Can you be more concrete with what you mean here?
>>>>
>>>
>>> By using an IOCTL, we define an API that allows to retrieve device
>>> properties from userspace. The way it is retrieved is handled by the
>>> kernel (For example for now if OF is unavailable, the kernel will
>>> retrieve the property using ACPI) and is totally transparent from the
>>> userspace point of view.
>>>
>>> My point is that we can go with the current naive implementation for
>>> now, and we might extend it later according to the needs of future
>>> devices, without changing anything from the userspace point of view.
>>
>> I don't really like "ioctl" and "naive implementation" in the same
>> thought.  The ioctl itself needs to be well thought out for current and
>> future needs, including independence from the source data type
>> (OF/DT/ACPI/etc), proven that it's necessary, it's not redundant to
>> existing interfaces, and vfio should be the one hosting it.  Thanks,
>>
> 
> The word we used, naive, does not mean a bad implementation of the
> IOCTL interface but rather it was referring to the fact that at this
> moment we export 1-to-1 the information available in the device tree
> via sys or proc file system. So one could obtain the same result
> without such interface, however our implementation is aready
> independent from the source of such information, since OF and ACPI are
> supported already, and there will be no problem in extending the
> interface to support more.
I rebooted my host in ACPI mode and I am not able to find the attribute
files anymore in sysfs. the of_node directory where the files were
located does not exist anymore (/sys/firmware/devicetree/base) and I
wonder whether we can get access to those same info somewhere in sysfs
(/sys/firmware). Obviously they are in the ACPI tables but ... Alex do
you know how it works on x86?

So I wonder whether the userspace has a unified way to fetch data in
sysfs whatever the type of HW description (DT, ACPI) or at least if it
not a unified way whether he have a simple way to achieve that on userspace.

As opposed to that the kernel offers that feature through the unified
device properties API, the one used in this series.

Best Regards

Eric
> 
> The next release will take into account your comments about argsz and flags.
> 
> Regards,
> Baptiste
> 
>> Alex
>>

Re: [RFC PATCH v3 1/3] vfio: platform: add device properties skeleton and user API

[Eric Auger]

On 12/19/2014 10:20 PM, Antonios Motakis wrote:
> This patch introduced the API to return device properties about
This patch introduces an API that allows to return device node
properties of a device bound to the vfio-platform/vfio-amba driver?
> a PLATFORM device (if described by a device tree or ACPI) and the
> skeleton of the implementation for VFIO_PLATFORM. Information about any
> device node bound by VFIO_PLATFORM should be queried via the introduced
> ioctl VFIO_DEVICE_GET_DEV_PROPERTY.
> 
> The user needs to know the name and the data type of the property he is
> accessing.
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/platform/Makefile                |  3 +-
>  drivers/vfio/platform/properties.c            | 61 +++++++++++++++++++++++++++
>  drivers/vfio/platform/vfio_platform_common.c  | 35 +++++++++++++++
>  drivers/vfio/platform/vfio_platform_private.h |  7 +++
>  include/uapi/linux/vfio.h                     | 26 ++++++++++++
>  5 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/vfio/platform/properties.c
> 
> diff --git a/drivers/vfio/platform/Makefile b/drivers/vfio/platform/Makefile
> index 81de144..99f3ba1 100644
> --- a/drivers/vfio/platform/Makefile
> +++ b/drivers/vfio/platform/Makefile
> @@ -1,5 +1,6 @@
>  
> -vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o
> +vfio-platform-y := vfio_platform.o vfio_platform_common.o vfio_platform_irq.o \
> +		   devtree.o
devtree.o or properties.o?
>  
>  obj-$(CONFIG_VFIO_PLATFORM) += vfio-platform.o
>  
> diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
> new file mode 100644
> index 0000000..8b90465
> --- /dev/null
> +++ b/drivers/vfio/platform/properties.c
missing copyright
> @@ -0,0 +1,61 @@
> +#include <linux/slab.h>
> +#include <linux/vfio.h>
> +#include <linux/property.h>
> +#include "vfio_platform_private.h"
> +
> +static int dev_property_get_strings(struct device *dev,
> +				    char *name, unsigned *lenp,
> +				    void __user *datap, unsigned long datasz)
> +{
> +	return -EINVAL;
> +}
> +
> +static int dev_property_get_uint(struct device *dev, char *name,
> +				 uint32_t type, unsigned *lenp,
> +				 void __user *datap, unsigned long datasz)
> +{
> +	return -EINVAL;
> +}
> +
> +int vfio_platform_dev_properties(struct device *dev,
> +				 uint32_t type, unsigned *lenp,
> +				 void __user *datap, unsigned long datasz)
> +{
> +	char *name;
> +	long namesz;
> +	int ret;
> +
> +	namesz = strnlen_user(datap, datasz);
> +	if (!namesz)
> +		return -EFAULT;
> +	if (namesz > datasz)
> +		return -EINVAL;
> +
> +	name = kzalloc(namesz, GFP_KERNEL);
> +	if (!name)
> +		return -ENOMEM;
> +	if (strncpy_from_user(name, datap, namesz) <= 0) {
> +		kfree(name);
> +		return -EFAULT;
> +	}
> +
> +	switch (type) {
> +	case VFIO_DEV_PROPERTY_TYPE_STRINGS:
> +		ret = dev_property_get_strings(dev, name, lenp, datap, datasz);
> +		break;
> +
> +	case VFIO_DEV_PROPERTY_TYPE_U64:
> +	case VFIO_DEV_PROPERTY_TYPE_U32:
> +	case VFIO_DEV_PROPERTY_TYPE_U16:
> +	case VFIO_DEV_PROPERTY_TYPE_U8:
> +		ret = dev_property_get_uint(dev, name, type, lenp,
> +					    datap, datasz);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	kfree(name);
> +	return ret;
> +}
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index a532a25..83ad4b74 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -19,6 +19,7 @@
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/vfio.h>
> +#include <linux/property.h>
>  
>  #include "vfio_platform_private.h"
>  
> @@ -251,6 +252,34 @@ static long vfio_platform_ioctl(void *device_data,
>  
>  		return ret;
>  
> +	} else if (cmd == VFIO_DEVICE_GET_DEV_PROPERTY) {
> +		struct vfio_dev_property info;
> +		void __user *datap;
> +		unsigned long datasz;
> +		int ret;
> +
> +		if (!vdev->dev)
> +			return -EINVAL;
> +
> +		minsz = offsetofend(struct vfio_dev_property, length);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		datap = (void __user *) arg + minsz;
> +		datasz = info.argsz - minsz;
> +
> +		ret = vfio_platform_dev_properties(vdev->dev, info.type,
> +						   &info.length, datap, datasz);
> +
> +		if (copy_to_user((void __user *)arg, &info, minsz))
> +			ret = -EFAULT;
> +
> +		return ret;
> +
>  	} else if (cmd == VFIO_DEVICE_RESET)
>  		return -EINVAL;
>  
> @@ -501,6 +530,12 @@ int vfio_platform_probe_common(struct vfio_platform_device *vdev,
>  		return ret;
>  	}
>  
> +	/* add device properties flag */
> +	if (device_property_present(dev, "name")) {
> +		vdev->dev = dev;
> +		vdev->flags |= VFIO_DEVICE_FLAGS_DEV_PROPERTIES;
> +	}
> +
>  	mutex_init(&vdev->igate);
>  
>  	return 0;
> diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h
> index c3d5b4b..fc6b1fb 100644
> --- a/drivers/vfio/platform/vfio_platform_private.h
> +++ b/drivers/vfio/platform/vfio_platform_private.h
> @@ -53,6 +53,7 @@ struct vfio_platform_device {
>  	u32				num_irqs;
>  	int				refcnt;
>  	struct mutex			igate;
> +	struct device			*dev;
>  
>  	/*
>  	 * These fields should be filled by the bus specific binder
> @@ -79,4 +80,10 @@ extern int vfio_platform_set_irqs_ioctl(struct vfio_platform_device *vdev,
>  					unsigned start, unsigned count,
>  					void *data);
>  
> +/* device properties support in devtree.c */
devtree.c to be removed
> +extern int vfio_platform_dev_properties(struct device *dev,
> +					uint32_t type, unsigned *lenp,
> +					void __user *datap,
> +					unsigned long datasz);
> +
>  #endif /* VFIO_PLATFORM_PRIVATE_H */
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 544d3d8..ac50ab3f 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -161,12 +161,38 @@ struct vfio_device_info {
>  #define VFIO_DEVICE_FLAGS_PCI	(1 << 1)	/* vfio-pci device */
>  #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)	/* vfio-platform device */
>  #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)	/* vfio-amba device */
> +#define VFIO_DEVICE_FLAGS_DEV_PROPERTIES (1 << 4) /* Device properties */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };
>  #define VFIO_DEVICE_GET_INFO		_IO(VFIO_TYPE, VFIO_BASE + 7)
>  
>  /**
> + * VFIO_DEVICE_GET_DEV_PROPERTY - _IOR(VFIO_TYPE, VFIO_BASE + 16,
> + *						struct vfio_devtree_info)
> + *
> + * Retrive a device property, e.g. from a device tree if available.
retrieve
from an HW description (device tree or ACPI)
> + * Caller will initialize data[] with a single string with the requested
needs to initialize data[] with the property name string?
> + * devicetree property name, and type depending on whether a array of strings
an array
> + * or an array of u32 values is expected. On success, data[] will be extended
> + * with the requested information, either as an array of u32, or with a list
> + * of strings sepparated by the NULL terminating character.
separated
> + * Return: 0 on success, -errno on failure.
> + */
> +struct vfio_dev_property {
> +	__u32	argsz;
> +	__u32	type;
> +#define VFIO_DEV_PROPERTY_TYPE_STRINGS	0
_STRING?
> +#define VFIO_DEV_PROPERTY_TYPE_U8	1
> +#define VFIO_DEV_PROPERTY_TYPE_U16	2
> +#define VFIO_DEV_PROPERTY_TYPE_U32	3
> +#define VFIO_DEV_PROPERTY_TYPE_U64	4
> +	__u32	length;
you may comment length value as initialized by the user (size of the
string including the nul char?
> +	__u8	data[];
> +};
> +#define VFIO_DEVICE_GET_DEV_PROPERTY	_IO(VFIO_TYPE, VFIO_BASE + 17)
17 does not match figure in above comment
> +
> +/**
>   * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
>   *				       struct vfio_region_info)
>   *
> 

Re: [RFC PATCH v3 2/3] vfio: platform: access device property as a list of strings

[Eric Auger]

On 12/19/2014 10:20 PM, Antonios Motakis wrote:
> Certain device properties (e.g. the device node name, the compatible
> string), are available as a list of strings (separated by the null
> terminating character). Let the VFIO user query this type of properties.
> 
> Signed-off-by: Antonios Motakis <a.motakis@virtualopensystems.com>
> ---
>  drivers/vfio/platform/properties.c | 43 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
> index 8b90465..39c6342 100644
> --- a/drivers/vfio/platform/properties.c
> +++ b/drivers/vfio/platform/properties.c
> @@ -7,7 +7,48 @@ static int dev_property_get_strings(struct device *dev,
>  				    char *name, unsigned *lenp,
>  				    void __user *datap, unsigned long datasz)
>  {
> -	return -EINVAL;
> +	const char **val;
> +	int n, i, ret;
> +
> +	*lenp = 0;
> +
> +	n = device_property_read_string_array(dev, name, NULL, 0);
> +	if (n < 0)
don't know if n==0 can happen? property found but not value?
> +		return n;
> +
> +	val = kcalloc(n, sizeof(char*), GFP_KERNEL);
char *, checkpatch error.
> +	if (!val)
> +		return -ENOMEM;
> +
> +	ret = device_property_read_string_array(dev, name, val, n);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = 0;
why not initializing ret at declaration time?
> +
> +	for (i = 0; i < n; i++) {
> +		size_t len = strlen(val[i]) + 1;
> +
> +		if (datasz < len) {
> +			ret = -EOVERFLOW;
> +			while (i < n)
> +				*lenp += strlen(val[i++]) + 1;
why do you set *lenp to the length of all strings even if they can't be
copied?
> +			goto out;
> +		}
> +
> +		if (copy_to_user(datap, val[i], len)) {
> +			ret = -EFAULT;
so in that case *lenp is not incremented?
> +			goto out;
> +		}
Can't you have a similar implementation as for uxx, see next patch
comments. Can't you copy to user in one shot. Can't that code be simplified.
> +
> +		*lenp += len;
> +		datap += len;
> +		datasz -= len;
> +	}
> +
> +out:
> +	kfree(val);
> +	return ret;
>  }
>  
>  static int dev_property_get_uint(struct device *dev, char *name,
> 

Re: [RFC PATCH v3 3/3] vfio: platform: return device properties as arrays of unsigned integers

[Eric Auger]

On 12/19/2014 10:20 PM, Antonios Motakis wrote:
> Certain properties of a device are accessible as an array of unsigned
> integers, either u64, u32, u16, or u8. Let the VFIO user query this
> type of device properties.
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/platform/properties.c | 62 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/platform/properties.c b/drivers/vfio/platform/properties.c
> index 39c6342..645f6e5c 100644
> --- a/drivers/vfio/platform/properties.c
> +++ b/drivers/vfio/platform/properties.c
> @@ -55,7 +55,67 @@ static int dev_property_get_uint(struct device *dev, char *name,
>  				 uint32_t type, unsigned *lenp,
>  				 void __user *datap, unsigned long datasz)
>  {
> -	return -EINVAL;
> +	int ret, n;
> +	u8 *out;
> +	size_t sz;
> +	int (*func)(const struct device *, const char *, void *, size_t)
> +		= NULL;
> +
> +	switch (type) {
> +	case VFIO_DEV_PROPERTY_TYPE_U64:
> +		sz = sizeof(u64);
> +		func = (int (*)(const struct device *,
> +				const char *, void *, size_t))
> +			device_property_read_u64_array;
> +		break;
> +	case VFIO_DEV_PROPERTY_TYPE_U32:
> +		sz = sizeof(u32);
> +		func = (int (*)(const struct device *,
> +				const char *, void *, size_t))
> +			device_property_read_u32_array;
> +		break;
> +	case VFIO_DEV_PROPERTY_TYPE_U16:
> +		sz = sizeof(u16);
> +		func = (int (*)(const struct device *,
> +				const char *, void *, size_t))
> +			device_property_read_u16_array;
> +		break;
> +	case VFIO_DEV_PROPERTY_TYPE_U8:
> +		sz = sizeof(u8);
> +		func = (int (*)(const struct device *,
> +				const char *, void *, size_t))
> +			device_property_read_u8_array;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* get size of array */
> +	n = func(dev, name, NULL, 0);
> +	if (n < 0)
> +		return n;
> +
> +	if (lenp)
> +		*lenp = n * sz;
is it really relevant to check lenp is allocated? also what if it is not...
> +
> +	if (n * sz > datasz)
> +		return -EOVERFLOW;
so you could have done that too for strings. Isn't it simpler? Note the
*lengp value is not consistent with what was done for strings.
> +
> +	out = kcalloc(n, sz, GFP_KERNEL);
> +	if (!out)
> +		return -EFAULT;
-ENOMEM?
> +
> +	ret = func(dev, name, out, n);
> +	if (ret)
> +		goto out;
> +
> +	if (copy_to_user(datap, out, n * sz))
> +		ret = -EFAULT;
> +
> +out:
> +	kfree(out);
> +	return ret;
>  }
>  
>  int vfio_platform_dev_properties(struct device *dev,
>