[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/5] xen: arm: fix guest register access.
On Tue, 18 Dec 2012, 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@xxxxxxxxxx> > --- > xen/arch/arm/traps.c | 62 ++++++++++++++++++++++++++++++++++++++++++- > xen/arch/arm/vgic.c | 4 +- > xen/arch/arm/vpl011.c | 4 +- > xen/arch/arm/vtimer.c | 8 +++--- > xen/include/asm-arm/regs.h | 6 ++++ > 5 files changed, 74 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 096dc0b..e3c0290 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_fiq + reg - 8; what's the point of this if? > + 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 3f7e757..59780d2 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 490b021..07994b2 100644 > --- a/xen/arch/arm/vtimer.c > +++ b/xen/arch/arm/vtimer.c > @@ -21,7 +21,7 @@ > #include <xen/lib.h> > #include <xen/timer.h> > #include <xen/sched.h> > -#include "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 +49,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 +101,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 > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |