On Fri, 15 Nov 2019 03:56:16 +0100
Eric Farman <farman@xxxxxxxxxxxxx> wrote:
> From: Farhan Ali <alifm@xxxxxxxxxxxxx>
>
> This region can be used by userspace to get channel report
> words from vfio-ccw driver.
I think this needs a bit more explanation; this is for channel report
words concerning vfio-ccw devices that are supposed to be relayed to
the guest, IIUC?
>
> We provide space for two CRWs, per the limit in the
> base driver (see crw_collect_info()).
That rationale seems a bit sketchy.
As far as I know, current systems provide at most two crws chained
together for an event (one for the ssid + one for the subchannel id in
case of a subchannel event, one crw for other events); and that's the
reason why we provide space for two crws (unless there's something
upcoming which would need three crws chained together?)
>
> Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
>
> Notes:
> 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
>
> drivers/s390/cio/vfio_ccw_chp.c | 53 +++++++++++++++++++++++++++++
> 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 | 5 +++
> 6 files changed, 86 insertions(+)
>
> diff --git a/drivers/s390/cio/vfio_ccw_chp.c b/drivers/s390/cio/vfio_ccw_chp.c
> index 826d08379fe3..d1e8bfef06be 100644
> --- a/drivers/s390/cio/vfio_ccw_chp.c
> +++ b/drivers/s390/cio/vfio_ccw_chp.c
> @@ -73,3 +73,56 @@ 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;
> +
> + mutex_lock(&private->io_mutex);
> + region = private->region[i].data;
> +
> + if (copy_to_user(buf, (void *)region + pos, count))
> + ret = -EFAULT;
> + else
> + ret = count;
> +
Can userspace read the same crw(s) multiple times? How can it find out
if there's something new in there?
> + mutex_unlock(&private->io_mutex);
> + return ret;
> +}
> +
(...)
> diff --git a/include/uapi/linux/vfio_ccw.h b/include/uapi/linux/vfio_ccw.h
> index 7c0a834e5d7a..88b125aad279 100644
> --- a/include/uapi/linux/vfio_ccw.h
> +++ b/include/uapi/linux/vfio_ccw.h
> @@ -39,4 +39,9 @@ struct ccw_schib_region {
> __u8 schib_area[SCHIB_AREA_SIZE];
> } __packed;
>
I think this one wants an explaining comment as well.
> +struct ccw_crw_region {
> + __u32 crw0;
> + __u32 crw1;
> +} __packed;
> +
> #endif
