linux-kernel - Re: [PATCH v2 5/8] s390: vfio-ap: wait for queue empty on queue reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Apr 2019 09:28:50 -0400
From:   Tony Krowiak <akrowiak@...ux.ibm.com>
To:     pmorel@...ux.ibm.com, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Cc:     freude@...ux.ibm.com, borntraeger@...ibm.com, cohuck@...hat.com,
        frankja@...ux.ibm.com, david@...hat.com, schwidefsky@...ibm.com,
        heiko.carstens@...ibm.com, pasic@...ux.ibm.com,
        alex.williamson@...hat.com, kwankhede@...dia.com
Subject: Re: [PATCH v2 5/8] s390: vfio-ap: wait for queue empty on queue reset

On 4/23/19 8:50 AM, Pierre Morel wrote:
> On 20/04/2019 23:49, Tony Krowiak wrote:
>> Refactors the AP queue reset function to wait until the queue is empty
>> after the PQAP(ZAPQ) instruction is executed to zero out the queue as
>> required by the AP architecture.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@...ux.ibm.com>
>> ---
>>   drivers/s390/crypto/vfio_ap_ops.c | 35 
>> ++++++++++++++++++++++++++++++++---
>>   1 file changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 091804317c5e..31ce30ee784d 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -744,15 +744,44 @@ static int vfio_ap_mdev_group_notifier(struct 
>> notifier_block *nb,
>>       return NOTIFY_OK;
>>   }
>> -static int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int 
>> apqi,
>> -                    unsigned int retry)
>> +static void vfio_ap_mdev_wait_for_qempty(unsigned long apid, unsigned 
>> long apqi)
>>   {
>>       struct ap_queue_status status;
>> +    ap_qid_t qid = AP_MKQID(apid, apqi);
>> +    int retry = 5;
>> +
>> +    do {
>> +        status = ap_tapq(qid, NULL);
>> +        switch (status.response_code) {
>> +        case AP_RESPONSE_NORMAL:
>> +            if (status.queue_empty)
>> +                return;
>> +            msleep(20);
>> +            break;
>> +        case AP_RESPONSE_RESET_IN_PROGRESS:
>> +        case AP_RESPONSE_BUSY:
>> +            msleep(20);
>> +            break;
>> +        default:
>> +            pr_warn("%s: tapq err %02x: 0x%04x may not be empty\n",
>> +                __func__, status.response_code, q->apqn);
> 
> q is not defined. use qid ?

You are right, it is not.

> 
> 
>> +            return;
>> +        }
>> +    } while (--retry);
>> +}
>> +
>> +int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi)
>> +{
>> +    struct ap_queue_status status;
>> +    int retry = 5;
>>       do {
>>           status = ap_zapq(AP_MKQID(apid, apqi));
>>           switch (status.response_code) {
>>           case AP_RESPONSE_NORMAL:
>> +            vfio_ap_mdev_wait_for_qempty(apid, apqi);
>> +            return 0;
>> +        case AP_RESPONSE_DECONFIGURED:
>>               return 0;
>>           case AP_RESPONSE_RESET_IN_PROGRESS:
>>           case AP_RESPONSE_BUSY:
>> @@ -778,7 +807,7 @@ static int vfio_ap_mdev_reset_queues(struct 
>> mdev_device *mdev)
>>                    matrix_mdev->matrix.apm_max + 1) {
>>           for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm,
>>                        matrix_mdev->matrix.aqm_max + 1) {
>> -            ret = vfio_ap_mdev_reset_queue(apid, apqi, 1);
>> +            ret = vfio_ap_mdev_reset_queue(apid, apqi);
>>               /*
>>                * Regardless whether a queue turns out to be busy, or
>>                * is not operational, we need to continue resetting
>>
> 
>