|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 01/13] xen/arm: basic PSCI support, implement cpu_on and cpu_off
On Wed, 2013-04-24 at 20:07 +0100, Stefano Stabellini wrote:
> Implement support for ARM Power State Coordination Interface, PSCI in
> short.
> Support HVC and SMC calls.
>
> Changes in v3:
> - move do_psci_* to psci.c;
> - trap SMC;
> - return PSCI error codes;
> - remove Linux boot procotol from secondary cpus.
>
> Changes in v2:
> - set is_initialised in arch_set_info_guest;
> - zero vcpu_guest_context before using it.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
> xen/arch/arm/Makefile | 1 +
> xen/arch/arm/domain.c | 2 +
> xen/arch/arm/psci.c | 87
> +++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/traps.c | 47 ++++++++++++++++++++-
> xen/include/asm-arm/processor.h | 1 +
> xen/include/asm-arm/psci.h | 24 +++++++++++
> 6 files changed, 161 insertions(+), 1 deletions(-)
> create mode 100644 xen/arch/arm/psci.c
> create mode 100644 xen/include/asm-arm/psci.h
>
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 2106a4f..8f75044 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -5,6 +5,7 @@ subdir-y += platforms
> obj-y += early_printk.o
> obj-y += cpu.o
> obj-y += domain.o
> +obj-y += psci.o
> obj-y += domctl.o
> obj-y += sysctl.o
> obj-y += domain_build.o
>
> diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> new file mode 100644
> index 0000000..28153ad
> --- /dev/null
> +++ b/xen/arch/arm/psci.c
> @@ -0,0 +1,87 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <xen/errno.h>
> +#include <xen/sched.h>
> +#include <xen/types.h>
> +
> +#include <asm/current.h>
> +#include <asm/psci.h>
> +
> +int do_psci_cpu_on(uint32_t vcpuid, uint32_t entry_point)
> +{
> + struct vcpu *v;
> + struct domain *d = current->domain;
> + struct vcpu_guest_context *ctxt;
> + int rc;
> +
> + if ( (vcpuid < 0) || (vcpuid >= MAX_VIRT_CPUS) )
> + return PSCI_EINVAL;
> +
> + if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> + return PSCI_EINVAL;
> +
> + if ( !v->is_initialised ) {
If this is true then we eventually return success, should this be
returning EINVAL or DENIED or something?
[...]
> +int do_psci_migrate(uint32_t vcpuid)
> +{
> + return PSCI_ENOSYS;
> +}
Is this required or should we just not expose it in device tree?
> + if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
> + return PSCI_DENIED;
> +
> + memset(ctxt, 0, sizeof(*ctxt));
> + ctxt->user_regs.pc32 = entry_point;
> + ctxt->sctlr = SCTLR_BASE;
> + ctxt->ttbr0 = 0;
> + ctxt->ttbr1 = 0;
> + ctxt->ttbcr = 0; /* Defined Reset Value */
> + ctxt->user_regs.cpsr =
> PSR_ABT_MASK|PSR_FIQ_MASK|PSR_IRQ_MASK|PSR_MODE_SVC;
This value is repeated in a couple of places now, perhaps we should have
a ctxt_init which sets up our vcpus defined "reset" values all in one
place?
> + ctxt->flags = VGCF_online;
> +
> + domain_lock(d);
> + rc = arch_set_info_guest(v, ctxt);
> + free_vcpu_guest_context(ctxt);
> +
> + if ( rc < 0 )
> + {
> + domain_unlock(d);
> + return PSCI_DENIED;
> + }
> + domain_unlock(d);
> + }
> +
> + clear_bit(_VPF_down, &v->pause_flags);
ctxt->flags = VGCF_online would cause this to happen, I'd rather do that
than repeat the logic here.
> @@ -647,6 +673,20 @@ static void do_debug_trap(struct cpu_user_regs *regs,
> unsigned int code)
> }
> }
>
> +static void do_trap_psci(struct cpu_user_regs *regs)
> +{
> + arm_psci_fn_t psci_call = NULL;
> +
> + if ( regs->r0 >= ARRAY_SIZE(arm_psci_table) )
> + {
> + regs->r0 = -ENOSYS;
> + return;
> + }
> +
> + psci_call = arm_psci_table[regs->r0].fn;
Might be NULL?
Since Xen passes the valid PSCI numbers to the guest I think it should
be illegal to call anything else, so rather than ENOSYS calling an
unimplemented slots should result in domain_crash_synchronous. It seems
like the general plan is for various firmware interfaces to use iss==0
and to communicate the valid r0 numbers via device tree, but I don't
think we can assume that all those different interfaces will have the
same ENOSYS like value.
> + regs->r0 = psci_call(regs->r1, regs->r2);
> +}
> +
> static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
> {
> arm_hypercall_fn_t call = NULL;
> @@ -959,8 +999,13 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs
> *regs)
> case HSR_EC_HVC:
> if ( (hsr.iss & 0xff00) == 0xff00 )
> return do_debug_trap(regs, hsr.iss & 0x00ff);
> + if ( hsr.iss == 0 )
> + return do_trap_psci(regs);
> do_trap_hypercall(regs, hsr.iss);
This is a pre-existing issue but this handles all no-zero iss as a Xen
hypercall, I think we should
switch ( hsr.iss )
case XEN_PSCI_TAG: ...
case XEN_HYPERCALL_TAG: ...
default: domain_crash_synchrous
and pull the domain crash out of do_trap_hypercall.
Maybe XEN_PSCI_TAG isn't the right name if other interfaces are going to
use it, but it's only an internal name anyway.
> break;
> + case HSR_EC_SMC:
> + do_trap_psci(regs);
> + break;
> case HSR_EC_DATA_ABORT_GUEST:
> do_trap_data_abort_guest(regs, hsr.dabt);
> break;
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 1681ebf..72f7f6b 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -85,6 +85,7 @@
> #define HSR_EC_CP14_64 0x0c
> #define HSR_EC_SVC 0x11
> #define HSR_EC_HVC 0x12
> +#define HSR_EC_SMC 0x13
> #define HSR_EC_INSTR_ABORT_GUEST 0x20
> #define HSR_EC_INSTR_ABORT_HYP 0x21
> #define HSR_EC_DATA_ABORT_GUEST 0x24
> diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h
> new file mode 100644
> index 0000000..8511eb1
> --- /dev/null
> +++ b/xen/include/asm-arm/psci.h
> @@ -0,0 +1,24 @@
> +#ifndef __ASM_PSCI_H__
> +#define __ASM_PSCI_H__
> +
> +#define PSCI_SUCCESS 0
> +#define PSCI_ENOSYS -1
> +#define PSCI_EINVAL -2
> +#define PSCI_DENIED -3
> +
> +int do_psci_cpu_on(uint32_t vcpuid, uint32_t entry_point);
> +int do_psci_cpu_off(uint32_t power_state);
> +int do_psci_cpu_suspend(uint32_t power_state, uint32_t entry_point);
> +int do_psci_migrate(uint32_t vcpuid);
> +
> +#endif /* __ASM_PSCI_H__ */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |