| Message ID | 20181122165432.4437-2-cohuck@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio-ccw: support hsch/csch (kernel part) | expand |
On 22/11/2018 17:54, Cornelia Huck wrote: > Allow to extend the regions used by vfio-ccw. The first user will be > handling of halt and clear subchannel. > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > drivers/s390/cio/vfio_ccw_ops.c | 182 ++++++++++++++++++++++++---- > drivers/s390/cio/vfio_ccw_private.h | 38 ++++++ > include/uapi/linux/vfio.h | 1 + > 3 files changed, 195 insertions(+), 26 deletions(-) > Halt and clear have no parameters (the sub-channel ID is obviously the one of the mediated device). Isn't adding a new sub-region for the purpose of handling halt and clear superfluous? What is the reason not to use simple ioctls ? Regards, Pierre
On Fri, 23 Nov 2018 13:28:25 +0100 Pierre Morel <pmorel@linux.ibm.com> wrote: > On 22/11/2018 17:54, Cornelia Huck wrote: > > Allow to extend the regions used by vfio-ccw. The first user will be > > handling of halt and clear subchannel. > > > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > drivers/s390/cio/vfio_ccw_ops.c | 182 ++++++++++++++++++++++++---- > > drivers/s390/cio/vfio_ccw_private.h | 38 ++++++ > > include/uapi/linux/vfio.h | 1 + > > 3 files changed, 195 insertions(+), 26 deletions(-) > > > > Halt and clear have no parameters (the sub-channel ID is obviously the > one of the mediated device). > > Isn't adding a new sub-region for the purpose of handling halt and clear > superfluous? > > What is the reason not to use simple ioctls ? Should it turn out that we missed something and need an enhanced interface, we can simply stop providing this subregion and add a new subregion, without breaking existing userspace. We can't do that with ioctls. And moreover, this is only the first user of this infrastructure. There's also that path handling series that Dong Jia had posted early this year -- that would be an obvious user as well.
On 23/11/2018 13:45, Cornelia Huck wrote: > On Fri, 23 Nov 2018 13:28:25 +0100 > Pierre Morel <pmorel@linux.ibm.com> wrote: > >> On 22/11/2018 17:54, Cornelia Huck wrote: >>> Allow to extend the regions used by vfio-ccw. The first user will be >>> handling of halt and clear subchannel. >>> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> drivers/s390/cio/vfio_ccw_ops.c | 182 ++++++++++++++++++++++++---- >>> drivers/s390/cio/vfio_ccw_private.h | 38 ++++++ >>> include/uapi/linux/vfio.h | 1 + >>> 3 files changed, 195 insertions(+), 26 deletions(-) >>> >> >> Halt and clear have no parameters (the sub-channel ID is obviously the >> one of the mediated device). >> >> Isn't adding a new sub-region for the purpose of handling halt and clear >> superfluous? >> >> What is the reason not to use simple ioctls ? > > Should it turn out that we missed something and need an enhanced > interface, we can simply stop providing this subregion and add a new > subregion, without breaking existing userspace. We can't do that with > ioctls. OK, it is a good reason, took me a while but I get the interest of capabilities for regions too. > > And moreover, this is only the first user of this infrastructure. > There's also that path handling series that Dong Jia had posted early > this year -- that would be an obvious user as well. > right. Thanks. Regards, Pierre
On 11/22/2018 11:54 AM, Cornelia Huck wrote: > diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h > index 078e46f9623d..a6f9f84526e2 100644 > --- a/drivers/s390/cio/vfio_ccw_private.h > +++ b/drivers/s390/cio/vfio_ccw_private.h > @@ -3,9 +3,11 @@ > * Private stuff for vfio_ccw driver > * > * Copyright IBM Corp. 2017 > + * Copyright Red Hat, Inc. 2018 > * > * Author(s): Dong Jia Shi<bjsdjshi@linux.vnet.ibm.com> > * Xiao Feng Ren<renxiaof@linux.vnet.ibm.com> > + * Cornelia Huck<cohuck@redhat.com> > */ > > #ifndef_VFIO_CCW_PRIVATE_H_ > @@ -19,6 +21,38 @@ > #include "css.h" > #include "vfio_ccw_cp.h" > > +#define VFIO_CCW_OFFSET_SHIFT 40 > +#define VFIO_CCW_OFFSET_TO_INDEX(off) (off >> VFIO_CCW_OFFSET_SHIFT) > +#define VFIO_CCW_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_CCW_OFFSET_SHIFT) > +#define VFIO_CCW_OFFSET_MASK (((u64)(1) << VFIO_CCW_OFFSET_SHIFT) - 1) > + Why is the offset shift 40? I know vfio-pci is also using the same offset shift, but I am curious about the reasoning behind why we are using this? :)
On Tue, 27 Nov 2018 14:04:49 -0500 Farhan Ali <alifm@linux.ibm.com> wrote: > On 11/22/2018 11:54 AM, Cornelia Huck wrote: > > diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h > > index 078e46f9623d..a6f9f84526e2 100644 > > --- a/drivers/s390/cio/vfio_ccw_private.h > > +++ b/drivers/s390/cio/vfio_ccw_private.h > > @@ -3,9 +3,11 @@ > > * Private stuff for vfio_ccw driver > > * > > * Copyright IBM Corp. 2017 > > + * Copyright Red Hat, Inc. 2018 > > * > > * Author(s): Dong Jia Shi<bjsdjshi@linux.vnet.ibm.com> > > * Xiao Feng Ren<renxiaof@linux.vnet.ibm.com> > > + * Cornelia Huck<cohuck@redhat.com> > > */ > > > > #ifndef_VFIO_CCW_PRIVATE_H_ > > @@ -19,6 +21,38 @@ > > #include "css.h" > > #include "vfio_ccw_cp.h" > > > > +#define VFIO_CCW_OFFSET_SHIFT 40 > > +#define VFIO_CCW_OFFSET_TO_INDEX(off) (off >> VFIO_CCW_OFFSET_SHIFT) > > +#define VFIO_CCW_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_CCW_OFFSET_SHIFT) > > +#define VFIO_CCW_OFFSET_MASK (((u64)(1) << VFIO_CCW_OFFSET_SHIFT) - 1) > > + > > Why is the offset shift 40? I know vfio-pci is also using the same > offset shift, but I am curious about the reasoning behind why we are > using this? :) > My entire reasoning was "hey, vfio-pci is using this, so it should not be bad" 8)
On 11/22/2018 11:54 AM, Cornelia Huck wrote: ...snip... > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 813102810f53..565669f95534 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { > > #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) > #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) > +#define VFIO_REGION_TYPE_CCW (1 << 30) Oof. So the existing VFIO_REGION_TYPE_PCI_VENDOR_TYPE gets OR'd with another value (e.g., 8086). But in 4.20, there was a VFIO_REGION_TYPE_GFX is added as simply "1" ... Which direction are these definitions being added from? I guess asked another way, is _TYPE_CCW going to be OR'd with anything else that necessitates its presence as an identifier with some Other Thing, or should this follow the TYPE_GFX enumeration? Perhaps the type field needs to be tidied up to help this sit more cleanly now? (Sorry!) - Eric > > /* 8086 Vendor sub-types */ > #define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1) >
On Mon, 17 Dec 2018 16:53:34 -0500 Eric Farman <farman@linux.ibm.com> wrote: > On 11/22/2018 11:54 AM, Cornelia Huck wrote: > > ...snip... > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > index 813102810f53..565669f95534 100644 > > --- a/include/uapi/linux/vfio.h > > +++ b/include/uapi/linux/vfio.h > > @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { > > > > #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) > > #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) > > +#define VFIO_REGION_TYPE_CCW (1 << 30) > > Oof. So the existing VFIO_REGION_TYPE_PCI_VENDOR_TYPE gets OR'd with > another value (e.g., 8086). But in 4.20, there was a > VFIO_REGION_TYPE_GFX is added as simply "1" ... Which direction are > these definitions being added from? I guess asked another way, is > _TYPE_CCW going to be OR'd with anything else that necessitates its > presence as an identifier with some Other Thing, or should this follow > the TYPE_GFX enumeration? Perhaps the type field needs to be tidied up > to help this sit more cleanly now? (Sorry!) The semantics of that type stuff are really a bit unclear to me :( I don't think we'll ever do any fancy mask handling for ccw. It is probably enough to have any kind of uniqueness within the different types, so maybe counting up would be indeed enough... > > - Eric > > > > > /* 8086 Vendor sub-types */ > > #define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1) > > >
On 12/18/2018 12:24 PM, Cornelia Huck wrote: > On Mon, 17 Dec 2018 16:53:34 -0500 > Eric Farman <farman@linux.ibm.com> wrote: > >> On 11/22/2018 11:54 AM, Cornelia Huck wrote: >> >> ...snip... >> >>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h >>> index 813102810f53..565669f95534 100644 >>> --- a/include/uapi/linux/vfio.h >>> +++ b/include/uapi/linux/vfio.h >>> @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { >>> >>> #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) >>> #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) >>> +#define VFIO_REGION_TYPE_CCW (1 << 30) >> >> Oof. So the existing VFIO_REGION_TYPE_PCI_VENDOR_TYPE gets OR'd with >> another value (e.g., 8086). But in 4.20, there was a >> VFIO_REGION_TYPE_GFX is added as simply "1" ... Which direction are >> these definitions being added from? I guess asked another way, is >> _TYPE_CCW going to be OR'd with anything else that necessitates its >> presence as an identifier with some Other Thing, or should this follow >> the TYPE_GFX enumeration? Perhaps the type field needs to be tidied up >> to help this sit more cleanly now? (Sorry!) > > The semantics of that type stuff are really a bit unclear to me :( +1 I was confused when I first looked at this. When I applied it to 4.20, I got another level of confusion. ;) > > I don't think we'll ever do any fancy mask handling for ccw. It is > probably enough to have any kind of uniqueness within the different > types, so maybe counting up would be indeed enough... Considering the subtype space, I think it would be fine too. But wanted to ask in case I've been out of the loop on something. > >> >> - Eric >> >>> >>> /* 8086 Vendor sub-types */ >>> #define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1) >>> >> >
On Tue, 18 Dec 2018 18:24:00 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > On Mon, 17 Dec 2018 16:53:34 -0500 > Eric Farman <farman@linux.ibm.com> wrote: > > > On 11/22/2018 11:54 AM, Cornelia Huck wrote: > > > > ...snip... > > > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > > index 813102810f53..565669f95534 100644 > > > --- a/include/uapi/linux/vfio.h > > > +++ b/include/uapi/linux/vfio.h > > > @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { > > > > > > #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) > > > #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) > > > +#define VFIO_REGION_TYPE_CCW (1 << 30) > > > > Oof. So the existing VFIO_REGION_TYPE_PCI_VENDOR_TYPE gets OR'd with > > another value (e.g., 8086). But in 4.20, there was a > > VFIO_REGION_TYPE_GFX is added as simply "1" ... Which direction are > > these definitions being added from? I guess asked another way, is > > _TYPE_CCW going to be OR'd with anything else that necessitates its > > presence as an identifier with some Other Thing, or should this follow > > the TYPE_GFX enumeration? Perhaps the type field needs to be tidied up > > to help this sit more cleanly now? (Sorry!) > > The semantics of that type stuff are really a bit unclear to me :( > > I don't think we'll ever do any fancy mask handling for ccw. It is > probably enough to have any kind of uniqueness within the different > types, so maybe counting up would be indeed enough... Just to confirm, this is the intended usage, simply reserve a new type following the GFX region example. We can define VFIO_REGION_TYPE_CCW as 2 and then there's a whole address space of sub-types to fill in within that. I might have over-engineered PCI a bit with the address space split, but it seemed like a good idea at the time to pre-define a type address space for each vendor, such that they only need to define the sub-types and we can avoid namespace collisions. Unfortunately this implicit definition for each PCI vendor also contributes to the confusion here. Sorry. Thanks, Alex
On Wed, 19 Dec 2018 09:28:00 -0700 Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 18 Dec 2018 18:24:00 +0100 > Cornelia Huck <cohuck@redhat.com> wrote: > > > On Mon, 17 Dec 2018 16:53:34 -0500 > > Eric Farman <farman@linux.ibm.com> wrote: > > > > > On 11/22/2018 11:54 AM, Cornelia Huck wrote: > > > > > > ...snip... > > > > > > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > > > > index 813102810f53..565669f95534 100644 > > > > --- a/include/uapi/linux/vfio.h > > > > +++ b/include/uapi/linux/vfio.h > > > > @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { > > > > > > > > #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) > > > > #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) > > > > +#define VFIO_REGION_TYPE_CCW (1 << 30) > > > > > > Oof. So the existing VFIO_REGION_TYPE_PCI_VENDOR_TYPE gets OR'd with > > > another value (e.g., 8086). But in 4.20, there was a > > > VFIO_REGION_TYPE_GFX is added as simply "1" ... Which direction are > > > these definitions being added from? I guess asked another way, is > > > _TYPE_CCW going to be OR'd with anything else that necessitates its > > > presence as an identifier with some Other Thing, or should this follow > > > the TYPE_GFX enumeration? Perhaps the type field needs to be tidied up > > > to help this sit more cleanly now? (Sorry!) > > > > The semantics of that type stuff are really a bit unclear to me :( > > > > I don't think we'll ever do any fancy mask handling for ccw. It is > > probably enough to have any kind of uniqueness within the different > > types, so maybe counting up would be indeed enough... > > Just to confirm, this is the intended usage, simply reserve a new type > following the GFX region example. We can define VFIO_REGION_TYPE_CCW > as 2 and then there's a whole address space of sub-types to fill in > within that. I might have over-engineered PCI a bit with the address > space split, but it seemed like a good idea at the time to pre-define a > type address space for each vendor, such that they only need to define > the sub-types and we can avoid namespace collisions. Unfortunately > this implicit definition for each PCI vendor also contributes to the > confusion here. Sorry. Thanks, > > Alex Thanks for the explanation. I'm simply switching VFIO_REGION_TYPE_CCW to 2 in the next version.
diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index f673e106c041..a5d731ed2a39 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -3,9 +3,11 @@ * Physical device callbacks for vfio_ccw * * Copyright IBM Corp. 2017 + * Copyright Red Hat, Inc. 2018 * * Author(s): Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> * Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> + * Cornelia Huck <cohuck@redhat.com> */ #include <linux/vfio.h> @@ -157,47 +159,76 @@ static void vfio_ccw_mdev_release(struct mdev_device *mdev) { struct vfio_ccw_private *private = dev_get_drvdata(mdev_parent_dev(mdev)); + int i; vfio_unregister_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY, &private->nb); + + for (i = 0; i < private->num_regions; i++) + private->region[i].ops->release(private, &private->region[i]); + + private->num_regions = 0; + kfree(private->region); + private->region = NULL; } -static ssize_t vfio_ccw_mdev_read(struct mdev_device *mdev, - char __user *buf, - size_t count, - loff_t *ppos) +static ssize_t vfio_ccw_mdev_read_io_region(struct vfio_ccw_private *private, + char __user *buf, size_t count, + loff_t *ppos) { - struct vfio_ccw_private *private; + loff_t pos = *ppos & VFIO_CCW_OFFSET_MASK; struct ccw_io_region *region; - if (*ppos + count > sizeof(*region)) + if (pos + count > sizeof(*region)) return -EINVAL; - private = dev_get_drvdata(mdev_parent_dev(mdev)); region = private->io_region; - if (copy_to_user(buf, (void *)region + *ppos, count)) + if (copy_to_user(buf, (void *)region + pos, count)) return -EFAULT; return count; } -static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, - const char __user *buf, - size_t count, - loff_t *ppos) +static ssize_t vfio_ccw_mdev_read(struct mdev_device *mdev, + char __user *buf, + size_t count, + loff_t *ppos) { + unsigned int index = VFIO_CCW_OFFSET_TO_INDEX(*ppos); struct vfio_ccw_private *private; + + private = dev_get_drvdata(mdev_parent_dev(mdev)); + + if (index >= VFIO_CCW_NUM_REGIONS + private->num_regions) + return -EINVAL; + + switch (index) { + case VFIO_CCW_CONFIG_REGION_INDEX: + return vfio_ccw_mdev_read_io_region(private, buf, count, ppos); + default: + index -= VFIO_CCW_NUM_REGIONS; + return private->region[index].ops->read(private, buf, count, + ppos); + } + + return -EINVAL; +} + +static ssize_t vfio_ccw_mdev_write_io_region(struct vfio_ccw_private *private, + const char __user *buf, + size_t count, loff_t *ppos) +{ + loff_t pos = *ppos & VFIO_CCW_OFFSET_MASK; struct ccw_io_region *region; - if (*ppos + count > sizeof(*region)) + if (pos + count > sizeof(*region)) return -EINVAL; - private = dev_get_drvdata(mdev_parent_dev(mdev)); if (private->state != VFIO_CCW_STATE_IDLE) return -EACCES; region = private->io_region; - if (copy_from_user((void *)region + *ppos, buf, count)) + if (copy_from_user((void *)region + pos, buf, count)) return -EFAULT; vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_IO_REQ); @@ -207,21 +238,55 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, } return count; + } -static int vfio_ccw_mdev_get_device_info(struct vfio_device_info *info) +static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, + const char __user *buf, + size_t count, + loff_t *ppos) { + unsigned int index = VFIO_CCW_OFFSET_TO_INDEX(*ppos); + struct vfio_ccw_private *private; + + private = dev_get_drvdata(mdev_parent_dev(mdev)); + + if (index >= VFIO_CCW_NUM_REGIONS + private->num_regions) + return -EINVAL; + + switch (index) { + case VFIO_CCW_CONFIG_REGION_INDEX: + return vfio_ccw_mdev_write_io_region(private, buf, count, ppos); + default: + index -= VFIO_CCW_NUM_REGIONS; + return private->region[index].ops->write(private, buf, count, + ppos); + } + + return -EINVAL; +} + +static int vfio_ccw_mdev_get_device_info(struct vfio_device_info *info, + struct mdev_device *mdev) +{ + struct vfio_ccw_private *private; + + private = dev_get_drvdata(mdev_parent_dev(mdev)); info->flags = VFIO_DEVICE_FLAGS_CCW | VFIO_DEVICE_FLAGS_RESET; - info->num_regions = VFIO_CCW_NUM_REGIONS; + info->num_regions = VFIO_CCW_NUM_REGIONS + private->num_regions; info->num_irqs = VFIO_CCW_NUM_IRQS; return 0; } static int vfio_ccw_mdev_get_region_info(struct vfio_region_info *info, - u16 *cap_type_id, - void **cap_type) + struct mdev_device *mdev, + unsigned long arg) { + struct vfio_ccw_private *private; + int i; + + private = dev_get_drvdata(mdev_parent_dev(mdev)); switch (info->index) { case VFIO_CCW_CONFIG_REGION_INDEX: info->offset = 0; @@ -229,9 +294,51 @@ static int vfio_ccw_mdev_get_region_info(struct vfio_region_info *info, info->flags = VFIO_REGION_INFO_FLAG_READ | VFIO_REGION_INFO_FLAG_WRITE; return 0; - default: - return -EINVAL; + default: /* all other regions are handled via capability chain */ + { + struct vfio_info_cap caps = { .buf = NULL, .size = 0 }; + struct vfio_region_info_cap_type cap_type = { + .header.id = VFIO_REGION_INFO_CAP_TYPE, + .header.version = 1 }; + int ret; + + if (info->index >= + VFIO_CCW_NUM_REGIONS + private->num_regions) + return -EINVAL; + + i = info->index - VFIO_CCW_NUM_REGIONS; + + info->offset = VFIO_CCW_INDEX_TO_OFFSET(info->index); + info->size = private->region[i].size; + info->flags = private->region[i].flags; + + cap_type.type = private->region[i].type; + cap_type.subtype = private->region[i].subtype; + + ret = vfio_info_add_capability(&caps, &cap_type.header, + sizeof(cap_type)); + if (ret) + return ret; + + info->flags |= VFIO_REGION_INFO_FLAG_CAPS; + if (info->argsz < sizeof(*info) + caps.size) { + info->argsz = sizeof(*info) + caps.size; + info->cap_offset = 0; + } else { + vfio_info_cap_shift(&caps, sizeof(*info)); + if (copy_to_user((void __user *)arg + sizeof(*info), + caps.buf, caps.size)) { + kfree(caps.buf); + return -EFAULT; + } + info->cap_offset = sizeof(*info); + } + + kfree(caps.buf); + + } } + return 0; } static int vfio_ccw_mdev_get_irq_info(struct vfio_irq_info *info) @@ -308,6 +415,32 @@ static int vfio_ccw_mdev_set_irqs(struct mdev_device *mdev, } } +int vfio_ccw_register_dev_region(struct vfio_ccw_private *private, + unsigned int subtype, + const struct vfio_ccw_regops *ops, + size_t size, u32 flags, void *data) +{ + struct vfio_ccw_region *region; + + region = krealloc(private->region, + (private->num_regions + 1) * sizeof(*region), + GFP_KERNEL); + if (!region) + return -ENOMEM; + + private->region = region; + private->region[private->num_regions].type = VFIO_REGION_TYPE_CCW; + private->region[private->num_regions].subtype = subtype; + private->region[private->num_regions].ops = ops; + private->region[private->num_regions].size = size; + private->region[private->num_regions].flags = flags; + private->region[private->num_regions].data = data; + + private->num_regions++; + + return 0; +} + static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device *mdev, unsigned int cmd, unsigned long arg) @@ -328,7 +461,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device *mdev, if (info.argsz < minsz) return -EINVAL; - ret = vfio_ccw_mdev_get_device_info(&info); + ret = vfio_ccw_mdev_get_device_info(&info, mdev); if (ret) return ret; @@ -337,8 +470,6 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device *mdev, case VFIO_DEVICE_GET_REGION_INFO: { struct vfio_region_info info; - u16 cap_type_id = 0; - void *cap_type = NULL; minsz = offsetofend(struct vfio_region_info, offset); @@ -348,8 +479,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct mdev_device *mdev, if (info.argsz < minsz) return -EINVAL; - ret = vfio_ccw_mdev_get_region_info(&info, &cap_type_id, - &cap_type); + ret = vfio_ccw_mdev_get_region_info(&info, mdev, arg); if (ret) return ret; diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 078e46f9623d..a6f9f84526e2 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -3,9 +3,11 @@ * Private stuff for vfio_ccw driver * * Copyright IBM Corp. 2017 + * Copyright Red Hat, Inc. 2018 * * Author(s): Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> * Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> + * Cornelia Huck <cohuck@redhat.com> */ #ifndef _VFIO_CCW_PRIVATE_H_ @@ -19,6 +21,38 @@ #include "css.h" #include "vfio_ccw_cp.h" +#define VFIO_CCW_OFFSET_SHIFT 40 +#define VFIO_CCW_OFFSET_TO_INDEX(off) (off >> VFIO_CCW_OFFSET_SHIFT) +#define VFIO_CCW_INDEX_TO_OFFSET(index) ((u64)(index) << VFIO_CCW_OFFSET_SHIFT) +#define VFIO_CCW_OFFSET_MASK (((u64)(1) << VFIO_CCW_OFFSET_SHIFT) - 1) + +/* capability chain handling similar to vfio-pci */ +struct vfio_ccw_private; +struct vfio_ccw_region; + +struct vfio_ccw_regops { + size_t (*read)(struct vfio_ccw_private *private, char __user *buf, + size_t count, loff_t *ppos); + size_t (*write)(struct vfio_ccw_private *private, + const char __user *buf, size_t count, loff_t *ppos); + void (*release)(struct vfio_ccw_private *private, + struct vfio_ccw_region *region); +}; + +struct vfio_ccw_region { + u32 type; + u32 subtype; + const struct vfio_ccw_regops *ops; + void *data; + size_t size; + u32 flags; +}; + +int vfio_ccw_register_dev_region(struct vfio_ccw_private *private, + unsigned int subtype, + const struct vfio_ccw_regops *ops, + size_t size, u32 flags, void *data); + /** * struct vfio_ccw_private * @sch: pointer to the subchannel @@ -28,6 +62,8 @@ * @mdev: pointer to the mediated device * @nb: notifier for vfio events * @io_region: MMIO region to input/output I/O arguments/results + * @region: additional regions for other subchannel operations + * @num_regions: number of additional regions * @cp: channel program for the current I/O operation * @irb: irb info received from interrupt * @scsw: scsw info @@ -42,6 +78,8 @@ struct vfio_ccw_private { struct mdev_device *mdev; struct notifier_block nb; struct ccw_io_region *io_region; + struct vfio_ccw_region *region; + int num_regions; struct channel_program cp; struct irb irb; diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 813102810f53..565669f95534 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -297,6 +297,7 @@ struct vfio_region_info_cap_type { #define VFIO_REGION_TYPE_PCI_VENDOR_TYPE (1 << 31) #define VFIO_REGION_TYPE_PCI_VENDOR_MASK (0xffff) +#define VFIO_REGION_TYPE_CCW (1 << 30) /* 8086 Vendor sub-types */ #define VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION (1)
Allow to extend the regions used by vfio-ccw. The first user will be handling of halt and clear subchannel. Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- drivers/s390/cio/vfio_ccw_ops.c | 182 ++++++++++++++++++++++++---- drivers/s390/cio/vfio_ccw_private.h | 38 ++++++ include/uapi/linux/vfio.h | 1 + 3 files changed, 195 insertions(+), 26 deletions(-)