s390 - Allow ECKD devices to be used with older controllers and emulation
Subsystems (2)
PATCH 1/1 s390 - Allow ECKD devices to be used with older controllers and emulation
Crowd,
Apologies if this is not formatted correctly, but I'm fairly new to this (submitting patches that is)..
**************
s390 dasd ECKD drivers issues a Perform Subsystem Function / Prepare
for Read SubSystem Data with a length of 16.
However, older hardware and documentation specifies a length of 12
leading to a possible Incorrect Length indication.
This patch activates the SLI CCW flag in order to avoid reporting
the Incorrect Length indication since it is possible that the DASD control unit may be expecting a length of 12, not 16.
-- Ivan Warren ([email protected])
drivers/s390/block/dasd_eckd.c +1 -0
@@ -810,6 +810,7 @@ static int dasd_eckd_read_features(struct dasd_device *device) ccw->cmd_code =DASD_ECKD_CCW_PSF ; ccw->count =sizeof (struct dasd_psf_prssd_data ); ccw->flags |=CCW_FLAG_CC ; + ccw->flags |=CCW_FLAG_SLI ; ccw->cda = (__u32 )(addr_t ) prssdp;/* Read Subsystem Data - feature codes */
**************
--Ivan
PATCH 1/1 s390 - Allow ECKD devices to be used with older controllers and emulation
Forwarding to s390 kernel list as there have been no replies so far.
(Patch may have whitespace damage because of forward.)
Ivan: You may want to read Documentation/SubmittingPatches in the kernel source tree.
--------------------------- Forwarded message --------------------------- Crowd,
Apologies if this is not formatted correctly, but I'm fairly new to this (submitting patches that is)..
**************
s390 dasd ECKD drivers issues a Perform Subsystem Function / Prepare
for Read SubSystem Data with a length of 16.
However, older hardware and documentation specifies a length of 12
leading to a possible Incorrect Length indication.
This patch activates the SLI CCW flag in order to avoid reporting
the Incorrect Length indication since it is possible that the DASD control unit may be expecting a length of 12, not 16.
-- Ivan Warren ([email protected])
drivers/s390/block/dasd_eckd.c +1 -0
@@ -810,6 +810,7 @@ static int dasd_eckd_read_features(struct dasd_device *device) ccw->cmd_code =DASD_ECKD_CCW_PSF ; ccw->count =sizeof (struct dasd_psf_prssd_data ); ccw->flags |=CCW_FLAG_CC ; + ccw->flags |=CCW_FLAG_SLI ; ccw->cda = (__u32 )(addr_t ) prssdp;/* Read Subsystem Data - feature codes */
**************
--Ivan
Quoted 13 lines · Frans Pop
s390 dasd ECKD drivers issues a Perform Subsystem Function / Prepare
for Read SubSystem Data with a length of 16.
However, older hardware and documentation specifies a length of 12
leading to a possible Incorrect Length indication.
This patch activates the SLI CCW flag in order to avoid reporting
the Incorrect Length indication since it is possible that the DASD control unit may be expecting a length of 12, not 16.
-- Ivan Warren ([email protected])
drivers/s390/block/dasd_eckd.c static int dasd_eckd_read_features(struct dasd_device *device) · line 816
Quoted 7 lines
drivers/s390/block/dasd_eckd.c +1
-0@@ -810,6 +810,7 @@ static int dasd_eckd_read_features(struct dasd_device *device) ccw->cmd_code =DASD_ECKD_CCW_PSF ; ccw->count =sizeof (struct dasd_psf_prssd_data ); ccw->flags |=CCW_FLAG_CC ; + ccw->flags |=CCW_FLAG_SLI ; ccw->cda = (__u32 )(addr_t ) prssdp;/* Read Subsystem Data - feature codes */
That makes sense, can I get a signed-off-by for the patch please?
Signature · 4 lines
blue skies, Martin. "Reality continues to ruin my life." - Calvin.
PATCH 1/1 S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation
s390 Dasd ECKD Driver issues a Perform Subsystem Function/Prepare for Read Subsystem Data with a length of 16.
However, some hardware (namely 3990/9390 and some version of ESS) only take 12 bytes and therefore, an Incorrect Length indication is returned, breaking CCW Command Chaining and leads to a failure to initialize the DASD. This patch sets the SLI CCW bit to prevent an incorrect length indication to be reported when the control unit expects a length of 12 instead of 16.
--- drivers/s390/block/dasd_eckd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/s390/block/dasd_eckd.c +1 -1
@@ -809,7 +809,7 @@ static int dasd_eckd_read_features(struct dasd_device *device) ccw = cqr->cpaddr; ccw->cmd_code =DASD_ECKD_CCW_PSF ; ccw->count =sizeof (struct dasd_psf_prssd_data ); - ccw->flags |=CCW_FLAG_CC ; + ccw->flags |=CCW_FLAG_CC |CCW_FLAG_SLI ; ccw->cda = (__u32 )(addr_t ) prssdp;/* Read Subsystem Data - feature codes */
Signature · 1 line
1.5.6.3
Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation
Quoted 8 lines · Ivan Warren
s390 Dasd ECKD Driver issues a Perform Subsystem Function/Prepare for Read Subsystem Data with a length of 16.
However, some hardware (namely 3990/9390 and some version of ESS) only take 12 bytes and therefore, an Incorrect Length indication is returned, breaking CCW Command Chaining and leads to a failure to initialize the DASD. This patch sets the SLI CCW bit to prevent an incorrect length indication to be reported when the control unit expects a length of 12 instead of 16.
Hi Ivan,
while I agree that your patch is technically correct, I think I'd rather like to fix the problematic data length instead of making the storage server ignore it. There is no need for the dasd_psf_prssd_data structure to be 16 bytes long as it is only used for suborders that require 12 bytes. The current layout of that structure was probably just a mistake. If we ever go to use a suborder that requires more data, we should either define a further structure specific to that purpose or find some other way to make sure that each suborder is called with the appropriate data length.
The following patch works fine on current hardware. Can you please verify that it indeed fixes your problem as well?
--- drivers/s390/block/dasd_eckd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/s390/block/dasd_eckd.h +1 -1
@@ -379,7 +379,7 @@ struct dasd_psf_prssd_data {unsigned char flags;unsigned char reserved[4 ];unsigned char suborder; -unsigned char varies[9 ]; +unsigned char varies[5 ]; } __attribute__ ((packed));/*
Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation
Quoted 27 lines · Stefan Weinhuber
Hi Ivan,
while I agree that your patch is technically correct, I think I'd rather like to fix the problematic data length instead of making the storage server ignore it. There is no need for the
dasd_psf_prssd_datastructure to be 16 bytes long as it is only used for suborders that require 12 bytes. The current layout of that structure was probably just a mistake. If we ever go to use a suborder that requires more data, we should either define a further structure specific to that purpose or find some other way to make sure that each suborder is called with the appropriate data length.The following patch works fine on current hardware. Can you please verify that it indeed fixes your problem as well?
Signed-off-by Stefan Weinhuber <[email protected]>--- drivers/s390/block/dasd_eckd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Index:
linux-2.6.26/drivers/s390/block/dasd_eckd.h
===================================================================
drivers/s390/block/dasd_eckd.h struct dasd_psf_prssd_data { · line 385
Quoted 8 lines
drivers/s390/block/dasd_eckd.h +1
-1@@ -379,7 +379,7 @@ struct dasd_psf_prssd_data {unsigned char flags;unsigned char reserved[4 ];unsigned char suborder; -unsigned char varies[9 ]; +unsigned char varies[5 ]; } __attribute__ ((packed));/*
Stefan,
Sorry for taking such a long time to respond..
I like the patch. You are right, it's probably better to set the right length then hide it ! My initial rationale to SLI instead of altering the length was that I couldn't know if there was any obscure reason for setting the length to 16 instead of 12 (I thought it could have been related to DS8K PAV/HyperPAV support)..
I also tried it on my rig, and it does seem to take care of the problem !
So I guess :
Thanks again !
--Ivan
Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation
Quoted 4 lines · Ivan Warren
Quoted 4 lines
The following patch works fine on current hardware. Can you please verify that it indeed fixes your problem as well?
Signed-off-by Stefan Weinhuber <[email protected]>
[...]
Quoted 8 lines
Sorry for taking such a long time to respond..
I like the patch. You are right, it's probably better to set the right length then hide it ! My initial rationale to SLI instead of altering the length was that I couldn't know if there was any obscure reason for setting the length to 16 instead of 12 (I thought it could have been related to DS8K PAV/HyperPAV support)..
Quoted 4 lines
I also tried it on my rig, and it does seem to take care of the problem
Reviewed-by Ivan Warren <[email protected]>Tested-by Ivan Warren <[email protected]>
Could this patch please also be pushed for the next stable update (2.6.26 and maybe even 2.6.25) by adding:
CC: stable <[email protected]>
?
TIA,
FJP
Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation
Quoted 16 lines · Frans Pop
Quoted 4 lines · Ivan Warren
Quoted 4 lines
The following patch works fine on current hardware. Can you please verify that it indeed fixes your problem as well?
Signed-off-by Stefan Weinhuber <[email protected]>[...]
Quoted 8 lines
Sorry for taking such a long time to respond..
I like the patch. You are right, it's probably better to set the right length then hide it ! My initial rationale to SLI instead of altering the length was that I couldn't know if there was any obscure reason for setting the length to 16 instead of 12 (I thought it could have been related to DS8K PAV/HyperPAV support)..
Quoted 8 lines
Quoted 4 lines
I also tried it on my rig, and it does seem to take care of the problem
Reviewed-by Ivan Warren <[email protected]>Tested-by Ivan Warren <[email protected]>Could this patch please also be pushed for the next stable update (2.6.26 and maybe even 2.6.25) by adding:
CC: stable <[email protected]>
Ivan,
thanks for reporting the problem and reviewing the patch. I've submitted the patch to our local repository, so it can be picked up by Martin, our maintainer, for further processing.
Frans,
thanks for your suggestion. I've added a respective CC: stable line as well.