From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 6/9] xen: arm: Handle CP14 32-bit register accesses from userspace Date: Tue, 17 Feb 2015 15:20:57 +0000 Message-ID: <54E35C59.6060104@linaro.org> References: <1423542956.5851.9.camel@citrix.com> <1423543523-8010-6-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1423543523-8010-6-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 10/02/15 04:45, 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. > > 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 > --- > xen/arch/arm/traps.c | 34 +++++++++++++++++++++++++++------- > 1 file changed, 27 insertions(+), 7 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 6045396..18f8332 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1721,10 +1721,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 > @@ -1737,15 +1739,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; > + The comment explaining why this field is RAZ/WI is useful. I would add it again here. Regards, -- Julien Grall