linux-kernel - Re: [PATCH v2] s390: vfio-ap: remove unnecessary calls to disable queue interrupts

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:   Wed, 4 Sep 2019 09:35:47 +0200
From:   Christian Borntraeger <borntraeger@...ibm.com>
To:     Halil Pasic <pasic@...ux.ibm.com>,
        Tony Krowiak <akrowiak@...ux.ibm.com>
Cc:     linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        freude@...ibm.com, cohuck@...hat.com, pasic@...ux.vnet.ibm.com,
        frankja@...ux.ibm.com, jjherne@...ux.ibm.com
Subject: Re: [PATCH v2] s390: vfio-ap: remove unnecessary calls to disable
 queue interrupts

Halil,

can you also send this patch as a separate mail. This also requires a much better
patch description about the why and it certainly should also have an agreement from
Anthony.

On 30.08.19 18:02, Halil Pasic wrote:
> From: Halil Pasic <pasic@...ux.ibm.com>
> Date: Fri, 30 Aug 2019 17:39:47 +0200
> Subject: [PATCH 2/2] s390: vfio-ap: don't wait after AQIC interpretation
> 
> Waiting for the asynchronous part of AQIC to complete as a part
> AQIC implementation is unnecessary and silly.
> 
> Let's get rid of vfio_ap_wait_for_irqclear().
> 
> Signed-off-by: Halil Pasic <pasic@...ux.ibm.com>
> ---
>  drivers/s390/crypto/vfio_ap_ops.c | 50 ++-------------------------------------
>  1 file changed, 2 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index dd07ebf..8d098f0 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -68,47 +68,6 @@ static struct vfio_ap_queue *vfio_ap_get_queue(
>  }
>  
>  /**
> - * vfio_ap_wait_for_irqclear
> - * @apqn: The AP Queue number
> - *
> - * Checks the IRQ bit for the status of this APQN using ap_tapq.
> - * Returns if the ap_tapq function succeeded and the bit is clear.
> - * Returns if ap_tapq function failed with invalid, deconfigured or
> - * checkstopped AP.
> - * Otherwise retries up to 5 times after waiting 20ms.
> - *
> - */
> -static void vfio_ap_wait_for_irqclear(int apqn)
> -{
> -	struct ap_queue_status status;
> -	int retry = 5;
> -
> -	do {
> -		status = ap_tapq(apqn, NULL);
> -		switch (status.response_code) {
> -		case AP_RESPONSE_NORMAL:
> -		case AP_RESPONSE_RESET_IN_PROGRESS:
> -			if (!status.irq_enabled)
> -				return;
> -			/* Fall through */
> -		case AP_RESPONSE_BUSY:
> -			msleep(20);
> -			break;
> -		case AP_RESPONSE_Q_NOT_AVAIL:
> -		case AP_RESPONSE_DECONFIGURED:
> -		case AP_RESPONSE_CHECKSTOPPED:
> -		default:
> -			WARN_ONCE(1, "%s: tapq rc %02x: %04x\n", __func__,
> -				  status.response_code, apqn);
> -			return;
> -		}
> -	} while (--retry);
> -
> -	WARN_ONCE(1, "%s: tapq rc %02x: %04x could not clear IR bit\n",
> -		  __func__, status.response_code, apqn);
> -}
> -
> -/**
>   * vfio_ap_free_aqic_resources
>   * @q: The vfio_ap_queue
>   *
> @@ -133,14 +92,10 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q)
>   * @q: The vfio_ap_queue
>   *
>   * Uses ap_aqic to disable the interruption and in case of success, reset
> - * in progress or IRQ disable command already proceeded: calls
> - * vfio_ap_wait_for_irqclear() to check for the IRQ bit to be clear
> - * and calls vfio_ap_free_aqic_resources() to free the resources associated
> + * in progress or IRQ disable command already proceeded :calls
> + * vfio_ap_free_aqic_resources() to free the resources associated
>   * with the AP interrupt handling.
>   *
> - * In the case the AP is busy, or a reset is in progress,
> - * retries after 20ms, up to 5 times.
> - *
>   * Returns if ap_aqic function failed with invalid, deconfigured or
>   * checkstopped AP.
>   */
> @@ -155,7 +110,6 @@ struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q)
>  		switch (status.response_code) {
>  		case AP_RESPONSE_OTHERWISE_CHANGED:
>  		case AP_RESPONSE_NORMAL:
> -			vfio_ap_wait_for_irqclear(q->apqn);
>  			goto end_free;
>  		case AP_RESPONSE_RESET_IN_PROGRESS:
>  		case AP_RESPONSE_BUSY:
> -- 2.5.5