| Message ID | 1410279788-27167-6-git-send-email-ian.campbell@citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Ian, On 09/09/14 09:23, Ian Campbell wrote: > Accesses to these from 32-bit userspace would cause a hypervisor exception > (host crash) when running a 64-bit kernel, which is worked around by the fix to > XSA-102. On 32-bit kernels they would be implemented as RAZ/WI which is > incorrect but harmless. > > Update as follows: > - DBGDSCRINT should be R/O. > - DBGDSCREXT should be EL1 only. > - DBGOSLAR is RO and EL1 only. > - DBGVCR, DBGB[VC]R*, DBGW[VC]R*, and DBGOSDLR are EL1 only. > > DBGDIDR and DBGDSCRINT are accessible from EL0 if DBGDSCRext.UDCCdis. Since we > emulate that as RAZ/WI we allow access. Shall we just set DBGDSCRext.UDCCdis to avoid taking care of EL0 access? Regards,
On Tue, 2014-09-09 at 16:45 -0700, Julien Grall wrote: > Hi Ian, > > On 09/09/14 09:23, Ian Campbell wrote: > > Accesses to these from 32-bit userspace would cause a hypervisor exception > > (host crash) when running a 64-bit kernel, which is worked around by the fix to > > XSA-102. On 32-bit kernels they would be implemented as RAZ/WI which is > > incorrect but harmless. > > > > Update as follows: > > - DBGDSCRINT should be R/O. > > - DBGDSCREXT should be EL1 only. > > - DBGOSLAR is RO and EL1 only. > > - DBGVCR, DBGB[VC]R*, DBGW[VC]R*, and DBGOSDLR are EL1 only. > > > > DBGDIDR and DBGDSCRINT are accessible from EL0 if DBGDSCRext.UDCCdis. Since we > > emulate that as RAZ/WI we allow access. > > Shall we just set DBGDSCRext.UDCCdis to avoid taking care of EL0 access? I'd need to lookup what the acceptable reset states for that bit are, but perhaps. Ian.
On 10/02/2015 11:40, Ian Campbell wrote: > On Wed, 2014-09-10 at 10:48 +0100, Ian Campbell wrote: >> On Tue, 2014-09-09 at 16:45 -0700, Julien Grall wrote: >>> Hi Ian, >>> >>> On 09/09/14 09:23, Ian Campbell wrote: >>>> Accesses to these from 32-bit userspace would cause a hypervisor exception >>>> (host crash) when running a 64-bit kernel, which is worked around by the fix to >>>> XSA-102. On 32-bit kernels they would be implemented as RAZ/WI which is >>>> incorrect but harmless. >>>> >>>> Update as follows: >>>> - DBGDSCRINT should be R/O. >>>> - DBGDSCREXT should be EL1 only. >>>> - DBGOSLAR is RO and EL1 only. >>>> - DBGVCR, DBGB[VC]R*, DBGW[VC]R*, and DBGOSDLR are EL1 only. >>>> >>>> DBGDIDR and DBGDSCRINT are accessible from EL0 if DBGDSCRext.UDCCdis. Since we >>>> emulate that as RAZ/WI we allow access. >>> >>> Shall we just set DBGDSCRext.UDCCdis to avoid taking care of EL0 access? >> >> I'd need to lookup what the acceptable reset states for that bit are, >> but perhaps. > > The AArch32 version of this bit resets to 0, so I think the code is OK > as it is, at least for now. > > I'd like to implement proper handling of dbg registers sooner rather > than later, but I think this series should go in first and the the dbg > stuff can be built on it later. I'm fine with that. Let's have a 32-bit userspace support on 64-bit kernel before :). Regards,
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index e7a2791..01cc3c0 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1600,10 +1600,12 @@ static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr) switch ( hsr.bits & HSR_CP32_REGS_MASK ) { case HSR_CPREG32(DBGDIDR): - - /* Read-only register */ + /* + * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis + * is set to 0, which we emulated below. + */ if ( !cp32.read ) - goto bad_cp; + goto undef_cp14_32; /* Implement the minimum requirements: * - Number of watchpoints: 1 @@ -1616,15 +1618,24 @@ static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr) break; case HSR_CPREG32(DBGDSCRINT): + if ( !cp32.read ) + goto undef_cp14_32; + + *r = 0; + break; + case HSR_CPREG32(DBGDSCREXT): + if ( usr_mode(regs) ) + goto undef_cp14_32; + /* Implement debug status and control register as RAZ/WI. * The OS won't use Hardware debug if MDBGen not set */ if ( cp32.read ) *r = 0; break; + case HSR_CPREG32(DBGVCR): - case HSR_CPREG32(DBGOSLAR): case HSR_CPREG32(DBGBVR0): case HSR_CPREG32(DBGBCR0): case HSR_CPREG32(DBGWVR0): @@ -1632,13 +1643,22 @@ static void do_cp14_32(struct cpu_user_regs *regs, union hsr hsr) case HSR_CPREG32(DBGBVR1): case HSR_CPREG32(DBGBCR1): case HSR_CPREG32(DBGOSDLR): + if ( usr_mode(regs) ) + goto undef_cp14_32; /* RAZ/WI */ if ( cp32.read ) *r = 0; break; + case HSR_CPREG32(DBGOSLAR): + if ( usr_mode(regs) ) + goto undef_cp14_32; + /* WO */ + if ( cp32.read ) + goto undef_cp14_32; + /* else: ignore */ + break; default: -bad_cp: #ifndef NDEBUG gdprintk(XENLOG_ERR, "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n", @@ -1647,6 +1667,7 @@ bad_cp: gdprintk(XENLOG_ERR, "unhandled 32-bit cp14 access %#x\n", hsr.bits & HSR_CP32_REGS_MASK); #endif +undef_cp14_32: inject_undef_exception(regs, hsr.len); return; } @@ -1939,8 +1960,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) do_cp15_64(regs, hsr); break; case HSR_EC_CP14_32: - if ( !is_32bit_domain(current->domain) ) - goto bad_trap; + BUG_ON(!psr_mode_is_32bit(regs->cpsr)); do_cp14_32(regs, hsr); break; case HSR_EC_CP14_DBG:
Accesses to these from 32-bit userspace would cause a hypervisor exception (host crash) when running a 64-bit kernel, which is worked around by the fix to XSA-102. On 32-bit kernels they would be implemented as RAZ/WI which is incorrect but harmless. Update as follows: - DBGDSCRINT should be R/O. - DBGDSCREXT should be EL1 only. - DBGOSLAR is RO and EL1 only. - DBGVCR, DBGB[VC]R*, DBGW[VC]R*, and DBGOSDLR are EL1 only. DBGDIDR and DBGDSCRINT are accessible from EL0 if DBGDSCRext.UDCCdis. Since we emulate that as RAZ/WI we allow access. When we do not allow an access we now silently inject an undef even in debug mode since the debugging messages are not helpful (we have handled the access, by explicitly choosing not to). Signed-off-by: Ian Campbell <ian.campbell@citrix.com> --- xen/arch/arm/traps.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-)