[PATCH v2 0/2] Introduce new SATA queued commands

[PATCH v2 0/2] Introduce new SATA queued commands

[Marc C]

From: Marc Carino <marc.ceeeee@gmail.com>

This patch series updates the libata driver with some additional
commands which are specified in the more recent versions of the
SATA and ATA specifications. These commands are:

        SEND FPDMA QUEUED
        RECEIVE FPDMA QUEUED.

The new queued commands augment the existing READ and WRITE FPDMA
QUEUED commands. The commands are intended to transport non-media
data to/from a device in a non-blocking manner.

One application of these commands include a queued-style DSM TRIM
operation. The current DSM TRIM command is non-queued, and requires
the initiator to empty out the queue before issuance. The new DSM TRIM
allows the host to keep the queue full.

These patches have been validated on an Intel SATA AHCI controller,
on a Micron M500 SSD, using "postmark," "fstrim," and "fsck."

Version history:

v2:
- rebased to 3.11-rc4
- moved auxiliary field to ata_queued_cmd struct
- updated signoff name

v1:
- initial

Marc Carino (2):
  libata: Add support for SEND/RECEIVE FPDMA QUEUED
  libata: Add support for queued DSM TRIM

 drivers/ata/ahci.h        |  8 ++++++++
 drivers/ata/libahci.c     |  1 +
 drivers/ata/libata-core.c | 15 +++++++++++++++
 drivers/ata/libata-scsi.c | 27 +++++++++++++++++++++------
 include/linux/ata.h       | 25 +++++++++++++++++++++++++
 include/linux/libata.h    |  6 ++++++
 6 files changed, 76 insertions(+), 6 deletions(-)

-- 
1.8.1.2

[PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Marc C]

From: Marc Carino <marc.ceeeee@gmail.com>

Add support for the following ATA opcodes, which are present
in SATA 3.1 and T13 ATA ACS-3:

        SEND FPDMA QUEUED
        RECEIVE FPDMA QUEUED

Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
---
 drivers/ata/ahci.h        |  8 ++++++++
 drivers/ata/libahci.c     |  1 +
 drivers/ata/libata-core.c | 15 +++++++++++++++
 include/linux/ata.h       | 25 +++++++++++++++++++++++++
 include/linux/libata.h    |  6 ++++++
 5 files changed, 55 insertions(+)

diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 1145637..f9e4fd8 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -388,4 +388,12 @@ static inline int ahci_nr_ports(u32 cap)
 	return (cap & 0x1f) + 1;
 }
 
+static inline void ahci_h2dfis_set_auxiliary(u8 *fis, u32 aux)
+{
+	fis[16] = aux & 0xff;
+	fis[17] = (aux >> 8) & 0xff;
+	fis[18] = (aux >> 16) & 0xff;
+	fis[19] = (aux >> 24) & 0xff;
+}
+
 #endif /* _AHCI_H */
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index acfd0f7..52668f4 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1499,6 +1499,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
 	cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ;
 
 	ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, cmd_tbl);
+	ahci_h2dfis_set_auxiliary(cmd_tbl, qc->auxiliary);
 	if (is_atapi) {
 		memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32);
 		memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c24354d..b0a0b5c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2103,6 +2103,7 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 	int hdepth = 0, ddepth = ata_id_queue_depth(dev->id);
 	unsigned int err_mask;
 	char *aa_desc = "";
+	u8 sata_setting[ATA_SECT_SIZE];
 
 	if (!ata_id_has_ncq(dev->id)) {
 		desc[0] = '\0';
@@ -2139,6 +2140,20 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 	else
 		snprintf(desc, desc_sz, "NCQ (depth %d/%d)%s", hdepth,
 			ddepth, aa_desc);
+
+	if (ata_id_has_ncq_send_and_recv(dev->id)) {
+		err_mask = ata_read_log_page(dev,
+					     ATA_LOG_NCQ_SEND_RECV,
+					     0,
+					     sata_setting,
+					     1);
+		if (!err_mask) {
+			dev->flags |= ATA_DFLAG_NCQ_SEND_RECV;
+			memcpy(dev->ncq_send_recv_cmds, sata_setting,
+				ATA_LOG_NCQ_SEND_RECV_SIZE);
+		}
+	}
+
 	return 0;
 }
 
diff --git a/include/linux/ata.h b/include/linux/ata.h
index ee0bd95..dd26211 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -239,6 +239,8 @@ enum {
 	ATA_CMD_WRITE_QUEUED_FUA_EXT = 0x3E,
 	ATA_CMD_FPDMA_READ	= 0x60,
 	ATA_CMD_FPDMA_WRITE	= 0x61,
+	ATA_CMD_FPDMA_SEND	= 0x64,
+	ATA_CMD_FPDMA_RECV	= 0x65,
 	ATA_CMD_PIO_READ	= 0x20,
 	ATA_CMD_PIO_READ_EXT	= 0x24,
 	ATA_CMD_PIO_WRITE	= 0x30,
@@ -293,8 +295,13 @@ enum {
 	/* marked obsolete in the ATA/ATAPI-7 spec */
 	ATA_CMD_RESTORE		= 0x10,
 
+	/* Subcmds for ATA_CMD_FPDMA_SEND */
+	ATA_SUBCMD_FPDMA_SEND_DSM            = 0x00,
+	ATA_SUBCMD_FPDMA_SEND_WR_LOG_DMA_EXT = 0x02,
+
 	/* READ_LOG_EXT pages */
 	ATA_LOG_SATA_NCQ	= 0x10,
+	ATA_LOG_NCQ_SEND_RECV	= 0x13,
 	ATA_LOG_SATA_ID_DEV_DATA  = 0x30,
 	ATA_LOG_SATA_SETTINGS	  = 0x08,
 	ATA_LOG_DEVSLP_OFFSET	  = 0x30,
@@ -305,6 +312,15 @@ enum {
 	ATA_LOG_DEVSLP_VALID	  = 0x07,
 	ATA_LOG_DEVSLP_VALID_MASK = 0x80,
 
+	/* NCQ send and receive log */
+	ATA_LOG_NCQ_SEND_RECV_SUBCMDS_OFFSET	= 0x00,
+	ATA_LOG_NCQ_SEND_RECV_SUBCMDS_DSM	= (1 << 0),
+	ATA_LOG_NCQ_SEND_RECV_DSM_OFFSET	= 0x04,
+	ATA_LOG_NCQ_SEND_RECV_DSM_TRIM          = (1 << 0),
+	ATA_LOG_NCQ_SEND_RECV_RD_LOG_OFFSET	= 0x08,
+	ATA_LOG_NCQ_SEND_RECV_WR_LOG_OFFSET	= 0x0C,
+	ATA_LOG_NCQ_SEND_RECV_SIZE		= 0x10,
+
 	/* READ/WRITE LONG (obsolete) */
 	ATA_CMD_READ_LONG	= 0x22,
 	ATA_CMD_READ_LONG_ONCE	= 0x23,
@@ -509,6 +525,8 @@ struct ata_taskfile {
 	u8			device;
 
 	u8			command;	/* IO operation */
+
+	u32			auxiliary;
 };
 
 /*
@@ -865,6 +883,13 @@ static inline int ata_id_rotation_rate(const u16 *id)
 	return val;
 }
 
+static inline bool ata_id_has_ncq_send_and_recv(const u16 *id)
+{
+	if (id[ATA_ID_SATA_CAPABILITY_2] & BIT(6))
+		return true;
+	return false;
+}
+
 static inline bool ata_id_has_trim(const u16 *id)
 {
 	if (ata_id_major_version(id) >= 7 &&
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 4ea55bb..7ec83b7 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -164,6 +164,7 @@ enum {
 	ATA_DFLAG_DA		= (1 << 26), /* device supports Device Attention */
 	ATA_DFLAG_DEVSLP	= (1 << 27), /* device supports Device Sleep */
 	ATA_DFLAG_ACPI_DISABLED = (1 << 28), /* ACPI for the device is disabled */
+	ATA_DFLAG_NCQ_SEND_RECV = (1 << 29), /* device supports NCQ SEND and RECV */
 
 	ATA_DEV_UNKNOWN		= 0,	/* unknown device */
 	ATA_DEV_ATA		= 1,	/* ATA device */
@@ -565,6 +566,7 @@ struct ata_queued_cmd {
 
 	struct ata_taskfile	tf;
 	u8			cdb[ATAPI_CDB_LEN];
+	u32			auxiliary;
 
 	unsigned long		flags;		/* ATA_QCFLAG_xxx */
 	unsigned int		tag;
@@ -660,6 +662,9 @@ struct ata_device {
 	/* DEVSLP Timing Variables from Identify Device Data Log */
 	u8			devslp_timing[ATA_LOG_DEVSLP_SIZE];
 
+	/* NCQ send and receive log subcommand support */
+	u8			ncq_send_recv_cmds[ATA_LOG_NCQ_SEND_RECV_SIZE];
+
 	/* error history */
 	int			spdn_cnt;
 	/* ering is CLEAR_END, read comment above CLEAR_END */
@@ -1556,6 +1561,7 @@ static inline void ata_qc_reinit(struct ata_queued_cmd *qc)
 	qc->n_elem = 0;
 	qc->err_mask = 0;
 	qc->sect_size = ATA_SECT_SIZE;
+	qc->auxiliary = 0;
 
 	ata_tf_init(qc->dev, &qc->tf);
 
-- 
1.8.1.2

[PATCH v2 2/2] libata: Add support for queued DSM TRIM

[Marc C]

From: Marc Carino <marc.ceeeee@gmail.com>

Some new SSDs support the queued version of the DSM TRIM command.
Let the driver use the new command if supported.

Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
---
 drivers/ata/libata-scsi.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index b1e880a..94b0f5e 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3100,12 +3100,27 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	buf = page_address(sg_page(scsi_sglist(scmd)));
 	size = ata_set_lba_range_entries(buf, 512, block, n_block);
 
-	tf->protocol = ATA_PROT_DMA;
-	tf->hob_feature = 0;
-	tf->feature = ATA_DSM_TRIM;
-	tf->hob_nsect = (size / 512) >> 8;
-	tf->nsect = size / 512;
-	tf->command = ATA_CMD_DSM;
+	if (ata_ncq_enabled(dev) &&
+	    (dev->ncq_send_recv_cmds[ATA_LOG_NCQ_SEND_RECV_DSM_OFFSET] &
+	     ATA_LOG_NCQ_SEND_RECV_DSM_TRIM)) {
+		/* Newer devices support queued TRIM commands */
+		tf->protocol = ATA_PROT_NCQ;
+		tf->command = ATA_CMD_FPDMA_SEND;
+		tf->hob_nsect = ATA_SUBCMD_FPDMA_SEND_DSM & 0x1f;
+		tf->nsect = qc->tag << 3;
+		tf->hob_feature = (size / 512) >> 8;
+		tf->feature = size / 512;
+
+		qc->auxiliary = 1;
+	} else {
+		tf->protocol = ATA_PROT_DMA;
+		tf->hob_feature = 0;
+		tf->feature = ATA_DSM_TRIM;
+		tf->hob_nsect = (size / 512) >> 8;
+		tf->nsect = size / 512;
+		tf->command = ATA_CMD_DSM;
+	}
+
 	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48 |
 		     ATA_TFLAG_WRITE;
 
-- 
1.8.1.2

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Sergei Shtylyov]

Hello.

On 08/08/2013 11:16 AM, Marc C wrote:

> From: Marc Carino <marc.ceeeee@gmail.com>

> Add support for the following ATA opcodes, which are present
> in SATA 3.1 and T13 ATA ACS-3:

>          SEND FPDMA QUEUED
>          RECEIVE FPDMA QUEUED

> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>

[...]

> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index ee0bd95..dd26211 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
[...]
> @@ -509,6 +525,8 @@ struct ata_taskfile {
>   	u8			device;
>
>   	u8			command;	/* IO operation */
> +
> +	u32			auxiliary;

    You are still basing your work on the wrong branch -- you should use Tejun 
Heo's libata.git, 'for-3.12' branch. And you haven't removed this field from 
'struct ata_taskfile'...

WBR, Sergei

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Sergei Shtylyov]

On 08/08/2013 11:16 AM, Marc C wrote:

> From: Marc Carino <marc.ceeeee@gmail.com>

> Add support for the following ATA opcodes, which are present
> in SATA 3.1 and T13 ATA ACS-3:

>          SEND FPDMA QUEUED
>          RECEIVE FPDMA QUEUED

> Signed-off-by: Marc Carino <marc.ceeeee@gmail.com>
[...]

> diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> index 1145637..f9e4fd8 100644
> --- a/drivers/ata/ahci.h
> +++ b/drivers/ata/ahci.h
> @@ -388,4 +388,12 @@ static inline int ahci_nr_ports(u32 cap)
>   	return (cap & 0x1f) + 1;
>   }
>
> +static inline void ahci_h2dfis_set_auxiliary(u8 *fis, u32 aux)
> +{
> +	fis[16] = aux & 0xff;
> +	fis[17] = (aux >> 8) & 0xff;
> +	fis[18] = (aux >> 16) & 0xff;
> +	fis[19] = (aux >> 24) & 0xff;
> +}
> +
>   #endif /* _AHCI_H */
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index acfd0f7..52668f4 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1499,6 +1499,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
>   	cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ;
>
>   	ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, cmd_tbl);
> +	ahci_h2dfis_set_auxiliary(cmd_tbl, qc->auxiliary);
>   	if (is_atapi) {
>   		memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32);
>   		memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len);

    How about non-AHCI FIS-based controllers?

WBR, Sergei

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Marc C]

Hello Sergei,

>    How about non-AHCI FIS-based controllers?
Right. Since it's cost prohibitive for me to test exhaustively on
non-AHCI FIS-based controllers, do you think it would be acceptable to
add a new ATA host flag... something like, ATA_FLAG_AHCI, which would
denote the controller as being AHCI-based? Then the flag could be used
to gate processing of READ/WRITE/SEND/RECEIVE FPDMA commands that have
the 'auxiliary' field set. Or, I could add a big fat warning print
whenever an ata_queued_cmd is passed to the drivers with a non-zero
'auxiliary' value.

Regards,
Marc

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Sergei Shtylyov]

Hello.

On 08/08/2013 10:58 PM, Marc C wrote:

>>     How about non-AHCI FIS-based controllers?

> Right. Since it's cost prohibitive for me to test exhaustively on
> non-AHCI FIS-based controllers, do you think it would be acceptable to

    You can mark your patch as RFT (request for testing) in this case.

> add a new ATA host flag... something like, ATA_FLAG_AHCI, which would
> denote the controller as being AHCI-based? Then the flag could be used
> to gate processing of READ/WRITE/SEND/RECEIVE FPDMA commands that have
> the 'auxiliary' field set. Or, I could add a big fat warning print
> whenever an ata_queued_cmd is passed to the drivers with a non-zero
> 'auxiliary' value.

    I was rather thinking about a flag (ATA_FLAG_FIS_BASED, maybe) marking a 
controller as FIS-based, so that libata would know whether it can issue the 
new commands using the 'auxiliary' value.

> Regards,
> Marc

WBR, Sergei

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Sergei Shtylyov]

On 08/08/2013 11:03 PM, Sergei Shtylyov wrote:

>>>     How about non-AHCI FIS-based controllers?

>> Right. Since it's cost prohibitive for me to test exhaustively on
>> non-AHCI FIS-based controllers, do you think it would be acceptable to

>     You can mark your patch as RFT (request for testing) in this case.

>> add a new ATA host flag... something like, ATA_FLAG_AHCI, which would
>> denote the controller as being AHCI-based? Then the flag could be used
>> to gate processing of READ/WRITE/SEND/RECEIVE FPDMA commands that have
>> the 'auxiliary' field set. Or, I could add a big fat warning print
>> whenever an ata_queued_cmd is passed to the drivers with a non-zero
>> 'auxiliary' value.

>     I was rather thinking about a flag (ATA_FLAG_FIS_BASED, maybe) marking a
> controller as FIS-based, so that libata would know whether it can issue the
> new commands using the 'auxiliary' value.

    Perhaps we can test for the presence of one of the sff_*() methods as a 
negative condition for the controller being FIS-based.

>> Regards,
>> Marc

WBR, Sergei

Re: [PATCH v2 1/2] libata: Add support for SEND/RECEIVE FPDMA QUEUED

[Tejun Heo]

On Thu, Aug 08, 2013 at 11:03:55PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 08/08/2013 10:58 PM, Marc C wrote:
> 
> >>    How about non-AHCI FIS-based controllers?
> 
> >Right. Since it's cost prohibitive for me to test exhaustively on
> >non-AHCI FIS-based controllers, do you think it would be acceptable to
> 
>    You can mark your patch as RFT (request for testing) in this case.
> 
> >add a new ATA host flag... something like, ATA_FLAG_AHCI, which would
> >denote the controller as being AHCI-based? Then the flag could be used
> >to gate processing of READ/WRITE/SEND/RECEIVE FPDMA commands that have
> >the 'auxiliary' field set. Or, I could add a big fat warning print
> >whenever an ata_queued_cmd is passed to the drivers with a non-zero
> >'auxiliary' value.
> 
>    I was rather thinking about a flag (ATA_FLAG_FIS_BASED, maybe)
> marking a controller as FIS-based, so that libata would know whether
> it can issue the new commands using the 'auxiliary' value.

I'm not sure whether checking whether a controller is FIS based would
be enough.  sil24 is clearly FIS based but it snoops the command code
and likely to choke on commands which it doesn't know about.  I think
the best way to deal with it would be having a feature flag on the
host / port and setting it on controllers known to work.  In practice,
just enabling it on ahci's should cover most use cases from now on
anyway.

Thanks.

-- 
tejun