* [PATCH] fix qla2xxx regression on sparc64
@ 2020-08-22 11:59 Rene Rebe
2020-08-22 15:53 ` Bart Van Assche
0 siblings, 1 reply; 11+ messages in thread
From: Rene Rebe @ 2020-08-22 11:59 UTC (permalink / raw)
To: linux-scsi; +Cc: Himanshu Madhani
Commit 98aee70d19a7e3203649fa2078464e4f402a0ad8 in 2014 broke qla2xxx on
sparc64, e.g. as in the Sun Blade 1000 / 2000. Unbreak by partial revert to
fix endianess in nvram firmware default initialization.
Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.")
Signed-off-by: René Rebe <rene@exactcode.de>
diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 57a2d76aa691..0916c33eb076 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -4603,18 +4603,18 @@ qla2x00_nvram_config(scsi_qla_host_t *vha)
nv->firmware_options[1] = BIT_7 | BIT_5;
nv->add_firmware_options[0] = BIT_5;
nv->add_firmware_options[1] = BIT_5 | BIT_4;
- nv->frame_payload_size = 2048;
+ nv->frame_payload_size = __constant_cpu_to_le16(2048);
nv->special_options[1] = BIT_7;
} else if (IS_QLA2200(ha)) {
nv->firmware_options[0] = BIT_2 | BIT_1;
nv->firmware_options[1] = BIT_7 | BIT_5;
nv->add_firmware_options[0] = BIT_5;
nv->add_firmware_options[1] = BIT_5 | BIT_4;
- nv->frame_payload_size = 1024;
+ nv->frame_payload_size = __constant_cpu_to_le16(1024);
} else if (IS_QLA2100(ha)) {
nv->firmware_options[0] = BIT_3 | BIT_1;
nv->firmware_options[1] = BIT_5;
- nv->frame_payload_size = 1024;
+ nv->frame_payload_size = __constant_cpu_to_le16(1024);
}
nv->max_iocb_allocation = cpu_to_le16(256);
--
René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
https://exactcode.com | https://t2sde.org | https://rene.rebe.de
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-22 11:59 [PATCH] fix qla2xxx regression on sparc64 Rene Rebe @ 2020-08-22 15:53 ` Bart Van Assche 0 siblings, 0 replies; 11+ messages in thread From: Bart Van Assche @ 2020-08-22 15:53 UTC (permalink / raw) To: Rene Rebe, linux-scsi; +Cc: Himanshu Madhani On 2020-08-22 04:59, Rene Rebe wrote: > Commit 98aee70d19a7e3203649fa2078464e4f402a0ad8 in 2014 broke qla2xxx on > sparc64, e.g. as in the Sun Blade 1000 / 2000. Unbreak by partial revert to > fix endianess in nvram firmware default initialization. > > Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.") > Signed-off-by: René Rebe <rene@exactcode.de> > > diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c > index 57a2d76aa691..0916c33eb076 100644 > --- a/drivers/scsi/qla2xxx/qla_init.c > +++ b/drivers/scsi/qla2xxx/qla_init.c > @@ -4603,18 +4603,18 @@ qla2x00_nvram_config(scsi_qla_host_t *vha) > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 2048; > + nv->frame_payload_size = __constant_cpu_to_le16(2048); > nv->special_options[1] = BIT_7; > } else if (IS_QLA2200(ha)) { > nv->firmware_options[0] = BIT_2 | BIT_1; > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = __constant_cpu_to_le16(1024); > } else if (IS_QLA2100(ha)) { > nv->firmware_options[0] = BIT_3 | BIT_1; > nv->firmware_options[1] = BIT_5; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = __constant_cpu_to_le16(1024); > } > > nv->max_iocb_allocation = cpu_to_le16(256); Drivers should use cpu_to_le16() instead of __constant_cpu_to_le16(). Additionally, this patch introduces the following new sparse warnings: $ make C=2 M=drivers/scsi/qla2xxx CC [M] drivers/scsi/qla2xxx/qla_os.o CHECK drivers/scsi/qla2xxx/qla_os.c CC [M] drivers/scsi/qla2xxx/qla_init.o CHECK drivers/scsi/qla2xxx/qla_init.c drivers/scsi/qla2xxx/qla_init.c:4606:48: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_init.c:4606:48: expected unsigned short [usertype] frame_payload_size drivers/scsi/qla2xxx/qla_init.c:4606:48: got restricted __le16 [usertype] drivers/scsi/qla2xxx/qla_init.c:4613:48: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_init.c:4613:48: expected unsigned short [usertype] frame_payload_size drivers/scsi/qla2xxx/qla_init.c:4613:48: got restricted __le16 [usertype] drivers/scsi/qla2xxx/qla_init.c:4617:48: warning: incorrect type in assignment (different base types) drivers/scsi/qla2xxx/qla_init.c:4617:48: expected unsigned short [usertype] frame_payload_size drivers/scsi/qla2xxx/qla_init.c:4617:48: got restricted __le16 [usertype] Please fix these. Bart. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] fix qla2xxx regression on sparc64
@ 2020-08-21 12:28 Rene Rebe
2020-08-21 13:49 ` Himanshu Madhani
2020-08-21 20:55 ` Bart Van Assche
0 siblings, 2 replies; 11+ messages in thread
From: Rene Rebe @ 2020-08-21 12:28 UTC (permalink / raw)
To: linux-scsi
Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke
qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000.
Unbreak by fixing endianess in nvram firmware defaults
initialization.
Signed-off-by: René Rebe <rene@exactcode.de>
--- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200
+++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200
@@ -4603,18 +4603,18 @@
nv->firmware_options[1] = BIT_7 | BIT_5;
nv->add_firmware_options[0] = BIT_5;
nv->add_firmware_options[1] = BIT_5 | BIT_4;
- nv->frame_payload_size = 2048;
+ nv->frame_payload_size = cpu_to_le16(2048);
nv->special_options[1] = BIT_7;
} else if (IS_QLA2200(ha)) {
nv->firmware_options[0] = BIT_2 | BIT_1;
nv->firmware_options[1] = BIT_7 | BIT_5;
nv->add_firmware_options[0] = BIT_5;
nv->add_firmware_options[1] = BIT_5 | BIT_4;
- nv->frame_payload_size = 1024;
+ nv->frame_payload_size = cpu_to_le16(1024);
} else if (IS_QLA2100(ha)) {
nv->firmware_options[0] = BIT_3 | BIT_1;
nv->firmware_options[1] = BIT_5;
- nv->frame_payload_size = 1024;
+ nv->frame_payload_size = cpu_to_le16(1024);
}
nv->max_iocb_allocation = cpu_to_le16(256);
--
René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin
https://exactcode.com | https://t2sde.org | https://rene.rebe.de
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 12:28 Rene Rebe @ 2020-08-21 13:49 ` Himanshu Madhani 2020-08-21 14:47 ` René Rebe 2020-08-21 20:55 ` Bart Van Assche 1 sibling, 1 reply; 11+ messages in thread From: Himanshu Madhani @ 2020-08-21 13:49 UTC (permalink / raw) To: Rene Rebe; +Cc: linux-scsi Hi Rene, > On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: > > > Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke > qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. > Unbreak by fixing endianess in nvram firmware defaults > initialization. > I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. Can you please provide details of the commit which introduced this regression. Also when you resubmit this patch please use Fixes tag. See documentation here for the correct format. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst > Signed-off-by: René Rebe <rene@exactcode.de> > > --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 > +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 > @@ -4603,18 +4603,18 @@ > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 2048; > + nv->frame_payload_size = cpu_to_le16(2048); > nv->special_options[1] = BIT_7; > } else if (IS_QLA2200(ha)) { > nv->firmware_options[0] = BIT_2 | BIT_1; > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = cpu_to_le16(1024); > } else if (IS_QLA2100(ha)) { > nv->firmware_options[0] = BIT_3 | BIT_1; > nv->firmware_options[1] = BIT_5; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = cpu_to_le16(1024); > } > > nv->max_iocb_allocation = cpu_to_le16(256); > > -- > René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin > https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ -- Himanshu Madhani Oracle Linux Engineering ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 13:49 ` Himanshu Madhani @ 2020-08-21 14:47 ` René Rebe 2020-08-21 15:17 ` Himanshu Madhani 0 siblings, 1 reply; 11+ messages in thread From: René Rebe @ 2020-08-21 14:47 UTC (permalink / raw) To: Himanshu Madhani; +Cc: linux-scsi Hey, > On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: > > Hi Rene, > > >> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >> >> >> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >> Unbreak by fixing endianess in nvram firmware defaults >> initialization. >> > > I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. Did I copy this wrong? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97 > Can you please provide details of the commit which introduced this regression. See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … Have a nice weekend, René > Also when you > resubmit this patch please use Fixes tag. See documentation here for the correct format. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst > >> Signed-off-by: René Rebe <rene@exactcode.de> >> >> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >> @@ -4603,18 +4603,18 @@ >> nv->firmware_options[1] = BIT_7 | BIT_5; >> nv->add_firmware_options[0] = BIT_5; >> nv->add_firmware_options[1] = BIT_5 | BIT_4; >> - nv->frame_payload_size = 2048; >> + nv->frame_payload_size = cpu_to_le16(2048); >> nv->special_options[1] = BIT_7; >> } else if (IS_QLA2200(ha)) { >> nv->firmware_options[0] = BIT_2 | BIT_1; >> nv->firmware_options[1] = BIT_7 | BIT_5; >> nv->add_firmware_options[0] = BIT_5; >> nv->add_firmware_options[1] = BIT_5 | BIT_4; >> - nv->frame_payload_size = 1024; >> + nv->frame_payload_size = cpu_to_le16(1024); >> } else if (IS_QLA2100(ha)) { >> nv->firmware_options[0] = BIT_3 | BIT_1; >> nv->firmware_options[1] = BIT_5; >> - nv->frame_payload_size = 1024; >> + nv->frame_payload_size = cpu_to_le16(1024); >> } >> >> nv->max_iocb_allocation = cpu_to_le16(256); >> >> -- >> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ > > -- > Himanshu Madhani Oracle Linux Engineering > -- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 14:47 ` René Rebe @ 2020-08-21 15:17 ` Himanshu Madhani 2020-08-21 17:49 ` René Rebe 0 siblings, 1 reply; 11+ messages in thread From: Himanshu Madhani @ 2020-08-21 15:17 UTC (permalink / raw) To: René Rebe; +Cc: linux-scsi Hi Rene, > On Aug 21, 2020, at 9:47 AM, René Rebe <rene@exactcode.de> wrote: > > Hey, > >> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >> >> Hi Rene, >> >> >>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >>> >>> >>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>> Unbreak by fixing endianess in nvram firmware defaults >>> initialization. >>> >> >> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. > > Did I copy this wrong? > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ > Yes. The actual commit that change those lines is following https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8 So your fixes tag should be Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760 Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. >> Can you please provide details of the commit which introduced this regression. > > See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … > > Have a nice weekend, > René > >> Also when you >> resubmit this patch please use Fixes tag. See documentation here for the correct format. >> >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >> >>> Signed-off-by: René Rebe <rene@exactcode.de> >>> >>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>> @@ -4603,18 +4603,18 @@ >>> nv->firmware_options[1] = BIT_7 | BIT_5; >>> nv->add_firmware_options[0] = BIT_5; >>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>> - nv->frame_payload_size = 2048; >>> + nv->frame_payload_size = cpu_to_le16(2048); >>> nv->special_options[1] = BIT_7; >>> } else if (IS_QLA2200(ha)) { >>> nv->firmware_options[0] = BIT_2 | BIT_1; >>> nv->firmware_options[1] = BIT_7 | BIT_5; >>> nv->add_firmware_options[0] = BIT_5; >>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>> - nv->frame_payload_size = 1024; >>> + nv->frame_payload_size = cpu_to_le16(1024); >>> } else if (IS_QLA2100(ha)) { >>> nv->firmware_options[0] = BIT_3 | BIT_1; >>> nv->firmware_options[1] = BIT_5; >>> - nv->frame_payload_size = 1024; >>> + nv->frame_payload_size = cpu_to_le16(1024); >>> } >>> >>> nv->max_iocb_allocation = cpu_to_le16(256); >>> >>> -- >>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >> >> -- >> Himanshu Madhani Oracle Linux Engineering >> > > -- > ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ > https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ -- Himanshu Madhani Oracle Linux Engineering ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 15:17 ` Himanshu Madhani @ 2020-08-21 17:49 ` René Rebe 2020-08-21 18:00 ` Himanshu Madhani 0 siblings, 1 reply; 11+ messages in thread From: René Rebe @ 2020-08-21 17:49 UTC (permalink / raw) To: Himanshu Madhani; +Cc: linux-scsi Hi, > On 21. Aug 2020, at 17:17, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: > > Hi Rene, > >> On Aug 21, 2020, at 9:47 AM, René Rebe <rene@exactcode.de> wrote: >> >> Hey, >> >>> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>> >>> Hi Rene, >>> >>> >>>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >>>> >>>> >>>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>>> Unbreak by fixing endianess in nvram firmware defaults >>>> initialization. >>>> >>> >>> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. >> >> Did I copy this wrong? >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ >> > > Yes. The actual commit that change those lines is following > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8 > > So your fixes tag should be > > Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) > > And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. > > Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760 > > Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. Well, linux kernels since 3.17 did not work on this adapter on sparc64: and I reverted the whole 9a50aaefc1b896e734bf7faf3d085f71a360ce97 and factored the changes out and tested what I sent with 5..8.1. This was also reported back in the say, but somehow never applied, ..? https://lore.kernel.org/lkml/86A17A977688E04689F40F6B1EC20EC601940E06A9@AVMB1.qlogic.org/ Regards, René >>> Can you please provide details of the commit which introduced this regression. >> >> See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … >> >> Have a nice weekend, >> René >> >>> Also when you >>> resubmit this patch please use Fixes tag. See documentation here for the correct format. >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >>> >>>> Signed-off-by: René Rebe <rene@exactcode.de> >>>> >>>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>>> @@ -4603,18 +4603,18 @@ >>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>> nv->add_firmware_options[0] = BIT_5; >>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>> - nv->frame_payload_size = 2048; >>>> + nv->frame_payload_size = cpu_to_le16(2048); >>>> nv->special_options[1] = BIT_7; >>>> } else if (IS_QLA2200(ha)) { >>>> nv->firmware_options[0] = BIT_2 | BIT_1; >>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>> nv->add_firmware_options[0] = BIT_5; >>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>> - nv->frame_payload_size = 1024; >>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>> } else if (IS_QLA2100(ha)) { >>>> nv->firmware_options[0] = BIT_3 | BIT_1; >>>> nv->firmware_options[1] = BIT_5; >>>> - nv->frame_payload_size = 1024; >>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>> } >>>> >>>> nv->max_iocb_allocation = cpu_to_le16(256); >>>> >>>> -- >>>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >>> >>> -- >>> Himanshu Madhani Oracle Linux Engineering >>> >> >> -- >> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ >> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ > > -- > Himanshu Madhani Oracle Linux Engineering > -- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 17:49 ` René Rebe @ 2020-08-21 18:00 ` Himanshu Madhani 2020-08-21 20:21 ` René Rebe 0 siblings, 1 reply; 11+ messages in thread From: Himanshu Madhani @ 2020-08-21 18:00 UTC (permalink / raw) To: René Rebe; +Cc: linux-scsi > On Aug 21, 2020, at 12:49 PM, René Rebe <rene@exactcode.de> wrote: > > Hi, > >> On 21. Aug 2020, at 17:17, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >> >> Hi Rene, >> >>> On Aug 21, 2020, at 9:47 AM, René Rebe <rene@exactcode.de> wrote: >>> >>> Hey, >>> >>>> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>>> >>>> Hi Rene, >>>> >>>> >>>>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >>>>> >>>>> >>>>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>>>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>>>> Unbreak by fixing endianess in nvram firmware defaults >>>>> initialization. >>>>> >>>> >>>> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. >>> >>> Did I copy this wrong? >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ >>> >> >> Yes. The actual commit that change those lines is following >> >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXzu7NMDs$ >> >> So your fixes tag should be >> >> Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) >> >> And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. >> >> Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 >> >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX5pZx4bu$ >> >> Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. > > Well, linux kernels since 3.17 did not work on this adapter on sparc64: > > and I reverted the whole 9a50aaefc1b896e734bf7faf3d085f71a360ce97 and factored the changes out and tested what I sent with 5..8.1. > > This was also reported back in the say, but somehow never applied, ..? > > https://urldefense.com/v3/__https://lore.kernel.org/lkml/86A17A977688E04689F40F6B1EC20EC601940E06A9@AVMB1.qlogic.org/__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX9w-_N_o$ > Wow. This should have been done the moment Author acknowledged the revert. I am stumped that we are still talking about this revert after 5+ yrs and its not done. Would you please send just plain revert request for commit 98aee70d19a7e rather than reverting whole merge window patch. > Regards, > René > >>>> Can you please provide details of the commit which introduced this regression. >>> >>> See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … >>> >>> Have a nice weekend, >>> René >>> >>>> Also when you >>>> resubmit this patch please use Fixes tag. See documentation here for the correct format. >>>> >>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >>>> >>>>> Signed-off-by: René Rebe <rene@exactcode.de> >>>>> >>>>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>>>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>>>> @@ -4603,18 +4603,18 @@ >>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>> nv->add_firmware_options[0] = BIT_5; >>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>> - nv->frame_payload_size = 2048; >>>>> + nv->frame_payload_size = cpu_to_le16(2048); >>>>> nv->special_options[1] = BIT_7; >>>>> } else if (IS_QLA2200(ha)) { >>>>> nv->firmware_options[0] = BIT_2 | BIT_1; >>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>> nv->add_firmware_options[0] = BIT_5; >>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>> - nv->frame_payload_size = 1024; >>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>> } else if (IS_QLA2100(ha)) { >>>>> nv->firmware_options[0] = BIT_3 | BIT_1; >>>>> nv->firmware_options[1] = BIT_5; >>>>> - nv->frame_payload_size = 1024; >>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>> } >>>>> >>>>> nv->max_iocb_allocation = cpu_to_le16(256); >>>>> >>>>> -- >>>>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>>>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >>>> >>>> -- >>>> Himanshu Madhani Oracle Linux Engineering >>>> >>> >>> -- >>> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ >>> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ >> >> -- >> Himanshu Madhani Oracle Linux Engineering >> > > -- > ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6haBk6D$ > https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX4W9vLaa$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX04ZQeL4$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6DRH0bV$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXz0D-Wul$ -- Himanshu Madhani Oracle Linux Engineering ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 18:00 ` Himanshu Madhani @ 2020-08-21 20:21 ` René Rebe 2020-08-21 20:36 ` Himanshu Madhani 0 siblings, 1 reply; 11+ messages in thread From: René Rebe @ 2020-08-21 20:21 UTC (permalink / raw) To: Himanshu Madhani; +Cc: linux-scsi Hey, > On 21. Aug 2020, at 20:00, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: > > > >> On Aug 21, 2020, at 12:49 PM, René Rebe <rene@exactcode.de> wrote: >> >> Hi, >> >>> On 21. Aug 2020, at 17:17, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>> >>> Hi Rene, >>> >>>> On Aug 21, 2020, at 9:47 AM, René Rebe <rene@exactcode.de> wrote: >>>> >>>> Hey, >>>> >>>>> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>>>> >>>>> Hi Rene, >>>>> >>>>> >>>>>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >>>>>> >>>>>> >>>>>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>>>>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>>>>> Unbreak by fixing endianess in nvram firmware defaults >>>>>> initialization. >>>>>> >>>>> >>>>> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. >>>> >>>> Did I copy this wrong? >>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ >>>> >>> >>> Yes. The actual commit that change those lines is following >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXzu7NMDs$ >>> >>> So your fixes tag should be >>> >>> Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) >>> >>> And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. >>> >>> Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 >>> >>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX5pZx4bu$ >>> >>> Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. >> >> Well, linux kernels since 3.17 did not work on this adapter on sparc64: >> >> and I reverted the whole 9a50aaefc1b896e734bf7faf3d085f71a360ce97 and factored the changes out and tested what I sent with 5..8.1. >> >> This was also reported back in the say, but somehow never applied, ..? >> >> https://urldefense.com/v3/__https://lore.kernel.org/lkml/86A17A977688E04689F40F6B1EC20EC601940E06A9@AVMB1.qlogic.org/__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX9w-_N_o$ >> > > Wow. This should have been done the moment Author acknowledged the revert. I am stumped that we are still talking about this revert after 5+ yrs and its not done. Would you please send just plain revert request for commit 98aee70d19a7e rather than reverting whole merge window patch. Well, how many people care about vintage Sun sparc64 hardware, ..? ;-) IMHO it makes no sense to revert this in full today. It does not revert cleanly and all the other __le16 conversions did not cause regressions that I know of. So instead having one not __le16 member, I suggest the partial revert I posted initially. I can resend it with the updated Fixes: reference as requested /* Firmware Initialization Control Block. */ - uint16_t version; + __le16 version; uint16_t reserved_1; ++<<<<<<< HEAD + __le16 frame_payload_size; + __le16 execution_throttle; + __le16 exchange_count; + __le16 hard_address; ++======= + uint16_t frame_payload_size; + uint16_t execution_throttle; + uint16_t exchange_count; + uint16_t hard_address; ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. … @@@ -8411,11 -5903,11 +8437,19 @@@ qla81xx_nvram_config(scsi_qla_host_t *v * Set default initialization control block. */ memset(nv, 0, ha->nvram_size); ++<<<<<<< HEAD + nv->nvram_version = cpu_to_le16(ICB_VERSION); + nv->version = cpu_to_le16(ICB_VERSION); + nv->frame_payload_size = cpu_to_le16(2048); + nv->execution_throttle = cpu_to_le16(0xFFFF); + nv->exchange_count = cpu_to_le16(0); ++======= + nv->nvram_version = __constant_cpu_to_le16(ICB_VERSION); + nv->version = __constant_cpu_to_le16(ICB_VERSION); + nv->frame_payload_size = __constant_cpu_to_le16(2048); + nv->execution_throttle = __constant_cpu_to_le16(0xFFFF); + nv->exchange_count = __constant_cpu_to_le16(0); ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. … >> Regards, >> René >> >>>>> Can you please provide details of the commit which introduced this regression. >>>> >>>> See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … >>>> >>>> Have a nice weekend, >>>> René >>>> >>>>> Also when you >>>>> resubmit this patch please use Fixes tag. See documentation here for the correct format. >>>>> >>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >>>>> >>>>>> Signed-off-by: René Rebe <rene@exactcode.de> >>>>>> >>>>>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>>>>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>>>>> @@ -4603,18 +4603,18 @@ >>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>> - nv->frame_payload_size = 2048; >>>>>> + nv->frame_payload_size = cpu_to_le16(2048); >>>>>> nv->special_options[1] = BIT_7; >>>>>> } else if (IS_QLA2200(ha)) { >>>>>> nv->firmware_options[0] = BIT_2 | BIT_1; >>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>> - nv->frame_payload_size = 1024; >>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>> } else if (IS_QLA2100(ha)) { >>>>>> nv->firmware_options[0] = BIT_3 | BIT_1; >>>>>> nv->firmware_options[1] = BIT_5; >>>>>> - nv->frame_payload_size = 1024; >>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>> } >>>>>> >>>>>> nv->max_iocb_allocation = cpu_to_le16(256); >>>>>> >>>>>> -- >>>>>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>>>>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >>>>> >>>>> -- >>>>> Himanshu Madhani Oracle Linux Engineering >>>>> >>>> >>>> -- >>>> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ >>>> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ >>> >>> -- >>> Himanshu Madhani Oracle Linux Engineering >>> >> >> -- >> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6haBk6D$ >> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX4W9vLaa$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX04ZQeL4$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6DRH0bV$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXz0D-Wul$ > > -- > Himanshu Madhani Oracle Linux Engineering > -- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 20:21 ` René Rebe @ 2020-08-21 20:36 ` Himanshu Madhani 0 siblings, 0 replies; 11+ messages in thread From: Himanshu Madhani @ 2020-08-21 20:36 UTC (permalink / raw) To: René Rebe; +Cc: linux-scsi > On Aug 21, 2020, at 3:21 PM, René Rebe <rene@exactcode.de> wrote: > > Hey, > >> On 21. Aug 2020, at 20:00, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >> >> >> >>> On Aug 21, 2020, at 12:49 PM, René Rebe <rene@exactcode.de> wrote: >>> >>> Hi, >>> >>>> On 21. Aug 2020, at 17:17, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>>> >>>> Hi Rene, >>>> >>>>> On Aug 21, 2020, at 9:47 AM, René Rebe <rene@exactcode.de> wrote: >>>>> >>>>> Hey, >>>>> >>>>>> On 21. Aug 2020, at 15:49, Himanshu Madhani <himanshu.madhani@oracle.com> wrote: >>>>>> >>>>>> Hi Rene, >>>>>> >>>>>> >>>>>>> On Aug 21, 2020, at 7:28 AM, Rene Rebe <rene@exactcode.com> wrote: >>>>>>> >>>>>>> >>>>>>> Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke >>>>>>> qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. >>>>>>> Unbreak by fixing endianess in nvram firmware defaults >>>>>>> initialization. >>>>>>> >>>>>> >>>>>> I could not find this commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in Linus or SCSI tree. >>>>> >>>>> Did I copy this wrong? >>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/scsi/qla2xxx?id=9a50aaefc1b896e734bf7faf3d085f71a360ce97__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n_89mkIf$ >>>>> >>>> >>>> Yes. The actual commit that change those lines is following >>>> >>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=98aee70d19a7e3203649fa2078464e4f402a0ad8__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXzu7NMDs$ >>>> >>>> So your fixes tag should be >>>> >>>> Fixes: 98aee70d19a7e ("qla2xxx: Add endianizer to max_payload_size modifier.”) >>>> >>>> And looking at the changes, I see that the frame_payload_size was changed to __le16 so this should not cause regression in your env. >>>> >>>> Interestingly this patch had fixed the regression and was originally tested on SunBlade-1000 >>>> >>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4e08df3f91837656c36712f559d5ce8d80852760__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX5pZx4bu$ >>>> >>>> Can you try reverting 98aee70d19a7e and retest to confirm that this patch indeed introduced regression for you. >>> >>> Well, linux kernels since 3.17 did not work on this adapter on sparc64: >>> >>> and I reverted the whole 9a50aaefc1b896e734bf7faf3d085f71a360ce97 and factored the changes out and tested what I sent with 5..8.1. >>> >>> This was also reported back in the say, but somehow never applied, ..? >>> >>> https://urldefense.com/v3/__https://lore.kernel.org/lkml/86A17A977688E04689F40F6B1EC20EC601940E06A9@AVMB1.qlogic.org/__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX9w-_N_o$ >>> >> >> Wow. This should have been done the moment Author acknowledged the revert. I am stumped that we are still talking about this revert after 5+ yrs and its not done. Would you please send just plain revert request for commit 98aee70d19a7e rather than reverting whole merge window patch. > > Well, how many people care about vintage Sun sparc64 hardware, ..? ;-) > > IMHO it makes no sense to revert this in full today. It does not revert cleanly and all the other __le16 conversions did not cause regressions that I know of. > So instead having one not __le16 member, I suggest the partial revert I posted initially. I can resend it with the updated Fixes: reference as requested > Okay. I am fine with the patch you send earlier with Fixes tag and also add Cc to Stable as well so that it get ported in stable release. I would also like you to add link for the past discussion in the commit message so that we remember why this patch was added. > /* Firmware Initialization Control Block. */ > - uint16_t version; > + __le16 version; > uint16_t reserved_1; > ++<<<<<<< HEAD > + __le16 frame_payload_size; > + __le16 execution_throttle; > + __le16 exchange_count; > + __le16 hard_address; > ++======= > + uint16_t frame_payload_size; > + uint16_t execution_throttle; > + uint16_t exchange_count; > + uint16_t hard_address; > ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. > > … > > @@@ -8411,11 -5903,11 +8437,19 @@@ qla81xx_nvram_config(scsi_qla_host_t *v > * Set default initialization control block. > */ > memset(nv, 0, ha->nvram_size); > ++<<<<<<< HEAD > + nv->nvram_version = cpu_to_le16(ICB_VERSION); > + nv->version = cpu_to_le16(ICB_VERSION); > + nv->frame_payload_size = cpu_to_le16(2048); > + nv->execution_throttle = cpu_to_le16(0xFFFF); > + nv->exchange_count = cpu_to_le16(0); > ++======= > + nv->nvram_version = __constant_cpu_to_le16(ICB_VERSION); > + nv->version = __constant_cpu_to_le16(ICB_VERSION); > + nv->frame_payload_size = __constant_cpu_to_le16(2048); > + nv->execution_throttle = __constant_cpu_to_le16(0xFFFF); > + nv->exchange_count = __constant_cpu_to_le16(0); > ++>>>>>>> parent of 98aee70d19a7... qla2xxx: Add endianizer to max_payload_size modifier. > > … > >>> Regards, >>> René >>> >>>>>> Can you please provide details of the commit which introduced this regression. >>>>> >>>>> See above, there was plenty of more reworks and code shuffling, and the quoted commit is also a condensed changes of other /14 series and other scsi work, … >>>>> >>>>> Have a nice weekend, >>>>> René >>>>> >>>>>> Also when you >>>>>> resubmit this patch please use Fixes tag. See documentation here for the correct format. >>>>>> >>>>>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n620C3iF$ >>>>>> >>>>>>> Signed-off-by: René Rebe <rene@exactcode.de> >>>>>>> >>>>>>> --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 >>>>>>> +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 >>>>>>> @@ -4603,18 +4603,18 @@ >>>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>>> - nv->frame_payload_size = 2048; >>>>>>> + nv->frame_payload_size = cpu_to_le16(2048); >>>>>>> nv->special_options[1] = BIT_7; >>>>>>> } else if (IS_QLA2200(ha)) { >>>>>>> nv->firmware_options[0] = BIT_2 | BIT_1; >>>>>>> nv->firmware_options[1] = BIT_7 | BIT_5; >>>>>>> nv->add_firmware_options[0] = BIT_5; >>>>>>> nv->add_firmware_options[1] = BIT_5 | BIT_4; >>>>>>> - nv->frame_payload_size = 1024; >>>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>>> } else if (IS_QLA2100(ha)) { >>>>>>> nv->firmware_options[0] = BIT_3 | BIT_1; >>>>>>> nv->firmware_options[1] = BIT_5; >>>>>>> - nv->frame_payload_size = 1024; >>>>>>> + nv->frame_payload_size = cpu_to_le16(1024); >>>>>>> } >>>>>>> >>>>>>> nv->max_iocb_allocation = cpu_to_le16(256); >>>>>>> >>>>>>> -- >>>>>>> René Rebe, ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin >>>>>>> https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxI-1EV4u$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxAAjd8CY$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!KU2euUgVJIRXqzU0RdhkxvMdgtotnSUfdk9KmK73n26lyGlTMhmSvyadcgXXxDUjMdad$ >>>>>> >>>>>> -- >>>>>> Himanshu Madhani Oracle Linux Engineering >>>>>> >>>>> >>>>> -- >>>>> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-uIVWXr$ >>>>> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n2dniKkK$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n-v2wVJD$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_n9g--eFu$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!L6-irbVsNXqZh_TSb_WyaBZ1S3Evo18GqWr8hX2Z-tNl3rZbWv0orEt7Igy_nxllUxx0$ >>>> >>>> -- >>>> Himanshu Madhani Oracle Linux Engineering >>>> >>> >>> -- >>> ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6haBk6D$ >>> https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX4W9vLaa$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX04ZQeL4$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXX6DRH0bV$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!Jsf7iuXhoEimJ56ynA6RrwNm3nkfk3ATaMfNVTUhnU9-NfIZT3eVBiXtsQHXXz0D-Wul$ >> >> -- >> Himanshu Madhani Oracle Linux Engineering >> > > -- > ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://urldefense.com/v3/__https://exactcode.com__;!!GqivPVa7Brio!Pzi_Eeyo1QdtyP0r5gMlCBHiq2DPWzBnz2dTVcfduR8O8gSkBXIZ3RI4DP0Q74GDxqJE$ > https://urldefense.com/v3/__https://exactscan.com__;!!GqivPVa7Brio!Pzi_Eeyo1QdtyP0r5gMlCBHiq2DPWzBnz2dTVcfduR8O8gSkBXIZ3RI4DP0Q76FLfX-m$ | https://urldefense.com/v3/__https://ocrkit.com__;!!GqivPVa7Brio!Pzi_Eeyo1QdtyP0r5gMlCBHiq2DPWzBnz2dTVcfduR8O8gSkBXIZ3RI4DP0Q70Zxm72q$ | https://urldefense.com/v3/__https://t2sde.org__;!!GqivPVa7Brio!Pzi_Eeyo1QdtyP0r5gMlCBHiq2DPWzBnz2dTVcfduR8O8gSkBXIZ3RI4DP0Q79_X62BE$ | https://urldefense.com/v3/__https://rene.rebe.de__;!!GqivPVa7Brio!Pzi_Eeyo1QdtyP0r5gMlCBHiq2DPWzBnz2dTVcfduR8O8gSkBXIZ3RI4DP0Q71A3vvWa$ > -- Himanshu Madhani Oracle Linux Engineering ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix qla2xxx regression on sparc64 2020-08-21 12:28 Rene Rebe 2020-08-21 13:49 ` Himanshu Madhani @ 2020-08-21 20:55 ` Bart Van Assche 1 sibling, 0 replies; 11+ messages in thread From: Bart Van Assche @ 2020-08-21 20:55 UTC (permalink / raw) To: Rene Rebe, linux-scsi On 8/21/20 5:28 AM, Rene Rebe wrote: > > Commit 9a50aaefc1b896e734bf7faf3d085f71a360ce97 in 2014 broke > qla2xxx on sparc64, e.g. as in the Sun Blade 1000 / 2000. > Unbreak by fixing endianess in nvram firmware defaults > initialization. > > Signed-off-by: René Rebe <rene@exactcode.de> > > --- linux-5.8/drivers/scsi/qla2xxx/qla_init.c.vanilla 2020-08-21 09:55:18.600926737 +0200 > +++ linux-5.8/drivers/scsi/qla2xxx/qla_init.c 2020-08-21 09:57:35.992926885 +0200 > @@ -4603,18 +4603,18 @@ > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 2048; > + nv->frame_payload_size = cpu_to_le16(2048); > nv->special_options[1] = BIT_7; > } else if (IS_QLA2200(ha)) { > nv->firmware_options[0] = BIT_2 | BIT_1; > nv->firmware_options[1] = BIT_7 | BIT_5; > nv->add_firmware_options[0] = BIT_5; > nv->add_firmware_options[1] = BIT_5 | BIT_4; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = cpu_to_le16(1024); > } else if (IS_QLA2100(ha)) { > nv->firmware_options[0] = BIT_3 | BIT_1; > nv->firmware_options[1] = BIT_5; > - nv->frame_payload_size = 1024; > + nv->frame_payload_size = cpu_to_le16(1024); > } > > nv->max_iocb_allocation = cpu_to_le16(256); Hi Rene, Please keep the qla2xxx driver endianness-clean and include the following in your patch: diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 8c92af5e4390..2182b2da43ed 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -1626,7 +1626,7 @@ typedef struct { */ uint8_t firmware_options[2]; - uint16_t frame_payload_size; + __le16 frame_payload_size; __le16 max_iocb_allocation; __le16 execution_throttle; uint8_t retry_count; Thanks, Bart. ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-08-22 15:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-22 11:59 [PATCH] fix qla2xxx regression on sparc64 Rene Rebe 2020-08-22 15:53 ` Bart Van Assche -- strict thread matches above, loose matches on Subject: below -- 2020-08-21 12:28 Rene Rebe 2020-08-21 13:49 ` Himanshu Madhani 2020-08-21 14:47 ` René Rebe 2020-08-21 15:17 ` Himanshu Madhani 2020-08-21 17:49 ` René Rebe 2020-08-21 18:00 ` Himanshu Madhani 2020-08-21 20:21 ` René Rebe 2020-08-21 20:36 ` Himanshu Madhani 2020-08-21 20:55 ` Bart Van Assche
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox