On Thu, 6 Feb 2020 22:38:21 +0100
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
> From: Farhan Ali <alifm@xxxxxxxxxxxxx>
>
> This region provides a mechanism to pass Channel Report Word(s)
> that affect vfio-ccw devices, and need to be passed to the guest
> for its awareness and/or processing.
>
> The base driver (see crw_collect_info()) provides space for two
> CRWs, as a subchannel event may have two CRWs chained together
> (one for the ssid, one for the subcahnnel). All other CRWs will
> only occupy the first one. Even though this support will also
> only utilize the first one, we'll provide space for two also.
>
> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
>
> Notes:
> v1-v2:
> - Add new region info to Documentation/s390/vfio-ccw.rst [CH]
> - Add a block comment to struct ccw_crw_region [CH]
>
> v0->v1: [EF]
> - Clean up checkpatch (whitespace) errors
> - Add ret=-ENOMEM in error path for new region
> - Add io_mutex for region read (originally in last patch)
> - Change crw1/crw2 to crw0/crw1
> - Reorder cleanup of regions
>
> Documentation/s390/vfio-ccw.rst | 15 ++++++++
> drivers/s390/cio/vfio_ccw_chp.c | 56 +++++++++++++++++++++++++++++
> drivers/s390/cio/vfio_ccw_drv.c | 20 +++++++++++
> drivers/s390/cio/vfio_ccw_ops.c | 4 +++
> drivers/s390/cio/vfio_ccw_private.h | 3 ++
> include/uapi/linux/vfio.h | 1 +
> include/uapi/linux/vfio_ccw.h | 9 +++++
> 7 files changed, 108 insertions(+)
>
(...)
> diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c
> index 826d08379fe3..8fde94552149 100644
> --- a/drivers/s390/cio/vfio_ccw_chp.c
> +++ b/drivers/s390/cio/vfio_ccw_chp.c
> @@ -73,3 +73,59 @@ int vfio_ccw_register_schib_dev_regions(struct vfio_ccw_private *private)
> VFIO_REGION_INFO_FLAG_READ,
> private->schib_region);
> }
> +
> +static ssize_t vfio_ccw_crw_region_read(struct vfio_ccw_private *private,
> + char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + unsigned int i = VFIO_CCW_OFFSET_TO_INDEX(*ppos) - VFIO_CCW_NUM_REGIONS;
> + loff_t pos = *ppos & VFIO_CCW_OFFSET_MASK;
> + struct ccw_crw_region *region;
> + int ret;
> +
> + if (pos + count > sizeof(*region))
> + return -EINVAL;
> +
> + if (list_empty(&private->crw))
> + return 0;
> +
> + mutex_lock(&private->io_mutex);
> + region = private->region[i].data;
> +
> + if (copy_to_user(buf, (void *)region + pos, count))
> + ret = -EFAULT;
> + else
> + ret = count;
> +
> + mutex_unlock(&private->io_mutex);
> + return ret;
> +}
Would it make sense to clear out the crw after it has been read by
userspace?
In patch 7, you add a notification for a new crw via eventfd, but
nothing is preventing userspace from reading this even if not
triggered. I also don't see the region being updated there until a new
crw is posted.
Or am I missing something?
