We weren''t taking the guest mode (CPSR) into account and would always access the user version of the registers. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- v2: Fix r8 vs r8_fiq thinko. --- xen/arch/arm/traps.c | 62 ++++++++++++++++++++++++++++++++++++++++++- xen/arch/arm/vgic.c | 4 +- xen/arch/arm/vpl011.c | 4 +- xen/arch/arm/vtimer.c | 7 +++-- xen/include/asm-arm/regs.h | 6 ++++ 5 files changed, 74 insertions(+), 9 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index bddd7d4..f42e4e9 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -73,6 +73,64 @@ static void print_xen_info(void) debug, print_tainted(taint_str)); } +uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg) +{ + BUG_ON( guest_mode(regs) ); + + /* + * We rely heavily on the layout of cpu_user_regs to avoid having + * to handle all of the registers individually. Use BUILD_BUG_ON to + * ensure that things which expect are contiguous actually are. + */ +#define REGOFFS(R) offsetof(struct cpu_user_regs, R) + + switch ( reg ) { + case 0 ... 7: /* Unbanked registers */ + BUILD_BUG_ON(REGOFFS(r0) + 7*sizeof(uint32_t) != REGOFFS(r7)); + return ®s->r0 + reg; + case 8 ... 12: /* Register banked in FIQ mode */ + BUILD_BUG_ON(REGOFFS(r8_fiq) + 4*sizeof(uint32_t) != REGOFFS(r12_fiq)); + if ( fiq_mode(regs) ) + return ®s->r8_fiq + reg - 8; + else + return ®s->r8 + reg - 8; + case 13 ... 14: /* Banked SP + LR registers */ + BUILD_BUG_ON(REGOFFS(sp_fiq) + 1*sizeof(uint32_t) != REGOFFS(lr_fiq)); + BUILD_BUG_ON(REGOFFS(sp_irq) + 1*sizeof(uint32_t) != REGOFFS(lr_irq)); + BUILD_BUG_ON(REGOFFS(sp_svc) + 1*sizeof(uint32_t) != REGOFFS(lr_svc)); + BUILD_BUG_ON(REGOFFS(sp_abt) + 1*sizeof(uint32_t) != REGOFFS(lr_abt)); + BUILD_BUG_ON(REGOFFS(sp_und) + 1*sizeof(uint32_t) != REGOFFS(lr_und)); + switch ( regs->cpsr & PSR_MODE_MASK ) + { + case PSR_MODE_USR: + case PSR_MODE_SYS: /* Sys regs are the usr regs */ + if ( reg == 13 ) + return ®s->sp_usr; + else /* lr_usr == lr in a user frame */ + return ®s->lr; + case PSR_MODE_FIQ: + return ®s->sp_fiq + reg - 13; + case PSR_MODE_IRQ: + return ®s->sp_irq + reg - 13; + case PSR_MODE_SVC: + return ®s->sp_svc + reg - 13; + case PSR_MODE_ABT: + return ®s->sp_abt + reg - 13; + case PSR_MODE_UND: + return ®s->sp_und + reg - 13; + case PSR_MODE_MON: + case PSR_MODE_HYP: + default: + BUG(); + } + case 15: /* PC */ + return ®s->pc; + default: + BUG(); + } +#undef REGOFFS +} + static const char *decode_fsc(uint32_t fsc, int *level) { const char *msg = NULL; @@ -448,7 +506,7 @@ static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code) switch ( code ) { case 0xe0 ... 0xef: reg = code - 0xe0; - r = ®s->r0 + reg; + r = select_user_reg(regs, reg); printk("DOM%d: R%d = %#010"PRIx32" at %#010"PRIx32"\n", domid, reg, *r, regs->pc); break; @@ -518,7 +576,7 @@ static void do_cp15_32(struct cpu_user_regs *regs, union hsr hsr) { struct hsr_cp32 cp32 = hsr.cp32; - uint32_t *r = ®s->r0 + cp32.reg; + uint32_t *r = select_user_reg(regs, cp32.reg); if ( !cp32.ccvalid ) { dprintk(XENLOG_ERR, "cp_15(32): need to handle invalid condition codes\n"); diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 7d1a5ad..39b9775 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -160,7 +160,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info) { struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); - uint32_t *r = ®s->r0 + dabt.reg; + uint32_t *r = select_user_reg(regs, dabt.reg); struct vgic_irq_rank *rank; int offset = (int)(info->gpa - VGIC_DISTR_BASE_ADDRESS); int gicd_reg = REG(offset); @@ -372,7 +372,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info) { struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); - uint32_t *r = ®s->r0 + dabt.reg; + uint32_t *r = select_user_reg(regs, dabt.reg); struct vgic_irq_rank *rank; int offset = (int)(info->gpa - VGIC_DISTR_BASE_ADDRESS); int gicd_reg = REG(offset); diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c index 1522667..7dcee90 100644 --- a/xen/arch/arm/vpl011.c +++ b/xen/arch/arm/vpl011.c @@ -92,7 +92,7 @@ static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info) { struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); - uint32_t *r = ®s->r0 + dabt.reg; + uint32_t *r = select_user_reg(regs, dabt.reg); int offset = (int)(info->gpa - UART0_START); switch ( offset ) @@ -114,7 +114,7 @@ static int uart0_mmio_write(struct vcpu *v, mmio_info_t *info) { struct hsr_dabt dabt = info->dabt; struct cpu_user_regs *regs = guest_cpu_user_regs(); - uint32_t *r = ®s->r0 + dabt.reg; + uint32_t *r = select_user_reg(regs, dabt.reg); int offset = (int)(info->gpa - UART0_START); switch ( offset ) diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c index 1c45f4a..fc452e3 100644 --- a/xen/arch/arm/vtimer.c +++ b/xen/arch/arm/vtimer.c @@ -22,6 +22,7 @@ #include <xen/timer.h> #include <xen/sched.h> #include <asm/gic.h> +#include <asm/regs.h> extern s_time_t ticks_to_ns(uint64_t ticks); extern uint64_t ns_to_ticks(s_time_t ns); @@ -49,7 +50,7 @@ static int vtimer_emulate_32(struct cpu_user_regs *regs, union hsr hsr) { struct vcpu *v = current; struct hsr_cp32 cp32 = hsr.cp32; - uint32_t *r = ®s->r0 + cp32.reg; + uint32_t *r = select_user_reg(regs, cp32.reg); s_time_t now; switch ( hsr.bits & HSR_CP32_REGS_MASK ) @@ -101,8 +102,8 @@ static int vtimer_emulate_64(struct cpu_user_regs *regs, union hsr hsr) { struct vcpu *v = current; struct hsr_cp64 cp64 = hsr.cp64; - uint32_t *r1 = ®s->r0 + cp64.reg1; - uint32_t *r2 = ®s->r0 + cp64.reg2; + uint32_t *r1 = select_user_reg(regs, cp64.reg1); + uint32_t *r2 = select_user_reg(regs, cp64.reg2); uint64_t ticks; s_time_t now; diff --git a/xen/include/asm-arm/regs.h b/xen/include/asm-arm/regs.h index 54f6ed8..7486944 100644 --- a/xen/include/asm-arm/regs.h +++ b/xen/include/asm-arm/regs.h @@ -30,6 +30,12 @@ #define return_reg(v) ((v)->arch.cpu_info->guest_cpu_user_regs.r0) +/* + * Returns a pointer to the given register value in regs, taking the + * processor mode (CPSR) into account. + */ +extern uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg); + #endif /* __ARM_REGS_H__ */ /* * Local variables: -- 1.7.2.5
At 14:56 +0000 on 19 Dec (1355929001), Ian Campbell wrote:> We weren''t taking the guest mode (CPSR) into account and would always > access the user version of the registers. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Tim Deegan <tim@xen.org>
On Thu, 2012-12-20 at 11:30 +0000, Tim Deegan wrote:> At 14:56 +0000 on 19 Dec (1355929001), Ian Campbell wrote: > > We weren''t taking the guest mode (CPSR) into account and would always > > access the user version of the registers. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Tim Deegan <tim@xen.org>Applied, thanks.
On Thu, 2012-12-20 at 11:30 +0000, Tim Deegan wrote:> At 14:56 +0000 on 19 Dec (1355929001), Ian Campbell wrote: > > We weren''t taking the guest mode (CPSR) into account and would always > > access the user version of the registers. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Tim Deegan <tim@xen.org>Thanks. I applied this last year and forgot to push. Now done. Ian.
On Wed, 2012-12-19 at 14:56 +0000, Ian Campbell wrote:> @@ -73,6 +73,64 @@ static void print_xen_info(void) > debug, print_tainted(taint_str)); > } > > +uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg) > +{ > + BUG_ON( guest_mode(regs) );I was obviously on crack here. 8<-------------------------- From a53486a9aa6c0bd9512ac5e213119c6bda27e3b6 Mon Sep 17 00:00:00 2001 From: Ian Campbell <ian.campbell@citrix.com> Date: Tue, 15 Jan 2013 11:25:05 +0000 Subject: [PATCH] xen: arm: fix assert in select_user_reg The condition was inverted. Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/traps.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index f42e4e9..8afcf0c 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -75,7 +75,7 @@ static void print_xen_info(void) uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg) { - BUG_ON( guest_mode(regs) ); + BUG_ON( !guest_mode(regs) ); /* * We rely heavily on the layout of cpu_user_regs to avoid having -- 1.7.2.5
Stefano Stabellini
2013-Jan-15 11:54 UTC
Re: [PATCH V2] xen: arm: fix guest register access.
On Tue, 15 Jan 2013, Ian Campbell wrote:> On Wed, 2012-12-19 at 14:56 +0000, Ian Campbell wrote: > > @@ -73,6 +73,64 @@ static void print_xen_info(void) > > debug, print_tainted(taint_str)); > > } > > > > +uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg) > > +{ > > + BUG_ON( guest_mode(regs) ); > > I was obviously on crack here. > > 8<-------------------------- > > From a53486a9aa6c0bd9512ac5e213119c6bda27e3b6 Mon Sep 17 00:00:00 2001 > From: Ian Campbell <ian.campbell@citrix.com> > Date: Tue, 15 Jan 2013 11:25:05 +0000 > Subject: [PATCH] xen: arm: fix assert in select_user_reg > > The condition was inverted. > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> xen/arch/arm/traps.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index f42e4e9..8afcf0c 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -75,7 +75,7 @@ static void print_xen_info(void) > > uint32_t *select_user_reg(struct cpu_user_regs *regs, int reg) > { > - BUG_ON( guest_mode(regs) ); > + BUG_ON( !guest_mode(regs) ); > > /* > * We rely heavily on the layout of cpu_user_regs to avoid having > -- > 1.7.2.5 > > > >
> > From a53486a9aa6c0bd9512ac5e213119c6bda27e3b6 Mon Sep 17 00:00:00 2001 > > From: Ian Campbell <ian.campbell@citrix.com> > > Date: Tue, 15 Jan 2013 11:25:05 +0000 > > Subject: [PATCH] xen: arm: fix assert in select_user_reg > > > > The condition was inverted. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>Thanks, applied.
Maybe Matching Threads
- [PATCH 00/45] initial arm v8 (64-bit) support
- [PATCH v3 00/46] initial arm v8 (64-bit) support
- [PATCH v5 5/6] xen/arm: Implement hypercall for dirty page tracing
- [PATCH 1/4] xen/arm: trap guest WFI
- [PATCH v4 00/25] xen: ARMv7 with virtualization extensions