Re: [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
On 10/2/2020 11:50 PM, Alex Williamson wrote:
On Tue, 29 Sep 2020 12:03:38 +0300
Diana Craciun wrote:
The software uses a memory-mapped I/O command interface (MC portals) to
communicate with the MC hardware. This command interface is used to
discover, enumerate, configure and remove DPAA2 objects. The DPAA2
objects use MSIs, so the command interface needs to be emulated
such that the correct MSI is configured in the hardware (the guest
has the virtual MSIs).
This patch is adding read/write support for fsl-mc devices. The mc
commands are emulated by the userspace. The host is just passing
the correct command to the hardware.
Also the current patch limits userspace to write complete
64byte command once and read 64byte response by one ioctl.
Signed-off-by: Bharat Bhushan
Signed-off-by: Diana Craciun
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 118 +-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
2 files changed, 116 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 82157837f37a..0aff99cdf722 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "vfio_fsl_mc_private.h"
@@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct vfio_fsl_mc_device *vdev)
!(vdev->regions[i].size & ~PAGE_MASK))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_MMAP;
-
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
+ if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
}
return 0;
@@ -123,6 +126,11 @@ static int vfio_fsl_mc_regions_init(struct
vfio_fsl_mc_device *vdev)
static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int i;
+
+ for (i = 0; i < mc_dev->obj_desc.region_count; i++)
+ iounmap(vdev->regions[i].ioaddr);
kfree(vdev->regions);
}
@@ -301,13 +309,117 @@ static long vfio_fsl_mc_ioctl(void *device_data, unsigned int cmd,
static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
size_t count, loff_t *ppos)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ struct vfio_fsl_mc_region *region;
+ u64 data[8];
+ int i;
+
+ if (index >= mc_dev->obj_desc.region_count)
+ return -EINVAL;
+
+ region = >regions[index];
+
+ if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
+ return -EINVAL;
Nit, there are no regions w/o read access according to the regions_init
code above. Maybe this is just for symmetry with write? Keep it if
you prefer. Thanks,
I would prefer to keep it like this for symmetry with write.
Thanks,
Diana
Alex
+
+ if (!region->ioaddr) {
+ region->ioaddr = ioremap(region->addr, region->size);
+ if (!region->ioaddr)
+ return -ENOMEM;
+ }
+
+ if (count != 64 || off != 0)
+ return -EINVAL;
+
+ for (i = 7; i >= 0; i--)
+ data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
+
+ if (copy_to_user(buf, data, 64))
+ return -EFAULT;
+
+ return count;
+}
+
+#define MC_CMD_COMPLETION_TIMEOUT_MS5000
+#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS500
+
+static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
+{
+ int i;
+ enum mc_cmd_status status;
+ unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
+
+ /* Write at command parameter into portal */
+ for (i = 7; i >= 1; i--)
+ writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
+
+ /* Write command header in the end */
+ writeq(cmd_data[0], ioaddr);
+
+ /* Wait for response before returning to user-space
+* This can be optimized in future to even prepare response
+* before returning to user-space and avoid read ioctl.
+*/
+ for (;;) {
+ u64 header;
+ struct mc_cmd_header *resp_hdr;
+
+ header = cpu_to_le64(readq_relaxed(ioaddr));
+
+ resp_hdr = (struct mc_cmd_header *)
+ status = (enum mc_cmd_status)resp_hdr->status;
+ if (status != MC_CMD_STATUS_READY)
+ break;
+
+ udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
+
Re: [PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
On Tue, 29 Sep 2020 12:03:38 +0300
Diana Craciun wrote:
> The software uses a memory-mapped I/O command interface (MC portals) to
> communicate with the MC hardware. This command interface is used to
> discover, enumerate, configure and remove DPAA2 objects. The DPAA2
> objects use MSIs, so the command interface needs to be emulated
> such that the correct MSI is configured in the hardware (the guest
> has the virtual MSIs).
>
> This patch is adding read/write support for fsl-mc devices. The mc
> commands are emulated by the userspace. The host is just passing
> the correct command to the hardware.
>
> Also the current patch limits userspace to write complete
> 64byte command once and read 64byte response by one ioctl.
>
> Signed-off-by: Bharat Bhushan
> Signed-off-by: Diana Craciun
> ---
> drivers/vfio/fsl-mc/vfio_fsl_mc.c | 118 +-
> drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
> 2 files changed, 116 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 82157837f37a..0aff99cdf722 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #include "vfio_fsl_mc_private.h"
>
> @@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct
> vfio_fsl_mc_device *vdev)
> !(vdev->regions[i].size & ~PAGE_MASK))
> vdev->regions[i].flags |=
> VFIO_REGION_INFO_FLAG_MMAP;
> -
> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
> + if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
> + vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
> }
>
> return 0;
> @@ -123,6 +126,11 @@ static int vfio_fsl_mc_regions_init(struct
> vfio_fsl_mc_device *vdev)
>
> static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
> {
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + int i;
> +
> + for (i = 0; i < mc_dev->obj_desc.region_count; i++)
> + iounmap(vdev->regions[i].ioaddr);
> kfree(vdev->regions);
> }
>
> @@ -301,13 +309,117 @@ static long vfio_fsl_mc_ioctl(void *device_data,
> unsigned int cmd,
> static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return -EINVAL;
> + struct vfio_fsl_mc_device *vdev = device_data;
> + unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
> + loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
> + struct fsl_mc_device *mc_dev = vdev->mc_dev;
> + struct vfio_fsl_mc_region *region;
> + u64 data[8];
> + int i;
> +
> + if (index >= mc_dev->obj_desc.region_count)
> + return -EINVAL;
> +
> + region = >regions[index];
> +
> + if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
> + return -EINVAL;
Nit, there are no regions w/o read access according to the regions_init
code above. Maybe this is just for symmetry with write? Keep it if
you prefer. Thanks,
Alex
> +
> + if (!region->ioaddr) {
> + region->ioaddr = ioremap(region->addr, region->size);
> + if (!region->ioaddr)
> + return -ENOMEM;
> + }
> +
> + if (count != 64 || off != 0)
> + return -EINVAL;
> +
> + for (i = 7; i >= 0; i--)
> + data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
> +
> + if (copy_to_user(buf, data, 64))
> + return -EFAULT;
> +
> + return count;
> +}
> +
> +#define MC_CMD_COMPLETION_TIMEOUT_MS5000
> +#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS500
> +
> +static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
> +{
> + int i;
> + enum mc_cmd_status status;
> + unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
> +
> + /* Write at command parameter into portal */
> + for (i = 7; i >= 1; i--)
> + writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
> +
> + /* Write command header in the end */
> + writeq(cmd_data[0], ioaddr);
> +
> + /* Wait for response before returning to user-space
> + * This can be optimized in future to even prepare response
> + * before returning to user-space and avoid read ioctl.
> + */
> + for (;;) {
> + u64 header;
> + struct mc_cmd_header *resp_hdr;
> +
> + header = cpu_to_le64(readq_relaxed(ioaddr));
> +
> + resp_hdr = (struct mc_cmd_header *)
> + status = (enum mc_cmd_status)resp_hdr->status;
> + if (status != MC_CMD_STATUS_READY)
> + break;
> +
> + udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
> + timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
> +
[PATCH v5 09/10] vfio/fsl-mc: Add read/write support for fsl-mc devices
The software uses a memory-mapped I/O command interface (MC portals) to
communicate with the MC hardware. This command interface is used to
discover, enumerate, configure and remove DPAA2 objects. The DPAA2
objects use MSIs, so the command interface needs to be emulated
such that the correct MSI is configured in the hardware (the guest
has the virtual MSIs).
This patch is adding read/write support for fsl-mc devices. The mc
commands are emulated by the userspace. The host is just passing
the correct command to the hardware.
Also the current patch limits userspace to write complete
64byte command once and read 64byte response by one ioctl.
Signed-off-by: Bharat Bhushan
Signed-off-by: Diana Craciun
---
drivers/vfio/fsl-mc/vfio_fsl_mc.c | 118 +-
drivers/vfio/fsl-mc/vfio_fsl_mc_private.h | 1 +
2 files changed, 116 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 82157837f37a..0aff99cdf722 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "vfio_fsl_mc_private.h"
@@ -115,7 +116,9 @@ static int vfio_fsl_mc_regions_init(struct
vfio_fsl_mc_device *vdev)
!(vdev->regions[i].size & ~PAGE_MASK))
vdev->regions[i].flags |=
VFIO_REGION_INFO_FLAG_MMAP;
-
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_READ;
+ if (!(mc_dev->regions[i].flags & IORESOURCE_READONLY))
+ vdev->regions[i].flags |= VFIO_REGION_INFO_FLAG_WRITE;
}
return 0;
@@ -123,6 +126,11 @@ static int vfio_fsl_mc_regions_init(struct
vfio_fsl_mc_device *vdev)
static void vfio_fsl_mc_regions_cleanup(struct vfio_fsl_mc_device *vdev)
{
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ int i;
+
+ for (i = 0; i < mc_dev->obj_desc.region_count; i++)
+ iounmap(vdev->regions[i].ioaddr);
kfree(vdev->regions);
}
@@ -301,13 +309,117 @@ static long vfio_fsl_mc_ioctl(void *device_data,
unsigned int cmd,
static ssize_t vfio_fsl_mc_read(void *device_data, char __user *buf,
size_t count, loff_t *ppos)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ unsigned int index = VFIO_FSL_MC_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_FSL_MC_OFFSET_MASK;
+ struct fsl_mc_device *mc_dev = vdev->mc_dev;
+ struct vfio_fsl_mc_region *region;
+ u64 data[8];
+ int i;
+
+ if (index >= mc_dev->obj_desc.region_count)
+ return -EINVAL;
+
+ region = >regions[index];
+
+ if (!(region->flags & VFIO_REGION_INFO_FLAG_READ))
+ return -EINVAL;
+
+ if (!region->ioaddr) {
+ region->ioaddr = ioremap(region->addr, region->size);
+ if (!region->ioaddr)
+ return -ENOMEM;
+ }
+
+ if (count != 64 || off != 0)
+ return -EINVAL;
+
+ for (i = 7; i >= 0; i--)
+ data[i] = readq(region->ioaddr + i * sizeof(uint64_t));
+
+ if (copy_to_user(buf, data, 64))
+ return -EFAULT;
+
+ return count;
+}
+
+#define MC_CMD_COMPLETION_TIMEOUT_MS5000
+#define MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS500
+
+static int vfio_fsl_mc_send_command(void __iomem *ioaddr, uint64_t *cmd_data)
+{
+ int i;
+ enum mc_cmd_status status;
+ unsigned long timeout_usecs = MC_CMD_COMPLETION_TIMEOUT_MS * 1000;
+
+ /* Write at command parameter into portal */
+ for (i = 7; i >= 1; i--)
+ writeq_relaxed(cmd_data[i], ioaddr + i * sizeof(uint64_t));
+
+ /* Write command header in the end */
+ writeq(cmd_data[0], ioaddr);
+
+ /* Wait for response before returning to user-space
+* This can be optimized in future to even prepare response
+* before returning to user-space and avoid read ioctl.
+*/
+ for (;;) {
+ u64 header;
+ struct mc_cmd_header *resp_hdr;
+
+ header = cpu_to_le64(readq_relaxed(ioaddr));
+
+ resp_hdr = (struct mc_cmd_header *)
+ status = (enum mc_cmd_status)resp_hdr->status;
+ if (status != MC_CMD_STATUS_READY)
+ break;
+
+ udelay(MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS);
+ timeout_usecs -= MC_CMD_COMPLETION_POLLING_MAX_SLEEP_USECS;
+ if (timeout_usecs == 0)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
}
static ssize_t vfio_fsl_mc_write(void *device_data, const char __user *buf,
size_t count, loff_t *ppos)
{
- return -EINVAL;
+ struct vfio_fsl_mc_device *vdev = device_data;
+ unsigned int index =
