On 9/10/26 10:48 PM, Tyrel Datwyler wrote: > From: Dave Marquardt <[email protected]> > > Wire the async sub-CRQ into the sub-CRQ lifecycle so it is allocated, > registered, and freed alongside the SCSI channel queues. > > In ibmvfc_init_sub_crqs(), allocate the async sub-CRQ queue buffer via > ibmvfc_alloc_queue() before allocating the SCSI channels. Register it > with the VIOS by calling ibmvfc_register_channel() with index -1 (the > negative-index sentinel introduced in the previous commit). Either > failure disables multi-queue and aborts init. > > In ibmvfc_release_sub_crqs(), ibmvfc_reenable_crq_queue(), and > ibmvfc_reset_crq(), deregister and re-register the async sub-CRQ > alongside the SCSI channel queues. > > In ibmvfc_channel_setup_done(), capture the async sub-CRQ handle > returned by the VIOS in the channel setup response and store it in > vhost->async_sub_crq.vios_cookie. > > In ibmvfc_set_login_info(), advertise IBMVFC_USE_ASYNC_SUBQ, > IBMVFC_CAN_HANDLE_FPIN, and IBMVFC_YES_SCSI capabilities whenever > multi-queue channels are enabled. IBMVFC_YES_SCSI was previously only > set for NVMe-enabled configurations; move it to the common multi-queue > path so it is always advertised when channels are in use. > > Fix a variable-shadowing bug in ibmvfc_register_channel() where the > irq_failed cleanup loop reused rc for the H_FREE_SUB_CRQ hcall result, > clobbering the error code returned to the caller. Introduce hcall_rc > for the cleanup loop instead.
I think this should be its own patch. > > Guard the memset() in ibmvfc_deregister_channel() behind a check of > scrq->msgs.handle to prevent a NULL dereference when the function is > called on a queue that was never allocated, such as async_sub_crq when > multi-queue is disabled or initialization failed before > ibmvfc_alloc_queue() was reached. Same with this chunk. -Tyrel > > Signed-off-by: Dave Marquardt <[email protected]> > Acked-by: Tyrel Datwyler <[email protected]> > --- > drivers/scsi/ibmvscsi/ibmvfc-core.c | 51 +++++++++++++++++++++++++---- > 1 file changed, 44 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c > b/drivers/scsi/ibmvscsi/ibmvfc-core.c > index c78e221f182a..70a3046135c7 100644 > --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c > +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c > @@ -192,6 +192,8 @@ static void ibmvfc_tgt_move_login(struct ibmvfc_target *); > > static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *, struct > ibmvfc_channels *); > static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *, struct ibmvfc_channels > *); > +static void ibmvfc_deregister_channel(struct ibmvfc_host *, struct > ibmvfc_channels *, int); > +static int ibmvfc_register_channel(struct ibmvfc_host *, struct > ibmvfc_channels *, int); > > static const char *unknown_error = "unknown error"; > > @@ -964,6 +966,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host > *vhost) > struct vio_dev *vdev = to_vio_dev(vhost->dev); > unsigned long flags; > > + ibmvfc_deregister_channel(vhost, &vhost->scsi_scrqs, -1); > ibmvfc_dereg_sub_crqs(vhost, &vhost->scsi_scrqs); > ibmvfc_dereg_sub_crqs(vhost, &vhost->nvme_scrqs); > > @@ -986,6 +989,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host > *vhost) > spin_unlock(vhost->crq.q_lock); > spin_unlock_irqrestore(&vhost->host->host_lock, flags); > > + ibmvfc_register_channel(vhost, &vhost->scsi_scrqs, -1); > ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs); > ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs); > > @@ -1006,6 +1010,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost) > struct vio_dev *vdev = to_vio_dev(vhost->dev); > struct ibmvfc_queue *crq = &vhost->crq; > > + ibmvfc_deregister_channel(vhost, &vhost->scsi_scrqs, -1); > ibmvfc_dereg_sub_crqs(vhost, &vhost->scsi_scrqs); > ibmvfc_dereg_sub_crqs(vhost, &vhost->nvme_scrqs); > > @@ -1042,6 +1047,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost) > spin_unlock(vhost->crq.q_lock); > spin_unlock_irqrestore(&vhost->host->host_lock, flags); > > + ibmvfc_register_channel(vhost, &vhost->scsi_scrqs, -1); > ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs); > ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs); > > @@ -1584,9 +1590,11 @@ static void ibmvfc_set_login_info(struct ibmvfc_host > *vhost) > > if (vhost->mq_enabled || vhost->using_channels) { > login_info->capabilities |= > cpu_to_be64(IBMVFC_CAN_USE_CHANNELS); > + login_info->capabilities |= cpu_to_be64(IBMVFC_USE_ASYNC_SUBQ); > + login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_HANDLE_FPIN); > + login_info->capabilities |= cpu_to_be64(IBMVFC_YES_SCSI); > if (vhost->nvme_enabled) { > login_info->capabilities |= > cpu_to_be64(IBMVFC_YES_NVMEOF); > - login_info->capabilities |= > cpu_to_be64(IBMVFC_YES_SCSI); > login_info->capabilities |= > cpu_to_be64(IBMVFC_CAN_USE_WWPN_ALL); > } > } > @@ -5811,6 +5819,7 @@ static void ibmvfc_channel_setup_done(struct > ibmvfc_event *evt) > for (i = 0; i < nvme->active_queues; i++) > nvme->scrqs[i].vios_cookie = > > be64_to_cpu(setup->channel_handles[scsi->active_queues + i]); > + vhost->async_sub_crq.vios_cookie = > be64_to_cpu(setup->async_sub_crq_handle); > > ibmvfc_dbg(vhost, "Using %u SCSI channels\n", > scsi->active_queues); > @@ -5870,6 +5879,7 @@ static void ibmvfc_channel_setup(struct ibmvfc_host > *vhost) > for (i = 0; i < nvme_channels; i++) > setup_buf->channel_handles[scsi_channels + i] = > cpu_to_be64(nvme->scrqs[i].cookie); > + setup_buf->async_sub_crq_handle = > cpu_to_be64(vhost->async_sub_crq.cookie); > } > > ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT); > @@ -6834,6 +6844,7 @@ static int ibmvfc_register_channel(struct ibmvfc_host > *vhost, > bool is_async = index < 0; > struct ibmvfc_queue *scrq = !is_async ? &channels->scrqs[index] : > &vhost->async_sub_crq; > int rc = -ENOMEM; > + int hcall_rc; > > ENTER; > > @@ -6902,8 +6913,8 @@ static int ibmvfc_register_channel(struct ibmvfc_host > *vhost, > > irq_failed: > do { > - rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, > scrq->cookie); > - } while (rc == H_BUSY || H_IS_LONG_BUSY(rc)); > + hcall_rc = plpar_hcall_norets(H_FREE_SUB_CRQ, > vdev->unit_address, scrq->cookie); > + } while (hcall_rc == H_BUSY || H_IS_LONG_BUSY(hcall_rc)); > reg_failed: > LEAVE; > return rc; > @@ -6953,8 +6964,10 @@ static void ibmvfc_deregister_channel(struct > ibmvfc_host *vhost, > } > > /* Clean out the queue */ > - memset(scrq->msgs.crq, 0, PAGE_SIZE); > - scrq->cur = 0; > + if (scrq->msgs.handle) { > + memset(scrq->msgs.crq, 0, PAGE_SIZE); > + scrq->cur = 0; > + } > > LEAVE; > } > @@ -6971,7 +6984,9 @@ static void ibmvfc_reg_sub_crqs(struct ibmvfc_host > *vhost, > for (i = 0; i < channels->max_queues; i++) { > if (ibmvfc_register_channel(vhost, channels, i)) { > for (j = i; j > 0; j--) > - ibmvfc_deregister_channel(vhost, channels, j - > 1); > + ibmvfc_deregister_channel( > + vhost, channels, j - 1); > + > vhost->do_enquiry = 0; > return; > } > @@ -7026,16 +7041,26 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host > *vhost, > > static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost) > { > + int rc = 0; > + > ENTER; > if (!vhost->mq_enabled) > return; > > - if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs)) { > + rc = ibmvfc_alloc_queue(vhost, &vhost->async_sub_crq, > IBMVFC_SUB_CRQ_FMT); > + if (rc) { > vhost->do_enquiry = 0; > vhost->mq_enabled = 0; > return; > } > > + /* register async_sub_crq channel */ > + if (ibmvfc_register_channel(vhost, &vhost->scsi_scrqs, -1)) > + goto free_async_sub_crq; > + > + if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs)) > + goto deregister_async_sub_crq; > + > ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs); > > if (vhost->nvme_enabled) { > @@ -7046,6 +7071,15 @@ static void ibmvfc_init_sub_crqs(struct ibmvfc_host > *vhost) > } > > LEAVE; > + return; > + > + deregister_async_sub_crq: > + ibmvfc_deregister_channel(vhost, &vhost->scsi_scrqs, -1); > +free_async_sub_crq: > + ibmvfc_free_queue(vhost, &vhost->async_sub_crq); > + vhost->do_enquiry = 0; > + vhost->mq_enabled = 0; > + return; > } > > static void ibmvfc_release_channels(struct ibmvfc_host *vhost, > @@ -7069,6 +7103,9 @@ static void ibmvfc_release_channels(struct ibmvfc_host > *vhost, > static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost) > { > ENTER; > + ibmvfc_deregister_channel(vhost, &vhost->scsi_scrqs, -1); > + ibmvfc_free_queue(vhost, &vhost->async_sub_crq); > + > if (!vhost->scsi_scrqs.scrqs) > return; >
