[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 02/16] exec.c: Allow target CPUs to define multipl
From: |
Edgar E. Iglesias |
Subject: |
Re: [Qemu-arm] [PATCH 02/16] exec.c: Allow target CPUs to define multiple AddressSpaces |
Date: |
Fri, 6 Nov 2015 14:21:15 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> ---
> exec.c | 28 ++++++++++++++++++----------
> include/qom/cpu.h | 2 ++
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> 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);
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.
> + }
> + 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 */
> --
> 1.9.1
>
[Qemu-arm] [PATCH 07/16] exec.c: Use cpu_get_phys_page_asidx_debug, Peter Maydell, 2015/11/05
[Qemu-arm] [PATCH 14/16] hw/arm/virt: Wire up memory region to CPUs explicitly, Peter Maydell, 2015/11/05
[Qemu-arm] [PATCH 04/16] exec.c: Add address space index to CPUIOTLBEntry, Peter Maydell, 2015/11/05