| Message ID | 20200220043441.20504-3-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | qla2xxx patches for kernel v5.7 | expand |
On Wed, Feb 19, 2020 at 08:34:38PM -0800, Bart Van Assche wrote: > Instead of changing endianness in-place, write the data in CPU endian format > in another buffer and copy that buffer back. This patch does not change any > functionality but silences some sparse endianness warnings. > > Reviewed-by: Daniel Wagner <dwagner@suse.de> > Cc: Quinn Tran <qutran@marvell.com> > Cc: Martin Wilck <mwilck@suse.com> > Cc: Roman Bolshakov <r.bolshakov@yadro.com> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/scsi/qla2xxx/qla_def.h | 2 +- > drivers/scsi/qla2xxx/qla_init.c | 11 +++++------ > 2 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c > index 9e6b56527b25..880f2be062a9 100644 > --- a/drivers/scsi/qla2xxx/qla_init.c > +++ b/drivers/scsi/qla2xxx/qla_init.c > @@ -5068,13 +5068,12 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha) > rval = qla24xx_get_port_login_templ(vha, > ha->init_cb_dma, (void *)ha->init_cb, sz); > if (rval == QLA_SUCCESS) { > + __be32 *q = &ha->plogi_els_payld.data[0]; > + > bp = (uint32_t *)ha->init_cb; > - for (i = 0; i < sz/4 ; i++, bp++) > - *bp = cpu_to_be32(*bp); > + cpu_to_be32_array(q, bp, sz / 4); > > - memcpy(&ha->plogi_els_payld.data, > - (void *)ha->init_cb, > - sizeof(ha->plogi_els_payld.data)); > + memcpy(bp, q, sizeof(ha->plogi_els_payld.data)); Hi Bart, Honestly, I'd prefer to drop the memcpy as it has no purpose, but since you replicated a side-effect of the previous code and fixed sparse warnings, Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> Thank you, Roman P.S. During the review, I looked again where init_cb is used. It seems the reuse and rewriting of init_cb contents by qla24xx_get_port_login_templ may corrupt init_cb values expected by qla2x00_fdmi_rpa and qla2x00_fdmiv2_rpa (they retrieve max frame size from Initialization Control Block), and qla2x00_async_event (recovers WWPN from init_cb in case of Fabric-assigned WWPN). That _rpa functions are invoked if a port is in fabric topology and FDMI registration is enabled, so a change of topology from P2P to Switched Fabric may result in FDMI registration with wrong max frame size. I don't know how FC switches use the value though. If the fabric assigns WWPNs (FA-WWPN) and a disconnect happens (Loop Down Event), the driver will try to recover WWPN from init_cb that is corrupted by qla24xx_get_port_login_templ. Probably, the wrong state may be recovered by reclaiming the underlying PCI device. init_cb is set to correct values in probe() handler.
diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index c5a067f45005..b04d334933ef 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -414,7 +414,7 @@ struct els_logo_payload { struct els_plogi_payload { uint8_t opcode; uint8_t rsvd[3]; - uint8_t data[112]; + __be32 data[112 / 4]; }; struct ct_arg { diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c index 9e6b56527b25..880f2be062a9 100644 --- a/drivers/scsi/qla2xxx/qla_init.c +++ b/drivers/scsi/qla2xxx/qla_init.c @@ -5060,7 +5060,7 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha) if (N2N_TOPO(ha)) { if (test_and_clear_bit(N2N_LOGIN_NEEDED, &vha->dpc_flags)) { /* borrowing */ - u32 *bp, i, sz; + u32 *bp, sz; memset(ha->init_cb, 0, ha->init_cb_size); sz = min_t(int, sizeof(struct els_plogi_payload), @@ -5068,13 +5068,12 @@ qla2x00_configure_local_loop(scsi_qla_host_t *vha) rval = qla24xx_get_port_login_templ(vha, ha->init_cb_dma, (void *)ha->init_cb, sz); if (rval == QLA_SUCCESS) { + __be32 *q = &ha->plogi_els_payld.data[0]; + bp = (uint32_t *)ha->init_cb; - for (i = 0; i < sz/4 ; i++, bp++) - *bp = cpu_to_be32(*bp); + cpu_to_be32_array(q, bp, sz / 4); - memcpy(&ha->plogi_els_payld.data, - (void *)ha->init_cb, - sizeof(ha->plogi_els_payld.data)); + memcpy(bp, q, sizeof(ha->plogi_els_payld.data)); } else { ql_dbg(ql_dbg_init, vha, 0x00d1, "PLOGI ELS param read fail.\n");