diff mbox

[3/9] memory: Add global-locking property to memory regions

Message ID 1435163110-2724-4-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini June 24, 2015, 4:25 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

This introduces the memory region property "global_locking". It is true
by default. By setting it to false, a device model can request BQL-free
dispatching of region accesses to its r/w handlers. The actual BQL
break-up will be provided in a separate patch.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Frederic Konrad <fred.konrad@greensocs.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1434646046-27150-4-git-send-email-pbonzini@redhat.com>
---
 include/exec/memory.h | 26 ++++++++++++++++++++++++++
 memory.c              | 11 +++++++++++
 2 files changed, 37 insertions(+)

Comments

Fam Zheng June 25, 2015, 3:44 a.m. UTC | #1
On Wed, 06/24 18:25, Paolo Bonzini wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This introduces the memory region property "global_locking". It is true
> by default. By setting it to false, a device model can request BQL-free
> dispatching of region accesses to its r/w handlers. The actual BQL
> break-up will be provided in a separate patch.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Frederic Konrad <fred.konrad@greensocs.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <1434646046-27150-4-git-send-email-pbonzini@redhat.com>
> ---
>  include/exec/memory.h | 26 ++++++++++++++++++++++++++
>  memory.c              | 11 +++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 8ae004e..fc33348 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -180,6 +180,7 @@ struct MemoryRegion {
>      bool rom_device;
>      bool warning_printed; /* For reservations */
>      bool flush_coalesced_mmio;
> +    bool global_locking;
>      MemoryRegion *alias;
>      hwaddr alias_offset;
>      int32_t priority;
> @@ -825,6 +826,31 @@ void memory_region_set_flush_coalesced(MemoryRegion *mr);
>  void memory_region_clear_flush_coalesced(MemoryRegion *mr);
>  
>  /**
> + * memory_region_set_global_locking: Declares the access processing requires
> + *                                   QEMU's global lock.
> + *
> + * When this is invoked, access to this memory regions will be processed while
> + * holding the global lock of QEMU. This is the default behavior of memory
> + * regions.
> + *
> + * @mr: the memory region to be updated.
> + */
> +void memory_region_set_global_locking(MemoryRegion *mr);
> +
> +/**
> + * memory_region_clear_global_locking: Declares that access processing does
> + *                                     not depend on the QEMU global lock.
> + *
> + * By clearing this property, accesses to the memory region will be processed

Inconsistent with singlar form (access) in the previous one. Otherwise,

Reviewed-by: Fam Zheng <famz@redhat.com>

> + * outside of QEMU's global lock (unless the lock is held on when issuing the
> + * access request). In this case, the device model implementing the access
> + * handlers is responsible for synchronization of concurrency.
> + *
> + * @mr: the memory region to be updated.
> + */
> +void memory_region_clear_global_locking(MemoryRegion *mr);
> +
> +/**
>   * memory_region_add_eventfd: Request an eventfd to be triggered when a word
>   *                            is written to a location.
>   *
> diff --git a/memory.c b/memory.c
> index 3ac0bd2..b0b8860 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1012,6 +1012,7 @@ static void memory_region_initfn(Object *obj)
>      mr->ram_addr = RAM_ADDR_INVALID;
>      mr->enabled = true;
>      mr->romd_mode = true;
> +    mr->global_locking = true;
>      mr->destructor = memory_region_destructor_none;
>      QTAILQ_INIT(&mr->subregions);
>      QTAILQ_INIT(&mr->coalesced);
> @@ -1646,6 +1647,16 @@ void memory_region_clear_flush_coalesced(MemoryRegion *mr)
>      }
>  }
>  
> +void memory_region_set_global_locking(MemoryRegion *mr)
> +{
> +    mr->global_locking = true;
> +}
> +
> +void memory_region_clear_global_locking(MemoryRegion *mr)
> +{
> +    mr->global_locking = false;
> +}
> +
>  void memory_region_add_eventfd(MemoryRegion *mr,
>                                 hwaddr addr,
>                                 unsigned size,
> -- 
> 1.8.3.1
> 
>
Paolo Bonzini June 25, 2015, 7:46 a.m. UTC | #2
On 25/06/2015 05:44, Fam Zheng wrote:
>> > + * memory_region_clear_global_locking: Declares that access processing does
>> > + *                                     not depend on the QEMU global lock.
>> > + *
>> > + * By clearing this property, accesses to the memory region will be processed
> Inconsistent with singlar form (access) in the previous one. Otherwise,

I think this is correct English actually.  "Access processing" in the
first line cannot use a plural because it behaves as a single word.
Let's ask a native speaker. :)

Paolo
Fam Zheng June 25, 2015, 10:59 a.m. UTC | #3
On Thu, 06/25 09:46, Paolo Bonzini wrote:
> 
> 
> On 25/06/2015 05:44, Fam Zheng wrote:
> >> > + * memory_region_clear_global_locking: Declares that access processing does
> >> > + *                                     not depend on the QEMU global lock.
> >> > + *
> >> > + * By clearing this property, accesses to the memory region will be processed
> > Inconsistent with singlar form (access) in the previous one. Otherwise,
> 
> I think this is correct English actually.  "Access processing" in the
> first line cannot use a plural because it behaves as a single word.
> Let's ask a native speaker. :)

I meant the above "When this is invoked, access to this memory regions will be
processed..." compared to this "accesses to the memory region".

Fam
Paolo Bonzini June 25, 2015, 11:10 a.m. UTC | #4
On 25/06/2015 12:59, Fam Zheng wrote:
> I meant the above "When this is invoked, access to this memory regions will be
> processed..." compared to this "accesses to the memory region".

Ah, I see what you mean now.  Fixed.

Paolo
diff mbox

Patch

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8ae004e..fc33348 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -180,6 +180,7 @@  struct MemoryRegion {
     bool rom_device;
     bool warning_printed; /* For reservations */
     bool flush_coalesced_mmio;
+    bool global_locking;
     MemoryRegion *alias;
     hwaddr alias_offset;
     int32_t priority;
@@ -825,6 +826,31 @@  void memory_region_set_flush_coalesced(MemoryRegion *mr);
 void memory_region_clear_flush_coalesced(MemoryRegion *mr);
 
 /**
+ * memory_region_set_global_locking: Declares the access processing requires
+ *                                   QEMU's global lock.
+ *
+ * When this is invoked, access to this memory regions will be processed while
+ * holding the global lock of QEMU. This is the default behavior of memory
+ * regions.
+ *
+ * @mr: the memory region to be updated.
+ */
+void memory_region_set_global_locking(MemoryRegion *mr);
+
+/**
+ * memory_region_clear_global_locking: Declares that access processing does
+ *                                     not depend on the QEMU global lock.
+ *
+ * By clearing this property, accesses to the memory region will be processed
+ * outside of QEMU's global lock (unless the lock is held on when issuing the
+ * access request). In this case, the device model implementing the access
+ * handlers is responsible for synchronization of concurrency.
+ *
+ * @mr: the memory region to be updated.
+ */
+void memory_region_clear_global_locking(MemoryRegion *mr);
+
+/**
  * memory_region_add_eventfd: Request an eventfd to be triggered when a word
  *                            is written to a location.
  *
diff --git a/memory.c b/memory.c
index 3ac0bd2..b0b8860 100644
--- a/memory.c
+++ b/memory.c
@@ -1012,6 +1012,7 @@  static void memory_region_initfn(Object *obj)
     mr->ram_addr = RAM_ADDR_INVALID;
     mr->enabled = true;
     mr->romd_mode = true;
+    mr->global_locking = true;
     mr->destructor = memory_region_destructor_none;
     QTAILQ_INIT(&mr->subregions);
     QTAILQ_INIT(&mr->coalesced);
@@ -1646,6 +1647,16 @@  void memory_region_clear_flush_coalesced(MemoryRegion *mr)
     }
 }
 
+void memory_region_set_global_locking(MemoryRegion *mr)
+{
+    mr->global_locking = true;
+}
+
+void memory_region_clear_global_locking(MemoryRegion *mr)
+{
+    mr->global_locking = false;
+}
+
 void memory_region_add_eventfd(MemoryRegion *mr,
                                hwaddr addr,
                                unsigned size,