VFIO returns a file descriptor which we can use to manipulate the memory
regions of the device. Since some memory regions we cannot mmap due to
security concerns, we also allow to read and write to this file descriptor
directly.
Signed-off-by: Antonios Motakis <[email protected]>
---
drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++-
1 file changed, 107 insertions(+), 3 deletions(-)
diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
index 5430cbe..7c01ced 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
region.addr = res->start;
region.size = resource_size(res);
- region.flags = 0;
+ region.flags = VFIO_REGION_INFO_FLAG_READ
+ | VFIO_REGION_INFO_FLAG_WRITE;
vdev->region[i] = region;
}
@@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data,
static ssize_t vfio_platform_read(void *device_data, char __user *buf,
size_t count, loff_t *ppos)
{
- return 0;
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
+ unsigned int done = 0;
+ void __iomem *io;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ io = ioremap_nocache(vdev->region[index].addr,
+ vdev->region[index].size);
+
+ while (count) {
+ size_t filled;
+
+ if (count >= 4 && !(off % 4)) {
+ u32 val;
+
+ val = ioread32(io + off);
+ if (copy_to_user(buf, &val, 4))
+ goto err;
+
+ filled = 4;
+ } else if (count >= 2 && !(off % 2)) {
+ u16 val;
+
+ val = ioread16(io + off);
+ if (copy_to_user(buf, &val, 2))
+ goto err;
+
+ filled = 2;
+ } else {
+ u8 val;
+
+ val = ioread8(io + off);
+ if (copy_to_user(buf, &val, 1))
+ goto err;
+
+ filled = 1;
+ }
+
+
+ count -= filled;
+ done += filled;
+ off += filled;
+ buf += filled;
+ }
+
+ iounmap(io);
+ return done;
+err:
+ iounmap(io);
+ return -EFAULT;
}
static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
size_t count, loff_t *ppos)
{
- return 0;
+ struct vfio_platform_device *vdev = device_data;
+ unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
+ loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
+ unsigned int done = 0;
+ void __iomem *io;
+
+ if (index >= vdev->num_regions)
+ return -EINVAL;
+
+ io = ioremap_nocache(vdev->region[index].addr,
+ vdev->region[index].size);
+
+ while (count) {
+ size_t filled;
+
+ if (count >= 4 && !(off % 4)) {
+ u32 val;
+
+ if (copy_from_user(&val, buf, 4))
+ goto err;
+ iowrite32(val, io + off);
+
+ filled = 4;
+ } else if (count >= 2 && !(off % 2)) {
+ u16 val;
+
+ if (copy_from_user(&val, buf, 2))
+ goto err;
+ iowrite16(val, io + off);
+
+ filled = 2;
+ } else {
+ u8 val;
+
+ if (copy_from_user(&val, buf, 1))
+ goto err;
+ iowrite8(val, io + off);
+
+ filled = 1;
+ }
+
+ count -= filled;
+ done += filled;
+ off += filled;
+ buf += filled;
+ }
+
+ iounmap(io);
+ return done;
+err:
+ iounmap(io);
+ return -EFAULT;
}
static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
--
1.8.3.2
On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote:
> VFIO returns a file descriptor which we can use to manipulate the memory
> regions of the device. Since some memory regions we cannot mmap due to
> security concerns, we also allow to read and write to this file descriptor
> directly.
If there are regions we cannot mmap for security, why do we provide full
read/write access to them?
> Signed-off-by: Antonios Motakis <[email protected]>
> ---
> drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++-
> 1 file changed, 107 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> index 5430cbe..7c01ced 100644
> --- a/drivers/vfio/platform/vfio_platform.c
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
>
> region.addr = res->start;
> region.size = resource_size(res);
> - region.flags = 0;
> + region.flags = VFIO_REGION_INFO_FLAG_READ
> + | VFIO_REGION_INFO_FLAG_WRITE;
>
> vdev->region[i] = region;
> }
> @@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data,
> static ssize_t vfio_platform_read(void *device_data, char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return 0;
> + struct vfio_platform_device *vdev = device_data;
> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
> + unsigned int done = 0;
> + void __iomem *io;
> +
> + if (index >= vdev->num_regions)
> + return -EINVAL;
> +
> + io = ioremap_nocache(vdev->region[index].addr,
> + vdev->region[index].size);
I haven't looked at ioremap on arm, but if it's remotely non-trivially,
it's probably a good idea to do this only on first access, cache it, and
unmap it on region cleanup. Thanks,
Alex
> +
> + while (count) {
> + size_t filled;
> +
> + if (count >= 4 && !(off % 4)) {
> + u32 val;
> +
> + val = ioread32(io + off);
> + if (copy_to_user(buf, &val, 4))
> + goto err;
> +
> + filled = 4;
> + } else if (count >= 2 && !(off % 2)) {
> + u16 val;
> +
> + val = ioread16(io + off);
> + if (copy_to_user(buf, &val, 2))
> + goto err;
> +
> + filled = 2;
> + } else {
> + u8 val;
> +
> + val = ioread8(io + off);
> + if (copy_to_user(buf, &val, 1))
> + goto err;
> +
> + filled = 1;
> + }
> +
> +
> + count -= filled;
> + done += filled;
> + off += filled;
> + buf += filled;
> + }
> +
> + iounmap(io);
> + return done;
> +err:
> + iounmap(io);
> + return -EFAULT;
> }
>
> static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return 0;
> + struct vfio_platform_device *vdev = device_data;
> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
> + unsigned int done = 0;
> + void __iomem *io;
> +
> + if (index >= vdev->num_regions)
> + return -EINVAL;
> +
> + io = ioremap_nocache(vdev->region[index].addr,
> + vdev->region[index].size);
> +
> + while (count) {
> + size_t filled;
> +
> + if (count >= 4 && !(off % 4)) {
> + u32 val;
> +
> + if (copy_from_user(&val, buf, 4))
> + goto err;
> + iowrite32(val, io + off);
> +
> + filled = 4;
> + } else if (count >= 2 && !(off % 2)) {
> + u16 val;
> +
> + if (copy_from_user(&val, buf, 2))
> + goto err;
> + iowrite16(val, io + off);
> +
> + filled = 2;
> + } else {
> + u8 val;
> +
> + if (copy_from_user(&val, buf, 1))
> + goto err;
> + iowrite8(val, io + off);
> +
> + filled = 1;
> + }
> +
> + count -= filled;
> + done += filled;
> + off += filled;
> + buf += filled;
> + }
> +
> + iounmap(io);
> + return done;
> +err:
> + iounmap(io);
> + return -EFAULT;
> }
>
> static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
On Mon, Apr 28, 2014 at 7:24 PM, Alex Williamson
<[email protected]> wrote:
> On Mon, 2014-04-28 at 17:52 +0200, Antonios Motakis wrote:
>> VFIO returns a file descriptor which we can use to manipulate the memory
>> regions of the device. Since some memory regions we cannot mmap due to
>> security concerns, we also allow to read and write to this file descriptor
>> directly.
>
> If there are regions we cannot mmap for security, why do we provide full
> read/write access to them?
What I refer to here is to the problem of mmaping an MMIO region that
is not page-sized. For example if it is just a small number of
registers which are not aligned on a page boundary, mmaping them would
still make the whole page accessible to the guest, with unpredictable
outcomes. But in these cases read/write is still OK.
>
>> Signed-off-by: Antonios Motakis <[email protected]>
>> ---
>> drivers/vfio/platform/vfio_platform.c | 110 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 107 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
>> index 5430cbe..7c01ced 100644
>> --- a/drivers/vfio/platform/vfio_platform.c
>> +++ b/drivers/vfio/platform/vfio_platform.c
>> @@ -55,7 +55,8 @@ static int vfio_platform_regions_init(struct vfio_platform_device *vdev)
>>
>> region.addr = res->start;
>> region.size = resource_size(res);
>> - region.flags = 0;
>> + region.flags = VFIO_REGION_INFO_FLAG_READ
>> + | VFIO_REGION_INFO_FLAG_WRITE;
>>
>> vdev->region[i] = region;
>> }
>> @@ -153,13 +154,116 @@ static long vfio_platform_ioctl(void *device_data,
>> static ssize_t vfio_platform_read(void *device_data, char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return 0;
>> + struct vfio_platform_device *vdev = device_data;
>> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
>> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
>> + unsigned int done = 0;
>> + void __iomem *io;
>> +
>> + if (index >= vdev->num_regions)
>> + return -EINVAL;
>> +
>> + io = ioremap_nocache(vdev->region[index].addr,
>> + vdev->region[index].size);
>
> I haven't looked at ioremap on arm, but if it's remotely non-trivially,
> it's probably a good idea to do this only on first access, cache it, and
> unmap it on region cleanup. Thanks,
Ack.
>
> Alex
>
>> +
>> + while (count) {
>> + size_t filled;
>> +
>> + if (count >= 4 && !(off % 4)) {
>> + u32 val;
>> +
>> + val = ioread32(io + off);
>> + if (copy_to_user(buf, &val, 4))
>> + goto err;
>> +
>> + filled = 4;
>> + } else if (count >= 2 && !(off % 2)) {
>> + u16 val;
>> +
>> + val = ioread16(io + off);
>> + if (copy_to_user(buf, &val, 2))
>> + goto err;
>> +
>> + filled = 2;
>> + } else {
>> + u8 val;
>> +
>> + val = ioread8(io + off);
>> + if (copy_to_user(buf, &val, 1))
>> + goto err;
>> +
>> + filled = 1;
>> + }
>> +
>> +
>> + count -= filled;
>> + done += filled;
>> + off += filled;
>> + buf += filled;
>> + }
>> +
>> + iounmap(io);
>> + return done;
>> +err:
>> + iounmap(io);
>> + return -EFAULT;
>> }
>>
>> static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return 0;
>> + struct vfio_platform_device *vdev = device_data;
>> + unsigned int index = VFIO_PLATFORM_OFFSET_TO_INDEX(*ppos);
>> + loff_t off = *ppos & VFIO_PLATFORM_OFFSET_MASK;
>> + unsigned int done = 0;
>> + void __iomem *io;
>> +
>> + if (index >= vdev->num_regions)
>> + return -EINVAL;
>> +
>> + io = ioremap_nocache(vdev->region[index].addr,
>> + vdev->region[index].size);
>> +
>> + while (count) {
>> + size_t filled;
>> +
>> + if (count >= 4 && !(off % 4)) {
>> + u32 val;
>> +
>> + if (copy_from_user(&val, buf, 4))
>> + goto err;
>> + iowrite32(val, io + off);
>> +
>> + filled = 4;
>> + } else if (count >= 2 && !(off % 2)) {
>> + u16 val;
>> +
>> + if (copy_from_user(&val, buf, 2))
>> + goto err;
>> + iowrite16(val, io + off);
>> +
>> + filled = 2;
>> + } else {
>> + u8 val;
>> +
>> + if (copy_from_user(&val, buf, 1))
>> + goto err;
>> + iowrite8(val, io + off);
>> +
>> + filled = 1;
>> + }
>> +
>> + count -= filled;
>> + done += filled;
>> + off += filled;
>> + buf += filled;
>> + }
>> +
>> + iounmap(io);
>> + return done;
>> +err:
>> + iounmap(io);
>> + return -EFAULT;
>> }
>>
>> static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
>
>
>
--
Antonios Motakis
Virtual Open Systems