Hello.

On 05-07-2012 13:40, Paolo Bonzini wrote:

The cache_type file in sysfs lets users configure the disk cache in
write-through or write-back modes.  However, ata disks do not support
writing to the file because they do not implement the MODE SELECT
command.

This patch adds a translation from MODE SELECT (for the caching page
only) to the ATA SET FEATURES command.  The set of changeable parameters
answered by MODE SENSE is also adjusted accordingly.

Cc: Sergei Shtylyov <redacted>
Cc: Jeff Garzik <redacted>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
  drivers/ata/libata-scsi.c |  185 +++++++++++++++++++++++++++++++++++++++++++--
  1 files changed, 179 insertions(+), 6 deletions(-)

[...]

+/**
+ *	ata_scsiop_mode_select - Simulate MODE SELECT 6, 10 commands
+ *	@qc: Storage for translated ATA taskfile
+ *
+ *	Converts a MODE SELECT command to an ATA SET FEATURES taskfile.
+ *	Assume this is invoked for direct access devices (e.g. disks) only.
+ *	There should be no block descriptor for other device types.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ */
+static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
+{
+	struct scsi_cmnd *scmd = qc->scsicmd;
+	const u8 *cdb = scmd->cmnd;
+	const u8 *p;
+	u8 pg, spg;
+	unsigned six_byte, pg_len, hdr_len;
+	int len;
+
+	VPRINTK("ENTER\n");
+
+	six_byte = (cdb[0] == MODE_SELECT);
+	if (six_byte) {
+		if (scmd->cmd_len < 5)
+			goto invalid_fld;

    Strictly speaking, it should be "invalid phase" error.

+
+		len = cdb[4];
+	} else {
+		if (scmd->cmd_len < 9)
+			goto invalid_fld;

    The same.

+
+		len = (cdb[7] << 8) + cdb[8];
+	}
+	/* Test early for possible overrun.  */
+	if (!scsi_sg_count(scmd) || scsi_sglist(scmd)->length < len)
+		goto invalid_param_len;

     Strictly speaking, it's "data underrun" error.

+	p = page_address(sg_page(scsi_sglist(scmd)));

    What if the S/G list spans multiple non-consecutive pages?

+
+	/* Move past header and block descriptors.  */
+	if (six_byte)
+		hdr_len = p[3] + 4;
+	else
+		hdr_len = (p[6] << 8) + p[7] + 8;
+
+	if (len < hdr_len)
+		goto invalid_param_len;
+
+	len -= hdr_len;
+	p += hdr_len;
+	if (len == 0)
+		goto skip;
+
+	/* Parse both possible formats for the mode page headers.  */
+	pg = p[0] & 0x3f;
+	if (p[0] & 0x40) {
+		if (len < 4)
+			goto invalid_param_len;
+
+		spg = p[1];
+		pg_len = (p[2] << 8) | p[3];
+		p += 4;
+		len -= 4;
+	} else {
+		if (len < 2)
+			goto invalid_param_len;
+
+		spg = 0;
+		pg_len = p[1];
+		p += 2;
+		len -= 2;
+	}
+
+	/*
+	 * Only one page has changeable data, so we only support setting one
+	 * page at a time.
+	 */
+	if (len < pg_len)
+		goto invalid_param;

     Not 'invalid_param_len'?

+
+	/*
+	 * No mode subpages supported (yet) but asking for _all_
+	 * subpages may be valid
+	 */
+	if (spg && (spg != ALL_SUB_MPAGES))
+		goto invalid_param;

    Rather "paramater not supported" (0x26/0x01)...

+	if (pg_len > len)
+		goto invalid_param_len;

    We have just checked this.

+	switch (pg) {
+	case CACHE_MPAGE:
+		if (ata_mselect_caching(qc, p, pg_len) < 0)
+			goto invalid_param;

    Rather "parameter not supported"?

+		break;
+
+	default:		/* invalid page code */
+		goto invalid_param;

    Rather "paramater not supported"...

+	}
+
+	return 0;

MBR, Sergei