On Mon, Dec 02, 2019 at 06:34:43PM -0800, Bart Van Assche wrote:
> On 2019-12-02 12:55, Roman Bolshakov wrote:
> > On Mon, Dec 02, 2019 at 08:40:17AM -0800, Bart Van Assche wrote:
> >> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> >> index b25f87ff8cde..e2e91b3f2e65 100644
> >> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> >> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> >> @@ -2656,10 +2656,16 @@ qla24xx_els_logo_iocb(srb_t *sp, struct els_entry_24xx *els_iocb)
> >> els_iocb->port_id[0] = sp->fcport->d_id.b.al_pa;
> >> els_iocb->port_id[1] = sp->fcport->d_id.b.area;
> >> els_iocb->port_id[2] = sp->fcport->d_id.b.domain;
> >> - /* For SID the byte order is different than DID */
> >> - els_iocb->s_id[1] = vha->d_id.b.al_pa;
> >> - els_iocb->s_id[2] = vha->d_id.b.area;
> >> - els_iocb->s_id[0] = vha->d_id.b.domain;
> >> + if (IS_QLA23XX(vha->hw) || IS_QLA24XX(vha->hw) || IS_QLA25XX(vha->hw)) {
> >> + els_iocb->s_id[0] = vha->d_id.b.al_pa;
> >> + els_iocb->s_id[1] = vha->d_id.b.area;
> >> + els_iocb->s_id[2] = vha->d_id.b.domain;
> >> + } else {
> >> + /* For SID the byte order is different than DID */
> >> + els_iocb->s_id[1] = vha->d_id.b.al_pa;
> >> + els_iocb->s_id[2] = vha->d_id.b.area;
> >> + els_iocb->s_id[0] = vha->d_id.b.domain;
> >> + }
> >>
> >> if (elsio->u.els_logo.els_cmd == ELS_DCMD_PLOGI) {
> >> els_iocb->control_flags = 0;
> >
> > Hi Bart,
> >
> > I'm fine as long as it works for old and new HBAs. I don't have docs to
> > 2300/2400 series and the HBAs. Are you sure it follows the same S_ID
> > order as 2500?
> >
> > Regardless of that, it should work for the last 4 series of the HBAs,
> > Reviewed-by: Roman Bolshakov <r.bolshakov@xxxxxxxxx>
>
> Hi Roman,
>
> Thanks for the review. In my copy of the 25xx firmware manual the s_id[]
> field has been marked as RESERVED. I have tried not to write into s_id[]
> but that was not sufficient to restore point-to-point mode.
>
Hi Bart,
IMO that means the field is undocumented rather than RESERVED on 2500
series chips.
> Older versions of the qla2xxx driver did not initialize s_id[]. I think
> that commit edd05de19759 ("scsi: qla2xxx: Changes to support N2N
> logins") is the commit that introduced initialization of s_id[]. Is that
> value passed to the target port? Did that commit perhaps introduce code
> that checks the value received by the target?
>
Firmware can do implicit login, and this is how it worked for a while.
Then explicit login was introduced in the commit you referenced by
setting bit 8 in IFCB fimwrare options 3 for 2600/2700 series and
issuing ELS IOCB. However, for 2500 series, bit 7 should be set to
disable implicit logins.
The latest commits that touches the bit is 8777e4314d397 ("scsi: qla2xxx:
Migrate NVME N2N handling into state machine"). It sets the bit in
qla24xx_nvram_config regadless of chip.
Does it help to set bit 7 in IFCB, firmware options 3 for 2500 series
and leave the RESERVED S_ID field untouched?
Thanks,
Roman
