This series corrects two issues related to enablement of interrupts in
response to interception of the PQAP(AQIC) command:
1. Returning a status response code 06 (Invalid address of AP-queue
notification byte) when the call to register a guest ISC fails makes no
sense.
2. The pages containing the interrupt notification-indicator byte are not
freed after a failure to register the guest ISC fails.
Anthony Krowiak (2):
s390/vfio-ap: unpin pages on gisc registration failure
s390/vfio-ap: set status response code to 06 on gisc registration
failure
Tony Krowiak (1):
s390/vfio-ap: improve reaction to response code 07 from PQAP(AQIC)
command
drivers/s390/crypto/vfio_ap_ops.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--
2.41.0
Let's improve the vfio_ap driver's reaction to reception of response code
07 from the PQAP(AQIC) command when enabling interrupts on behalf of a
guest:
* Unregister the guest's ISC before the pages containing the notification
indicator bytes are unpinned.
* Capture the return code from the kvm_s390_gisc_unregister function and
log a DBF warning if it fails.
Suggested-by: Matthew Rosato <[email protected]>
Signed-off-by: Tony Krowiak <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 25d7ce2094f8..4e80c211ba47 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -476,8 +476,11 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
break;
case AP_RESPONSE_OTHERWISE_CHANGED:
/* We could not modify IRQ settings: clear new configuration */
+ ret = kvm_s390_gisc_unregister(kvm, isc);
+ if (ret)
+ VFIO_AP_DBF_WARN("%s: kvm_s390_gisc_unregister: rc=%d isc=%d, apqn=%#04x\n",
+ __func__, ret, isc, q->apqn);
vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
- kvm_s390_gisc_unregister(kvm, isc);
break;
default:
pr_warn("%s: apqn %04x: response: %02x\n", __func__, q->apqn,
--
2.41.0
From: Anthony Krowiak <[email protected]>
In the vfio_ap_irq_enable function, after the page containing the
notification indicator byte (NIB) is pinned, the function attempts
to register the guest ISC. If registration fails, the function sets the
status response code and returns without unpinning the page containing
the NIB. In order to avoid a memory leak, the NIB should be unpinned before
returning from the vfio_ap_irq_enable function.
Signed-off-by: Janosch Frank <[email protected]>
Signed-off-by: Anthony Krowiak <[email protected]>
Reviewed-by: Matthew Rosato <[email protected]>
Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
Cc: <[email protected]>
---
drivers/s390/crypto/vfio_ap_ops.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index 4db538a55192..9cb28978c186 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
__func__, nisc, isc, q->apqn);
+ vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
status.response_code = AP_RESPONSE_INVALID_GISA;
return status;
}
--
2.41.0
From: Anthony Krowiak <[email protected]>
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.
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 <[email protected]>
Suggested-by: Halil Pasic <[email protected]>
---
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;
}
--
2.41.0
Am 18.10.23 um 15:38 schrieb Tony Krowiak:
> From: Anthony Krowiak <[email protected]>
>
> In the vfio_ap_irq_enable function, after the page containing the
> notification indicator byte (NIB) is pinned, the function attempts
> to register the guest ISC. If registration fails, the function sets the
> status response code and returns without unpinning the page containing
> the NIB. In order to avoid a memory leak, the NIB should be unpinned before
> returning from the vfio_ap_irq_enable function.
>
> Signed-off-by: Janosch Frank <[email protected]>
Where is Janoschs signed off coming from here?
> Signed-off-by: Anthony Krowiak <[email protected]>
> Reviewed-by: Matthew Rosato <[email protected]>
> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
> Cc: <[email protected]>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 4db538a55192..9cb28978c186 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
> VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
> __func__, nisc, isc, q->apqn);
>
> + vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
> status.response_code = AP_RESPONSE_INVALID_GISA;
> return status;
> }
On 10/26/23 08:18, Christian Borntraeger wrote:
>
>
> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>> From: Anthony Krowiak <[email protected]>
>>
>> In the vfio_ap_irq_enable function, after the page containing the
>> notification indicator byte (NIB) is pinned, the function attempts
>> to register the guest ISC. If registration fails, the function sets the
>> status response code and returns without unpinning the page containing
>> the NIB. In order to avoid a memory leak, the NIB should be unpinned
>> before
>> returning from the vfio_ap_irq_enable function.
>>
>> Signed-off-by: Janosch Frank <[email protected]>
>
> Where is Janoschs signed off coming from here?
Janosch found this and composed the patch originally. I just tweaked the
description and posted it.
>
>> Signed-off-by: Anthony Krowiak <[email protected]>
>> Reviewed-by: Matthew Rosato <[email protected]>
>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the
>> vfio_ap_irq_enable function")
>> Cc: <[email protected]>
>> ---
>> drivers/s390/crypto/vfio_ap_ops.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 4db538a55192..9cb28978c186 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -457,6 +457,7 @@ static struct ap_queue_status
>> vfio_ap_irq_enable(struct vfio_ap_queue *q,
>> VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d,
>> isc=%d, apqn=%#04x\n",
>> __func__, nisc, isc, q->apqn);
>> + vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>> status.response_code = AP_RESPONSE_INVALID_GISA;
>> return status;
>> }
Am 26.10.23 um 15:16 schrieb Tony Krowiak:
>
>
> On 10/26/23 08:18, Christian Borntraeger wrote:
>>
>>
>> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>>> From: Anthony Krowiak <[email protected]>
>>>
>>> In the vfio_ap_irq_enable function, after the page containing the
>>> notification indicator byte (NIB) is pinned, the function attempts
>>> to register the guest ISC. If registration fails, the function sets the
>>> status response code and returns without unpinning the page containing
>>> the NIB. In order to avoid a memory leak, the NIB should be unpinned before
>>> returning from the vfio_ap_irq_enable function.
>>>
>>> Signed-off-by: Janosch Frank <[email protected]>
>>
>> Where is Janoschs signed off coming from here?
>
> Janosch found this and composed the patch originally. I just tweaked the description and posted it.
So we should add
Co-developed-by: Janosch Frank <[email protected]>
in front of Janoschs signoff.
>
>>
>>> Signed-off-by: Anthony Krowiak <[email protected]>
>>> Reviewed-by: Matthew Rosato <[email protected]>
>>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the vfio_ap_irq_enable function")
>>> Cc: <[email protected]>
>>> ---
>>> drivers/s390/crypto/vfio_ap_ops.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>>> index 4db538a55192..9cb28978c186 100644
>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>>> @@ -457,6 +457,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q,
>>> VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d, isc=%d, apqn=%#04x\n",
>>> __func__, nisc, isc, q->apqn);
>>> + vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>>> status.response_code = AP_RESPONSE_INVALID_GISA;
>>> return status;
>>> }
On 10/18/23 9:38 AM, Tony Krowiak wrote:
> Let's improve the vfio_ap driver's reaction to reception of response code
> 07 from the PQAP(AQIC) command when enabling interrupts on behalf of a
> guest:
>
> * Unregister the guest's ISC before the pages containing the notification
> indicator bytes are unpinned.
>
> * Capture the return code from the kvm_s390_gisc_unregister function and
> log a DBF warning if it fails.
>
> Suggested-by: Matthew Rosato <[email protected]>
> Signed-off-by: Tony Krowiak <[email protected]>
Reviewed-by: Matthew Rosato <[email protected]>
I went back-and-forth on whether this should be a stable/fixes candidate but I think no... I happened to notice it while reviewing other code, I'm not aware that it's ever created a visible issue, and it's on a pretty immediate error path. If anyone thinks it should be a stable candidate I have no objection but in that case would suggest to break the patch up to separate the new WARN from the fix.
On 10/26/23 09:25, Christian Borntraeger wrote:
>
>
> Am 26.10.23 um 15:16 schrieb Tony Krowiak:
>>
>>
>> On 10/26/23 08:18, Christian Borntraeger wrote:
>>>
>>>
>>> Am 18.10.23 um 15:38 schrieb Tony Krowiak:
>>>> From: Anthony Krowiak <[email protected]>
>>>>
>>>> In the vfio_ap_irq_enable function, after the page containing the
>>>> notification indicator byte (NIB) is pinned, the function attempts
>>>> to register the guest ISC. If registration fails, the function sets the
>>>> status response code and returns without unpinning the page containing
>>>> the NIB. In order to avoid a memory leak, the NIB should be unpinned
>>>> before
>>>> returning from the vfio_ap_irq_enable function.
>>>>
>>>> Signed-off-by: Janosch Frank <[email protected]>
>>>
>>> Where is Janoschs signed off coming from here?
>>
>> Janosch found this and composed the patch originally. I just tweaked
>> the description and posted it.
>
> So we should add
>
> Co-developed-by: Janosch Frank <[email protected]>
>
> in front of Janoschs signoff.
Will do.
>
>>
>>>
>>>> Signed-off-by: Anthony Krowiak <[email protected]>
>>>> Reviewed-by: Matthew Rosato <[email protected]>
>>>> Fixes: 783f0a3ccd79 ("s390/vfio-ap: add s390dbf logging to the
>>>> vfio_ap_irq_enable function")
>>>> Cc: <[email protected]>
>>>> ---
>>>> drivers/s390/crypto/vfio_ap_ops.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>>>> b/drivers/s390/crypto/vfio_ap_ops.c
>>>> index 4db538a55192..9cb28978c186 100644
>>>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>>>> @@ -457,6 +457,7 @@ static struct ap_queue_status
>>>> vfio_ap_irq_enable(struct vfio_ap_queue *q,
>>>> VFIO_AP_DBF_WARN("%s: gisc registration failed: nisc=%d,
>>>> isc=%d, apqn=%#04x\n",
>>>> __func__, nisc, isc, q->apqn);
>>>> + vfio_unpin_pages(&q->matrix_mdev->vdev, nib, 1);
>>>> status.response_code = AP_RESPONSE_INVALID_GISA;
>>>> return status;
>>>> }
On 10/26/23 10:15, Matthew Rosato wrote:
> On 10/18/23 9:38 AM, Tony Krowiak wrote:
>> Let's improve the vfio_ap driver's reaction to reception of response code
>> 07 from the PQAP(AQIC) command when enabling interrupts on behalf of a
>> guest:
>>
>> * Unregister the guest's ISC before the pages containing the notification
>> indicator bytes are unpinned.
>>
>> * Capture the return code from the kvm_s390_gisc_unregister function and
>> log a DBF warning if it fails.
>>
>> Suggested-by: Matthew Rosato <[email protected]>
>> Signed-off-by: Tony Krowiak <[email protected]>
>
> Reviewed-by: Matthew Rosato <[email protected]>
>
> I went back-and-forth on whether this should be a stable/fixes candidate but I think no... I happened to notice it while reviewing other code, I'm not aware that it's ever created a visible issue, and it's on a pretty immediate error path. If anyone thinks it should be a stable candidate I have no objection but in that case would suggest to break the patch up to separate the new WARN from the fix.
Nothing has ever been reported and is probably very unlikely to be
reported; so, I agree it should not be a stable/fixes candidate.
>
>
>
On Wed, 18 Oct 2023 09:38:24 -0400
Tony Krowiak <[email protected]> wrote:
> From: Anthony Krowiak <[email protected]>
>
> 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 <[email protected]>
> Suggested-by: Halil Pasic <[email protected]>
> ---
> 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 Fri, 27 Oct 2023 09:36:26 -0400
Tony Krowiak <[email protected]> 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.
Regards,
Halil
On 10/27/23 06:56, Halil Pasic wrote:
> On Wed, 18 Oct 2023 09:38:24 -0400
> Tony Krowiak <[email protected]> wrote:
>
>> From: Anthony Krowiak <[email protected]>
>>
>> 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.
>
>>
>> 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 <[email protected]>
>> Suggested-by: Halil Pasic <[email protected]>
>
>
>> ---
>> 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 <[email protected]> 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