Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Robert Hancock wrote: > Tejun Heo wrote: >> Tejun Heo wrote: >>> If so, can you please add that switching into register mode is okay as >>> long as there's no other ADMA commands in flight and add >>> WARN_ON((qc->flags & ATA_QCFLAG_RESULT_TF) && link->sactive)? >> >> More accurately, link->sactive test can be substituted with >> (ap->qc_allocated & ~(1 << qc->tag)). > > Unfortunately we only get the ata_port and ata_taskfile in the tf_read > callback, so I'm not sure if we can do the equivalent of the qc->flags & > ATA_QCFLAG_RESULT_TF test (i.e. distinguishing between the > error-handling case where we care if we abort outstanding commands and > the normal case with a RESULT_TF command where we do).. You can test it in ->qc_issue(), no? -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Robert Hancock wrote: Tejun Heo wrote: Tejun Heo wrote: If so, can you please add that switching into register mode is okay as long as there's no other ADMA commands in flight and add WARN_ON((qc-flags ATA_QCFLAG_RESULT_TF) link-sactive)? More accurately, link-sactive test can be substituted with (ap-qc_allocated ~(1 qc-tag)). Unfortunately we only get the ata_port and ata_taskfile in the tf_read callback, so I'm not sure if we can do the equivalent of the qc-flags ATA_QCFLAG_RESULT_TF test (i.e. distinguishing between the error-handling case where we care if we abort outstanding commands and the normal case with a RESULT_TF command where we do).. You can test it in -qc_issue(), no? -- tejun - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Tejun Heo wrote: Tejun Heo wrote: If so, can you please add that switching into register mode is okay as long as there's no other ADMA commands in flight and add WARN_ON((qc->flags & ATA_QCFLAG_RESULT_TF) && link->sactive)? More accurately, link->sactive test can be substituted with (ap->qc_allocated & ~(1 << qc->tag)). Unfortunately we only get the ata_port and ata_taskfile in the tf_read callback, so I'm not sure if we can do the equivalent of the qc->flags & ATA_QCFLAG_RESULT_TF test (i.e. distinguishing between the error-handling case where we care if we abort outstanding commands and the normal case with a RESULT_TF command where we do).. -- Robert Hancock Saskatoon, SK, Canada To email, remove "nospam" from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Tejun Heo wrote: Tejun Heo wrote: If so, can you please add that switching into register mode is okay as long as there's no other ADMA commands in flight and add WARN_ON((qc-flags ATA_QCFLAG_RESULT_TF) link-sactive)? More accurately, link-sactive test can be substituted with (ap-qc_allocated ~(1 qc-tag)). Unfortunately we only get the ata_port and ata_taskfile in the tf_read callback, so I'm not sure if we can do the equivalent of the qc-flags ATA_QCFLAG_RESULT_TF test (i.e. distinguishing between the error-handling case where we care if we abort outstanding commands and the normal case with a RESULT_TF command where we do).. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Tejun Heo wrote: > If so, can you please add that switching into register mode is okay as > long as there's no other ADMA commands in flight and add > WARN_ON((qc->flags & ATA_QCFLAG_RESULT_TF) && link->sactive)? More accurately, link->sactive test can be substituted with (ap->qc_allocated & ~(1 << qc->tag)). -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Hello,
Robert Hancock wrote:
> We need to run any DMA command with result taskfile requested in ADMA mode
> when the port is in ADMA mode, otherwise it may try to use the legacy DMA
> engine
> in ADMA mode which is not allowed. Enforce this with BUG_ON() since data
> corruption could potentially result if this happened.
>
> Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
>
> --- linux-2.6.24-rc1-git10/drivers/ata/sata_nv.c 2007-11-01
> 20:01:32.0 -0600
> +++ linux-2.6.24-rc1-git10edit/drivers/ata/sata_nv.c 2007-11-13
> 19:01:09.0 -0600
> @@ -791,11 +797,13 @@
>
> static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
> {
> - /* Since commands where a result TF is requested are not
> -executed in ADMA mode, the only time this function will be called
> -in ADMA mode will be if a command fails. In this case we
> -don't care about going into register mode with ADMA commands
> -pending, as the commands will all shortly be aborted anyway. */
> + /* Other than when internal or pass-through commands are executed,
> +the only time this function will be called in ADMA mode will be
> +if a command fails. In the failure case we don't care about going
> +into register mode with ADMA commands pending, as the commands will
> +all shortly be aborted anyway. We assume that NCQ commands are not
> +issued via passthrough and so this will not abort any commands in
> +that case. */
> nv_adma_register_mode(ap);
So, now if an ATA DMA command is issued w/ RESULT_TF set, it's issued
using ADMA. Then when nv_adma_tf_read() is called on success path, it
switches into register mode and read TF which is okay as long as there's
no other ADMA commands in flight and that's why you wrote about not
issuing NCQ commands via NCQ. Am I understanding it correctly?
If so, can you please add that switching into register mode is okay as
long as there's no other ADMA commands in flight and add
WARN_ON((qc->flags & ATA_QCFLAG_RESULT_TF) && link->sactive)?
Thanks.
--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Hello,
Robert Hancock wrote:
We need to run any DMA command with result taskfile requested in ADMA mode
when the port is in ADMA mode, otherwise it may try to use the legacy DMA
engine
in ADMA mode which is not allowed. Enforce this with BUG_ON() since data
corruption could potentially result if this happened.
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
--- linux-2.6.24-rc1-git10/drivers/ata/sata_nv.c 2007-11-01
20:01:32.0 -0600
+++ linux-2.6.24-rc1-git10edit/drivers/ata/sata_nv.c 2007-11-13
19:01:09.0 -0600
@@ -791,11 +797,13 @@
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
{
- /* Since commands where a result TF is requested are not
-executed in ADMA mode, the only time this function will be called
-in ADMA mode will be if a command fails. In this case we
-don't care about going into register mode with ADMA commands
-pending, as the commands will all shortly be aborted anyway. */
+ /* Other than when internal or pass-through commands are executed,
+the only time this function will be called in ADMA mode will be
+if a command fails. In the failure case we don't care about going
+into register mode with ADMA commands pending, as the commands will
+all shortly be aborted anyway. We assume that NCQ commands are not
+issued via passthrough and so this will not abort any commands in
+that case. */
nv_adma_register_mode(ap);
So, now if an ATA DMA command is issued w/ RESULT_TF set, it's issued
using ADMA. Then when nv_adma_tf_read() is called on success path, it
switches into register mode and read TF which is okay as long as there's
no other ADMA commands in flight and that's why you wrote about not
issuing NCQ commands via NCQ. Am I understanding it correctly?
If so, can you please add that switching into register mode is okay as
long as there's no other ADMA commands in flight and add
WARN_ON((qc-flags ATA_QCFLAG_RESULT_TF) link-sactive)?
Thanks.
--
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
Tejun Heo wrote: If so, can you please add that switching into register mode is okay as long as there's no other ADMA commands in flight and add WARN_ON((qc-flags ATA_QCFLAG_RESULT_TF) link-sactive)? More accurately, link-sactive test can be substituted with (ap-qc_allocated ~(1 qc-tag)). -- tejun - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
We need to run any DMA command with result taskfile requested in ADMA mode
when the port is in ADMA mode, otherwise it may try to use the legacy DMA engine
in ADMA mode which is not allowed. Enforce this with BUG_ON() since data
corruption could potentially result if this happened.
Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
--- linux-2.6.24-rc1-git10/drivers/ata/sata_nv.c2007-11-01
20:01:32.0 -0600
+++ linux-2.6.24-rc1-git10edit/drivers/ata/sata_nv.c2007-11-13
19:01:09.0 -0600
@@ -791,11 +797,13 @@
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
{
- /* Since commands where a result TF is requested are not
- executed in ADMA mode, the only time this function will be called
- in ADMA mode will be if a command fails. In this case we
- don't care about going into register mode with ADMA commands
- pending, as the commands will all shortly be aborted anyway. */
+ /* Other than when internal or pass-through commands are executed,
+ the only time this function will be called in ADMA mode will be
+ if a command fails. In the failure case we don't care about going
+ into register mode with ADMA commands pending, as the commands will
+ all shortly be aborted anyway. We assume that NCQ commands are not
+ issued via passthrough and so this will not abort any commands in
+ that case. */
nv_adma_register_mode(ap);
ata_tf_read(ap, tf);
@@ -1359,11 +1376,9 @@
struct nv_adma_port_priv *pp = qc->ap->private_data;
/* ADMA engine can only be used for non-ATAPI DMA commands,
- or interrupt-driven no-data commands, where a result taskfile
- is not required. */
+ or interrupt-driven no-data commands. */
if ((pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) ||
- (qc->tf.flags & ATA_TFLAG_POLLING) ||
- (qc->flags & ATA_QCFLAG_RESULT_TF))
+ (qc->tf.flags & ATA_TFLAG_POLLING))
return 1;
if ((qc->flags & ATA_QCFLAG_DMAMAP) ||
@@ -1381,6 +1396,8 @@
NV_CPB_CTL_IEN;
if (nv_adma_use_reg_mode(qc)) {
+ BUG_ON(!(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) &&
+ (qc->flags & ATA_QCFLAG_DMAMAP));
nv_adma_register_mode(qc->ap);
ata_qc_prep(qc);
return;
@@ -1428,6 +1445,8 @@
if (nv_adma_use_reg_mode(qc)) {
/* use ATA register mode */
VPRINTK("using ATA register mode: 0x%lx\n", qc->flags);
+ BUG_ON(!(pp->flags & NV_ADMA_ATAPI_SETUP_COMPLETE) &&
+ (qc->flags & ATA_QCFLAG_DMAMAP));
nv_adma_register_mode(qc->ap);
return ata_qc_issue_prot(qc);
} else
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] sata_nv: don't use legacy DMA in ADMA mode
We need to run any DMA command with result taskfile requested in ADMA mode
when the port is in ADMA mode, otherwise it may try to use the legacy DMA engine
in ADMA mode which is not allowed. Enforce this with BUG_ON() since data
corruption could potentially result if this happened.
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
--- linux-2.6.24-rc1-git10/drivers/ata/sata_nv.c2007-11-01
20:01:32.0 -0600
+++ linux-2.6.24-rc1-git10edit/drivers/ata/sata_nv.c2007-11-13
19:01:09.0 -0600
@@ -791,11 +797,13 @@
static void nv_adma_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
{
- /* Since commands where a result TF is requested are not
- executed in ADMA mode, the only time this function will be called
- in ADMA mode will be if a command fails. In this case we
- don't care about going into register mode with ADMA commands
- pending, as the commands will all shortly be aborted anyway. */
+ /* Other than when internal or pass-through commands are executed,
+ the only time this function will be called in ADMA mode will be
+ if a command fails. In the failure case we don't care about going
+ into register mode with ADMA commands pending, as the commands will
+ all shortly be aborted anyway. We assume that NCQ commands are not
+ issued via passthrough and so this will not abort any commands in
+ that case. */
nv_adma_register_mode(ap);
ata_tf_read(ap, tf);
@@ -1359,11 +1376,9 @@
struct nv_adma_port_priv *pp = qc-ap-private_data;
/* ADMA engine can only be used for non-ATAPI DMA commands,
- or interrupt-driven no-data commands, where a result taskfile
- is not required. */
+ or interrupt-driven no-data commands. */
if ((pp-flags NV_ADMA_ATAPI_SETUP_COMPLETE) ||
- (qc-tf.flags ATA_TFLAG_POLLING) ||
- (qc-flags ATA_QCFLAG_RESULT_TF))
+ (qc-tf.flags ATA_TFLAG_POLLING))
return 1;
if ((qc-flags ATA_QCFLAG_DMAMAP) ||
@@ -1381,6 +1396,8 @@
NV_CPB_CTL_IEN;
if (nv_adma_use_reg_mode(qc)) {
+ BUG_ON(!(pp-flags NV_ADMA_ATAPI_SETUP_COMPLETE)
+ (qc-flags ATA_QCFLAG_DMAMAP));
nv_adma_register_mode(qc-ap);
ata_qc_prep(qc);
return;
@@ -1428,6 +1445,8 @@
if (nv_adma_use_reg_mode(qc)) {
/* use ATA register mode */
VPRINTK(using ATA register mode: 0x%lx\n, qc-flags);
+ BUG_ON(!(pp-flags NV_ADMA_ATAPI_SETUP_COMPLETE)
+ (qc-flags ATA_QCFLAG_DMAMAP));
nv_adma_register_mode(qc-ap);
return ata_qc_issue_prot(qc);
} else
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
