|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] arm: implement hypercall continuations
On Thu, 19 Jul 2012, Ian Campbell wrote:
> Largely cribbed from x86, register names differ and the return value is r0 ==
> the first argument rather than the hypercall number (which is r12).
>
> Multicall variant is untested, arms do_multicall_call is currently a BUG() so
> we obviously don't use that yet. I have left a BUG in the hypercall
> continuation path too since it will need validation once multicalls are
> implemented.
>
> Since the multicall state is local we do not need a globally atomic
> {test,set}_bit. However we do need to be atomic WRT interrupts so can't just
> use the naive RMW version. Stick with the global atomic implementation for now
> but keep the __ as documentaion of the intention.
>
> We cannot clobber all argument registers in a debug build anymore
> because continuations expect them to be preserved. Add nr_args field to the
> hypercall dispatch array and use it to only clobber the unused hypercall
> arguments.
>
> While debugging this I noticed that hypercall dispatch will happily run off
> the
> end of the hypercall dispatch array, add a suitable bounds check. Also make
> the
> use of an hvc immediate argument other than XEN_HYPERCALL_TAG fatal to the
> guest.
>
> Signed-off-by: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> ---
> xen/arch/arm/domain.c | 87
> ++++++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/dummy.S | 1 -
> xen/arch/arm/traps.c | 61 ++++++++++++++++++-----------
> xen/include/asm-arm/bitops.h | 9 ++++
> 4 files changed, 134 insertions(+), 24 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index f61568b..80be0cd 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -5,6 +5,7 @@
> #include <xen/softirq.h>
> #include <xen/wait.h>
> #include <xen/errno.h>
> +#include <xen/bitops.h>
>
> #include <asm/current.h>
> #include <asm/regs.h>
> @@ -224,6 +225,92 @@ void sync_vcpu_execstate(struct vcpu *v)
> /* Nothing to do -- no lazy switching */
> }
>
> +#define next_arg(fmt, args) ({ \
> + unsigned long __arg; \
> + switch ( *(fmt)++ ) \
> + { \
> + case 'i': __arg = (unsigned long)va_arg(args, unsigned int); break; \
> + case 'l': __arg = (unsigned long)va_arg(args, unsigned long); break; \
> + case 'h': __arg = (unsigned long)va_arg(args, void *); break; \
> + default: __arg = 0; BUG(); \
> + } \
> + __arg; \
> +})
> +
> +void hypercall_cancel_continuation(void)
> +{
> + struct cpu_user_regs *regs = guest_cpu_user_regs();
> + struct mc_state *mcs = ¤t->mc_state;
> +
> + if ( test_bit(_MCSF_in_multicall, &mcs->flags) )
> + {
> + __clear_bit(_MCSF_call_preempted, &mcs->flags);
> + }
> + else
> + {
> + regs->pc += 4; /* undo re-execute 'hvc #XEN_HYPERCALL_TAG' */
> + }
> +}
> +
> +unsigned long hypercall_create_continuation(
> + unsigned int op, const char *format, ...)
> +{
> + struct mc_state *mcs = ¤t->mc_state;
> + struct cpu_user_regs *regs;
> + const char *p = format;
> + unsigned long arg, rc;
> + unsigned int i;
> + va_list args;
> +
> + /* All hypercalls take at least one argument */
> + BUG_ON( !p || *p == '\0' );
> +
> + va_start(args, format);
> +
> + if ( test_bit(_MCSF_in_multicall, &mcs->flags) )
> + {
> + BUG(); /* XXX multicalls not implemented yet. */
> +
> + __set_bit(_MCSF_call_preempted, &mcs->flags);
> +
> + for ( i = 0; *p != '\0'; i++ )
> + mcs->call.args[i] = next_arg(p, args);
> +
> + /* Return value gets written back to mcs->call.result */
> + rc = mcs->call.result;
> + }
> + else
> + {
> + regs = guest_cpu_user_regs();
> + regs->r12 = op;
> +
> + /* Ensure the hypercall trap instruction is re-executed. */
> + regs->pc -= 4; /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
> +
> + for ( i = 0; *p != '\0'; i++ )
> + {
> + arg = next_arg(p, args);
> +
> + switch ( i )
> + {
> + case 0: regs->r0 = arg; break;
wrong alignment
> + case 1: regs->r1 = arg; break;
> + case 2: regs->r2 = arg; break;
> + case 3: regs->r3 = arg; break;
> + case 4: regs->r4 = arg; break;
> + case 5: regs->r5 = arg; break;
> + }
> + }
> +
> + /* Return value gets written back to r0 */
> + rc = regs->r0;
> + }
> +
> + va_end(args);
> +
> + return rc;
> +}
> +
> void startup_cpu_idle_loop(void)
> {
> struct vcpu *v = current;
> diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
> index cab9522..5406077 100644
> --- a/xen/arch/arm/dummy.S
> +++ b/xen/arch/arm/dummy.S
> @@ -46,7 +46,6 @@ DUMMY(domain_relinquish_resources);
> DUMMY(domain_set_time_offset);
> DUMMY(dom_cow);
> DUMMY(gmfn_to_mfn);
> -DUMMY(hypercall_create_continuation);
> DUMMY(send_timer_event);
> DUMMY(share_xen_page_with_privileged_guests);
> DUMMY(wallclock_time);
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f6e6807..50b62c0 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -413,24 +413,31 @@ unsigned long do_arch_0(unsigned int cmd, unsigned long
> long value)
> return 0;
> }
>
> -typedef unsigned long arm_hypercall_t(
> +typedef unsigned long (*arm_hypercall_fn_t)(
> unsigned int, unsigned int, unsigned int, unsigned int, unsigned int);
>
> -#define HYPERCALL(x) \
> - [ __HYPERVISOR_ ## x ] = (arm_hypercall_t *) do_ ## x
> -
> -static arm_hypercall_t *arm_hypercall_table[] = {
> - HYPERCALL(memory_op),
> - HYPERCALL(domctl),
> - HYPERCALL(arch_0),
> - HYPERCALL(sched_op),
> - HYPERCALL(console_io),
> - HYPERCALL(xen_version),
> - HYPERCALL(event_channel_op),
> - HYPERCALL(memory_op),
> - HYPERCALL(physdev_op),
> - HYPERCALL(sysctl),
> - HYPERCALL(hvm_op),
> +typedef struct {
> + arm_hypercall_fn_t fn;
> + int nr_args;
> +} arm_hypercall_t;
> +
> +#define HYPERCALL(_name, _nr_args) \
> + [ __HYPERVISOR_ ## _name ] = { \
> + .fn = (arm_hypercall_fn_t) &do_ ## _name, \
> + .nr_args = _nr_args, \
> + }
> +
> +static arm_hypercall_t arm_hypercall_table[] = {
> + HYPERCALL(memory_op, 2),
> + HYPERCALL(domctl, 1),
> + HYPERCALL(arch_0, 2),
> + HYPERCALL(sched_op, 2),
> + HYPERCALL(console_io, 3),
> + HYPERCALL(xen_version, 2),
> + HYPERCALL(event_channel_op, 2),
> + HYPERCALL(physdev_op, 2),
> + HYPERCALL(sysctl, 2),
> + HYPERCALL(hvm_op, 2),
> };
>
> static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
> @@ -462,17 +469,18 @@ static void do_debug_trap(struct cpu_user_regs *regs,
> unsigned int code)
>
> static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
> {
> - arm_hypercall_t *call = NULL;
> + arm_hypercall_fn_t call = NULL;
>
> if ( iss != XEN_HYPERCALL_TAG )
> + domain_crash_synchronous();
Why did you change the behavior of the iss != XEN_HYPERCALL_TAG case?
> + if ( regs->r12 > ARRAY_SIZE(arm_hypercall_table) )
> {
> - printk("%s %d: received an alien hypercall iss=%lx\n", __func__ ,
> - __LINE__ , iss);
> - regs->r0 = -EINVAL;
> + regs->r0 = -ENOSYS;
> return;
> }
>
> - call = arm_hypercall_table[regs->r12];
> + call = arm_hypercall_table[regs->r12].fn;
> if ( call == NULL )
> {
> regs->r0 = -ENOSYS;
> @@ -482,8 +490,15 @@ static void do_trap_hypercall(struct cpu_user_regs
> *regs, unsigned long iss)
> regs->r0 = call(regs->r0, regs->r1, regs->r2, regs->r3, regs->r4);
>
> #ifndef NDEBUG
> - /* clobber registers */
> - regs->r1 = regs->r2 = regs->r3 = regs->r4 = regs->r12 = 0xDEADBEEF;
> + /* clobber non-argument registers */
> + switch ( arm_hypercall_table[regs->r12].nr_args ) {
> + case 1: regs->r1 = 0xDEADBEEF;
> + case 2: regs->r2 = 0xDEADBEEF;
> + case 3: regs->r3 = 0xDEADBEEF;
> + case 4: regs->r4 = 0xDEADBEEF;
> + break;
> + default: BUG();
> + }
> #endif
> }
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |