| Message ID | 20231018133829.147226-3-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | a couple of corrections to the IRQ enablement function | expand |
On Wed, 18 Oct 2023 09:38:24 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > From: Anthony Krowiak <akrowiak@linux.ibm.com> > > The interception handler for the PQAP(AQIC) command calls the > kvm_s390_gisc_register function to register the guest ISC with the channel > subsystem. If that call fails, the status response code 08 - indicating > Invalid ZONE/GISA designation - is returned to the guest. This response > code does not make sense because the non-zero return code from the > kvm_s390_gisc_register function can be due one of two things: Either the > ISC passed as a parameter by the guest to the PQAP(AQIC) command is greater > than the maximum ISC value allowed, or the guest is not using a GISA. The "ISC passed as a parameter by the guest to the PQAP(AQIC) command is greater than the maximum ISC value allowed" is not possible. The isc is 3 bits wide and all 8 values that can be represented on 3 bits are valid. This is only possible if the hypervisor was to mess up, or if the machine was broken. > > Since this scenario is very unlikely to happen and there is no status > response code to indicate an invalid ISC value, let's set the > response code to 06 indicating 'Invalid address of AP-queue notification > byte'. While this is not entirely accurate, it is better than indicating > that the ZONE/GISA designation is invalid which is something the guest > can do nothing about since those values are set by the hypervisor. > > Signed-off-by: Anthony Krowiak <akrowiak@linux.ibm.com> > Suggested-by: Halil Pasic <pasic@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index 9cb28978c186..25d7ce2094f8 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -393,8 +393,8 @@ static int ensure_nib_shared(unsigned long addr, struct gmap *gmap) > * Register the guest ISC to GIB interface and retrieve the > * host ISC to issue the host side PQAP/AQIC > * > - * Response.status may be set to AP_RESPONSE_INVALID_ADDRESS in case the > - * vfio_pin_pages failed. > + * status.response_code may be set to AP_RESPONSE_INVALID_ADDRESS in case the > + * vfio_pin_pages or kvm_s390_gisc_register failed. > * > * Otherwise return the ap_queue_status returned by the ap_aqic(), > * all retry handling will be done by the guest. > @@ -458,7 +458,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q, > __func__, nisc, isc, q->apqn); > > vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1); > - status.response_code = AP_RESPONSE_INVALID_GISA; > + status.response_code = AP_RESPONSE_INVALID_ADDRESS; > return status; > } >
On 10/27/23 10:03, Halil Pasic wrote: > On Fri, 27 Oct 2023 09:36:26 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >>>> The interception handler for the PQAP(AQIC) command calls the >>>> kvm_s390_gisc_register function to register the guest ISC with the channel >>>> subsystem. If that call fails, the status response code 08 - indicating >>>> Invalid ZONE/GISA designation - is returned to the guest. This response >>>> code does not make sense because the non-zero return code from the >>>> kvm_s390_gisc_register function can be due one of two things: Either the >>>> ISC passed as a parameter by the guest to the PQAP(AQIC) command is greater >>>> than the maximum ISC value allowed, or the guest is not using a GISA. >>> >>> The "ISC passed as a parameter by the guest to the PQAP(AQIC) command is >>> greater than the maximum ISC value allowed" is not possible. The isc is >>> 3 bits wide and all 8 values that can be represented on 3 bits are valid. >>> >>> This is only possible if the hypervisor was to mess up, or if the machine >>> was broken. >> >> kvm_s390_gisc_register(struct kvm *kvm, u32 gisc) >> { >> struct kvm_s390_gisa_interrupt *gi = &kvm->arch.gisa_int; >> >> if (!gi->origin) >> return -ENODEV; >> if (gisc > MAX_ISC) >> return -ERANGE; >> ... >> >> Just quoting what is in the code. > > Right! But it is not the guest that calls this function directly. This > function is called by the vfio_ap code. > > The guest passes ISC in bits 61, 62 and 63 of GR1. > > So the guest can't give you an invalid value. Yes, I got it the first time you said that. > > Regards, > Halil
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 9cb28978c186..25d7ce2094f8 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -393,8 +393,8 @@ static int ensure_nib_shared(unsigned long addr, struct gmap *gmap) * Register the guest ISC to GIB interface and retrieve the * host ISC to issue the host side PQAP/AQIC * - * Response.status may be set to AP_RESPONSE_INVALID_ADDRESS in case the - * vfio_pin_pages failed. + * status.response_code may be set to AP_RESPONSE_INVALID_ADDRESS in case the + * vfio_pin_pages or kvm_s390_gisc_register failed. * * Otherwise return the ap_queue_status returned by the ap_aqic(), * all retry handling will be done by the guest. @@ -458,7 +458,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q, __func__, nisc, isc, q->apqn); vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1); - status.response_code = AP_RESPONSE_INVALID_GISA; + status.response_code = AP_RESPONSE_INVALID_ADDRESS; return status; }