nvme_admin_passthru DMA failure

nvme_admin_passthru DMA failure

[Patrick Sheridan]

There is a problem when using an NVME_IOCTL_ADMIN_CMD to submit an
nvme_admin_passthru struct such that user data
(nvme_passthru_cmd.addr) does not get DMA'd to the drive correctly.

The problem first appeared in kernel tags/v4.5.1 and was absent in
tags/v4.5.  The only relevant difference between those was in
block/blk-core.c, so this is likely a block layer problem:

diff --git a/block/blk-core.c b/block/blk-core.c
index b83d297..45f4d7e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2198,7 +2198,7 @@ int blk_insert_cloned_request(struct
request_queue *q, struct request *rq)
        if (q->mq_ops) {
                if (blk_queue_io_stat(q))
                        blk_account_io_start(rq, true);
-               blk_mq_insert_request(rq, false, true, true);
+               blk_mq_insert_request(rq, false, true, false);
                return 0;
        }

Reverting the blk_mq_insert_request to use async = true resolves the
issue in 4.5.1, but the latest (sha:  c8d2bc9) no longer uses this
pathway.

There's been a lot of changes since 4.5.1, so I haven't been able to
pin down where the problem is, and would appreciate any help.

Regards,
Patrick

---
Patrick Sheridan, PhD
Senior Engineer
Seagate Technology
389 Disc Dr. Longmont, CO 80503

nvme_admin_passthru DMA failure

[Busch, Keith]

Is this a vendor specific opcode? If so, what is the opcode value and what direction are you expecting the data transfer to go?


> -----Original Message-----
> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On Behalf Of Patrick Sheridan
> Sent: Friday, October 7, 2016 3:15 PM
> To: linux-nvme at lists.infradead.org
> Subject: nvme_admin_passthru DMA failure
>
> There is a problem when using an NVME_IOCTL_ADMIN_CMD to submit an
> nvme_admin_passthru struct such that user data
> (nvme_passthru_cmd.addr) does not get DMA'd to the drive correctly.
>
> The problem first appeared in kernel tags/v4.5.1 and was absent in
> tags/v4.5.  The only relevant difference between those was in
> block/blk-core.c, so this is likely a block layer problem:
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index b83d297..45f4d7e 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2198,7 +2198,7 @@ int blk_insert_cloned_request(struct
> request_queue *q, struct request *rq)
>         if (q->mq_ops) {
>                 if (blk_queue_io_stat(q))
>                         blk_account_io_start(rq, true);
> -               blk_mq_insert_request(rq, false, true, true);
> +               blk_mq_insert_request(rq, false, true, false);
>                 return 0;
>         }

nvme_admin_passthru DMA failure

[Patrick Sheridan]

Hi Keith,

It is a vendor specific command with opcode 0xC0 and is expected to
copy data from the host to the drive.
I think you hit the nail on the head:  nvme_is_write would indicate
that it's a read.
Out of curiosity, do you know why this worked in previous nvme drivers?

- Patrick

On Sat, Oct 8, 2016@1:00 PM, Busch, Keith <keith.busch@intel.com> wrote:
> Is this a vendor specific opcode? If so, what is the opcode value and what direction are you expecting the data transfer to go?
>
>
>> -----Original Message-----
>> From: Linux-nvme [mailto:linux-nvme-bounces at lists.infradead.org] On Behalf Of Patrick Sheridan
>> Sent: Friday, October 7, 2016 3:15 PM
>> To: linux-nvme at lists.infradead.org
>> Subject: nvme_admin_passthru DMA failure
>>
>> There is a problem when using an NVME_IOCTL_ADMIN_CMD to submit an
>> nvme_admin_passthru struct such that user data
>> (nvme_passthru_cmd.addr) does not get DMA'd to the drive correctly.
>>
>> The problem first appeared in kernel tags/v4.5.1 and was absent in
>> tags/v4.5.  The only relevant difference between those was in
>> block/blk-core.c, so this is likely a block layer problem:
>>
>> diff --git a/block/blk-core.c b/block/blk-core.c
>> index b83d297..45f4d7e 100644
>> --- a/block/blk-core.c
>> +++ b/block/blk-core.c
>> @@ -2198,7 +2198,7 @@ int blk_insert_cloned_request(struct
>> request_queue *q, struct request *rq)
>>         if (q->mq_ops) {
>>                 if (blk_queue_io_stat(q))
>>                         blk_account_io_start(rq, true);
>> -               blk_mq_insert_request(rq, false, true, true);
>> +               blk_mq_insert_request(rq, false, true, false);
>>                 return 0;
>>         }

nvme_admin_passthru DMA failure

[Keith Busch]

On Mon, Oct 10, 2016@08:58:36AM -0600, Patrick Sheridan wrote:
> Hi Keith,
> 
> It is a vendor specific command with opcode 0xC0 and is expected to
> copy data from the host to the drive.
> I think you hit the nail on the head:  nvme_is_write would indicate
> that it's a read.
> Out of curiosity, do you know why this worked in previous nvme drivers?

Before 4.1, the nvme driver mapped user pages and setup the prp
directly. It didn't care about the alignment.

Now we use blk_rq_map_user, and this uses a bounce buffer if your user
buffer is not aligned. If the data direction is wrong, the data will be
copied to/from the bounce buffer in the wrong direction.

I think you probably will want to adjust your vendor specific opcode.
The driver's ioctl doesn't have a data transfer field, so it always used
the opcode to determine which direction to DMA map a buffer. A C0 is a
command with no data transfer according to the spec. I know the spec
explicitly defines this direction only for defined commands rather
than vendor specific ones, but I don't think that was the intention
(potential for an ECN here).

If you have to stick with the current opcode, you should be able to
work-around the bounce buffer if you allocate your buffers on pages,
but that's conditional on your architecture not enforcing read/write
DMA direction.