Re: [PATCH v2 2/3] memory: Allow a MemoryRegion to be marked no_vhost

[Alex Williamson]

On Tue, 14 Jan 2020 02:19:39 -0500
"Michael S. Tsirkin" <address@hidden> wrote:

> On Mon, Jan 13, 2020 at 05:36:46PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > Allow a memory region to be marked as 'no_vhost' and
> > exclude that region from vhost's list build.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>  
> 
> I thought we agreed vfio needs this as well?
> I'd rather this had some meaning not just "no vhost" ...
> no_dma?

Sorry, why does vfio need this?  Are we intending to disallow
peer-to-peer DMA?  A vfio device MMIO range could certainly be a DMA
target, but we need to stick to basic CPU operations to access it (ie.
no multimedia extensions like used in memcpy), that's what brought
about the ram device memory region type.  Thanks,

Alex

> > ---
> >  hw/virtio/vhost.c     |  3 ++-
> >  include/exec/memory.h | 21 +++++++++++++++++++++
> >  memory.c              | 15 +++++++++++++++
> >  3 files changed, 38 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 774d87d98e..462498bfe6 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -402,7 +402,8 @@ static bool vhost_section(struct vhost_dev *dev, 
> > MemoryRegionSection *section)
> >      bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
> >                       ~(1 << DIRTY_MEMORY_MIGRATION);
> >      result = memory_region_is_ram(section->mr) &&
> > -        !memory_region_is_rom(section->mr);
> > +             !memory_region_is_rom(section->mr) &&
> > +             !memory_region_get_no_vhost(section->mr);
> >  
> >      /* Vhost doesn't handle any block which is doing dirty-tracking other
> >       * than migration; this typically fires on VGA areas.
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index aef8123d48..f475c06d63 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -394,6 +394,7 @@ struct MemoryRegion {
> >      bool ram_device;
> >      bool enabled;
> >      bool warning_printed; /* For reservations */
> > +    bool no_vhost;
> >      uint8_t vga_logging_count;
> >      MemoryRegion *alias;
> >      hwaddr alias_offset;
> > @@ -1625,6 +1626,26 @@ void memory_region_set_readonly(MemoryRegion *mr, 
> > bool readonly);
> >   */
> >  void memory_region_set_nonvolatile(MemoryRegion *mr, bool nonvolatile);
> >  
> > +/**
> > + * memory_region_set_no_vhost: Make vhost ignore a memory region
> > + *
> > + * Makes vhost ignore a memory region, useful if it isn't real
> > + * DMAble memory and is at inconvenient addresses
> > + *
> > + * @mr: the region being updated.
> > + * @no_vhost: true to ignore
> > + */
> > +void memory_region_set_no_vhost(MemoryRegion *mr, bool no_vhost);
> > +
> > +/**
> > + * memory_region_set_no_vhost: Test if memory region is marked no vhost
> > + *
> > + * Test if the no_vhost flag is set on the memory region
> > + *
> > + * @mr: the region being tested.
> > + */
> > +bool memory_region_get_no_vhost(const MemoryRegion *mr);
> > +
> >  /**
> >   * memory_region_rom_device_set_romd: enable/disable ROMD mode
> >   *
> > diff --git a/memory.c b/memory.c
> > index d7b9bb6951..9371998e30 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -2136,6 +2136,21 @@ void memory_region_set_nonvolatile(MemoryRegion *mr, 
> > bool nonvolatile)
> >      }
> >  }
> >  
> > +void memory_region_set_no_vhost(MemoryRegion *mr, bool no_vhost)
> > +{
> > +    if (mr->no_vhost != no_vhost) {
> > +        memory_region_transaction_begin();
> > +        mr->no_vhost = no_vhost;
> > +        memory_region_update_pending |= mr->enabled;
> > +        memory_region_transaction_commit();
> > +    }
> > +}
> > +
> > +bool memory_region_get_no_vhost(const MemoryRegion *mr)
> > +{
> > +    return mr->no_vhost;
> > +}
> > +
> >  void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
> >  {
> >      if (mr->romd_mode != romd_mode) {
> > -- 
> > 2.24.1  
> 
>