| Message ID | 1398379556-1132-5-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote: > XSA-93 adds a couple of new functions to trap coprocessor registers. They > unconditonally inject an undefined instruction to guest. "unconditionally" > When debugging an OS at early stage, it may be hard to know why the guest > received an UNDEFINED. Add some debug message to help the developper when Xen "developer" > is built in debug mode. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/traps.c | 18 ++++++++++++++++++ > xen/include/asm-arm/processor.h | 15 +++++++++++++-- > 2 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 1f61e6e..c04f53f 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1539,23 +1539,41 @@ bad_cp: > > static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr) > { > + struct hsr_cp64 cp64 = hsr.cp64; Won't this be unused in debug=n builds and therefore not build? > diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h > index 9267c1b..bc29de1 100644 > --- a/xen/include/asm-arm/processor.h > +++ b/xen/include/asm-arm/processor.h > @@ -289,12 +289,23 @@ union hsr { > unsigned long reg2:5; /* Rt2 */ > unsigned long sbzp2:1; > unsigned long op1:4; /* Op1 */ > - unsigned long cc:4; /* Condition Code */ > - unsigned long ccvalid:1;/* CC Valid */ > + unsigned long cc:4; /* condition code */ > + unsigned long ccvalid:1;/* cc valid */ This seems a bit gratuitous, especially given it appears 3 times and you only change one. I'd prefer if you just made the new version match the existing ones than change everything. > unsigned long len:1; /* Instruction length */ > unsigned long ec:6; /* Exception Class */ > } cp64; /* HSR_EC_CP15_64, HSR_EC_CP14_64 */ > > + struct hsr_cp { > + unsigned long coproc:4; /* Number of coproc accessed */ > + unsigned long sbz0p:1; > + unsigned long tas:1; /* Trapped Advanced SIMD */ > + unsigned long res0:14; > + unsigned long cc:4; /* condition code */ > + unsigned long ccvalid:1;/* cc valid */ > + unsigned long len:1; /* Instruction length */ > + unsigned long ec:6; /* Exception Class */ > + } cp; /* HSR_EC_CP */ > + > #ifdef CONFIG_ARM_64 > struct hsr_sysreg { > unsigned long read:1; /* Direction */
On 05/02/2014 12:12 PM, Ian Campbell wrote: > On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote: >> XSA-93 adds a couple of new functions to trap coprocessor registers. They >> unconditonally inject an undefined instruction to guest. > > "unconditionally" > >> When debugging an OS at early stage, it may be hard to know why the guest >> received an UNDEFINED. Add some debug message to help the developper when Xen > > "developer" I will fix both typo on the next version. >> is built in debug mode. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> --- >> xen/arch/arm/traps.c | 18 ++++++++++++++++++ >> xen/include/asm-arm/processor.h | 15 +++++++++++++-- >> 2 files changed, 31 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c >> index 1f61e6e..c04f53f 100644 >> --- a/xen/arch/arm/traps.c >> +++ b/xen/arch/arm/traps.c >> @@ -1539,23 +1539,41 @@ bad_cp: >> >> static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr) >> { >> + struct hsr_cp64 cp64 = hsr.cp64; > > Won't this be unused in debug=n builds and therefore not build? Right. I will add #ifndef NDEBUG >> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h >> index 9267c1b..bc29de1 100644 >> --- a/xen/include/asm-arm/processor.h >> +++ b/xen/include/asm-arm/processor.h >> @@ -289,12 +289,23 @@ union hsr { >> unsigned long reg2:5; /* Rt2 */ >> unsigned long sbzp2:1; >> unsigned long op1:4; /* Op1 */ >> - unsigned long cc:4; /* Condition Code */ >> - unsigned long ccvalid:1;/* CC Valid */ >> + unsigned long cc:4; /* condition code */ >> + unsigned long ccvalid:1;/* cc valid */ > > This seems a bit gratuitous, especially given it appears 3 times and you > only change one. I'd prefer if you just made the new version match the > existing ones than change everything. This change has been added by mistake. I will remove it in next version. Regards,
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 1f61e6e..c04f53f 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1539,23 +1539,41 @@ bad_cp: static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr) { + struct hsr_cp64 cp64 = hsr.cp64; + if ( !check_conditional_instr(regs, hsr) ) { advance_pc(regs, hsr); return; } +#ifndef NDEBUG + gdprintk(XENLOG_ERR, + "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n", + cp64.read ? "mrrc" : "mcrr", + cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc); + gdprintk(XENLOG_ERR, "unhandled 64-bit CP14 access %#x\n", + hsr.bits & HSR_CP64_REGS_MASK); +#endif inject_undef32_exception(regs); } static void do_cp(struct cpu_user_regs *regs, union hsr hsr) { +#ifndef NDEBUG + struct hsr_cp cp = hsr.cp; +#endif + if ( !check_conditional_instr(regs, hsr) ) { advance_pc(regs, hsr); return; } +#ifndef NDEBUG + ASSERT(!cp.tas); /* We don't trap SIMD instruction */ + gdprintk(XENLOG_ERR, "unhandled CP%d access\n", cp.coproc); +#endif inject_undef32_exception(regs); } diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index 9267c1b..bc29de1 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -289,12 +289,23 @@ union hsr { unsigned long reg2:5; /* Rt2 */ unsigned long sbzp2:1; unsigned long op1:4; /* Op1 */ - unsigned long cc:4; /* Condition Code */ - unsigned long ccvalid:1;/* CC Valid */ + unsigned long cc:4; /* condition code */ + unsigned long ccvalid:1;/* cc valid */ unsigned long len:1; /* Instruction length */ unsigned long ec:6; /* Exception Class */ } cp64; /* HSR_EC_CP15_64, HSR_EC_CP14_64 */ + struct hsr_cp { + unsigned long coproc:4; /* Number of coproc accessed */ + unsigned long sbz0p:1; + unsigned long tas:1; /* Trapped Advanced SIMD */ + unsigned long res0:14; + unsigned long cc:4; /* condition code */ + unsigned long ccvalid:1;/* cc valid */ + unsigned long len:1; /* Instruction length */ + unsigned long ec:6; /* Exception Class */ + } cp; /* HSR_EC_CP */ + #ifdef CONFIG_ARM_64 struct hsr_sysreg { unsigned long read:1; /* Direction */
XSA-93 adds a couple of new functions to trap coprocessor registers. They unconditonally inject an undefined instruction to guest. When debugging an OS at early stage, it may be hard to know why the guest received an UNDEFINED. Add some debug message to help the developper when Xen is built in debug mode. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/traps.c | 18 ++++++++++++++++++ xen/include/asm-arm/processor.h | 15 +++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)