Re: [PATCH 2/5] ibmvfc: Add NOOP command support

[Tyrel Datwyler <tyreld@linux.ibm.com>]

On 4/8/26 10:07 AM, Dave Marquardt via B4 Relay wrote:
> From: Dave Marquardt <davemarq@linux.ibm.com>

As with patch 1 this requires a slightly more detailed commit log message and
developer sign off tag.

> 
> - Add VFC_NOOP command support
> - Add KUnit tests for VFC_NOOP command
> ---
>  drivers/scsi/ibmvscsi/ibmvfc.c       | 23 +++++++++++++----------
>  drivers/scsi/ibmvscsi/ibmvfc.h       | 13 +++++++++++++
>  drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 27 +++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 3ac376ba2c62..808301fa452d 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -189,13 +189,6 @@ static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
>  	return rc;
>  }
>  
> -static int ibmvfc_check_caps(struct ibmvfc_host *vhost, unsigned long cap_flags)
> -{
> -	u64 host_caps = be64_to_cpu(vhost->login_buf->resp.capabilities);
> -
> -	return (host_caps & cap_flags) ? 1 : 0;
> -}
> -

It appears you are moving this to ibmvfc.h? Is there reasoning outside making it
visible to kunit?

>  static struct ibmvfc_fcp_cmd_iu *ibmvfc_get_fcp_iu(struct ibmvfc_host *vhost,
>  						   struct ibmvfc_cmd *vfc_cmd)
>  {
> @@ -1512,7 +1505,9 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
>  		login_info->flags |= cpu_to_be16(IBMVFC_CLIENT_MIGRATED);
>  
>  	login_info->max_cmds = cpu_to_be32(max_cmds);
> -	login_info->capabilities = cpu_to_be64(IBMVFC_CAN_MIGRATE | IBMVFC_CAN_SEND_VF_WWPN);
> +	login_info->capabilities =
> +		cpu_to_be64(IBMVFC_CAN_MIGRATE | IBMVFC_CAN_SEND_VF_WWPN |
> +			    IBMVFC_CAN_USE_NOOP_CMD);
>  
>  	if (vhost->mq_enabled || vhost->using_channels)
>  		login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_USE_CHANNELS);
> @@ -3461,8 +3456,8 @@ EXPORT_SYMBOL_IF_KUNIT(ibmvfc_handle_async);
>   * @evt_doneq:	Event done queue
>   *
>  **/
> -static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
> -			      struct list_head *evt_doneq)
> +VISIBLE_IF_KUNIT void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
> +					struct list_head *evt_doneq)
>  {
>  	long rc;
>  	struct ibmvfc_event *evt = (struct ibmvfc_event *)be64_to_cpu(crq->ioba);
> @@ -3520,6 +3515,13 @@ static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
>  	if (crq->format == IBMVFC_ASYNC_EVENT)
>  		return;
>  
> +	if (crq->format == IBMVFC_VFC_NOOP) {
> +		if (!ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_NOOP_CMD))
> +			dev_err(vhost->dev,
> +				"Received unexpected NOOP command from partner\n");

If we have a misbahaved VIOS partner we may want to ratelimit this dev_err so
that we don't flood the log. Probably a corner case, but I don't think it hurts.

-Tyrel

> +		return;
> +	}
> +
>  	/* The only kind of payload CRQs we should get are responses to
>  	 * things we send. Make sure this response is to something we
>  	 * actually sent