| Message ID | 20190518073905.17178-1-klaus@birkelund.eu (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | nvme: fix copy direction in DMA reads going to CMB | expand |
On 5/18/2019 1:39 AM, Klaus Birkelund Jensen wrote: > `nvme_dma_read_prp` erronously used `qemu_iovec_*to*_buf` instead of > `qemu_iovec_*from*_buf` when the request involved the controller memory > buffer. > > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> > --- > hw/block/nvme.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 7caf92532a09..63a5b58849fb 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -238,7 +238,7 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len, > } > qemu_sglist_destroy(&qsg); > } else { > - if (unlikely(qemu_iovec_to_buf(&iov, 0, ptr, len) != len)) { > + if (unlikely(qemu_iovec_from_buf(&iov, 0, ptr, len) != len)) { > trace_nvme_err_invalid_dma(); > status = NVME_INVALID_FIELD | NVME_DNR; > } > Reviewed-by: Kenneth Heitke <kenneth.heitke@intel.com>
Am 18.05.2019 um 09:39 hat Klaus Birkelund Jensen geschrieben: > `nvme_dma_read_prp` erronously used `qemu_iovec_*to*_buf` instead of > `qemu_iovec_*from*_buf` when the request involved the controller memory > buffer. > > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> Thanks, applied to the block branch. Kevin
On Sat, May 18, 2019 at 09:39:05AM +0200, Klaus Birkelund Jensen wrote: > `nvme_dma_read_prp` erronously used `qemu_iovec_*to*_buf` instead of > `qemu_iovec_*from*_buf` when the request involved the controller memory > buffer. > > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> I was wondering how this mistake got by for so long, and it looks like the only paths here require an admin command with dev->host transfer to CMB. That's just not done in any host implementation I'm aware of since it'd make it more difficult to use for no particular gain AFAICS, so I'd be curious to hear if you have a legit implementation doing this. Anyway, thanks for the fix.
On Mon, May 20, 2019 at 09:05:57AM -0600, Keith Busch wrote: > On Sat, May 18, 2019 at 09:39:05AM +0200, Klaus Birkelund Jensen wrote: > > `nvme_dma_read_prp` erronously used `qemu_iovec_*to*_buf` instead of > > `qemu_iovec_*from*_buf` when the request involved the controller memory > > buffer. > > > > Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> > > I was wondering how this mistake got by for so long, and it looks like > the only paths here require an admin command with dev->host transfer > to CMB. That's just not done in any host implementation I'm aware of > since it'd make it more difficult to use for no particular gain AFAICS, > so I'd be curious to hear if you have a legit implementation doing this. > I'm just trying to get the device to be as compliant as possible, but I don't know why you'd have any reason to do a, say Get Feature, to the CMB.
diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 7caf92532a09..63a5b58849fb 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -238,7 +238,7 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len, } qemu_sglist_destroy(&qsg); } else { - if (unlikely(qemu_iovec_to_buf(&iov, 0, ptr, len) != len)) { + if (unlikely(qemu_iovec_from_buf(&iov, 0, ptr, len) != len)) { trace_nvme_err_invalid_dma(); status = NVME_INVALID_FIELD | NVME_DNR; }
`nvme_dma_read_prp` erronously used `qemu_iovec_*to*_buf` instead of `qemu_iovec_*from*_buf` when the request involved the controller memory buffer. Signed-off-by: Klaus Birkelund Jensen <klaus.jensen@cnexlabs.com> --- hw/block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)