Re: [PATCH] util/vfio-helpers.c: Use ram_block_discard_disable() in qemu_vfio_open_pci()

[David Hildenbrand]

On 16.11.20 14:14, Philippe Mathieu-Daudé wrote:
> Hi David,
>
> Is this aiming at 5.2?

Hi,

Good point!

If possible, we want this in 5.2.

Thanks!

> On 11/16/20 11:59 AM, David Hildenbrand wrote:
> > Currently, when using "nvme://" for a block device, like
> >      -drive file=nvme://0000:01:00.0/1,if=none,id=drive0 \
> >      -device virtio-blk,drive=drive0 \
> >
> > VFIO may pin all guest memory, and discarding of RAM no longer works as
> > expected. I was able to reproduce this easily with my
> >      01:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
> >              NVMe SSD Controller SM981/PM981/PM983
> >
> > Similar to common VFIO, we have to disable it, making sure that:
> > a) virtio-balloon won't discard any memory ("silently disabled")
> > b) virtio-mem and nvme:// run mutually exclusive
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Alex Williamson <alex.williamson@redhat.com>
> > Cc: Wei Yang <richardw.yang@linux.intel.com>
> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> >   stubs/ram-block.c   |  6 ++++++
> >   util/vfio-helpers.c | 14 ++++++++++++++
> >   2 files changed, 20 insertions(+)
> >
> > diff --git a/stubs/ram-block.c b/stubs/ram-block.c
> > index 73c0a3ee08..108197683b 100644
> > --- a/stubs/ram-block.c
> > +++ b/stubs/ram-block.c
> > @@ -1,6 +1,7 @@
> >   #include "qemu/osdep.h"
> >   #include "exec/ramlist.h"
> >   #include "exec/cpu-common.h"
> > +#include "exec/memory.h"
> >   
> >   void *qemu_ram_get_host_addr(RAMBlock *rb)
> >   {
> > @@ -29,3 +30,8 @@ int qemu_ram_foreach_block(RAMBlockIterFunc func, void 
> > *opaque)
> >   {
> >       return 0;
> >   }
> > +
> > +int ram_block_discard_disable(bool state)
> > +{
> > +    return 0;
> > +}
> > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
> > index c469beb061..2bec48e163 100644
> > --- a/util/vfio-helpers.c
> > +++ b/util/vfio-helpers.c
> > @@ -16,6 +16,7 @@
> >   #include "qapi/error.h"
> >   #include "exec/ramlist.h"
> >   #include "exec/cpu-common.h"
> > +#include "exec/memory.h"
> >   #include "trace.h"
> >   #include "qemu/error-report.h"
> >   #include "standard-headers/linux/pci_regs.h"
> > @@ -494,8 +495,20 @@ QEMUVFIOState *qemu_vfio_open_pci(const char *device, 
> > Error **errp)
> >       int r;
> >       QEMUVFIOState *s = g_new0(QEMUVFIOState, 1);
> >   
> > +    /*
> > +     * VFIO may pin all memory inside mappings, resulting it in pinning
> > +     * all memory inside RAM blocks unconditionally.
> > +     */
> > +    r = ram_block_discard_disable(true);
> > +    if (r) {
> > +        error_setg_errno(errp, -r, "Cannot set discarding of RAM broken");
> > +        g_free(s);
> > +        return NULL;
> > +    }
> > +
> >       r = qemu_vfio_init_pci(s, device, errp);
> >       if (r) {
> > +        ram_block_discard_disable(false);
> >           g_free(s);
> >           return NULL;
> >       }
> > @@ -837,4 +850,5 @@ void qemu_vfio_close(QEMUVFIOState *s)
> >       close(s->device);
> >       close(s->group);
> >       close(s->container);
> > +    ram_block_discard_disable(false);
> >   }


--
Thanks,

David / dhildenb