[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v11 20/24] target-arm/powerctl: defer cpu reset wo
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [PATCH v11 20/24] target-arm/powerctl: defer cpu reset work to CPU context |
Date: |
Fri, 10 Feb 2017 14:46:29 +0000 |
On 9 February 2017 at 17:09, Alex Bennée <address@hidden> wrote:
> When switching a new vCPU on we want to complete a bunch of the setup
> work before we start scheduling the vCPU thread. To do this cleanly we
> defer vCPU setup to async work which will run the vCPUs execution
> context as the thread is woken up. The scheduling of the work will kick
> the vCPU awake.
>
> This avoids potential races in MTTCG system emulation.
>
> Signed-off-by: Alex Bennée <address@hidden>
> Reviewed-by: Richard Henderson <address@hidden>
>
> ---
> v7
> - add const to static mode_for_el[] array
> - fix checkpatch long lines
> v10
> - use async work for arm_set_cpu_off, arm_reset_cpu as well
> - model ON_PENDING to deal with racing arm_set_cpu_on
> v11
> - move to single cpu->power_state protected by BQL
> - bump migration format
> ---
> target/arm/arm-powerctl.c | 202
> +++++++++++++++++++++++++++++++---------------
> target/arm/arm-powerctl.h | 2 +
> target/arm/cpu.c | 4 +-
> target/arm/cpu.h | 13 ++-
> target/arm/kvm.c | 7 +-
> target/arm/machine.c | 6 +-
> target/arm/psci.c | 16 +++-
> 7 files changed, 174 insertions(+), 76 deletions(-)
>
> diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
> index fbb7a15daa..3b4c51978c 100644
> --- a/target/arm/arm-powerctl.c
> +++ b/target/arm/arm-powerctl.c
> @@ -14,6 +14,7 @@
> #include "internals.h"
> #include "arm-powerctl.h"
> #include "qemu/log.h"
> +#include "qemu/main-loop.h"
> #include "exec/exec-all.h"
>
> #ifndef DEBUG_ARM_POWERCTL
> @@ -48,11 +49,93 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
> return NULL;
> }
>
> +struct cpu_on_info {
Since you'll need to respin this anyway, our coding
style likes CamelCase for struct names.
> diff --git a/target/arm/machine.c b/target/arm/machine.c
> index fa5ec76090..15619a0430 100644
> --- a/target/arm/machine.c
> +++ b/target/arm/machine.c
> @@ -286,8 +286,8 @@ static int cpu_post_load(void *opaque, int version_id)
>
> const VMStateDescription vmstate_arm_cpu = {
> .name = "cpu",
> - .version_id = 22,
> - .minimum_version_id = 22,
> + .version_id = 23,
> + .minimum_version_id = 23,
Sorry, you can't do this, it will break migration. You need
to do something cleverer with a subsection or other trick.
David Gilbert's recent patch on list to docs/migration.txt
should have some examples of how to do this.
> .pre_save = cpu_pre_save,
> .post_load = cpu_post_load,
> .fields = (VMStateField[]) {
> @@ -329,7 +329,7 @@ const VMStateDescription vmstate_arm_cpu = {
> VMSTATE_UINT64(env.exception.vaddress, ARMCPU),
> VMSTATE_TIMER_PTR(gt_timer[GTIMER_PHYS], ARMCPU),
> VMSTATE_TIMER_PTR(gt_timer[GTIMER_VIRT], ARMCPU),
> - VMSTATE_BOOL(powered_off, ARMCPU),
> + VMSTATE_UINT32(power_state, ARMCPU),
> VMSTATE_END_OF_LIST()
> },
> .subsections = (const VMStateDescription*[]) {
I think the rest is OK.
thanks
-- PMM