Re: [PATCH v1] block/nvme: introduce PMR support from NVMe 1.4 spec

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1835 bytes --]

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.

> @@ -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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]