From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, Haozhong Zhang <haozhong.zhang@intel.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
keith.busch@intel.com, Zhang Yi <yi.z.zhang@linux.intel.com>,
Andrzej Jakowski <andrzej.jakowski@linux.intel.com>,
Junyan He <junyan.he@intel.com>
Subject: Re: [PATCH v1] block/nvme: introduce PMR support from NVMe 1.4 spec
Date: Fri, 21 Feb 2020 18:45:01 +0000 [thread overview]
Message-ID: <20200221184501.GL2931@work-vm> (raw)
In-Reply-To: <20200221134555.GK1484511@stefanha-x1.localdomain>
* Stefan Hajnoczi (stefanha@gmail.com) wrote:
> On Tue, Feb 18, 2020 at 03:48:11PM -0700, Andrzej Jakowski wrote:
> > This patch introduces support for PMR that has been defined as part of NVMe 1.4
> > spec. User can now specify a pmr_file which will be mmap'ed into qemu address
> > space and subsequently in PCI BAR 2. Guest OS can perform mmio read and writes
> > to the PMR region that will stay persistent accross system reboot.
> >
> > Signed-off-by: Andrzej Jakowski <andrzej.jakowski@linux.intel.com>
> > ---
> > hw/block/nvme.c | 145 ++++++++++++++++++++++++++++++++++-
> > hw/block/nvme.h | 5 ++
> > hw/block/trace-events | 5 ++
> > include/block/nvme.h | 172 ++++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 326 insertions(+), 1 deletion(-)
>
> NVDIMM folks, please take a look. There seems to be commonality here.
>
> Can this use -object memory-backend-file instead of manually opening and
> mapping a file?
>
> Also CCing David Gilbert because there is some similarity with the
> vhost-user-fs's DAX Window feature where QEMU mmaps regions of files
> into a BAR.
I guess the biggest difference here is that the read can have the side
effect; in my world I don't have to set read/write/endian ops - I just
map a chunk of memory and use memory_region_add_subregion, so all the
read/writes are native read/writes - I assume that would be a lot
faster - I guess it depends if NVME_PMRCAP_PMRWBM is something constant
you know early on; if you know that you don't need to do side effects in
the read you could do the same trick and avoid the IO ops altogether.
Isn't there also a requirement that BARs are powers of two? Wouldn't
you need to ensure the PMR file is a power of 2 in size?
Dave
> > @@ -1303,6 +1327,38 @@ static const MemoryRegionOps nvme_cmb_ops = {
> > },
> > };
> >
> > +static void nvme_pmr_write(void *opaque, hwaddr addr, uint64_t data,
> > + unsigned size)
> > +{
> > + NvmeCtrl *n = (NvmeCtrl *)opaque;
> > + stn_le_p(&n->pmrbuf[addr], size, data);
> > +}
> > +
> > +static uint64_t nvme_pmr_read(void *opaque, hwaddr addr, unsigned size)
> > +{
> > + NvmeCtrl *n = (NvmeCtrl *)opaque;
> > + if (!NVME_PMRCAP_PMRWBM(n->bar.pmrcap)) {
> > + int ret;
> > + ret = msync(n->pmrbuf, n->f_pmr_size, MS_SYNC);
> > + if (!ret) {
> > + NVME_GUEST_ERR(nvme_ub_mmiowr_pmrread_barrier,
> > + "error while persisting data");
> > + }
> > + }
>
> Why is msync(2) done on memory loads instead of stores?
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2020-02-21 18:46 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-18 22:48 [PATCH v1] block/nvme: introduce PMR support from NVMe 1.4 spec Andrzej Jakowski
2020-02-19 1:07 ` no-reply
2020-02-19 20:25 ` Andrzej Jakowski
2020-02-21 13:45 ` Stefan Hajnoczi
2020-02-21 15:36 ` Andrzej Jakowski
2020-02-21 17:31 ` Stefan Hajnoczi
2020-02-21 17:50 ` Dr. David Alan Gilbert
2020-02-21 18:29 ` Stefan Hajnoczi
2020-02-21 19:32 ` Stefan Hajnoczi
2020-02-21 20:20 ` Andrzej Jakowski
2020-02-21 18:45 ` Dr. David Alan Gilbert [this message]
2020-02-21 20:19 ` Andrzej Jakowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200221184501.GL2931@work-vm \
--to=dgilbert@redhat.com \
--cc=andrzej.jakowski@linux.intel.com \
--cc=haozhong.zhang@intel.com \
--cc=junyan.he@intel.com \
--cc=keith.busch@intel.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.com \
--cc=yi.z.zhang@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).