[Qemu-devel] [RFC PATCH 0/3] v2.2 RCU Implementation for QEMU

[Qemu-devel] [RFC PATCH 0/3] v2.2 RCU Implementation for QEMU

[Mike Day]

This patch set merges Paolo's conversion of address spaces to enable
RCU. There is one more patchset coming for TLB access that I'm
debugging right now. After I submit the last one I'll start working on
enabling RCU more widely - and the series will need to be refactored.

The series is availale online at: 

https://github.com/ncultra/qemu/tree/rcu-for-1.7

Mike Day (2):
  fixup changes from commit f63ca950
  fixup changes from commit f62a6b2f from Paulo Bonzini

Paolo Bonzini (1):
  exec: change iotlb APIs to take AddressSpaceDispatch

 aio-posix.c            |  1 +
 aio-win32.c            |  1 +
 cpus.c                 |  3 +++
 cputlb.c               |  7 ++++---
 exec.c                 | 13 ++++++++-----
 include/exec/cpu-all.h |  3 +++
 include/exec/cputlb.h  |  9 ++++++---
 7 files changed, 26 insertions(+), 11 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] fixup changes from commit f63ca950

[Mike Day]

From: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Mike Day <ncmike@ncultra.org>
---
 aio-posix.c | 1 +
 aio-win32.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/aio-posix.c b/aio-posix.c
index 562add6..7f27ea5 100644
--- a/aio-posix.c
+++ b/aio-posix.c
@@ -176,6 +176,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
     int ret;
     bool busy, progress;
 
+    rcu_quiescent_state();
     progress = false;
 
     /*
diff --git a/aio-win32.c b/aio-win32.c
index 8a6abb0..9744fa5 100644
--- a/aio-win32.c
+++ b/aio-win32.c
@@ -99,6 +99,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
     bool busy, progress;
     int count;
 
+    rcu_quiescent_state();
     progress = false;
 
     /*
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] fixup changes from commit f62a6b2f from Paulo Bonzini

[Mike Day]

From: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Mike Day <ncmike@ncultra.org>
---
 cpus.c                 | 3 +++
 exec.c                 | 4 +++-
 include/exec/cpu-all.h | 3 +++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 624658e..d6f2775 100644
--- a/cpus.c
+++ b/cpus.c
@@ -764,6 +764,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     CPUState *cpu = arg;
     int r;
 
+    tls_alloc_cpu_single_env_var();
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
@@ -804,6 +805,7 @@ static void *qemu_dummy_cpu_thread_fn(void *arg)
     sigset_t waitset;
     int r;
 
+    tls_alloc_cpu_single_env_var();
     qemu_mutex_lock_iothread();
     qemu_thread_get_self(cpu->thread);
     cpu->thread_id = qemu_get_thread_id();
@@ -850,6 +852,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *cpu = arg;
 
+    tls_alloc_cpu_single_env_var();
     qemu_tcg_init_cpu_signals();
     qemu_thread_get_self(cpu->thread);
 
diff --git a/exec.c b/exec.c
index 3ca9381..52e7fd5 100644
--- a/exec.c
+++ b/exec.c
@@ -45,7 +45,7 @@
 #include "trace.h"
 #endif
 #include "exec/cpu-all.h"
-
+#include "qemu/tls.h"
 #include "exec/cputlb.h"
 #include "translate-all.h"
 
@@ -72,6 +72,7 @@ static MemoryRegion io_mem_unassigned;
 CPUState *first_cpu;
 /* current CPU in the current thread. It is only valid inside
    cpu_exec() */
+DEFINE_TLS(CPUArchState *, cpu_single_env_var);
 DEFINE_TLS(CPUState *, current_cpu);
 /* 0 = Do not count executed instructions.
    1 = Precise instruction counting.
@@ -313,6 +314,7 @@ address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr *xlat,
 
 void cpu_exec_init_all(void)
 {
+    tls_alloc_cpu_single_env_var();
 #if !defined(CONFIG_USER_ONLY)
     qemu_mutex_init(&ram_list.mutex);
     memory_map_init();
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index a407b50..b961e58 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "exec/cpu-common.h"
 #include "qemu/thread.h"
+#include "qemu/tls.h"
 #include "qom/cpu.h"
 
 /* some important defines:
@@ -356,6 +357,8 @@ int page_get_flags(target_ulong address);
 void page_set_flags(target_ulong start, target_ulong end, int flags);
 int page_check_range(target_ulong start, target_ulong len, int flags);
 #endif
+DECLARE_TLS(CPUArchState *, cpu_single_env_var);
+#define cpu_single_env (*tls_get_cpu_single_env_var())
 
 CPUArchState *cpu_copy(CPUArchState *env);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] exec: change iotlb APIs to take AddressSpaceDispatch

[Mike Day]

From: Paolo Bonzini <pbonzini@redhat.com>

This makes it possible to start following RCU rules, which require
not dereferencing as->dispatch more than once.  It is not covering
the whole of TCG, since the TLB data structures are not RCU-friendly,
but it is enough for exec.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Mike Day <ncmike@ncultra.org>
---
 cputlb.c              | 7 ++++---
 exec.c                | 9 +++++----
 include/exec/cputlb.h | 9 ++++++---
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 977c0ca..1f74d08 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -255,6 +255,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
                   hwaddr paddr, int prot,
                   int mmu_idx, target_ulong size)
 {
+    AddressSpaceDispatch *d;
     MemoryRegionSection *section;
     unsigned int index;
     target_ulong address;
@@ -269,8 +270,8 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
     }
 
     sz = size;
-    section = address_space_translate_for_iotlb(&address_space_memory, paddr,
-                                                &xlat, &sz);
+    d = address_space_memory.dispatch;
+    section = address_space_translate_for_iotlb(d, paddr, &xlat, &sz);
     assert(sz >= TARGET_PAGE_SIZE);
 
 #if defined(DEBUG_TLB)
@@ -290,7 +291,7 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
     }
 
     code_address = address;
-    iotlb = memory_region_section_get_iotlb(env, section, vaddr, paddr, xlat,
+    iotlb = memory_region_section_get_iotlb(d, env, section, vaddr, paddr, xlat,
                                             prot, &address);
 
     index = (vaddr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
diff --git a/exec.c b/exec.c
index 52e7fd5..c194bad 100644
--- a/exec.c
+++ b/exec.c
@@ -301,11 +301,11 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
 }
 
 MemoryRegionSection *
-address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr *xlat,
+address_space_translate_for_iotlb(AddressSpaceDispatch *d, hwaddr addr, hwaddr *xlat,
                                   hwaddr *plen)
 {
     MemoryRegionSection *section;
-    section = address_space_translate_internal(as->dispatch, addr, xlat, plen, false);
+    section = address_space_translate_internal(d, addr, xlat, plen, false);
 
     assert(!section->mr->iommu_ops);
     return section;
@@ -719,7 +719,8 @@ static int cpu_physical_memory_set_dirty_tracking(int enable)
     return ret;
 }
 
-hwaddr memory_region_section_get_iotlb(CPUArchState *env,
+hwaddr memory_region_section_get_iotlb(AddressSpaceDispatch *d,
+                                       CPUArchState *env,
                                        MemoryRegionSection *section,
                                        target_ulong vaddr,
                                        hwaddr paddr, hwaddr xlat,
@@ -739,7 +740,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
             iotlb |= PHYS_SECTION_ROM;
         }
     } else {
-        iotlb = section - address_space_memory.dispatch->sections;
+        iotlb = section - d->sections;
         iotlb += xlat;
     }
 
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index e21cb60..bd7ff2f 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -19,6 +19,8 @@
 #ifndef CPUTLB_H
 #define CPUTLB_H
 
+#include <exec/memory-internal.h>
+
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_protect_code(ram_addr_t ram_addr);
@@ -34,9 +36,10 @@ extern int tlb_flush_count;
 void tb_flush_jmp_cache(CPUArchState *env, target_ulong addr);
 
 MemoryRegionSection *
-address_space_translate_for_iotlb(AddressSpace *as, hwaddr addr, hwaddr *xlat,
-                                  hwaddr *plen);
-hwaddr memory_region_section_get_iotlb(CPUArchState *env,
+address_space_translate_for_iotlb(AddressSpaceDispatch *d, hwaddr addr,
+                                  hwaddr *xlat, hwaddr *plen);
+hwaddr memory_region_section_get_iotlb(AddressSpaceDispatch *d,
+                                       CPUArchState *env,
                                        MemoryRegionSection *section,
                                        target_ulong vaddr,
                                        hwaddr paddr, hwaddr xlat,
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 0/3] v2.2 RCU Implementation for QEMU

[Andreas Färber]

Am 16.08.2013 16:41, schrieb Mike Day:
> This patch set merges Paolo's conversion of address spaces to enable
> RCU. There is one more patchset coming for TLB access that I'm
> debugging right now. After I submit the last one I'll start working on
> enabling RCU more widely - and the series will need to be refactored.

Please see http://wiki.qemu.org/Contribute/SubmitAPatch.

In particular you need to sign off any patches you submit, Reviewed-by
is insufficient.

> The series is availale online at: 
> 
> https://github.com/ncultra/qemu/tree/rcu-for-1.7
> 
> Mike Day (2):
>   fixup changes from commit f63ca950
>   fixup changes from commit f62a6b2f from Paulo Bonzini

These two patches are certainly not acceptable for upstream, lacking any
textual explanation and with those subjects.

Regards,
Andreas

> Paolo Bonzini (1):
>   exec: change iotlb APIs to take AddressSpaceDispatch
> 
>  aio-posix.c            |  1 +
>  aio-win32.c            |  1 +
>  cpus.c                 |  3 +++
>  cputlb.c               |  7 ++++---
>  exec.c                 | 13 ++++++++-----
>  include/exec/cpu-all.h |  3 +++
>  include/exec/cputlb.h  |  9 ++++++---
>  7 files changed, 26 insertions(+), 11 deletions(-)
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/3] v2.2 RCU Implementation for QEMU

[Mike Day]

Andreas Färber <afaerber@suse.de> writes:


>> https://github.com/ncultra/qemu/tree/rcu-for-1.7
>> 
>> Mike Day (2):
>>   fixup changes from commit f63ca950
>>   fixup changes from commit f62a6b2f from Paulo Bonzini
>
> These two patches are certainly not acceptable for upstream, lacking any
> textual explanation and with those subjects.

Thanks. The entire series is not ready yet - it needs to be
refactored and tested, hence the RFC tag. I'm trying to keep an rebased
version of the Paolo's May RCU tree available for those who are
interested. When its ready to be considered for an upstream merge it
will be as expected. 

The the next step for me is to start working on leveraging the RCU code. 

Mike

-- 

Mike Day | + 1 919 371-8786 | ncmike@ncultra.org
"Endurance is a Virtue"

Re: [Qemu-devel] [RFC PATCH 0/3] v2.2 RCU Implementation for QEMU

[Paolo Bonzini]

Il 16/08/2013 16:41, Mike Day ha scritto:
> This patch set merges Paolo's conversion of address spaces to enable
> RCU. There is one more patchset coming for TLB access that I'm
> debugging right now. After I submit the last one I'll start working on
> enabling RCU more widely - and the series will need to be refactored.
> 
> The series is availale online at: 
> 
> https://github.com/ncultra/qemu/tree/rcu-for-1.7
> 
> Mike Day (2):
>   fixup changes from commit f63ca950
>   fixup changes from commit f62a6b2f from Paulo Bonzini
> 
> Paolo Bonzini (1):
>   exec: change iotlb APIs to take AddressSpaceDispatch
> 
>  aio-posix.c            |  1 +
>  aio-win32.c            |  1 +
>  cpus.c                 |  3 +++
>  cputlb.c               |  7 ++++---
>  exec.c                 | 13 ++++++++-----
>  include/exec/cpu-all.h |  3 +++
>  include/exec/cputlb.h  |  9 ++++++---
>  7 files changed, 26 insertions(+), 11 deletions(-)
> 

Thanks Mike!  I think iotlb APIs can be made RCU-friendly by:

1) storing an AddressSpaceDispatch in the CPUArchState (which
iotlb_to_region can then use);

2) doing tcg_commit's TLB flush in the VCPU thread.

I'll pick up your changes and post the RCU patches in a few weeks.

Paolo