[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestam
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support |
Date: |
Mon, 20 May 2019 11:49:41 +0200 |
User-agent: |
Mutt/1.11.3 (2019-02-01) |
Am 18.05.2019 um 03:49 hat Heitke, Kenneth geschrieben:
>
>
> On 5/17/2019 12:24 AM, Klaus Birkelund wrote:
> > On Fri, May 17, 2019 at 07:35:04AM +0200, Klaus Birkelund wrote:
> > > Hi Kenneth,
> > >
> > > On Thu, May 16, 2019 at 05:24:47PM -0600, Heitke, Kenneth wrote:
> > > > Hi Klaus, thank you for you review. I have one comment inline
> > > >
> > > > On 5/14/2019 12:02 AM, Klaus Birkelund wrote:
> > > > > On Fri, Apr 05, 2019 at 03:41:17PM -0600, Kenneth Heitke wrote:
> > > > > > Signed-off-by: Kenneth Heitke <address@hidden>
> > > > > > ---
> > > > > > hw/block/nvme.c | 120
> > > > > > +++++++++++++++++++++++++++++++++++++++++-
> > > > > > hw/block/nvme.h | 3 ++
> > > > > > hw/block/trace-events | 2 +
> > > > > > include/block/nvme.h | 2 +
> > > > > > 4 files changed, 125 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> > > > > > index 7caf92532a..e775e89299 100644
> > > > > > --- a/hw/block/nvme.c
> > > > > > +++ b/hw/block/nvme.c
> > > > > > @@ -219,6 +219,30 @@ static uint16_t nvme_map_prp(QEMUSGList *qsg,
> > > > > > QEMUIOVector *iov, uint64_t prp1,
> > > > > > return NVME_INVALID_FIELD | NVME_DNR;
> > > > > > }
> > > > > > +static uint16_t nvme_dma_write_prp(NvmeCtrl *n, uint8_t *ptr,
> > > > > > uint32_t len,
> > > > > > + uint64_t prp1, uint64_t prp2)
> > > > > > +{
> > > > > > + QEMUSGList qsg;
> > > > > > + QEMUIOVector iov;
> > > > > > + uint16_t status = NVME_SUCCESS;
> > > > > > +
> > > > > > + if (nvme_map_prp(&qsg, &iov, prp1, prp2, len, n)) {
> > > > > > + return NVME_INVALID_FIELD | NVME_DNR;
> > > > > > + }
> > > > > > + if (qsg.nsg > 0) {
> > > > > > + if (dma_buf_write(ptr, len, &qsg)) {
> > > > > > + status = NVME_INVALID_FIELD | NVME_DNR;
> > > > > > + }
> > > > > > + qemu_sglist_destroy(&qsg);
> > > > > > + } else {
> > > > > > + if (qemu_iovec_from_buf(&iov, 0, ptr, len) != len) {
> > > > >
> > > > > This should be `qemu_iovec_to_buf`.
> > > > >
> > > >
> > > > This function is transferring data from the "host" to the device so I
> > > > believe I am using the correct function.
> > > >
> > >
> > > Exactly, but this means that you need to populate `ptr` with data
> > > described by the prps, hence dma_buf_*write* and qemu_iovec_*to*_buf. In
> > > this case `ptr` is set to the address of the uint64_t timestamp, and
> > > that is what we need to write to.
> > >
> >
> > I was going to argue with the fact that nvme_dma_read_prp uses
> > qemu_iovec_from_buf. But it uses _to_buf which as far as I can tell is
> > also wrong.
> >
>
> Okay, I'm onboard. You're correct. I'll update my patch and re-submit. I can
> also submit a patch to fix nvme_dma_read_prp() unless you or someone else
> wants to.
Maybe it would be time to extend tests/nvme-test.c a bit? So far it only
tests an error case, but it would be good to have tests for successful
use of the basic operations.
Kevin
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, John Snow, 2019/05/13
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Klaus Birkelund, 2019/05/14
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Heitke, Kenneth, 2019/05/16
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Klaus Birkelund, 2019/05/17
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Klaus Birkelund, 2019/05/17
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Heitke, Kenneth, 2019/05/17
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Heitke, Kenneth, 2019/05/17
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support, Klaus Birkelund, 2019/05/18
- Re: [Qemu-devel] [Qemu-block] [PATCH] nvme: add Get/Set Feature Timestamp support,
Kevin Wolf <=