diff mbox

[v3,06/12] ARM: Factor out ARM on/off PSCI control functions

Message ID adadf2671fd72e1146060c1f3e5f954e7f49aa91.1456868959.git.jcd@tribudubois.net (mailing list archive)
State New, archived
Headers show

Commit Message

Jean-Christophe Dubois March 1, 2016, 10:27 p.m. UTC
Split ARM on/off function from PSCI support code.

This will allow to reuse these functions in other code.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since V1:
 * Not present on V1

Changes since V2:
 * Not present on V2

 target-arm/Makefile.objs  |   1 +
 target-arm/arm-powerctl.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/arm-powerctl.h |  22 +++++++
 target-arm/psci.c         |  72 ++-------------------
 4 files changed, 184 insertions(+), 67 deletions(-)
 create mode 100644 target-arm/arm-powerctl.c
 create mode 100644 target-arm/arm-powerctl.h

Comments

Peter Maydell March 10, 2016, 10:14 a.m. UTC | #1
On 2 March 2016 at 05:27, Jean-Christophe Dubois <jcd@tribudubois.net> wrote:
> Split ARM on/off function from PSCI support code.
>
> This will allow to reuse these functions in other code.
>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since V1:
>  * Not present on V1
>
> Changes since V2:
>  * Not present on V2
>
>  target-arm/Makefile.objs  |   1 +
>  target-arm/arm-powerctl.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/arm-powerctl.h |  22 +++++++
>  target-arm/psci.c         |  72 ++-------------------
>  4 files changed, 184 insertions(+), 67 deletions(-)
>  create mode 100644 target-arm/arm-powerctl.c
>  create mode 100644 target-arm/arm-powerctl.h
>
> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
> index a80eb39..60fd1dd 100644
> --- a/target-arm/Makefile.objs
> +++ b/target-arm/Makefile.objs
> @@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
>  obj-y += gdbstub.o
>  obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
>  obj-y += crypto_helper.o
> +obj-y += arm-powerctl.o
> diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c
> new file mode 100644
> index 0000000..99c2af2
> --- /dev/null
> +++ b/target-arm/arm-powerctl.c
> @@ -0,0 +1,156 @@
> +/*
> + * QEMU support -- ARM Power Control specific functions.
> + *
> + * Copyright (c) 2016 Jean-Christophe Dubois
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include <cpu.h>
> +#include <cpu-qom.h>
> +#include "arm-powerctl.h"
> +
> +#ifndef DEBUG_ARM_POWERCTL
> +#define DEBUG_ARM_POWERCTL 0
> +#endif
> +
> +#define DPRINTF(fmt, args...) \
> +    do { \
> +        if (DEBUG_ARM_POWERCTL) { \
> +            fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
> +        } \
> +    } while (0)
> +
> +CPUState *arm_get_cpu_by_id(uint64_t id)
> +{
> +    CPUState *cpu;
> +
> +    DPRINTF("cpu %" PRId64 "\n", id);
> +
> +    CPU_FOREACH(cpu) {
> +        ARMCPU *armcpu = ARM_CPU(cpu);
> +
> +        if (armcpu->mp_affinity == id) {
> +            return cpu;
> +        }
> +    }
> +
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "[ARM]%s: Resquesting unknown CPU %" PRId64 "\n",
> +                  __func__, id);

"Requesting"

> +
> +    return NULL;
> +}
> +
> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +    CPUClass *target_cpu_class;
> +    CPUARMState *env = &ARM_CPU(current_cpu)->env;
> +
> +    DPRINTF("cpu %" PRId64 " @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n",
> +            cpuid, entry, context_id);
> +
> +    /* change to the cpu we are powering up */
> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return -1;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (!target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "[ARM]%s: CPU %" PRId64 " is already running\n",
> +                      __func__, cpuid);
> +        return -1;
> +    }
> +    target_cpu_class = CPU_GET_CLASS(target_cpu);
> +
> +    /* Initialize the cpu we are turning on */
> +    cpu_reset(target_cpu_state);
> +    target_cpu->powered_off = false;
> +    target_cpu_state->halted = 0;
> +
> +    /*
> +     * The PSCI spec mandates that newly brought up CPUs enter the
> +     * exception level of the caller in the same execution mode as
> +     * the caller, with context_id in x0/r0, respectively.

This is a generic function so it's a bit out of place for it to
talk about PSCI spec details. What are the semantics provided by
this helper function?

> +     *
> +     * For now, it is sufficient to assert() that CPUs come out of
> +     * reset in the same mode as the calling CPU, since we only
> +     * implement EL1, which means that

It's no longer true that we only implement EL1, incidentally.

> +     * (a) there is no EL2 for the calling CPU to trap into to change
> +     *     its state
> +     * (b) the newly brought up CPU enters EL1 immediately after coming
> +     *     out of reset in the default state
> +     */


> +    assert(is_a64(env) == is_a64(&target_cpu->env));
> +    if (is_a64(env)) {
> +        if (entry & 1) {
> +            return -1;
> +        }
> +        target_cpu->env.xregs[0] = context_id;
> +    } else {
> +        target_cpu->env.regs[0] = context_id;
> +        target_cpu->env.thumb = entry & 1;
> +    }
> +
> +    target_cpu_class->set_pc(target_cpu_state, entry);
> +
> +    return 0;
> +}
> +
> +void arm_set_cpu_off(uint64_t cpuid)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +
> +    DPRINTF("cpu %" PRId64 "\n", cpuid);
> +
> +    /* change to the cpu we are powering up */
> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "[ARM]%s: CPU %" PRId64 " is already off\n",
> +                      __func__, cpuid);
> +        return;
> +    }
> +
> +    target_cpu->powered_off = true;
> +    target_cpu_state->halted = 1;
> +    target_cpu_state->exception_index = EXCP_HLT;
> +    cpu_loop_exit(target_cpu_state);
> +}
> +
> +int arm_reset_cpu(uint64_t cpuid)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +
> +    DPRINTF("cpu %" PRId64 "\n", cpuid);
> +
> +    /* change to the cpu we are resetting */
> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return -1;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "[ARM]%s: CPU %" PRId64 " is off\n",
> +                      __func__, cpuid);

Whether requesting a power-off of a powered off CPU
is a guest error depends on the API used to request
it, so you can't log this as a guest error in the
generic helper function.

> +        return -1;
> +    }
> +
> +    /* Reset the cpu */
> +    cpu_reset(target_cpu_state);
> +
> +    return 0;
> +}
> diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h
> new file mode 100644
> index 0000000..13fb526
> --- /dev/null
> +++ b/target-arm/arm-powerctl.h
> @@ -0,0 +1,22 @@
> +/*
> + * QEMU support -- ARM Power Control specific functions.
> + *
> + * Copyright (c) 2016 Jean-Christophe Dubois
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_ARM_POWERCTL_H
> +#define QEMU_ARM_POWERCTL_H
> +
> +CPUState *arm_get_cpu_by_id(uint64_t id);
> +
> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id);
> +
> +void arm_set_cpu_off(uint64_t cpuid);
> +
> +int arm_reset_cpu(uint64_t cpuid);

Please provide doc comments for new globally-visible functions.

> +
> +#endif
> diff --git a/target-arm/psci.c b/target-arm/psci.c
> index c55487f..362105f 100644
> --- a/target-arm/psci.c
> +++ b/target-arm/psci.c
> @@ -22,6 +22,7 @@
>  #include <kvm-consts.h>
>  #include <sysemu/sysemu.h>
>  #include "internals.h"
> +#include "arm-powerctl.h"
>
>  bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>  {
> @@ -73,21 +74,6 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>      }
>  }
>
> -static CPUState *get_cpu_by_id(uint64_t id)
> -{
> -    CPUState *cpu;
> -
> -    CPU_FOREACH(cpu) {
> -        ARMCPU *armcpu = ARM_CPU(cpu);
> -
> -        if (armcpu->mp_affinity == id) {
> -            return cpu;
> -        }
> -    }
> -
> -    return NULL;
> -}
> -
>  void arm_handle_psci_call(ARMCPU *cpu)
>  {
>      /*
> @@ -98,7 +84,6 @@ void arm_handle_psci_call(ARMCPU *cpu)
>       * Additional information about the calling convention used is available in
>       * the document 'SMC Calling Convention' (ARM DEN 0028)
>       */
> -    CPUState *cs = CPU(cpu);
>      CPUARMState *env = &cpu->env;
>      uint64_t param[4];
>      uint64_t context_id, mpidr;
> @@ -123,7 +108,6 @@ void arm_handle_psci_call(ARMCPU *cpu)
>      switch (param[0]) {
>          CPUState *target_cpu_state;
>          ARMCPU *target_cpu;
> -        CPUClass *target_cpu_class;
>
>      case QEMU_PSCI_0_2_FN_PSCI_VERSION:
>          ret = QEMU_PSCI_0_2_RET_VERSION_0_2;
> @@ -137,7 +121,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
>
>          switch (param[2]) {
>          case 0:
> -            target_cpu_state = get_cpu_by_id(mpidr);
> +            target_cpu_state = arm_get_cpu_by_id(mpidr);
>              if (!target_cpu_state) {
>                  ret = QEMU_PSCI_RET_INVALID_PARAMS;
>                  break;
> @@ -168,51 +152,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
>          entry = param[2];
>          context_id = param[3];
>
> -        /* change to the cpu we are powering up */
> -        target_cpu_state = get_cpu_by_id(mpidr);
> -        if (!target_cpu_state) {
> -            ret = QEMU_PSCI_RET_INVALID_PARAMS;
> -            break;
> -        }
> -        target_cpu = ARM_CPU(target_cpu_state);
> -        if (!target_cpu->powered_off) {
> -            ret = QEMU_PSCI_RET_ALREADY_ON;
> -            break;
> -        }

You've lost the distinction between these two failure
modes -- the PSCI error codes are returned to the guest
and are mandated by the PSCI spec so you need to distinguish
them.


> -        target_cpu_class = CPU_GET_CLASS(target_cpu);
> -
> -        /* Initialize the cpu we are turning on */
> -        cpu_reset(target_cpu_state);
> -        target_cpu->powered_off = false;
> -        target_cpu_state->halted = 0;
> -
> -        /*
> -         * The PSCI spec mandates that newly brought up CPUs enter the
> -         * exception level of the caller in the same execution mode as
> -         * the caller, with context_id in x0/r0, respectively.
> -         *
> -         * For now, it is sufficient to assert() that CPUs come out of
> -         * reset in the same mode as the calling CPU, since we only
> -         * implement EL1, which means that
> -         * (a) there is no EL2 for the calling CPU to trap into to change
> -         *     its state
> -         * (b) the newly brought up CPU enters EL1 immediately after coming
> -         *     out of reset in the default state
> -         */
> -        assert(is_a64(env) == is_a64(&target_cpu->env));
> -        if (is_a64(env)) {
> -            if (entry & 1) {
> -                ret = QEMU_PSCI_RET_INVALID_PARAMS;
> -                break;
> -            }
> -            target_cpu->env.xregs[0] = context_id;
> -        } else {
> -            target_cpu->env.regs[0] = context_id;
> -            target_cpu->env.thumb = entry & 1;
> -        }
> -        target_cpu_class->set_pc(target_cpu_state, entry);
> -
> -        ret = 0;
> +        ret = arm_set_cpu_on(mpidr, entry, context_id) ?
> +                             QEMU_PSCI_RET_INVALID_PARAMS : 0;
>          break;
>      case QEMU_PSCI_0_1_FN_CPU_OFF:
>      case QEMU_PSCI_0_2_FN_CPU_OFF:
> @@ -250,9 +191,6 @@ err:
>      return;
>
>  cpu_off:
> -    cpu->powered_off = true;
> -    cs->halted = 1;
> -    cs->exception_index = EXCP_HLT;
> -    cpu_loop_exit(cs);
> +    arm_set_cpu_off(cpu->mp_affinity);
>      /* notreached */
>  }
> --
> 2.5.0
>

thanks
-- PMM
Jean-Christophe Dubois March 16, 2016, 10:19 p.m. UTC | #2
Hi Peter,

I am wondering what is the "correct" (simple?) way in QEMU to put a CPU 
in a particular state (HYP, SVC, MON, ...) after I reset it (using 
cpu_reset()).

For example, if I reset a core that has "has_el3" property it will start 
in EL3 mode.

Now what is the simple way to transition this CPU to EL1 before starting 
running it with target_cpu_class->set_pc(target_cpu_state, entry);

Thanks

JC

Le 10/03/2016 11:14, Peter Maydell a écrit :
> On 2 March 2016 at 05:27, Jean-Christophe Dubois <jcd@tribudubois.net> wrote:
>> Split ARM on/off function from PSCI support code.
>>
>> This will allow to reuse these functions in other code.
>>
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>> ---
>>
>> Changes since V1:
>>   * Not present on V1
>>
>> Changes since V2:
>>   * Not present on V2
>>
>>   target-arm/Makefile.objs  |   1 +
>>   target-arm/arm-powerctl.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++
>>   target-arm/arm-powerctl.h |  22 +++++++
>>   target-arm/psci.c         |  72 ++-------------------
>>   4 files changed, 184 insertions(+), 67 deletions(-)
>>   create mode 100644 target-arm/arm-powerctl.c
>>   create mode 100644 target-arm/arm-powerctl.h
>>
>> diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
>> index a80eb39..60fd1dd 100644
>> --- a/target-arm/Makefile.objs
>> +++ b/target-arm/Makefile.objs
>> @@ -9,3 +9,4 @@ obj-y += neon_helper.o iwmmxt_helper.o
>>   obj-y += gdbstub.o
>>   obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
>>   obj-y += crypto_helper.o
>> +obj-y += arm-powerctl.o
>> diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c
>> new file mode 100644
>> index 0000000..99c2af2
>> --- /dev/null
>> +++ b/target-arm/arm-powerctl.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * QEMU support -- ARM Power Control specific functions.
>> + *
>> + * Copyright (c) 2016 Jean-Christophe Dubois
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include <cpu.h>
>> +#include <cpu-qom.h>
>> +#include "arm-powerctl.h"
>> +
>> +#ifndef DEBUG_ARM_POWERCTL
>> +#define DEBUG_ARM_POWERCTL 0
>> +#endif
>> +
>> +#define DPRINTF(fmt, args...) \
>> +    do { \
>> +        if (DEBUG_ARM_POWERCTL) { \
>> +            fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
>> +        } \
>> +    } while (0)
>> +
>> +CPUState *arm_get_cpu_by_id(uint64_t id)
>> +{
>> +    CPUState *cpu;
>> +
>> +    DPRINTF("cpu %" PRId64 "\n", id);
>> +
>> +    CPU_FOREACH(cpu) {
>> +        ARMCPU *armcpu = ARM_CPU(cpu);
>> +
>> +        if (armcpu->mp_affinity == id) {
>> +            return cpu;
>> +        }
>> +    }
>> +
>> +    qemu_log_mask(LOG_GUEST_ERROR,
>> +                  "[ARM]%s: Resquesting unknown CPU %" PRId64 "\n",
>> +                  __func__, id);
> "Requesting"
>
>> +
>> +    return NULL;
>> +}
>> +
>> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id)
>> +{
>> +    CPUState *target_cpu_state;
>> +    ARMCPU *target_cpu;
>> +    CPUClass *target_cpu_class;
>> +    CPUARMState *env = &ARM_CPU(current_cpu)->env;
>> +
>> +    DPRINTF("cpu %" PRId64 " @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n",
>> +            cpuid, entry, context_id);
>> +
>> +    /* change to the cpu we are powering up */
>> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
>> +    if (!target_cpu_state) {
>> +        return -1;
>> +    }
>> +    target_cpu = ARM_CPU(target_cpu_state);
>> +    if (!target_cpu->powered_off) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "[ARM]%s: CPU %" PRId64 " is already running\n",
>> +                      __func__, cpuid);
>> +        return -1;
>> +    }
>> +    target_cpu_class = CPU_GET_CLASS(target_cpu);
>> +
>> +    /* Initialize the cpu we are turning on */
>> +    cpu_reset(target_cpu_state);
>> +    target_cpu->powered_off = false;
>> +    target_cpu_state->halted = 0;
>> +
>> +    /*
>> +     * The PSCI spec mandates that newly brought up CPUs enter the
>> +     * exception level of the caller in the same execution mode as
>> +     * the caller, with context_id in x0/r0, respectively.
> This is a generic function so it's a bit out of place for it to
> talk about PSCI spec details. What are the semantics provided by
> this helper function?
>
>> +     *
>> +     * For now, it is sufficient to assert() that CPUs come out of
>> +     * reset in the same mode as the calling CPU, since we only
>> +     * implement EL1, which means that
> It's no longer true that we only implement EL1, incidentally.
>
>> +     * (a) there is no EL2 for the calling CPU to trap into to change
>> +     *     its state
>> +     * (b) the newly brought up CPU enters EL1 immediately after coming
>> +     *     out of reset in the default state
>> +     */
>
>> +    assert(is_a64(env) == is_a64(&target_cpu->env));
>> +    if (is_a64(env)) {
>> +        if (entry & 1) {
>> +            return -1;
>> +        }
>> +        target_cpu->env.xregs[0] = context_id;
>> +    } else {
>> +        target_cpu->env.regs[0] = context_id;
>> +        target_cpu->env.thumb = entry & 1;
>> +    }
>> +
>> +    target_cpu_class->set_pc(target_cpu_state, entry);
>> +
>> +    return 0;
>> +}
>> +
>> +void arm_set_cpu_off(uint64_t cpuid)
>> +{
>> +    CPUState *target_cpu_state;
>> +    ARMCPU *target_cpu;
>> +
>> +    DPRINTF("cpu %" PRId64 "\n", cpuid);
>> +
>> +    /* change to the cpu we are powering up */
>> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
>> +    if (!target_cpu_state) {
>> +        return;
>> +    }
>> +    target_cpu = ARM_CPU(target_cpu_state);
>> +    if (target_cpu->powered_off) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "[ARM]%s: CPU %" PRId64 " is already off\n",
>> +                      __func__, cpuid);
>> +        return;
>> +    }
>> +
>> +    target_cpu->powered_off = true;
>> +    target_cpu_state->halted = 1;
>> +    target_cpu_state->exception_index = EXCP_HLT;
>> +    cpu_loop_exit(target_cpu_state);
>> +}
>> +
>> +int arm_reset_cpu(uint64_t cpuid)
>> +{
>> +    CPUState *target_cpu_state;
>> +    ARMCPU *target_cpu;
>> +
>> +    DPRINTF("cpu %" PRId64 "\n", cpuid);
>> +
>> +    /* change to the cpu we are resetting */
>> +    target_cpu_state = arm_get_cpu_by_id(cpuid);
>> +    if (!target_cpu_state) {
>> +        return -1;
>> +    }
>> +    target_cpu = ARM_CPU(target_cpu_state);
>> +    if (target_cpu->powered_off) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "[ARM]%s: CPU %" PRId64 " is off\n",
>> +                      __func__, cpuid);
> Whether requesting a power-off of a powered off CPU
> is a guest error depends on the API used to request
> it, so you can't log this as a guest error in the
> generic helper function.
>
>> +        return -1;
>> +    }
>> +
>> +    /* Reset the cpu */
>> +    cpu_reset(target_cpu_state);
>> +
>> +    return 0;
>> +}
>> diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h
>> new file mode 100644
>> index 0000000..13fb526
>> --- /dev/null
>> +++ b/target-arm/arm-powerctl.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * QEMU support -- ARM Power Control specific functions.
>> + *
>> + * Copyright (c) 2016 Jean-Christophe Dubois
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef QEMU_ARM_POWERCTL_H
>> +#define QEMU_ARM_POWERCTL_H
>> +
>> +CPUState *arm_get_cpu_by_id(uint64_t id);
>> +
>> +int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id);
>> +
>> +void arm_set_cpu_off(uint64_t cpuid);
>> +
>> +int arm_reset_cpu(uint64_t cpuid);
> Please provide doc comments for new globally-visible functions.
>
>> +
>> +#endif
>> diff --git a/target-arm/psci.c b/target-arm/psci.c
>> index c55487f..362105f 100644
>> --- a/target-arm/psci.c
>> +++ b/target-arm/psci.c
>> @@ -22,6 +22,7 @@
>>   #include <kvm-consts.h>
>>   #include <sysemu/sysemu.h>
>>   #include "internals.h"
>> +#include "arm-powerctl.h"
>>
>>   bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>>   {
>> @@ -73,21 +74,6 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
>>       }
>>   }
>>
>> -static CPUState *get_cpu_by_id(uint64_t id)
>> -{
>> -    CPUState *cpu;
>> -
>> -    CPU_FOREACH(cpu) {
>> -        ARMCPU *armcpu = ARM_CPU(cpu);
>> -
>> -        if (armcpu->mp_affinity == id) {
>> -            return cpu;
>> -        }
>> -    }
>> -
>> -    return NULL;
>> -}
>> -
>>   void arm_handle_psci_call(ARMCPU *cpu)
>>   {
>>       /*
>> @@ -98,7 +84,6 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>        * Additional information about the calling convention used is available in
>>        * the document 'SMC Calling Convention' (ARM DEN 0028)
>>        */
>> -    CPUState *cs = CPU(cpu);
>>       CPUARMState *env = &cpu->env;
>>       uint64_t param[4];
>>       uint64_t context_id, mpidr;
>> @@ -123,7 +108,6 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>       switch (param[0]) {
>>           CPUState *target_cpu_state;
>>           ARMCPU *target_cpu;
>> -        CPUClass *target_cpu_class;
>>
>>       case QEMU_PSCI_0_2_FN_PSCI_VERSION:
>>           ret = QEMU_PSCI_0_2_RET_VERSION_0_2;
>> @@ -137,7 +121,7 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>
>>           switch (param[2]) {
>>           case 0:
>> -            target_cpu_state = get_cpu_by_id(mpidr);
>> +            target_cpu_state = arm_get_cpu_by_id(mpidr);
>>               if (!target_cpu_state) {
>>                   ret = QEMU_PSCI_RET_INVALID_PARAMS;
>>                   break;
>> @@ -168,51 +152,8 @@ void arm_handle_psci_call(ARMCPU *cpu)
>>           entry = param[2];
>>           context_id = param[3];
>>
>> -        /* change to the cpu we are powering up */
>> -        target_cpu_state = get_cpu_by_id(mpidr);
>> -        if (!target_cpu_state) {
>> -            ret = QEMU_PSCI_RET_INVALID_PARAMS;
>> -            break;
>> -        }
>> -        target_cpu = ARM_CPU(target_cpu_state);
>> -        if (!target_cpu->powered_off) {
>> -            ret = QEMU_PSCI_RET_ALREADY_ON;
>> -            break;
>> -        }
> You've lost the distinction between these two failure
> modes -- the PSCI error codes are returned to the guest
> and are mandated by the PSCI spec so you need to distinguish
> them.
>
>
>> -        target_cpu_class = CPU_GET_CLASS(target_cpu);
>> -
>> -        /* Initialize the cpu we are turning on */
>> -        cpu_reset(target_cpu_state);
>> -        target_cpu->powered_off = false;
>> -        target_cpu_state->halted = 0;
>> -
>> -        /*
>> -         * The PSCI spec mandates that newly brought up CPUs enter the
>> -         * exception level of the caller in the same execution mode as
>> -         * the caller, with context_id in x0/r0, respectively.
>> -         *
>> -         * For now, it is sufficient to assert() that CPUs come out of
>> -         * reset in the same mode as the calling CPU, since we only
>> -         * implement EL1, which means that
>> -         * (a) there is no EL2 for the calling CPU to trap into to change
>> -         *     its state
>> -         * (b) the newly brought up CPU enters EL1 immediately after coming
>> -         *     out of reset in the default state
>> -         */
>> -        assert(is_a64(env) == is_a64(&target_cpu->env));
>> -        if (is_a64(env)) {
>> -            if (entry & 1) {
>> -                ret = QEMU_PSCI_RET_INVALID_PARAMS;
>> -                break;
>> -            }
>> -            target_cpu->env.xregs[0] = context_id;
>> -        } else {
>> -            target_cpu->env.regs[0] = context_id;
>> -            target_cpu->env.thumb = entry & 1;
>> -        }
>> -        target_cpu_class->set_pc(target_cpu_state, entry);
>> -
>> -        ret = 0;
>> +        ret = arm_set_cpu_on(mpidr, entry, context_id) ?
>> +                             QEMU_PSCI_RET_INVALID_PARAMS : 0;
>>           break;
>>       case QEMU_PSCI_0_1_FN_CPU_OFF:
>>       case QEMU_PSCI_0_2_FN_CPU_OFF:
>> @@ -250,9 +191,6 @@ err:
>>       return;
>>
>>   cpu_off:
>> -    cpu->powered_off = true;
>> -    cs->halted = 1;
>> -    cs->exception_index = EXCP_HLT;
>> -    cpu_loop_exit(cs);
>> +    arm_set_cpu_off(cpu->mp_affinity);
>>       /* notreached */
>>   }
>> --
>> 2.5.0
>>
> thanks
> -- PMM
>
Peter Maydell March 17, 2016, 8:46 a.m. UTC | #3
On 16 March 2016 at 22:19, Jean-Christophe DUBOIS <jcd@tribudubois.net> wrote:
> Hi Peter,
>
> I am wondering what is the "correct" (simple?) way in QEMU to put a CPU in a
> particular state (HYP, SVC, MON, ...) after I reset it (using cpu_reset()).
>
> For example, if I reset a core that has "has_el3" property it will start in
> EL3 mode.

If it's AArch64, yes. If it's AArch32 then it will start in Secure-SVC
(which is EL3, but the distinction is that if you wanted NS-SVC then
what you need to flip is the NS bit.)

> Now what is the simple way to transition this CPU to EL1 before starting
> running it with target_cpu_class->set_pc(target_cpu_state, entry);

There is no convenient API provided for this -- if you look at boot.c
it just directly tweaks registers. We might want to abstract this out
a bit better.

thanks
-- PMM
diff mbox

Patch

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index a80eb39..60fd1dd 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -9,3 +9,4 @@  obj-y += neon_helper.o iwmmxt_helper.o
 obj-y += gdbstub.o
 obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
 obj-y += crypto_helper.o
+obj-y += arm-powerctl.o
diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c
new file mode 100644
index 0000000..99c2af2
--- /dev/null
+++ b/target-arm/arm-powerctl.c
@@ -0,0 +1,156 @@ 
+/*
+ * QEMU support -- ARM Power Control specific functions.
+ *
+ * Copyright (c) 2016 Jean-Christophe Dubois
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include <cpu.h>
+#include <cpu-qom.h>
+#include "arm-powerctl.h"
+
+#ifndef DEBUG_ARM_POWERCTL
+#define DEBUG_ARM_POWERCTL 0
+#endif
+
+#define DPRINTF(fmt, args...) \
+    do { \
+        if (DEBUG_ARM_POWERCTL) { \
+            fprintf(stderr, "[ARM]%s: " fmt , __func__, ##args); \
+        } \
+    } while (0)
+
+CPUState *arm_get_cpu_by_id(uint64_t id)
+{
+    CPUState *cpu;
+
+    DPRINTF("cpu %" PRId64 "\n", id);
+
+    CPU_FOREACH(cpu) {
+        ARMCPU *armcpu = ARM_CPU(cpu);
+
+        if (armcpu->mp_affinity == id) {
+            return cpu;
+        }
+    }
+
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "[ARM]%s: Resquesting unknown CPU %" PRId64 "\n",
+                  __func__, id);
+
+    return NULL;
+}
+
+int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id)
+{
+    CPUState *target_cpu_state;
+    ARMCPU *target_cpu;
+    CPUClass *target_cpu_class;
+    CPUARMState *env = &ARM_CPU(current_cpu)->env;
+
+    DPRINTF("cpu %" PRId64 " @ 0x%" PRIx64 " with R0 = 0x%" PRIx64 "\n",
+            cpuid, entry, context_id);
+
+    /* change to the cpu we are powering up */
+    target_cpu_state = arm_get_cpu_by_id(cpuid);
+    if (!target_cpu_state) {
+        return -1;
+    }
+    target_cpu = ARM_CPU(target_cpu_state);
+    if (!target_cpu->powered_off) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "[ARM]%s: CPU %" PRId64 " is already running\n",
+                      __func__, cpuid);
+        return -1;
+    }
+    target_cpu_class = CPU_GET_CLASS(target_cpu);
+
+    /* Initialize the cpu we are turning on */
+    cpu_reset(target_cpu_state);
+    target_cpu->powered_off = false;
+    target_cpu_state->halted = 0;
+
+    /*
+     * The PSCI spec mandates that newly brought up CPUs enter the
+     * exception level of the caller in the same execution mode as
+     * the caller, with context_id in x0/r0, respectively.
+     *
+     * For now, it is sufficient to assert() that CPUs come out of
+     * reset in the same mode as the calling CPU, since we only
+     * implement EL1, which means that
+     * (a) there is no EL2 for the calling CPU to trap into to change
+     *     its state
+     * (b) the newly brought up CPU enters EL1 immediately after coming
+     *     out of reset in the default state
+     */
+    assert(is_a64(env) == is_a64(&target_cpu->env));
+    if (is_a64(env)) {
+        if (entry & 1) {
+            return -1;
+        }
+        target_cpu->env.xregs[0] = context_id;
+    } else {
+        target_cpu->env.regs[0] = context_id;
+        target_cpu->env.thumb = entry & 1;
+    }
+
+    target_cpu_class->set_pc(target_cpu_state, entry);
+
+    return 0;
+}
+
+void arm_set_cpu_off(uint64_t cpuid)
+{
+    CPUState *target_cpu_state;
+    ARMCPU *target_cpu;
+
+    DPRINTF("cpu %" PRId64 "\n", cpuid);
+
+    /* change to the cpu we are powering up */
+    target_cpu_state = arm_get_cpu_by_id(cpuid);
+    if (!target_cpu_state) {
+        return;
+    }
+    target_cpu = ARM_CPU(target_cpu_state);
+    if (target_cpu->powered_off) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "[ARM]%s: CPU %" PRId64 " is already off\n",
+                      __func__, cpuid);
+        return;
+    }
+
+    target_cpu->powered_off = true;
+    target_cpu_state->halted = 1;
+    target_cpu_state->exception_index = EXCP_HLT;
+    cpu_loop_exit(target_cpu_state);
+}
+
+int arm_reset_cpu(uint64_t cpuid)
+{
+    CPUState *target_cpu_state;
+    ARMCPU *target_cpu;
+
+    DPRINTF("cpu %" PRId64 "\n", cpuid);
+
+    /* change to the cpu we are resetting */
+    target_cpu_state = arm_get_cpu_by_id(cpuid);
+    if (!target_cpu_state) {
+        return -1;
+    }
+    target_cpu = ARM_CPU(target_cpu_state);
+    if (target_cpu->powered_off) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "[ARM]%s: CPU %" PRId64 " is off\n",
+                      __func__, cpuid);
+        return -1;
+    }
+
+    /* Reset the cpu */
+    cpu_reset(target_cpu_state);
+
+    return 0;
+}
diff --git a/target-arm/arm-powerctl.h b/target-arm/arm-powerctl.h
new file mode 100644
index 0000000..13fb526
--- /dev/null
+++ b/target-arm/arm-powerctl.h
@@ -0,0 +1,22 @@ 
+/*
+ * QEMU support -- ARM Power Control specific functions.
+ *
+ * Copyright (c) 2016 Jean-Christophe Dubois
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_ARM_POWERCTL_H
+#define QEMU_ARM_POWERCTL_H
+
+CPUState *arm_get_cpu_by_id(uint64_t id);
+
+int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id);
+
+void arm_set_cpu_off(uint64_t cpuid);
+
+int arm_reset_cpu(uint64_t cpuid);
+
+#endif
diff --git a/target-arm/psci.c b/target-arm/psci.c
index c55487f..362105f 100644
--- a/target-arm/psci.c
+++ b/target-arm/psci.c
@@ -22,6 +22,7 @@ 
 #include <kvm-consts.h>
 #include <sysemu/sysemu.h>
 #include "internals.h"
+#include "arm-powerctl.h"
 
 bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
 {
@@ -73,21 +74,6 @@  bool arm_is_psci_call(ARMCPU *cpu, int excp_type)
     }
 }
 
-static CPUState *get_cpu_by_id(uint64_t id)
-{
-    CPUState *cpu;
-
-    CPU_FOREACH(cpu) {
-        ARMCPU *armcpu = ARM_CPU(cpu);
-
-        if (armcpu->mp_affinity == id) {
-            return cpu;
-        }
-    }
-
-    return NULL;
-}
-
 void arm_handle_psci_call(ARMCPU *cpu)
 {
     /*
@@ -98,7 +84,6 @@  void arm_handle_psci_call(ARMCPU *cpu)
      * Additional information about the calling convention used is available in
      * the document 'SMC Calling Convention' (ARM DEN 0028)
      */
-    CPUState *cs = CPU(cpu);
     CPUARMState *env = &cpu->env;
     uint64_t param[4];
     uint64_t context_id, mpidr;
@@ -123,7 +108,6 @@  void arm_handle_psci_call(ARMCPU *cpu)
     switch (param[0]) {
         CPUState *target_cpu_state;
         ARMCPU *target_cpu;
-        CPUClass *target_cpu_class;
 
     case QEMU_PSCI_0_2_FN_PSCI_VERSION:
         ret = QEMU_PSCI_0_2_RET_VERSION_0_2;
@@ -137,7 +121,7 @@  void arm_handle_psci_call(ARMCPU *cpu)
 
         switch (param[2]) {
         case 0:
-            target_cpu_state = get_cpu_by_id(mpidr);
+            target_cpu_state = arm_get_cpu_by_id(mpidr);
             if (!target_cpu_state) {
                 ret = QEMU_PSCI_RET_INVALID_PARAMS;
                 break;
@@ -168,51 +152,8 @@  void arm_handle_psci_call(ARMCPU *cpu)
         entry = param[2];
         context_id = param[3];
 
-        /* change to the cpu we are powering up */
-        target_cpu_state = get_cpu_by_id(mpidr);
-        if (!target_cpu_state) {
-            ret = QEMU_PSCI_RET_INVALID_PARAMS;
-            break;
-        }
-        target_cpu = ARM_CPU(target_cpu_state);
-        if (!target_cpu->powered_off) {
-            ret = QEMU_PSCI_RET_ALREADY_ON;
-            break;
-        }
-        target_cpu_class = CPU_GET_CLASS(target_cpu);
-
-        /* Initialize the cpu we are turning on */
-        cpu_reset(target_cpu_state);
-        target_cpu->powered_off = false;
-        target_cpu_state->halted = 0;
-
-        /*
-         * The PSCI spec mandates that newly brought up CPUs enter the
-         * exception level of the caller in the same execution mode as
-         * the caller, with context_id in x0/r0, respectively.
-         *
-         * For now, it is sufficient to assert() that CPUs come out of
-         * reset in the same mode as the calling CPU, since we only
-         * implement EL1, which means that
-         * (a) there is no EL2 for the calling CPU to trap into to change
-         *     its state
-         * (b) the newly brought up CPU enters EL1 immediately after coming
-         *     out of reset in the default state
-         */
-        assert(is_a64(env) == is_a64(&target_cpu->env));
-        if (is_a64(env)) {
-            if (entry & 1) {
-                ret = QEMU_PSCI_RET_INVALID_PARAMS;
-                break;
-            }
-            target_cpu->env.xregs[0] = context_id;
-        } else {
-            target_cpu->env.regs[0] = context_id;
-            target_cpu->env.thumb = entry & 1;
-        }
-        target_cpu_class->set_pc(target_cpu_state, entry);
-
-        ret = 0;
+        ret = arm_set_cpu_on(mpidr, entry, context_id) ?
+                             QEMU_PSCI_RET_INVALID_PARAMS : 0;
         break;
     case QEMU_PSCI_0_1_FN_CPU_OFF:
     case QEMU_PSCI_0_2_FN_CPU_OFF:
@@ -250,9 +191,6 @@  err:
     return;
 
 cpu_off:
-    cpu->powered_off = true;
-    cs->halted = 1;
-    cs->exception_index = EXCP_HLT;
-    cpu_loop_exit(cs);
+    arm_set_cpu_off(cpu->mp_affinity);
     /* notreached */
 }