qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH RESEND v2] block/nvme: introduce PMR support from NVMe 1.4 sp


From: Stefan Hajnoczi
Subject: Re: [PATCH RESEND v2] block/nvme: introduce PMR support from NVMe 1.4 spec
Date: Wed, 11 Mar 2020 09:20:18 +0000

On Tue, Mar 10, 2020 at 8:09 PM Andrzej Jakowski
<address@hidden> wrote:
> On 3/10/20 2:51 AM, Stefan Hajnoczi wrote:
> > On Fri, Mar 06, 2020 at 03:38:53PM -0700, Andrzej Jakowski wrote:
> >> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> >> index d28335cbf3..ff7e74d765 100644
> >> --- a/hw/block/nvme.c
> >> +++ b/hw/block/nvme.c
> >> @@ -19,10 +19,14 @@
> >>   *      -drive file=<file>,if=none,id=<drive_id>
> >>   *      -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \
> >>   *              cmb_size_mb=<cmb_size_mb[optional]>, \
> >> + *              [pmr_file=<pmr_file_path>,] \
> >>   *              num_queues=<N[optional]>
> >>   *
> >>   * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
> >>   * offset 0 in BAR2 and supports only WDS, RDS and SQS for now.
> >> + *
> >> + * Either cmb or pmr - due to limitation in avaialbe BAR indexes.
> >
> > s/avaialbe/available/
> >
> >> + * pmr_file file needs to be preallocated and power of two in size.
> >
> > Why does it need to be preallocated?
>
> PMR file is mmaped into address space. If memory accesses are made outside of
> file then SIGBUS signal is raised. Preallocation requirement was introduced
> to prevent this situation.

Oh, I think I see what you mean.  That is not how the term
"preallocated" is usually used in POSIX file systems.  File systems
have sparse files by default and the term preallocation is used in the
context of fadvise(2) for reserving space.

In this case I think you're saying the file cannot grow.  That is
implicit since the BAR can't grow either so you could drop the comment
about preallocation.

> >
> >>   */
> >>
> >>  #include "qemu/osdep.h"
> >> @@ -1141,6 +1145,28 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr 
> >> offset, uint64_t data,
> >>          NVME_GUEST_ERR(nvme_ub_mmiowr_cmbsz_readonly,
> >>                         "invalid write to read only CMBSZ, ignored");
> >>          return;
> >> +#ifndef _WIN32
> >
> > This ifdef is a hint that the layering is not right.  QEMU device models
> > usually only implement the "frontend" device registers, interrupts, and
> > request processing logic.  The platform-specific host "backend"
> > (mmapping files, sending network packets, audio/graphics APIs, etc) is
> > implemented separately.
>
> Agree. I couldn't find QEMU backend ensuring persistence - thus decided to
> go with mmap.

Please try:

  $ git grep pmem

backends/hostmem-file.c is the backend that can be used and the
pmem_persist() API can be used to flush writes.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]