On Wed, 8 Nov 2017 12:04:22 +0100 Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote: > On 11/08/2017 10:19 AM, Cornelia Huck wrote: > > On Wed, 8 Nov 2017 09:41:43 +0100 > > Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote: > > > >> From: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx> > >> > >> There is a chance to delete not yet delivered I/O interrupts if an > >> exploiter uses the subsystem identification word 0x0000 while > >> processing a KVM_DEV_FLIC_CLEAR_IO_IRQ ioctl. -EINVAL will be returned > >> now instead in that case. > >> > >> Classic interrupts will always have bit 0x10000 set in the schid while > >> adapter interrupts have a zero schid. The clear_io_irq interface is > >> only useful for classic interrupts (as adapter interrupts belong to > >> many devices). Let's make this interface more strict and forbid a schid > >> of 0. > > > > I'm wondering: Is there any possible use case to clear adapter > > interrupts? This interface was introduced to handle the case where a > > CRW was made pending for a subchannel (which implies any pending > > interrupt needs to be cleared.) > > The problem with clearing adapter interrupts is that is actually a summary > interrupt for every potential device. So I somewhat consider an adapter interrupt > pending when the summary indicator went from 0 to 1. So I dont think clearing > a single one makes not much sense. (And this interface would be wrong for > that I think) Yes, this interface would be problematic. I'm not sure what's supposed to happen with crws vs. pending adapter interrupts, though. > The only use cases I can imagine for clearing adapter interrupts (e.g. reset) > is already covered by KVM_DEV_FLIC_CLEAR_IRQS > > > > > Alas, I cannot check the adapter interrupt question myself, as the > > public doc is lacking :( But qdio as another adapter interrupt user > > comes to mind (not that we support it in qemu, but still...) > > > >> > >> Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx> > >> Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxxxxxxx> > >> Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxx> > >> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx> > >> --- > >> Documentation/virtual/kvm/devices/s390_flic.txt | 3 +++ > >> arch/s390/kvm/interrupt.c | 2 ++ > >> 2 files changed, 5 insertions(+) > >> > >> diff --git a/Documentation/virtual/kvm/devices/s390_flic.txt b/Documentation/virtual/kvm/devices/s390_flic.txt > >> index 2f1cbf1..27ad53c 100644 > >> --- a/Documentation/virtual/kvm/devices/s390_flic.txt > >> +++ b/Documentation/virtual/kvm/devices/s390_flic.txt > >> @@ -156,3 +156,6 @@ FLIC with an unknown group or attribute gives the error code EINVAL (instead of > >> ENXIO, as specified in the API documentation). It is not possible to conclude > >> that a FLIC operation is unavailable based on the error code resulting from a > >> usage attempt. > >> + > >> +Note: The KVM_DEV_FLIC_CLEAR_IO_IRQ ioctl will return EINVAL in case a zero > >> +schid is specified. > >> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c > >> index a3da4f3..c8aacce 100644 > >> --- a/arch/s390/kvm/interrupt.c > >> +++ b/arch/s390/kvm/interrupt.c > >> @@ -2191,6 +2191,8 @@ static int clear_io_irq(struct kvm *kvm, struct kvm_device_attr *attr) > >> return -EINVAL; > >> if (copy_from_user(&schid, (void __user *) attr->addr, sizeof(schid))) > >> return -EFAULT; > >> + if (!schid) > >> + return -EINVAL; > >> kfree(kvm_s390_get_io_int(kvm, isc_mask, schid)); > >> /* > >> * If userspace is conforming to the architecture, we can have at most > > > As this particular interface would not be a good match for whatever we need to do for adapter interrupts, let's go with this patch. Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
