Skip to contentSkip to conversation

ALUA support for scsi_debug

PATCH In review Hannes Reinecke 3 participants 9 messages Oct 18, 2006 – Oct 20, 2006 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ALUA support for scsi_debug

Hi all,

I've hacked up scsi_debug to support REPORT TARGET PORT GROUPS. In combination with vpd_use_hostno=0 it'll now support multipathing nicely.
Doug, there is reference to port_a and port_b in the VPD page support. However, port_b appearently is just a fake entry with no (virtual) device backing.
We could eg use the 'channel' value to refer the relative port number, so that host & channel act as a fan-out for multipathing.

James, please apply.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstra�e 5				+49 911 74053 688
90409 N�rnberg				http://www.suse.de

scsi_debug: support REPORT TARGET PORT GROUPS

This patch adds support for REPORT TARGET PORT GROUPS. This is used eg for the multipathing priority callout to determine the path priority.
With this patch multipath-tools can use the existing mpath_prio_alua callout to exercise the path priority grouping.

drivers/scsi/scsi_debug.c +97 -9
index 9c0f358..e8c72ea 100644
@@ -254,6 +254,8 @@ static int resp_requests(struct scsi_cmn
 			 struct sdebug_dev_info * devip);
 static int resp_start_stop(struct scsi_cmnd * scp,
 			   struct sdebug_dev_info * devip);
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip);
 static int resp_readcap(struct scsi_cmnd * SCpnt,
 			struct sdebug_dev_info * devip);
 static int resp_readcap16(struct scsi_cmnd * SCpnt,
@@ -287,9 +289,9 @@ static void __init sdebug_build_parts(un
 static void __init init_all_queued(void);
 static void stop_all_queued(void);
 static int stop_queued_cmnd(struct scsi_cmnd * cmnd);
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
-			   int dev_id_str_len);
+static int inquiry_evpd_83(unsigned char * arr, int target_port_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str, int dev_id_str_len);
 static int inquiry_evpd_88(unsigned char * arr, int target_dev_id);
 static int do_create_driverfs_files(void);
 static void do_remove_driverfs_files(void);
@@ -422,6 +424,15 @@ int scsi_debug_queuecommand(struct scsi_
 		}
 		errsts = resp_readcap16(SCpnt, devip);
 		break;
+	case MAINTENANCE_IN:
+		if (MI_REPORT_TARGET_PGS != cmd[1]) {
+			mk_sense_buffer(devip, ILLEGAL_REQUEST,
+					INVALID_OPCODE, 0);
+			errsts = check_condition_result;
+			break;
+		}
+		errsts = resp_report_tgtpgs(SCpnt, devip);
+		break;
 	case READ_16:
 	case READ_12:
 	case READ_10:
@@ -665,8 +676,9 @@ static const char * inq_vendor_id = "Lin
 static const char * inq_product_id = "scsi_debug      ";
 static const char * inq_product_rev = "0004";
 
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
+static int inquiry_evpd_83(unsigned char * arr, int target_port_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str,
 			   int dev_id_str_len)
 {
 	int num, port_a;
@@ -707,7 +719,7 @@ static int inquiry_evpd_83(unsigned char
 		arr[num++] = 0x0;
 		arr[num++] = 0x1;	/* relative port A */
 	}
-	/* NAA-5, Target port identifier */
+	/* NAA-5, Relative Target port identifier */
 	arr[num++] = 0x61;	/* proto=sas, binary */
 	arr[num++] = 0x93;	/* piv=1, target port, naa */
 	arr[num++] = 0x0;
@@ -720,6 +732,15 @@ static int inquiry_evpd_83(unsigned char
 	arr[num++] = (port_a >> 16) & 0xff;
 	arr[num++] = (port_a >> 8) & 0xff;
 	arr[num++] = port_a & 0xff;
+	/* NAA-5, Target port group identifier */
+	arr[num++] = 0x61;	/* proto=sas, binary */
+	arr[num++] = 0x95;	/* piv=1, target port group id */
+	arr[num++] = 0x0;
+	arr[num++] = 0x4;
+	arr[num++] = 0;
+	arr[num++] = 0;
+	arr[num++] = (target_port_id >> 8) & 0xff;
+	arr[num++] = target_port_id & 0xff;
 	/* NAA-5, Target device identifier */
 	arr[num++] = 0x61;	/* proto=sas, binary */
 	arr[num++] = 0xa3;	/* piv=1, target device, naa */
@@ -946,10 +967,11 @@ static int resp_inquiry(struct scsi_cmnd
 			       	0);
 		return check_condition_result;
 	} else if (0x1 & cmd[1]) {  /* EVPD bit set */
-		int lu_id_num, target_dev_id, len;
+		int lu_id_num, target_port_id, target_dev_id, len;
 		char lu_id_str[6];
 		int host_no = devip->sdbg_host->shost->host_no;
 		
+		target_port_id = (host_no + 1);
 		if (0 == scsi_debug_vpd_use_hostno)
 			host_no = 0;
 		lu_id_num = devip->wlun ? -1 : (((host_no + 1) * 2000) +
@@ -977,8 +999,9 @@ static int resp_inquiry(struct scsi_cmnd
 			memcpy(&arr[4], lu_id_str, len);
 		} else if (0x83 == cmd[2]) { /* device identification */
 			arr[1] = cmd[2];	/*sanity */
-			arr[3] = inquiry_evpd_83(&arr[4], target_dev_id,
-						 lu_id_num, lu_id_str, len);
+			arr[3] = inquiry_evpd_83(&arr[4], target_port_id,
+						 target_dev_id, lu_id_num,
+						 lu_id_str, len);
 		} else if (0x84 == cmd[2]) { /* Software interface ident. */
 			arr[1] = cmd[2];	/*sanity */
 			arr[3] = inquiry_evpd_84(&arr[4]);
@@ -1023,6 +1046,8 @@ static int resp_inquiry(struct scsi_cmnd
 	arr[2] = scsi_debug_scsi_level;
 	arr[3] = 2;    /* response_data_format==2 */
 	arr[4] = SDEBUG_LONG_INQ_SZ - 5;
+	if (0 == scsi_debug_vpd_use_hostno)
+		arr[5] = 0x10; /* claim: implicit TGPS */
 	arr[6] = 0x10; /* claim: MultiP */
 	/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
 	arr[7] = 0xa; /* claim: LINKED + CMDQUE */
@@ -1171,6 +1196,69 @@ static int resp_readcap16(struct scsi_cm
 				    min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
 }
 
+#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
+
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip)
+{
+	unsigned char *cmd = (unsigned char *)scp->cmnd;
+	unsigned char arr[SDEBUG_MAX_TGTPGS_ARR_SZ];
+	int n, alloc_len, target_port_id, target_dev_id, port_a;
+
+	alloc_len = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
+		     + cmd[9]);
+	if (alloc_len < 4 + 11) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
+			       	0);
+		return check_condition_result;
+	}
+	
+	memset(arr, 0, SDEBUG_MAX_TGTPGS_ARR_SZ);
+	n = 4;
+	/*
+	 * Each host has it's own target port group.
+	 * The asymmetric access state is cycled according to the host_id.
+	 */
+
+	/* Construct target port group descriptors */
+	target_dev_id = ((devip->sdbg_host->shost->host_no + 1) * 2000) +
+			(devip->target * 1000) - 3;
+	port_a = target_dev_id + 1;
+	target_port_id = devip->sdbg_host->shost->host_no + 1;
+	/* We can only support ALUA if the hostno is ignored */
+	if (0 == scsi_debug_vpd_use_hostno) {
+		arr[n++] = target_port_id % 4; /* Asymm access state */
+		arr[n++] = 0x0F; /* claim: all states are supported */
+	} else {
+		arr[n++] = 0x0; /* Optimal path */
+		arr[n++] = 0x08; /* We only have optimal paths */
+	}
+	arr[n++] = (target_port_id >> 8) & 0xff;
+	arr[n++] = target_port_id & 0xff;
+	arr[n++] = 0;   /* Reserved */
+	arr[n++] = 0;   /* Status code */
+	arr[n++] = 0;   /* Vendor unique */
+	arr[n++] = 0x1; /* count. Why not scsi_debug_num_tgts? */
+	arr[n++] = 0;   /* Reserved */
+	arr[n++] = 0;   /* Reserved */
+	arr[n++] = (port_a >> 8) & 0xff;
+	arr[n++] = port_a & 0xff;
+
+	arr[0] = (n >> 24) & 0xff;
+	arr[1] = (n >> 16) & 0xff;
+	arr[2] = (n >> 8) & 0xff;
+	arr[3] = n & 0xff;
+
+	if (alloc_len < n) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
+			       	0);
+		return check_condition_result;
+	}
+	
+	return fill_from_dev_buffer(scp, arr,
+				    min(n, SDEBUG_MAX_TGTPGS_ARR_SZ));
+}
+
 /* <<Following mode page info copied from ST318451LW>> */
 
 static int resp_err_recov_pg(unsigned char * p, int pcontrol, int target)
include/scsi/scsi.h +3 -0
index 84a6d5f..8a3f0bd 100644
@@ -97,6 +97,7 @@ #define MODE_SENSE_10         0x5a
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define REPORT_LUNS           0xa0
+#define MAINTENANCE_IN        0xa3
 #define MOVE_MEDIUM           0xa5
 #define EXCHANGE_MEDIUM       0xa6
 #define READ_12               0xa8
@@ -114,6 +115,8 @@ #define VERIFY_16	      0x8f
 #define SERVICE_ACTION_IN     0x9e
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
+/* values for maintenance in */
+#define MI_REPORT_TARGET_PGS  0x0a
 
 /* Values for T10/04-262r7 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */
#1 lore raw
Quoted 8 lines · Hannes Reinecke

Hi all,

I've hacked up scsi_debug to support REPORT TARGET PORT GROUPS. In combination with vpd_use_hostno=0 it'll now support multipathing nicely.
Doug, there is reference to port_a and port_b in the VPD page support. However, port_b appearently is just a fake entry with no (virtual) device backing.

Hannes,
Yes, I was mimicking a SAS drive with the secondary port unconnected.

Quoted 2 lines

We could eg use the 'channel' value to refer the relative port number, so that host & channel act as a fan-out for multipathing.

I'll have a look, perhaps it could be wired when vpd_use_hostno=0 .

Quoted 1 line

James, please apply.

Umm, I would like to find out why that patch oopses my machine when I load the scsi_debug module.

lk 2.6.19-rc2 has regressed in a few areas, for example after an oops, inane messages from atkbd repeat ad nauseum, making the oops hard to analyse. And my SMC2835W (prism54 driver) no longer works.

Doug Gilbert

#2 lore raw
Quoted 10 lines · Douglas Gilbert
Quoted 8 lines · Hannes Reinecke

Hi all,

I've hacked up scsi_debug to support REPORT TARGET PORT GROUPS. In combination with vpd_use_hostno=0 it'll now support multipathing nicely.
Doug, there is reference to port_a and port_b in the VPD page support. However, port_b appearently is just a fake entry with no (virtual) device backing.

Quoted 4 lines

Hannes,
Yes, I was mimicking a SAS drive with the secondary port unconnected.

Hmm. We could to that, too. Just set the secondary port to unavailable. Let's see ...

Quoted 6 lines
Quoted 2 lines

We could eg use the 'channel' value to refer the relative port number, so that host & channel act as a fan-out for multipathing.

I'll have a look, perhaps it could be wired when
vpd_use_hostno=0 .

Yep, that would be possible.

Quoted 5 lines
Quoted 1 line

James, please apply.

Umm, I would like to find out why that patch oopses my machine when I load the scsi_debug module.

Not my fault (I hope). Patch works when compiling under 2.6.18 ...

Cheers,

Hannes

Signature · 8 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw
In reply to Hannes Reinecke
Quoted 11 lines · Hannes Reinecke

Hi all,

I've hacked up scsi_debug to support REPORT TARGET PORT GROUPS. In combination with vpd_use_hostno=0 it'll now support multipathing nicely.
Doug, there is reference to port_a and port_b in the VPD page support. However, port_b appearently is just a fake entry with no (virtual) device backing.
We could eg use the 'channel' value to refer the relative port number, so that host & channel act as a fan-out for multipathing.

Quoted 1 line

James, please apply.

Not quite yet ...

Hannes,
Here is my counter patch for you to consider. I suspect my machine was running into stack problems so I moved some arrays onto the heap.

You will see some other small changes if you compare the two patches. My patch is against lk 2.6.19-rc2

Doug Gilbert

include/scsi/scsi.h2619rc2alua renamed from include/scsi/scsi.h +3 -0
@@ -97,6 +97,7 @@
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define REPORT_LUNS           0xa0
+#define MAINTENANCE_IN        0xa3
 #define MOVE_MEDIUM           0xa5
 #define EXCHANGE_MEDIUM       0xa6
 #define READ_12               0xa8
@@ -114,6 +115,8 @@
 #define SERVICE_ACTION_IN     0x9e
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
+/* values for maintenance in */
+#define MI_REPORT_TARGET_PGS  0x0a
 
 /* Values for T10/04-262r7 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */
drivers/scsi/scsi_debug.c2619rc2alua2 renamed from drivers/scsi/scsi_debug.c +112 -14

217 lines not on this page · raw message

#5 lore raw
Quoted 12 lines · Douglas Gilbert
Quoted 11 lines · Hannes Reinecke

Hi all,

I've hacked up scsi_debug to support REPORT TARGET PORT GROUPS. In combination with vpd_use_hostno=0 it'll now support multipathing nicely.
Doug, there is reference to port_a and port_b in the VPD page support. However, port_b appearently is just a fake entry with no (virtual) device backing.
We could eg use the 'channel' value to refer the relative port number, so that host & channel act as a fan-out for multipathing.

Quoted 9 lines
Quoted 1 line

James, please apply.

Not quite yet ...

Hannes,
Here is my counter patch for you to consider. I suspect my machine was running into stack problems so I moved some arrays onto the heap.

Ah. Good. To tell the truth I was a bit concerned once I've discovered that all information pages are infact statically allocated ...

Quoted 4 lines

You will see some other small changes if you compare the two patches. My patch is against lk 2.6.19-rc2

Looks good. Only there was a small glitch in my implementation (0x08 actually means that only the 'unavailable' status is supported :-( ), so I've re-rolled it and added support for you unconnected port_b, too. So we now have two port groups per host, one with the actual device and the other one with the unconnected port_b's.

What do you think?

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
drivers/scsi/scsi_debug.c +127 -14
index 9c0f358..10385e7 100644
@@ -52,7 +52,7 @@ #include "scsi_logging.h"
 #include "scsi_debug.h"
 
 #define SCSI_DEBUG_VERSION "1.80"
-static const char * scsi_debug_version_date = "20060914";
+static const char * scsi_debug_version_date = "20061018";
 
 /* Additional Sense Code (ASC) used */
 #define NO_ADDITIONAL_SENSE 0x0
@@ -254,6 +254,8 @@ static int resp_requests(struct scsi_cmn
 			 struct sdebug_dev_info * devip);
 static int resp_start_stop(struct scsi_cmnd * scp,
 			   struct sdebug_dev_info * devip);
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip);
 static int resp_readcap(struct scsi_cmnd * SCpnt,
 			struct sdebug_dev_info * devip);
 static int resp_readcap16(struct scsi_cmnd * SCpnt,
@@ -287,9 +289,9 @@ static void __init sdebug_build_parts(un
 static void __init init_all_queued(void);
 static void stop_all_queued(void);
 static int stop_queued_cmnd(struct scsi_cmnd * cmnd);
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
-			   int dev_id_str_len);
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str, int dev_id_str_len);
 static int inquiry_evpd_88(unsigned char * arr, int target_dev_id);
 static int do_create_driverfs_files(void);
 static void do_remove_driverfs_files(void);
@@ -422,6 +424,15 @@ int scsi_debug_queuecommand(struct scsi_
 		}
 		errsts = resp_readcap16(SCpnt, devip);
 		break;
+	case MAINTENANCE_IN:
+		if (MI_REPORT_TARGET_PGS != cmd[1]) {
+			mk_sense_buffer(devip, ILLEGAL_REQUEST,
+					INVALID_OPCODE, 0);
+			errsts = check_condition_result;
+			break;
+		}
+		errsts = resp_report_tgtpgs(SCpnt, devip);
+		break;
 	case READ_16:
 	case READ_12:
 	case READ_10:
@@ -665,8 +676,9 @@ static const char * inq_vendor_id = "Lin
 static const char * inq_product_id = "scsi_debug      ";
 static const char * inq_product_rev = "0004";
 
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str,
 			   int dev_id_str_len)
 {
 	int num, port_a;
@@ -720,6 +732,15 @@ static int inquiry_evpd_83(unsigned char
 	arr[num++] = (port_a >> 16) & 0xff;
 	arr[num++] = (port_a >> 8) & 0xff;
 	arr[num++] = port_a & 0xff;
+	/* NAA-5, Target port group identifier */
+	arr[num++] = 0x61;	/* proto=sas, binary */
+	arr[num++] = 0x95;	/* piv=1, target port group id */
+	arr[num++] = 0x0;
+	arr[num++] = 0x4;
+	arr[num++] = 0;
+	arr[num++] = 0;
+	arr[num++] = (port_group_id >> 8) & 0xff;
+	arr[num++] = port_group_id & 0xff;
 	/* NAA-5, Target device identifier */
 	arr[num++] = 0x61;	/* proto=sas, binary */
 	arr[num++] = 0xa3;	/* piv=1, target device, naa */
@@ -928,12 +949,12 @@ static int resp_inquiry(struct scsi_cmnd
 			struct sdebug_dev_info * devip)
 {
 	unsigned char pq_pdt;
-	unsigned char arr[SDEBUG_MAX_INQ_ARR_SZ];
+	unsigned char * arr;
 	unsigned char *cmd = (unsigned char *)scp->cmnd;
-	int alloc_len, n;
+	int alloc_len, n, ret;
 
 	alloc_len = (cmd[3] << 8) + cmd[4];
-	memset(arr, 0, SDEBUG_MAX_INQ_ARR_SZ);
+	arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_KERNEL);
 	if (devip->wlun)
 		pq_pdt = 0x1e;	/* present, wlun */
 	else if (scsi_debug_no_lun_0 && (0 == devip->lun))
@@ -944,12 +965,14 @@ static int resp_inquiry(struct scsi_cmnd
 	if (0x2 & cmd[1]) {  /* CMDDT bit set */
 		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
 			       	0);
+		kfree(arr);
 		return check_condition_result;
 	} else if (0x1 & cmd[1]) {  /* EVPD bit set */
-		int lu_id_num, target_dev_id, len;
+		int lu_id_num, port_group_id, target_dev_id, len;
 		char lu_id_str[6];
 		int host_no = devip->sdbg_host->shost->host_no;
 		
+		port_group_id = (((host_no + 1) & 0x7f) << 8) + (devip->channel & 0xff);
 		if (0 == scsi_debug_vpd_use_hostno)
 			host_no = 0;
 		lu_id_num = devip->wlun ? -1 : (((host_no + 1) * 2000) +
@@ -977,8 +1000,9 @@ static int resp_inquiry(struct scsi_cmnd
 			memcpy(&arr[4], lu_id_str, len);
 		} else if (0x83 == cmd[2]) { /* device identification */
 			arr[1] = cmd[2];	/*sanity */
-			arr[3] = inquiry_evpd_83(&arr[4], target_dev_id,
-						 lu_id_num, lu_id_str, len);
+			arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
+						 target_dev_id, lu_id_num,
+						 lu_id_str, len);
 		} else if (0x84 == cmd[2]) { /* Software interface ident. */
 			arr[1] = cmd[2];	/*sanity */
 			arr[3] = inquiry_evpd_84(&arr[4]);
@@ -1012,17 +1036,22 @@ static int resp_inquiry(struct scsi_cmnd
 			/* Illegal request, invalid field in cdb */
 			mk_sense_buffer(devip, ILLEGAL_REQUEST,
 					INVALID_FIELD_IN_CDB, 0);
+			kfree(arr);
 			return check_condition_result;
 		}
 		len = min(((arr[2] << 8) + arr[3]) + 4, alloc_len);
-		return fill_from_dev_buffer(scp, arr,
+		ret = fill_from_dev_buffer(scp, arr,
 			    min(len, SDEBUG_MAX_INQ_ARR_SZ));
+		kfree(arr);
+		return ret;
 	}
 	/* drops through here for a standard inquiry */
 	arr[1] = DEV_REMOVEABLE(target) ? 0x80 : 0;	/* Removable disk */
 	arr[2] = scsi_debug_scsi_level;
 	arr[3] = 2;    /* response_data_format==2 */
 	arr[4] = SDEBUG_LONG_INQ_SZ - 5;
+	if (0 == scsi_debug_vpd_use_hostno)
+		arr[5] = 0x10; /* claim: implicit TGPS */
 	arr[6] = 0x10; /* claim: MultiP */
 	/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
 	arr[7] = 0xa; /* claim: LINKED + CMDQUE */
@@ -1039,8 +1068,10 @@ static int resp_inquiry(struct scsi_cmnd
 		arr[n++] = 0x3; arr[n++] = 0x60; /* SSC-2 no version */
 	}
 	arr[n++] = 0xc; arr[n++] = 0xf;  /* SAS-1.1 rev 10 */
-	return fill_from_dev_buffer(scp, arr,
+	ret = fill_from_dev_buffer(scp, arr,
 			    min(alloc_len, SDEBUG_LONG_INQ_SZ));
+	kfree(arr);
+	return ret;
 }
 
 static int resp_requests(struct scsi_cmnd * scp,
@@ -1171,6 +1202,88 @@ static int resp_readcap16(struct scsi_cm
 				    min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
 }
 
+#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
+
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip)
+{
+	unsigned char *cmd = (unsigned char *)scp->cmnd;
+	unsigned char * arr;
+	int host_no = devip->sdbg_host->shost->host_no;
+	int n, ret, alloc_len, rlen;
+	int target_dev_id, port_group_a, port_group_b, port_a, port_b;
+
+	alloc_len = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
+		     + cmd[9]);
+	if (alloc_len < 4 + (11 * 2)) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
+			       	0);
+		return check_condition_result;
+	}
+	
+	arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_KERNEL);
+	n = 4;
+	/*
+	 * Each host has it's own target port group.
+	 * The asymmetric access state is cycled according to the host_id.
+	 */
+	target_dev_id = ((host_no + 1) * 2000) +
+			(devip->target * 1000) - 3;
+	port_a = target_dev_id + 1;
+	port_b = target_dev_id + 2;
+	port_group_a = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0xff);
+	port_group_b = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0xff) + 0x80;
+	if (0 == scsi_debug_vpd_use_hostno) {
+	    arr[n++] = host_no % 3; /* Asymm access state */
+	    arr[n++] = 0x0F; /* claim: all states are supported */
+	} else {
+	    arr[n++] = 0x0; /* Active/Optimized path */
+	    arr[n++] = 0x01; /* claim: only support active/optimized paths */
+	}
+	arr[n++] = (port_group_a >> 8) & 0xff;
+	arr[n++] = port_group_a & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_a >> 8) & 0xff;
+	arr[n++] = port_a & 0xff;
+	arr[n++] = 3;    /* Port unavailable */
+	arr[n++] = 0x0F; /* claim: all states are supported */
+	arr[n++] = (port_group_b >> 8) & 0xff;
+	arr[n++] = port_group_b & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_b >> 8) & 0xff;
+	arr[n++] = port_b & 0xff;
+
+	rlen = n - 4;
+	arr[0] = (rlen >> 24) & 0xff;
+	arr[1] = (rlen >> 16) & 0xff;
+	arr[2] = (rlen >> 8) & 0xff;
+	arr[3] = rlen & 0xff;
+
+	if (alloc_len < n) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
+			       	0);
+		kfree(arr);
+		return check_condition_result;
+	}
+	
+	ret = fill_from_dev_buffer(scp, arr,
+				   min(n, SDEBUG_MAX_TGTPGS_ARR_SZ));
+	kfree(arr);
+	return ret;
+}
+
 /* <<Following mode page info copied from ST318451LW>> */
 
 static int resp_err_recov_pg(unsigned char * p, int pcontrol, int target)
include/scsi/scsi.h +3 -0
index 84a6d5f..8a3f0bd 100644
@@ -97,6 +97,7 @@ #define MODE_SENSE_10         0x5a
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define REPORT_LUNS           0xa0
+#define MAINTENANCE_IN        0xa3
 #define MOVE_MEDIUM           0xa5
 #define EXCHANGE_MEDIUM       0xa6
 #define READ_12               0xa8
@@ -114,6 +115,8 @@ #define VERIFY_16	      0x8f
 #define SERVICE_ACTION_IN     0x9e
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
+/* values for maintenance in */
+#define MI_REPORT_TARGET_PGS  0x0a
 
 /* Values for T10/04-262r7 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */
#6 lore raw
Quoted 8 lines · Hannes Reinecke
Quoted 5 lines · Douglas Gilbert

Hannes,
Here is my counter patch for you to consider.
I suspect my machine was running into stack
problems so I moved some arrays onto the heap.

Ah. Good. To tell the truth I was a bit concerned once I've discovered that all information pages are infact statically allocated ...

I wasn't aware that statically allocated data (file or local scope) would add to stack problems on a driver loaded as a module. The arrays I moved to the heap were "auto" (i.e. non-static).

Most of the data scsi_debug keeps static are (manufacturer's) default mode pages. They are constant. Keeping the current mode page in static store is a bit more problematic: it may be logically correct depending on the mode page policy (a VPD page) but accesses probably should be locked if the page can be modified (e.g. MRIE in the IEC mode page).

Over zealous error processing ....
I have made this mistake often enough, along with many others, so hopefully some readers will take note:

<snip>

Quoted 71 lines
drivers/scsi/scsi_debug.c +71 -0
+#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
+
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip)
+{
+	unsigned char *cmd = (unsigned char *)scp->cmnd;
+	unsigned char * arr;
+	int host_no = devip->sdbg_host->shost->host_no;
+	int n, ret, alloc_len, rlen;
+	int target_dev_id, port_group_a, port_group_b, port_a, port_b;
+
+	alloc_len = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
+		     + cmd[9]);
+	if (alloc_len < 4 + (11 * 2)) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
+			       	0);
+		return check_condition_result;
+	}
+	
+	arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_KERNEL);
+	n = 4;
+	/*
+	 * Each host has it's own target port group.
+	 * The asymmetric access state is cycled according to the host_id.
+	 */
+	target_dev_id = ((host_no + 1) * 2000) +
+			(devip->target * 1000) - 3;
+	port_a = target_dev_id + 1;
+	port_b = target_dev_id + 2;
+	port_group_a = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0xff);
+	port_group_b = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0xff) + 0x80;
+	if (0 == scsi_debug_vpd_use_hostno) {
+	    arr[n++] = host_no % 3; /* Asymm access state */
+	    arr[n++] = 0x0F; /* claim: all states are supported */
+	} else {
+	    arr[n++] = 0x0; /* Active/Optimized path */
+	    arr[n++] = 0x01; /* claim: only support active/optimized paths */
+	}
+	arr[n++] = (port_group_a >> 8) & 0xff;
+	arr[n++] = port_group_a & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_a >> 8) & 0xff;
+	arr[n++] = port_a & 0xff;
+	arr[n++] = 3;    /* Port unavailable */
+	arr[n++] = 0x0F; /* claim: all states are supported */
+	arr[n++] = (port_group_b >> 8) & 0xff;
+	arr[n++] = port_group_b & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_b >> 8) & 0xff;
+	arr[n++] = port_b & 0xff;
+
+	rlen = n - 4;
+	arr[0] = (rlen >> 24) & 0xff;
+	arr[1] = (rlen >> 16) & 0xff;
+	arr[2] = (rlen >> 8) & 0xff;
+	arr[3] = rlen & 0xff;
+
+	if (alloc_len < n) {
+		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,

drivers/scsi/scsi_debug.c static int resp_readcap16(struct scsi_cm · line 1285

Quoted 10 lines
drivers/scsi/scsi_debug.c +10 -0
+			       	0);
+		kfree(arr);
+		return check_condition_result;
+	}
+	
+	ret = fill_from_dev_buffer(scp, arr,
+				   min(n, SDEBUG_MAX_TGTPGS_ARR_SZ));
+	kfree(arr);
+	return ret;
+}

<snip>

The above code is building a response to the REPORT TARGET PORT GROUPS command. The first 4 bytes of the response are the length of the response irrespective of whether the response is truncated by alloc_len (a field in the RTPG command). It is _not_ an error if alloc_len < n (see spc4r07a.pdf section 4.3.4.6). This allows the application client to ask for a 4 byte response if it is worried about how big the response will be, then issue the command again with an appropriate allocation length. This is a common paradigm with mode and log pages.

So the "if (alloc_len < 4 + (11 * 2))" block should be removed. Also the "if (alloc_len < n)" block should be removed and "min" should be (possibly a cleaner version of):
min(min(alloc_len, n), SDEBUG_MAX_TGTPGS_ARR_SZ))

Doug Gilbert

#7 lore raw
Quoted 14 lines · Douglas Gilbert
Quoted 8 lines · Hannes Reinecke
Quoted 5 lines · Douglas Gilbert

Hannes,
Here is my counter patch for you to consider.
I suspect my machine was running into stack
problems so I moved some arrays onto the heap.

Ah. Good. To tell the truth I was a bit concerned once I've discovered that all information pages are infact statically allocated ...

I wasn't aware that statically allocated data (file or local scope) would add to stack problems on a driver loaded as a module. The arrays I moved to the heap were "auto" (i.e. non-static).

[ .. ]

Quoted 5 lines

Over zealous error processing ....
I have made this mistake often enough, along with many others, so hopefully some readers will take note:

I know. Been there before, too :-)

Quoted 1 line

<snip>

[ .. ]

Quoted 5 lines

So the "if (alloc_len < 4 + (11 * 2))" block should be removed. Also the "if (alloc_len < n)" block should be removed and "min" should be (possibly a cleaner version of):
min(min(alloc_len, n), SDEBUG_MAX_TGTPGS_ARR_SZ))

Ok, did so. And returned the proper (relative !) port number in the port descriptor list.

Is this more to your liking?

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
drivers/scsi/scsi_debug.c +127 -14
index 9c0f358..8e71fd2 100644
@@ -52,7 +52,7 @@ #include "scsi_logging.h"
 #include "scsi_debug.h"
 
 #define SCSI_DEBUG_VERSION "1.80"
-static const char * scsi_debug_version_date = "20060914";
+static const char * scsi_debug_version_date = "20061018";
 
 /* Additional Sense Code (ASC) used */
 #define NO_ADDITIONAL_SENSE 0x0
@@ -254,6 +254,8 @@ static int resp_requests(struct scsi_cmn
 			 struct sdebug_dev_info * devip);
 static int resp_start_stop(struct scsi_cmnd * scp,
 			   struct sdebug_dev_info * devip);
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip);
 static int resp_readcap(struct scsi_cmnd * SCpnt,
 			struct sdebug_dev_info * devip);
 static int resp_readcap16(struct scsi_cmnd * SCpnt,
@@ -287,9 +289,9 @@ static void __init sdebug_build_parts(un
 static void __init init_all_queued(void);
 static void stop_all_queued(void);
 static int stop_queued_cmnd(struct scsi_cmnd * cmnd);
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
-			   int dev_id_str_len);
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str, int dev_id_str_len);
 static int inquiry_evpd_88(unsigned char * arr, int target_dev_id);
 static int do_create_driverfs_files(void);
 static void do_remove_driverfs_files(void);
@@ -422,6 +424,15 @@ int scsi_debug_queuecommand(struct scsi_
 		}
 		errsts = resp_readcap16(SCpnt, devip);
 		break;
+	case MAINTENANCE_IN:
+		if (MI_REPORT_TARGET_PGS != cmd[1]) {
+			mk_sense_buffer(devip, ILLEGAL_REQUEST,
+					INVALID_OPCODE, 0);
+			errsts = check_condition_result;
+			break;
+		}
+		errsts = resp_report_tgtpgs(SCpnt, devip);
+		break;
 	case READ_16:
 	case READ_12:
 	case READ_10:
@@ -665,8 +676,9 @@ static const char * inq_vendor_id = "Lin
 static const char * inq_product_id = "scsi_debug      ";
 static const char * inq_product_rev = "0004";
 
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str,
 			   int dev_id_str_len)
 {
 	int num, port_a;
@@ -720,6 +732,15 @@ static int inquiry_evpd_83(unsigned char
 	arr[num++] = (port_a >> 16) & 0xff;
 	arr[num++] = (port_a >> 8) & 0xff;
 	arr[num++] = port_a & 0xff;
+	/* NAA-5, Target port group identifier */
+	arr[num++] = 0x61;	/* proto=sas, binary */
+	arr[num++] = 0x95;	/* piv=1, target port group id */
+	arr[num++] = 0x0;
+	arr[num++] = 0x4;
+	arr[num++] = 0;
+	arr[num++] = 0;
+	arr[num++] = (port_group_id >> 8) & 0xff;
+	arr[num++] = port_group_id & 0xff;
 	/* NAA-5, Target device identifier */
 	arr[num++] = 0x61;	/* proto=sas, binary */
 	arr[num++] = 0xa3;	/* piv=1, target device, naa */
@@ -928,12 +949,12 @@ static int resp_inquiry(struct scsi_cmnd
 			struct sdebug_dev_info * devip)
 {
 	unsigned char pq_pdt;
-	unsigned char arr[SDEBUG_MAX_INQ_ARR_SZ];
+	unsigned char * arr;
 	unsigned char *cmd = (unsigned char *)scp->cmnd;
-	int alloc_len, n;
+	int alloc_len, n, ret;
 
 	alloc_len = (cmd[3] << 8) + cmd[4];
-	memset(arr, 0, SDEBUG_MAX_INQ_ARR_SZ);
+	arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_KERNEL);
 	if (devip->wlun)
 		pq_pdt = 0x1e;	/* present, wlun */
 	else if (scsi_debug_no_lun_0 && (0 == devip->lun))
@@ -944,12 +965,15 @@ static int resp_inquiry(struct scsi_cmnd
 	if (0x2 & cmd[1]) {  /* CMDDT bit set */
 		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
 			       	0);
+		kfree(arr);
 		return check_condition_result;
 	} else if (0x1 & cmd[1]) {  /* EVPD bit set */
-		int lu_id_num, target_dev_id, len;
+		int lu_id_num, port_group_id, target_dev_id, len;
 		char lu_id_str[6];
 		int host_no = devip->sdbg_host->shost->host_no;
 		
+		port_group_id = (((host_no + 1) & 0x7f) << 8) + 
+		    (devip->channel & 0x7f);
 		if (0 == scsi_debug_vpd_use_hostno)
 			host_no = 0;
 		lu_id_num = devip->wlun ? -1 : (((host_no + 1) * 2000) +
@@ -977,8 +1001,9 @@ static int resp_inquiry(struct scsi_cmnd
 			memcpy(&arr[4], lu_id_str, len);
 		} else if (0x83 == cmd[2]) { /* device identification */
 			arr[1] = cmd[2];	/*sanity */
-			arr[3] = inquiry_evpd_83(&arr[4], target_dev_id,
-						 lu_id_num, lu_id_str, len);
+			arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
+						 target_dev_id, lu_id_num,
+						 lu_id_str, len);
 		} else if (0x84 == cmd[2]) { /* Software interface ident. */
 			arr[1] = cmd[2];	/*sanity */
 			arr[3] = inquiry_evpd_84(&arr[4]);
@@ -1012,17 +1037,22 @@ static int resp_inquiry(struct scsi_cmnd
 			/* Illegal request, invalid field in cdb */
 			mk_sense_buffer(devip, ILLEGAL_REQUEST,
 					INVALID_FIELD_IN_CDB, 0);
+			kfree(arr);
 			return check_condition_result;
 		}
 		len = min(((arr[2] << 8) + arr[3]) + 4, alloc_len);
-		return fill_from_dev_buffer(scp, arr,
+		ret = fill_from_dev_buffer(scp, arr,
 			    min(len, SDEBUG_MAX_INQ_ARR_SZ));
+		kfree(arr);
+		return ret;
 	}
 	/* drops through here for a standard inquiry */
 	arr[1] = DEV_REMOVEABLE(target) ? 0x80 : 0;	/* Removable disk */
 	arr[2] = scsi_debug_scsi_level;
 	arr[3] = 2;    /* response_data_format==2 */
 	arr[4] = SDEBUG_LONG_INQ_SZ - 5;
+	if (0 == scsi_debug_vpd_use_hostno)
+		arr[5] = 0x10; /* claim: implicit TGPS */
 	arr[6] = 0x10; /* claim: MultiP */
 	/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
 	arr[7] = 0xa; /* claim: LINKED + CMDQUE */
@@ -1039,8 +1069,10 @@ static int resp_inquiry(struct scsi_cmnd
 		arr[n++] = 0x3; arr[n++] = 0x60; /* SSC-2 no version */
 	}
 	arr[n++] = 0xc; arr[n++] = 0xf;  /* SAS-1.1 rev 10 */
-	return fill_from_dev_buffer(scp, arr,
+	ret = fill_from_dev_buffer(scp, arr,
 			    min(alloc_len, SDEBUG_LONG_INQ_SZ));
+	kfree(arr);
+	return ret;
 }
 
 static int resp_requests(struct scsi_cmnd * scp,
@@ -1171,6 +1203,87 @@ static int resp_readcap16(struct scsi_cm
 				    min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
 }
 
+#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
+
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip)
+{
+	unsigned char *cmd = (unsigned char *)scp->cmnd;
+	unsigned char * arr;
+	int host_no = devip->sdbg_host->shost->host_no;
+	int n, ret, alen, rlen;
+	int port_group_a, port_group_b, port_a, port_b;
+
+	alen = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
+		+ cmd[9]);
+
+	arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_KERNEL);
+	/*
+	 * EVPD page 0x88 states we have two ports, one
+	 * real and a fake port with no device connected.
+	 * So we create two port groups with one port each
+	 * and set the group with port B to unavailable.
+	 */
+	port_a = 0x1; /* relative port A */
+	port_b = 0x2; /* relative port B */
+	port_group_a = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0x7f);
+	port_group_b = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0x7f) + 0x80;
+
+	/*
+	 * The asymmetric access state is cycled according to the host_id.
+	 */
+	n = 4;
+	if (0 == scsi_debug_vpd_use_hostno) {
+	    arr[n++] = host_no % 3; /* Asymm access state */
+	    arr[n++] = 0x0F; /* claim: all states are supported */
+	} else {
+	    arr[n++] = 0x0; /* Active/Optimized path */
+	    arr[n++] = 0x01; /* claim: only support active/optimized paths */
+	}
+	arr[n++] = (port_group_a >> 8) & 0xff;
+	arr[n++] = port_group_a & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_a >> 8) & 0xff;
+	arr[n++] = port_a & 0xff;
+	arr[n++] = 3;    /* Port unavailable */
+	arr[n++] = 0x08; /* claim: only unavailalbe paths are supported */
+	arr[n++] = (port_group_b >> 8) & 0xff;
+	arr[n++] = port_group_b & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_b >> 8) & 0xff;
+	arr[n++] = port_b & 0xff;
+
+	rlen = n - 4;
+	arr[0] = (rlen >> 24) & 0xff;
+	arr[1] = (rlen >> 16) & 0xff;
+	arr[2] = (rlen >> 8) & 0xff;
+	arr[3] = rlen & 0xff;
+
+	/*
+	 * Return the smallest value of either
+	 * - The allocated length
+	 * - The constructed command length
+	 * - The maximum array size
+	 */
+	rlen = min(alen,n);
+	ret = fill_from_dev_buffer(scp, arr,
+				   min(rlen, SDEBUG_MAX_TGTPGS_ARR_SZ));
+	kfree(arr);
+	return ret;
+}
+
 /* <<Following mode page info copied from ST318451LW>> */
 
 static int resp_err_recov_pg(unsigned char * p, int pcontrol, int target)
include/scsi/scsi.h +3 -0
index 84a6d5f..8a3f0bd 100644
@@ -97,6 +97,7 @@ #define MODE_SENSE_10         0x5a
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define REPORT_LUNS           0xa0
+#define MAINTENANCE_IN        0xa3
 #define MOVE_MEDIUM           0xa5
 #define EXCHANGE_MEDIUM       0xa6
 #define READ_12               0xa8
@@ -114,6 +115,8 @@ #define VERIFY_16	      0x8f
 #define SERVICE_ACTION_IN     0x9e
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
+/* values for maintenance in */
+#define MI_REPORT_TARGET_PGS  0x0a
 
 /* Values for T10/04-262r7 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */
#8 lore raw
Quoted 31 lines · Hannes Reinecke
Quoted 15 lines · Douglas Gilbert
Quoted 9 lines · Hannes Reinecke
Quoted 5 lines · Douglas Gilbert

Hannes,
Here is my counter patch for you to consider.
I suspect my machine was running into stack
problems so I moved some arrays onto the heap.

Ah. Good. To tell the truth I was a bit concerned once I've discovered that all information pages are infact statically allocated
...

I wasn't aware that statically allocated data (file or local scope) would add to stack problems on a driver loaded as a module. The arrays I moved to the heap were "auto" (i.e. non-static).

[ .. ]

Quoted 5 lines

Over zealous error processing ....
I have made this mistake often enough, along with many others, so hopefully some readers will take note:

I know. Been there before, too :-)

Quoted 1 line

<snip>

[ .. ]

Quoted 5 lines

So the "if (alloc_len < 4 + (11 * 2))" block should be removed. Also the "if (alloc_len < n)" block should be removed and "min" should be (possibly a cleaner version of):
min(min(alloc_len, n), SDEBUG_MAX_TGTPGS_ARR_SZ))

Quoted 4 lines

Ok, did so. And returned the proper (relative !) port number in the port descriptor list.

Is this more to your liking?

Hannes,
Looks goods, tests well.

Doug Gilbert

Quoted 271 lines

------------------------------------------------------------------------

drivers/scsi/scsi_debug.c +127 -14
index 9c0f358..8e71fd2 100644
@@ -52,7 +52,7 @@ #include "scsi_logging.h"
 #include "scsi_debug.h"
 
 #define SCSI_DEBUG_VERSION "1.80"
-static const char * scsi_debug_version_date = "20060914";
+static const char * scsi_debug_version_date = "20061018";
 
 /* Additional Sense Code (ASC) used */
 #define NO_ADDITIONAL_SENSE 0x0
@@ -254,6 +254,8 @@ static int resp_requests(struct scsi_cmn
 			 struct sdebug_dev_info * devip);
 static int resp_start_stop(struct scsi_cmnd * scp,
 			   struct sdebug_dev_info * devip);
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip);
 static int resp_readcap(struct scsi_cmnd * SCpnt,
 			struct sdebug_dev_info * devip);
 static int resp_readcap16(struct scsi_cmnd * SCpnt,
@@ -287,9 +289,9 @@ static void __init sdebug_build_parts(un
 static void __init init_all_queued(void);
 static void stop_all_queued(void);
 static int stop_queued_cmnd(struct scsi_cmnd * cmnd);
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
-			   int dev_id_str_len);
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str, int dev_id_str_len);
 static int inquiry_evpd_88(unsigned char * arr, int target_dev_id);
 static int do_create_driverfs_files(void);
 static void do_remove_driverfs_files(void);
@@ -422,6 +424,15 @@ int scsi_debug_queuecommand(struct scsi_
 		}
 		errsts = resp_readcap16(SCpnt, devip);
 		break;
+	case MAINTENANCE_IN:
+		if (MI_REPORT_TARGET_PGS != cmd[1]) {
+			mk_sense_buffer(devip, ILLEGAL_REQUEST,
+					INVALID_OPCODE, 0);
+			errsts = check_condition_result;
+			break;
+		}
+		errsts = resp_report_tgtpgs(SCpnt, devip);
+		break;
 	case READ_16:
 	case READ_12:
 	case READ_10:
@@ -665,8 +676,9 @@ static const char * inq_vendor_id = "Lin
 static const char * inq_product_id = "scsi_debug      ";
 static const char * inq_product_rev = "0004";
 
-static int inquiry_evpd_83(unsigned char * arr, int target_dev_id,
-			   int dev_id_num, const char * dev_id_str,
+static int inquiry_evpd_83(unsigned char * arr, int port_group_id,
+			   int target_dev_id, int dev_id_num,
+			   const char * dev_id_str,
 			   int dev_id_str_len)
 {
 	int num, port_a;
@@ -720,6 +732,15 @@ static int inquiry_evpd_83(unsigned char
 	arr[num++] = (port_a >> 16) & 0xff;
 	arr[num++] = (port_a >> 8) & 0xff;
 	arr[num++] = port_a & 0xff;
+	/* NAA-5, Target port group identifier */
+	arr[num++] = 0x61;	/* proto=sas, binary */
+	arr[num++] = 0x95;	/* piv=1, target port group id */
+	arr[num++] = 0x0;
+	arr[num++] = 0x4;
+	arr[num++] = 0;
+	arr[num++] = 0;
+	arr[num++] = (port_group_id >> 8) & 0xff;
+	arr[num++] = port_group_id & 0xff;
 	/* NAA-5, Target device identifier */
 	arr[num++] = 0x61;	/* proto=sas, binary */
 	arr[num++] = 0xa3;	/* piv=1, target device, naa */
@@ -928,12 +949,12 @@ static int resp_inquiry(struct scsi_cmnd
 			struct sdebug_dev_info * devip)
 {
 	unsigned char pq_pdt;
-	unsigned char arr[SDEBUG_MAX_INQ_ARR_SZ];
+	unsigned char * arr;
 	unsigned char *cmd = (unsigned char *)scp->cmnd;
-	int alloc_len, n;
+	int alloc_len, n, ret;
 
 	alloc_len = (cmd[3] << 8) + cmd[4];
-	memset(arr, 0, SDEBUG_MAX_INQ_ARR_SZ);
+	arr = kzalloc(SDEBUG_MAX_INQ_ARR_SZ, GFP_KERNEL);
 	if (devip->wlun)
 		pq_pdt = 0x1e;	/* present, wlun */
 	else if (scsi_debug_no_lun_0 && (0 == devip->lun))
@@ -944,12 +965,15 @@ static int resp_inquiry(struct scsi_cmnd
 	if (0x2 & cmd[1]) {  /* CMDDT bit set */
 		mk_sense_buffer(devip, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB,
 			       	0);
+		kfree(arr);
 		return check_condition_result;
 	} else if (0x1 & cmd[1]) {  /* EVPD bit set */
-		int lu_id_num, target_dev_id, len;
+		int lu_id_num, port_group_id, target_dev_id, len;
 		char lu_id_str[6];
 		int host_no = devip->sdbg_host->shost->host_no;
 
+		port_group_id = (((host_no + 1) & 0x7f) << 8) + 
+		    (devip->channel & 0x7f);
 		if (0 == scsi_debug_vpd_use_hostno)
 			host_no = 0;
 		lu_id_num = devip->wlun ? -1 : (((host_no + 1) * 2000) +
@@ -977,8 +1001,9 @@ static int resp_inquiry(struct scsi_cmnd
 			memcpy(&arr[4], lu_id_str, len);
 		} else if (0x83 == cmd[2]) { /* device identification */
 			arr[1] = cmd[2];	/*sanity */
-			arr[3] = inquiry_evpd_83(&arr[4], target_dev_id,
-						 lu_id_num, lu_id_str, len);
+			arr[3] = inquiry_evpd_83(&arr[4], port_group_id,
+						 target_dev_id, lu_id_num,
+						 lu_id_str, len);
 		} else if (0x84 == cmd[2]) { /* Software interface ident. */
 			arr[1] = cmd[2];	/*sanity */
 			arr[3] = inquiry_evpd_84(&arr[4]);
@@ -1012,17 +1037,22 @@ static int resp_inquiry(struct scsi_cmnd
 			/* Illegal request, invalid field in cdb */
 			mk_sense_buffer(devip, ILLEGAL_REQUEST,
 					INVALID_FIELD_IN_CDB, 0);
+			kfree(arr);
 			return check_condition_result;
 		}
 		len = min(((arr[2] << 8) + arr[3]) + 4, alloc_len);
-		return fill_from_dev_buffer(scp, arr,
+		ret = fill_from_dev_buffer(scp, arr,
 			    min(len, SDEBUG_MAX_INQ_ARR_SZ));
+		kfree(arr);
+		return ret;
 	}
 	/* drops through here for a standard inquiry */
 	arr[1] = DEV_REMOVEABLE(target) ? 0x80 : 0;	/* Removable disk */
 	arr[2] = scsi_debug_scsi_level;
 	arr[3] = 2;    /* response_data_format==2 */
 	arr[4] = SDEBUG_LONG_INQ_SZ - 5;
+	if (0 == scsi_debug_vpd_use_hostno)
+		arr[5] = 0x10; /* claim: implicit TGPS */
 	arr[6] = 0x10; /* claim: MultiP */
 	/* arr[6] |= 0x40; ... claim: EncServ (enclosure services) */
 	arr[7] = 0xa; /* claim: LINKED + CMDQUE */
@@ -1039,8 +1069,10 @@ static int resp_inquiry(struct scsi_cmnd
 		arr[n++] = 0x3; arr[n++] = 0x60; /* SSC-2 no version */
 	}
 	arr[n++] = 0xc; arr[n++] = 0xf;  /* SAS-1.1 rev 10 */
-	return fill_from_dev_buffer(scp, arr,
+	ret = fill_from_dev_buffer(scp, arr,
 			    min(alloc_len, SDEBUG_LONG_INQ_SZ));
+	kfree(arr);
+	return ret;
 }
 
 static int resp_requests(struct scsi_cmnd * scp,
@@ -1171,6 +1203,87 @@ static int resp_readcap16(struct scsi_cm
 				    min(alloc_len, SDEBUG_READCAP16_ARR_SZ));
 }
 
+#define SDEBUG_MAX_TGTPGS_ARR_SZ 1412
+
+static int resp_report_tgtpgs(struct scsi_cmnd * scp,
+			      struct sdebug_dev_info * devip)
+{
+	unsigned char *cmd = (unsigned char *)scp->cmnd;
+	unsigned char * arr;
+	int host_no = devip->sdbg_host->shost->host_no;
+	int n, ret, alen, rlen;
+	int port_group_a, port_group_b, port_a, port_b;
+
+	alen = ((cmd[6] << 24) + (cmd[7] << 16) + (cmd[8] << 8)
+		+ cmd[9]);
+
+	arr = kzalloc(SDEBUG_MAX_TGTPGS_ARR_SZ, GFP_KERNEL);
+	/*
+	 * EVPD page 0x88 states we have two ports, one
+	 * real and a fake port with no device connected.
+	 * So we create two port groups with one port each
+	 * and set the group with port B to unavailable.
+	 */
+	port_a = 0x1; /* relative port A */
+	port_b = 0x2; /* relative port B */
+	port_group_a = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0x7f);
+	port_group_b = (((host_no + 1) & 0x7f) << 8) + 
+	    (devip->channel & 0x7f) + 0x80;
+
+	/*
+	 * The asymmetric access state is cycled according to the host_id.
+	 */
+	n = 4;
+	if (0 == scsi_debug_vpd_use_hostno) {
+	    arr[n++] = host_no % 3; /* Asymm access state */
+	    arr[n++] = 0x0F; /* claim: all states are supported */
+	} else {
+	    arr[n++] = 0x0; /* Active/Optimized path */
+	    arr[n++] = 0x01; /* claim: only support active/optimized paths */
+	}
+	arr[n++] = (port_group_a >> 8) & 0xff;
+	arr[n++] = port_group_a & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_a >> 8) & 0xff;
+	arr[n++] = port_a & 0xff;
+	arr[n++] = 3;    /* Port unavailable */
+	arr[n++] = 0x08; /* claim: only unavailalbe paths are supported */
+	arr[n++] = (port_group_b >> 8) & 0xff;
+	arr[n++] = port_group_b & 0xff;
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Status code */
+	arr[n++] = 0;    /* Vendor unique */
+	arr[n++] = 0x1;  /* One port per group */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = 0;    /* Reserved */
+	arr[n++] = (port_b >> 8) & 0xff;
+	arr[n++] = port_b & 0xff;
+
+	rlen = n - 4;
+	arr[0] = (rlen >> 24) & 0xff;
+	arr[1] = (rlen >> 16) & 0xff;
+	arr[2] = (rlen >> 8) & 0xff;
+	arr[3] = rlen & 0xff;
+
+	/*
+	 * Return the smallest value of either
+	 * - The allocated length
+	 * - The constructed command length
+	 * - The maximum array size
+	 */
+	rlen = min(alen,n);
+	ret = fill_from_dev_buffer(scp, arr,
+				   min(rlen, SDEBUG_MAX_TGTPGS_ARR_SZ));
+	kfree(arr);
+	return ret;
+}
+
 /* <<Following mode page info copied from ST318451LW>> */
 
 static int resp_err_recov_pg(unsigned char * p, int pcontrol, int target)
include/scsi/scsi.h +3 -0
index 84a6d5f..8a3f0bd 100644
@@ -97,6 +97,7 @@ #define MODE_SENSE_10         0x5a
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
 #define REPORT_LUNS           0xa0
+#define MAINTENANCE_IN        0xa3
 #define MOVE_MEDIUM           0xa5
 #define EXCHANGE_MEDIUM       0xa6
 #define READ_12               0xa8
@@ -114,6 +115,8 @@ #define VERIFY_16	      0x8f
 #define SERVICE_ACTION_IN     0x9e
 /* values for service action in */
 #define	SAI_READ_CAPACITY_16  0x10
+/* values for maintenance in */
+#define MI_REPORT_TARGET_PGS  0x0a
 
 /* Values for T10/04-262r7 */
 #define	ATA_16		      0x85	/* 16-byte pass-thru */