diff mbox

[2/2] mem: highlight the listener's priority as enum

Message ID 1368060022-16911-2-git-send-email-qemulist@gmail.com
State New
Headers show

Commit Message

pingfan liu May 9, 2013, 12:40 a.m. UTC
From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

It will make the priority prominent, when new listener added.
All the priority's value are kept unchanged, except for vhost
and hostmem.(Changes introduced by prev patch)

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 exec.c                        |    4 ++--
 hw/virtio/dataplane/hostmem.c |    2 +-
 hw/virtio/vhost.c             |    2 +-
 include/exec/memory.h         |   12 +++++++++++-
 kvm-all.c                     |    4 ++--
 xen-all.c                     |    2 +-
 6 files changed, 18 insertions(+), 8 deletions(-)

Comments

Stefan Hajnoczi May 9, 2013, 8:31 a.m. UTC | #1
On Thu, May 09, 2013 at 08:40:22AM +0800, Liu Ping Fan wrote:
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 9e88320..77e0d40 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -192,6 +192,16 @@ struct MemoryRegionSection {
>  
>  typedef struct MemoryListener MemoryListener;
>  
> +/* The list of priority, ex, vhost should have higher priority (less num) than
> + * kvm, ie PRI_VRING < PRI_HYPV

s/PRI_HYPV/PRI_HYPERV/

> + */
> +typedef enum {
> +    PRI_DEFAULT = 0,
> +    PRI_CORE = 1,
> +    PRI_VRING = 9,
> +    PRI_HYPERV = 10,

HYPERV == hypervisor?  Easy to confuse with Microsoft Hyper-V.  Please
use PRI_ACCEL or PRI_HYPERVISOR.
pingfan liu May 9, 2013, 9:05 a.m. UTC | #2
On Thu, May 9, 2013 at 4:31 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Thu, May 09, 2013 at 08:40:22AM +0800, Liu Ping Fan wrote:
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index 9e88320..77e0d40 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -192,6 +192,16 @@ struct MemoryRegionSection {
>>
>>  typedef struct MemoryListener MemoryListener;
>>
>> +/* The list of priority, ex, vhost should have higher priority (less num) than
>> + * kvm, ie PRI_VRING < PRI_HYPV
>
> s/PRI_HYPV/PRI_HYPERV/
>
Will fix.
>> + */
>> +typedef enum {
>> +    PRI_DEFAULT = 0,
>> +    PRI_CORE = 1,
>> +    PRI_VRING = 9,
>> +    PRI_HYPERV = 10,
>
> HYPERV == hypervisor?  Easy to confuse with Microsoft Hyper-V.  Please
> use PRI_ACCEL or PRI_HYPERVISOR.

Ok.

Regards,
Pingfan
Peter Maydell May 9, 2013, 9:21 a.m. UTC | #3
On 9 May 2013 01:40, Liu Ping Fan <qemulist@gmail.com> wrote:
> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>
> It will make the priority prominent, when new listener added.
> All the priority's value are kept unchanged, except for vhost
> and hostmem.(Changes introduced by prev patch)

Any chance of something somewhere which explains why
these various things care about the order and thus
need these particular priorities? (ie what are the
dependencies between the listeners?)

thanks
-- PMM
pingfan liu May 9, 2013, 9:30 a.m. UTC | #4
On Thu, May 9, 2013 at 5:21 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 May 2013 01:40, Liu Ping Fan <qemulist@gmail.com> wrote:
>> From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>
>> It will make the priority prominent, when new listener added.
>> All the priority's value are kept unchanged, except for vhost
>> and hostmem.(Changes introduced by prev patch)
>
> Any chance of something somewhere which explains why
> these various things care about the order and thus
> need these particular priorities? (ie what are the
> dependencies between the listeners?)
>
Sorry, I do not go so deeply, so leave the original value as they are.
 As far as I know, the most prominent one is the kvm and the core. If
a region removed from core earlier than kvm, then
cpu_physical_memory_rw() can use an address valid in kvm, but invalid
for radix-tree, and cause error.

> thanks
> -- PMM
diff mbox

Patch

diff --git a/exec.c b/exec.c
index 19725db..aef0349 100644
--- a/exec.c
+++ b/exec.c
@@ -1766,13 +1766,13 @@  static MemoryListener core_memory_listener = {
     .begin = core_begin,
     .log_global_start = core_log_global_start,
     .log_global_stop = core_log_global_stop,
-    .priority = 1,
+    .priority = PRI_CORE,
 };
 
 static MemoryListener io_memory_listener = {
     .region_add = io_region_add,
     .region_del = io_region_del,
-    .priority = 0,
+    .priority = PRI_DEFAULT,
 };
 
 static MemoryListener tcg_memory_listener = {
diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index 67cbce1..6be182c 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -158,7 +158,7 @@  void hostmem_init(HostMem *hostmem)
         .eventfd_del = hostmem_listener_eventfd_dummy,
         .coalesced_mmio_add = hostmem_listener_coalesced_mmio_dummy,
         .coalesced_mmio_del = hostmem_listener_coalesced_mmio_dummy,
-        .priority = 9,
+        .priority = PRI_VRING,
     };
 
     memory_listener_register(&hostmem->listener, &address_space_memory);
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 91c313b..df6d8c5 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -856,7 +856,7 @@  int vhost_dev_init(struct vhost_dev *hdev, int devfd, const char *devpath,
         .log_global_stop = vhost_log_global_stop,
         .eventfd_add = vhost_eventfd_add,
         .eventfd_del = vhost_eventfd_del,
-        .priority = 9
+        .priority = PRI_VRING
     };
     hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
     hdev->n_mem_sections = 0;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9e88320..77e0d40 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -192,6 +192,16 @@  struct MemoryRegionSection {
 
 typedef struct MemoryListener MemoryListener;
 
+/* The list of priority, ex, vhost should have higher priority (less num) than
+ * kvm, ie PRI_VRING < PRI_HYPV
+ */
+typedef enum {
+    PRI_DEFAULT = 0,
+    PRI_CORE = 1,
+    PRI_VRING = 9,
+    PRI_HYPERV = 10,
+} MemListenerPriority;
+
 /**
  * MemoryListener: callbacks structure for updates to the physical memory map
  *
@@ -218,7 +228,7 @@  struct MemoryListener {
     void (*coalesced_mmio_del)(MemoryListener *listener, MemoryRegionSection *section,
                                hwaddr addr, hwaddr len);
     /* Lower = earlier (during add), later (during del) */
-    unsigned priority;
+    MemListenerPriority priority;
     AddressSpace *address_space_filter;
     QTAILQ_ENTRY(MemoryListener) link;
 };
diff --git a/kvm-all.c b/kvm-all.c
index 3a31602..2794dee 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -875,13 +875,13 @@  static MemoryListener kvm_memory_listener = {
     .eventfd_del = kvm_mem_ioeventfd_del,
     .coalesced_mmio_add = kvm_coalesce_mmio_region,
     .coalesced_mmio_del = kvm_uncoalesce_mmio_region,
-    .priority = 10,
+    .priority = PRI_HYPERV,
 };
 
 static MemoryListener kvm_io_listener = {
     .eventfd_add = kvm_io_ioeventfd_add,
     .eventfd_del = kvm_io_ioeventfd_del,
-    .priority = 10,
+    .priority = PRI_HYPERV,
 };
 
 static void kvm_handle_interrupt(CPUState *cpu, int mask)
diff --git a/xen-all.c b/xen-all.c
index 539a154..7062420 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -562,7 +562,7 @@  static MemoryListener xen_memory_listener = {
     .log_sync = xen_log_sync,
     .log_global_start = xen_log_global_start,
     .log_global_stop = xen_log_global_stop,
-    .priority = 10,
+    .priority = PRI_HYPERV,
 };
 
 void qmp_xen_set_global_dirty_log(bool enable, Error **errp)