lkml.org 
[lkml]   [2012]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/2] ata: implement MODE SELECT command
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 <sshtylyov@mvista.com>
> Cc: Jeff Garzik <jgarzik@pobox.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> drivers/ata/libata-scsi.c | 185 +++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 179 insertions(+), 6 deletions(-)

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index d5aeb26..1cb88df 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
[...]
> +/**
> + * 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


\
 
 \ /
  Last update: 2012-07-05 14:21    [W:0.078 / U:0.076 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site