* Fwd: [Patch 2.6] s390 - Allow ECKD devices to be used with older controllers and emulation [not found] <489C9494.9070504@vmfacility.fr> @ 2008-08-13 23:07 ` Frans Pop 2008-08-14 15:07 ` Martin Schwidefsky 0 siblings, 1 reply; 7+ messages in thread From: Frans Pop @ 2008-08-13 23:07 UTC (permalink / raw) To: linux-s390; +Cc: Ivan Warren, linux-kernel 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 (ivan@vmfacility.fr) diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index 773b3fe..c4e3935 100644 --- a/drivers/s390/block/dasd_eckd.c +++ b/drivers/s390/block/dasd_eckd.c @@ -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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Fwd: [Patch 2.6] s390 - Allow ECKD devices to be used with older controllers and emulation 2008-08-13 23:07 ` Fwd: [Patch 2.6] s390 - Allow ECKD devices to be used with older controllers and emulation Frans Pop @ 2008-08-14 15:07 ` Martin Schwidefsky 2008-08-14 18:26 ` [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation Ivan Warren 0 siblings, 1 reply; 7+ messages in thread From: Martin Schwidefsky @ 2008-08-14 15:07 UTC (permalink / raw) To: Frans Pop; +Cc: linux-s390, Ivan Warren, linux-kernel On Thu, 2008-08-14 at 01:07 +0200, Frans Pop wrote: > 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 (ivan@vmfacility.fr) > > diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index 773b3fe..c4e3935 100644 > --- a/drivers/s390/block/dasd_eckd.c > +++ b/drivers/s390/block/dasd_eckd.c > @@ -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? -- blue skies, Martin. "Reality continues to ruin my life." - Calvin. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation 2008-08-14 15:07 ` Martin Schwidefsky @ 2008-08-14 18:26 ` Ivan Warren 2008-08-15 14:15 ` Stefan Weinhuber 0 siblings, 1 reply; 7+ messages in thread From: Ivan Warren @ 2008-08-14 18:26 UTC (permalink / raw) To: schwidefsky; +Cc: elendil, linux-s390, linux-kernel, 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. Signed-off-by: Ivan Warren <ivan@vmfacility.fr> --- drivers/s390/block/dasd_eckd.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/s390/block/dasd_eckd.c b/drivers/s390/block/dasd_eckd.c index 773b3fe..d644715 100644 --- a/drivers/s390/block/dasd_eckd.c +++ b/drivers/s390/block/dasd_eckd.c @@ -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 */ -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation 2008-08-14 18:26 ` [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation Ivan Warren @ 2008-08-15 14:15 ` Stefan Weinhuber 2008-08-18 17:08 ` Ivan Warren 0 siblings, 1 reply; 7+ messages in thread From: Stefan Weinhuber @ 2008-08-15 14:15 UTC (permalink / raw) To: Ivan Warren; +Cc: schwidefsky, elendil, linux-s390, linux-kernel Ivan Warren wrote: > 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? Signed-off-by: Stefan Weinhuber <wein@de.ibm.com> --- 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 =================================================================== --- linux-2.6.26.orig/drivers/s390/block/dasd_eckd.h +++ linux-2.6.26/drivers/s390/block/dasd_eckd.h @@ -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)); /* ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation 2008-08-15 14:15 ` Stefan Weinhuber @ 2008-08-18 17:08 ` Ivan Warren 2008-08-18 17:56 ` Frans Pop 0 siblings, 1 reply; 7+ messages in thread From: Ivan Warren @ 2008-08-18 17:08 UTC (permalink / raw) To: Stefan Weinhuber; +Cc: schwidefsky, elendil, linux-s390, linux-kernel Stefan Weinhuber wrote: > 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? > > Signed-off-by: Stefan Weinhuber <wein@de.ibm.com> > --- > 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 > =================================================================== > --- linux-2.6.26.orig/drivers/s390/block/dasd_eckd.h > +++ linux-2.6.26/drivers/s390/block/dasd_eckd.h > @@ -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 : Reviewed-by: Ivan Warren <ivan@vmfacility.fr> Tested-by: Ivan Warren <ivan@vmfacility.fr> Thanks again ! --Ivan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation 2008-08-18 17:08 ` Ivan Warren @ 2008-08-18 17:56 ` Frans Pop 2008-08-21 16:32 ` Stefan Weinhuber 0 siblings, 1 reply; 7+ messages in thread From: Frans Pop @ 2008-08-18 17:56 UTC (permalink / raw) To: Stefan Weinhuber; +Cc: Ivan Warren, schwidefsky, linux-s390, linux-kernel On Monday 18 August 2008, Ivan Warren wrote: > > 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 <wein@de.ibm.com> [...] > 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 > > Reviewed-by: Ivan Warren <ivan@vmfacility.fr> > Tested-by: Ivan Warren <ivan@vmfacility.fr> 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 <stable@kernel.org> ? TIA, FJP ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation 2008-08-18 17:56 ` Frans Pop @ 2008-08-21 16:32 ` Stefan Weinhuber 0 siblings, 0 replies; 7+ messages in thread From: Stefan Weinhuber @ 2008-08-21 16:32 UTC (permalink / raw) To: Frans Pop; +Cc: Ivan Warren, schwidefsky, linux-s390, linux-kernel On Mon, Aug 18, 2008 at 07:56:49PM +0200, Frans Pop wrote: > On Monday 18 August 2008, Ivan Warren wrote: > > > 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 <wein@de.ibm.com> > > [...] > > > 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 > > > > Reviewed-by: Ivan Warren <ivan@vmfacility.fr> > > Tested-by: Ivan Warren <ivan@vmfacility.fr> > > 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 <stable@kernel.org> 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-08-21 16:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <489C9494.9070504@vmfacility.fr>
2008-08-13 23:07 ` Fwd: [Patch 2.6] s390 - Allow ECKD devices to be used with older controllers and emulation Frans Pop
2008-08-14 15:07 ` Martin Schwidefsky
2008-08-14 18:26 ` [PATCH] S390 : Set SLI on PSF/PRSSD on Dasd ECKD initialisation Ivan Warren
2008-08-15 14:15 ` Stefan Weinhuber
2008-08-18 17:08 ` Ivan Warren
2008-08-18 17:56 ` Frans Pop
2008-08-21 16:32 ` Stefan Weinhuber
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox