diff mbox series

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

Message ID 20200113173647.84842-3-dgilbert@redhat.com
State New
Headers show
Series exclude hyperv synic sections from vhost | expand

Commit Message

Dr. David Alan Gilbert Jan. 13, 2020, 5:36 p.m. UTC 
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

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 <dgilbert@redhat.com>
---
 hw/virtio/vhost.c     |  3 ++-
 include/exec/memory.h | 21 +++++++++++++++++++++
 memory.c              | 15 +++++++++++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

Comments

Michael S. Tsirkin Jan. 14, 2020, 7:19 a.m. UTC | #1 
On Mon, Jan 13, 2020 at 05:36:46PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> 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 <dgilbert@redhat.com>

I thought we agreed vfio needs this as well?
I'd rather this had some meaning not just "no vhost" ...
no_dma?

> ---
>  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
Dr. David Alan Gilbert Jan. 14, 2020, 11:26 a.m. UTC | #2 
* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Jan 13, 2020 at 05:36:46PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > 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 <dgilbert@redhat.com>
> 
> I thought we agreed vfio needs this as well?
> I'd rather this had some meaning not just "no vhost" ...
> no_dma?

VFIO tends to be unplugged on migration anyway (at the moment);
but I'm happy to change the name if everyone is happy with the rest of
the series and we can agree on the new name,

Dave

> > ---
> >  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
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Michael S. Tsirkin Jan. 14, 2020, 2:52 p.m. UTC | #3 
On Tue, Jan 14, 2020 at 11:26:08AM +0000, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (mst@redhat.com) wrote:
> > On Mon, Jan 13, 2020 at 05:36:46PM +0000, Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > 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 <dgilbert@redhat.com>
> > 
> > I thought we agreed vfio needs this as well?
> > I'd rather this had some meaning not just "no vhost" ...
> > no_dma?
> 
> VFIO tends to be unplugged on migration anyway (at the moment);

But it still should ignore synic, right?

> but I'm happy to change the name if everyone is happy with the rest of
> the series and we can agree on the new name,
> 
> Dave
> 
> > > ---
> > >  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
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Alex Williamson Jan. 14, 2020, 3:02 p.m. UTC | #4 
On Tue, 14 Jan 2020 02:19:39 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 13, 2020 at 05:36:46PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > 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 <dgilbert@redhat.com>  
> 
> 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  
> 
>
diff mbox series

Patch

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) {