On 03/16/2016 02:26 PM, Gabriel Krisman Bertazi wrote:
> Some new enclosures, like ESLS only supports a limited buffer of 4k
> bytes during SCSI Write Buffer calls. This means that doing a single
> Write Buffer with mode 0Eh to download a large microcode to these
> enclosures is likely to fail with an Illegal Request. This patch
> modifies the code that writes the microcode to account for this
> limitation, by downloading the microcode in segments of size 4k bytes,
> instead of a single large buffer.
>
> This means that now, we need to do several Write Buffers to update
> firmware in enclosures like 5887, which don't have this buffer size
> limitation. From my tests, this is not an issue with 5887, and the
> performance impact is irrelevant.
I think I'd prefer if we managed to only change the download behavior
on the new drawer rather than affecting all older SAS enclosures as well,
since this then impacts 5802, 5803, 5886, 5887, 5888, EDR1, as well as embedded
enclosures in a whole bunch of systems going back quite a ways.
What happens if we try a write buffer with the large buffer on the new drawer?
Do we get an illegal request sense key back? If so, we could change the logic to
something like:
1. Try sending mode E, with the full buffer
2. If we get an Illegal Request, try sending mode E with the smaller buffer
3. If we get an Illegal Request, revert to mode5
>
> Signed-off-by: Gabriel Krisman Bertazi <kris...@linux.vnet.ibm.com>
> ---
> iprlib.c | 51 ++++++++++++++++++++++++++++++++-------------------
> iprlib.h | 2 ++
> 2 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/iprlib.c b/iprlib.c
> index eb077a5..0ede679 100644
> --- a/iprlib.c
> +++ b/iprlib.c
> @@ -4655,8 +4655,8 @@ int ipr_resume_device_bus(struct ipr_dev *dev,
> * Returns:
> * 0 if success / non-zero on failure
> **/
> -static int __ipr_write_buffer(struct ipr_dev *dev, u8 mode, void *buff, int
> length,
> - struct sense_data_t *sense_data)
> +static int __ipr_write_buffer(struct ipr_dev *dev, u8 mode, void *buff,
> + int offset, int length, struct sense_data_t
> *sense_data)
> {
> u32 direction = length ? SG_DXFER_TO_DEV : SG_DXFER_NONE;
> u8 cdb[IPR_CCB_CDB_LEN];
> @@ -4670,6 +4670,9 @@ static int __ipr_write_buffer(struct ipr_dev *dev, u8
> mode, void *buff, int leng
>
> cdb[0] = WRITE_BUFFER;
> cdb[1] = mode;
> + cdb[3] = (offset & 0xff0000) >> 16;
> + cdb[4] = (offset & 0x00ff00) >> 8;
> + cdb[5] = (offset & 0x0000ff);
> cdb[6] = (length & 0xff0000) >> 16;
> cdb[7] = (length & 0x00ff00) >> 8;
> cdb[8] = length & 0x0000ff;
> @@ -4746,19 +4752,26 @@ int ipr_write_buffer(struct ipr_dev *dev, void *buff,
> int length)
>
> if (sas_ses) {
> if (!mode5) {
> - for (i = 0; i < (sas_ses_retries + 1); i++) {
> - rc = __ipr_write_buffer(dev, 0x0e, buff,
> length, &sense_data);
> - if (!rc) {
> - break;
> - } else if (rc && sense_data.sense_key ==
> UNIT_ATTENTION &&
> - sense_data.add_sense_code == 0x29 &&
> - sense_data.add_sense_code_qual ==
> 0x00) {
> - continue;
> - } else if (rc && sense_data.sense_key ==
> ILLEGAL_REQUEST) {
> - mode5 = 1;
> - break;
> - } else if (rc) {
> - goto out;
> + for (offset = 0; offset < length; offset += chunk_size)
> {
I think we want to add !mode5 as a loop conditional here as well. If we end up
setting mode5 = 1
as highlighted below, we want to break out immediately of this new outer loop
rather than
sending a whole bunch of write buffer commands and hitting a whole bunch of
illegal requests.
> + remain = length - offset;
> + chunk_size = (remain < MAX_BUFFER_CHUNK)?
> + remain : MAX_BUFFER_CHUNK;
> +
> + for (i = 0; i < (sas_ses_retries + 1); i++) {
> + rc = __ipr_write_buffer(dev, 0x0e, buff
> + offset, offset,
> + chunk_size,
> &sense_data);
> + if (!rc) {
> + break;
> + } else if (rc && sense_data.sense_key
> == UNIT_ATTENTION &&
> + sense_data.add_sense_code ==
> 0x29 &&
> +
> sense_data.add_sense_code_qual == 0x00) {
> + continue;
> + } else if (rc && sense_data.sense_key
> == ILLEGAL_REQUEST) {
> + mode5 = 1;
> + break;
^^^^^
> + } else if (rc) {
> + goto out;
> + }
> }
> }
> }
> diff --git a/iprlib.h b/iprlib.h
> index ee024b0..b8c0e39 100644
> --- a/iprlib.h
> +++ b/iprlib.h
> @@ -198,6 +198,8 @@ typedef uint64_t u64;
> #define REPORT_LUNS 0xA0
> #endif
>
> +#define MAX_BUFFER_CHUNK 4096
WRITE_BUFFER_CHUNK_SZ might be a more descriptive name
> +
> #define IPR_XLATE_DEV_FMT_RC(rc) ((((rc) & 127) == 51) ? -EIO : 0)
> #define IPR_TYPE_AF_DISK 0xC
> #define IPR_TYPE_ADAPTER 0x1f
>
--
Brian King
Power Linux I/O
IBM Linux Technology Center
------------------------------------------------------------------------------
Transform Data into Opportunity.
Accelerate data analysis in your applications with
Intel Data Analytics Acceleration Library.
Click to learn more.
http://pubads.g.doubleclick.net/gampad/clk?id=278785351&iu=/4140
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
