Received-SPF: pass (google.com: domain of linux-kernel+bounces-83311-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <3b5a812d-3832-6f34-a528-c67e59d76e65@linux.ibm.com>
Date: Tue, 27 Feb 2024 08:25:21 -0500
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH] s390/vfio-ap: handle hardware checkstop state on queue
 reset operation
From: "Jason J. Herne" <jjherne@linux.ibm.com>
To: linux-s390@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, pasic@linux.ibm.com, akrowiak@linux.ibm.com,
        borntraeger@de.ibm.com, agordeev@linux.ibm.com, gor@linux.ibm.com
References: <20240215153144.14747-1-jjherne@linux.ibm.com>
Content-Language: en-US
In-Reply-To: <20240215153144.14747-1-jjherne@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Polite Ping :)

Patch already has R-b.
Thanks for taking a look.

On 2/15/24 10:31 AM, Jason J. Herne wrote:
> Update vfio_ap_mdev_reset_queue() to handle an unexpected checkstop (hardware error) the
> same as the deconfigured case. This prevents unexpected and unhelpful warnings in the
> event of a hardware error.
> 
> We also stop lying about a queue's reset response code. This was originally done so we
> could force vfio_ap_mdev_filter_matrix to pass a deconfigured device through to the guest
> for the hotplug scenario. vfio_ap_mdev_filter_matrix is instead modified to allow
> passthrough for all queues with reset state normal, deconfigured, or checkstopped. In the
> checkstopped case we choose to pass the device through and let the error state be
> reflected at the guest level.
> 
> Signed-off-by: Jason J. Herne <jjherne@linux.ibm.com>
> Reviewed-by: Anthony Krowiak <akrowiak@linux.ibm.com>
> ---
>   drivers/s390/crypto/vfio_ap_ops.c | 35 ++++++++++++++++---------------
>   1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 983b3b16196c..fc169bc61593 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -659,6 +659,21 @@ static bool vfio_ap_mdev_filter_cdoms(struct ap_matrix_mdev *matrix_mdev)
>   			     AP_DOMAINS);
>   }
>   
> +static bool _queue_passable(struct vfio_ap_queue *q)
> +{
> +	if (!q)
> +		return false;
> +
> +	switch (q->reset_status.response_code) {
> +	case AP_RESPONSE_NORMAL:
> +	case AP_RESPONSE_DECONFIGURED:
> +	case AP_RESPONSE_CHECKSTOPPED:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
>   /*
>    * vfio_ap_mdev_filter_matrix - filter the APQNs assigned to the matrix mdev
>    *				to ensure no queue devices are passed through to
> @@ -687,7 +702,6 @@ static bool vfio_ap_mdev_filter_matrix(struct ap_matrix_mdev *matrix_mdev,
>   	unsigned long apid, apqi, apqn;
>   	DECLARE_BITMAP(prev_shadow_apm, AP_DEVICES);
>   	DECLARE_BITMAP(prev_shadow_aqm, AP_DOMAINS);
> -	struct vfio_ap_queue *q;
>   
>   	bitmap_copy(prev_shadow_apm, matrix_mdev->shadow_apcb.apm, AP_DEVICES);
>   	bitmap_copy(prev_shadow_aqm, matrix_mdev->shadow_apcb.aqm, AP_DOMAINS);
> @@ -716,8 +730,7 @@ static bool vfio_ap_mdev_filter_matrix(struct ap_matrix_mdev *matrix_mdev,
>   			 * hardware device.
>   			 */
>   			apqn = AP_MKQID(apid, apqi);
> -			q = vfio_ap_mdev_get_queue(matrix_mdev, apqn);
> -			if (!q || q->reset_status.response_code) {
> +			if (!_queue_passable(vfio_ap_mdev_get_queue(matrix_mdev, apqn))) {
>   				clear_bit_inv(apid, matrix_mdev->shadow_apcb.apm);
>   
>   				/*
> @@ -1691,6 +1704,7 @@ static int apq_status_check(int apqn, struct ap_queue_status *status)
>   	switch (status->response_code) {
>   	case AP_RESPONSE_NORMAL:
>   	case AP_RESPONSE_DECONFIGURED:
> +	case AP_RESPONSE_CHECKSTOPPED:
>   		return 0;
>   	case AP_RESPONSE_RESET_IN_PROGRESS:
>   	case AP_RESPONSE_BUSY:
> @@ -1747,14 +1761,6 @@ static void apq_reset_check(struct work_struct *reset_work)
>   				memcpy(&q->reset_status, &status, sizeof(status));
>   				continue;
>   			}
> -			/*
> -			 * When an AP adapter is deconfigured, the
> -			 * associated queues are reset, so let's set the
> -			 * status response code to 0 so the queue may be
> -			 * passed through (i.e., not filtered)
> -			 */
> -			if (status.response_code == AP_RESPONSE_DECONFIGURED)
> -				q->reset_status.response_code = 0;
>   			if (q->saved_isc != VFIO_AP_ISC_INVALID)
>   				vfio_ap_free_aqic_resources(q);
>   			break;
> @@ -1781,12 +1787,7 @@ static void vfio_ap_mdev_reset_queue(struct vfio_ap_queue *q)
>   		queue_work(system_long_wq, &q->reset_work);
>   		break;
>   	case AP_RESPONSE_DECONFIGURED:
> -		/*
> -		 * When an AP adapter is deconfigured, the associated
> -		 * queues are reset, so let's set the status response code to 0
> -		 * so the queue may be passed through (i.e., not filtered).
> -		 */
> -		q->reset_status.response_code = 0;
> +	case AP_RESPONSE_CHECKSTOPPED:
>   		vfio_ap_free_aqic_resources(q);
>   		break;
>   	default: