diff mbox

[02/16] exec.c: Allow target CPUs to define multiple AddressSpaces

Message ID 1446747358-18214-3-git-send-email-peter.maydell@linaro.org
State Accepted
Headers show

Commit Message

Peter Maydell Nov. 5, 2015, 6:15 p.m. UTC 
Allow multiple calls to cpu_address_space_init(); each
call adds an entry to the cpu->ases array at the specified
index. It is up to the target-specific CPU code to actually use
these extra address spaces.

Since this multiple AddressSpace support won't work with
KVM, add an assertion to avoid confusing failures.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 exec.c            | 28 ++++++++++++++++++----------
 include/qom/cpu.h |  2 ++
 2 files changed, 20 insertions(+), 10 deletions(-)

-- 
1.9.1

Comments

Peter Maydell Nov. 6, 2015, 1:34 p.m. UTC | #1 
On 6 November 2015 at 13:21, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Nov 05, 2015 at 06:15:44PM +0000, Peter Maydell wrote:

>> Allow multiple calls to cpu_address_space_init(); each

>> call adds an entry to the cpu->ases array at the specified

>> index. It is up to the target-specific CPU code to actually use

>> these extra address spaces.

>>

>> Since this multiple AddressSpace support won't work with

>> KVM, add an assertion to avoid confusing failures.

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


>> +    if (asidx >= cpu->num_ases) {

>> +        if (cpu->num_ases == 0) {

>> +            cpu->cpu_ases = g_new(CPUAddressSpace, asidx + 1);

>> +        } else {

>> +            cpu->cpu_ases = g_renew(CPUAddressSpace, cpu->cpu_ases, asidx + 1);

>

> IIUC, g_renew may move the entire cpu_ases area. The internals of

> memory_listener_register (called below) seem to put away the pointers to listeners

> so a renew+move would leave invalid pointers to listeners in memory.c wouldn't it?

>

> There are various ways of solving this, (e.g dynamic allocation of the listener,

> static allocation of the cpu_ases, invalidate all listeners and restore them after

> each as init and more). I'm sure you'll figure something out.


Oops, yes, you're right.

Maybe we should just have the target CPU say in advance what the
maximum number of AddressSpaces it will have is -- my expectation
is that this will be (a) small (b) known in advance anyway.

thanks
-- PMM
diff mbox

Patch

diff --git a/exec.c b/exec.c
index b5490c8..6a2a694 100644
--- a/exec.c
+++ b/exec.c
@@ -552,25 +552,32 @@  CPUState *qemu_get_cpu(int index)
 #if !defined(CONFIG_USER_ONLY)
 void cpu_address_space_init(CPUState *cpu, AddressSpace *as, int asidx)
 {
+    CPUAddressSpace *newas;
+
     if (asidx == 0) {
         /* address space 0 gets the convenience alias */
         cpu->as = as;
     }
 
-    /* We only support one address space per cpu at the moment.  */
-    assert(cpu->as == as);
+    /* KVM cannot currently support multiple address spaces. */
+    assert(asidx == 0 || !kvm_enabled());
 
-    if (cpu->cpu_ases) {
-        /* We've already registered the listener for our only AS */
-        return;
+    if (asidx >= cpu->num_ases) {
+        if (cpu->num_ases == 0) {
+            cpu->cpu_ases = g_new(CPUAddressSpace, asidx + 1);
+        } else {
+            cpu->cpu_ases = g_renew(CPUAddressSpace, cpu->cpu_ases, asidx + 1);
+        }
+        cpu->num_ases = asidx + 1;
     }
 
-    cpu->cpu_ases = g_new0(CPUAddressSpace, 1);
-    cpu->cpu_ases[0].cpu = cpu;
-    cpu->cpu_ases[0].as = as;
+    newas = &cpu->cpu_ases[asidx];
+    memset(newas, 0, sizeof(*newas));
+    newas->cpu = cpu;
+    newas->as = as;
     if (tcg_enabled()) {
-        cpu->cpu_ases[0].tcg_as_listener.commit = tcg_commit;
-        memory_listener_register(&cpu->cpu_ases[0].tcg_as_listener, as);
+        newas->tcg_as_listener.commit = tcg_commit;
+        memory_listener_register(&newas->tcg_as_listener, as);
     }
 }
 #endif
@@ -627,6 +634,7 @@  void cpu_exec_init(CPUState *cpu, Error **errp)
     Error *local_err = NULL;
 
     cpu->as = NULL;
+    cpu->num_ases = 0;
 
 #ifndef CONFIG_USER_ONLY
     cpu->thread_id = qemu_get_thread_id();
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 51a1323..ae17932 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -236,6 +236,7 @@  struct kvm_run;
  * so that interrupts take effect immediately.
  * @cpu_ases: Pointer to array of CPUAddressSpaces (which define the
  *            AddressSpaces this CPU has)
+ * @num_ases: number of CPUAddressSpaces in @cpu_ases
  * @as: Pointer to the first AddressSpace, for the convenience of targets which
  *      only have a single AddressSpace
  * @env_ptr: Pointer to subclass-specific CPUArchState field.
@@ -285,6 +286,7 @@  struct CPUState {
     struct qemu_work_item *queued_work_first, *queued_work_last;
 
     CPUAddressSpace *cpu_ases;
+    int num_ases;
     AddressSpace *as;
 
     void *env_ptr; /* CPUArchState */