From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for-4.5 v6 14/17] xen/arm: Instruction prefetch abort (X) mem_event handling Date: Thu, 18 Sep 2014 19:59:56 +0100 Message-ID: <1411066796.1920.27.camel@citrix.com> References: <1410789775-24197-1-git-send-email-tklengyel@sec.in.tum.de> <1410789775-24197-15-git-send-email-tklengyel@sec.in.tum.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1410789775-24197-15-git-send-email-tklengyel@sec.in.tum.de> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel Cc: tim@xen.org, julien.grall@linaro.org, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, andres@lagarcavilla.org, jbeulich@suse.com, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On Mon, 2014-09-15 at 16:02 +0200, Tamas K Lengyel wrote: > Add missing structure definition for iabt and update the trap handling > mechanism to only inject the exception if the mem_access checker > decides to do so. > > Signed-off-by: Tamas K Lengyel > --- > v6: - Make npfec a const. > v4: - Don't mark instruction fetch violation as read violation. > - Use new struct npfec to pass violation info. > v2: - Add definition for instruction abort instruction fetch status codes > (enum iabt_ifsc) and only call p2m_mem_access_check for traps triggered > for permission violations. > --- > xen/arch/arm/traps.c | 39 ++++++++++++++++++++++++++++++++++++++- > xen/include/asm-arm/processor.h | 40 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 77 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > index 5bfcdf3..71c087f 100644 > --- a/xen/arch/arm/traps.c > +++ b/xen/arch/arm/traps.c > @@ -1828,7 +1828,44 @@ done: > static void do_trap_instr_abort_guest(struct cpu_user_regs *regs, > union hsr hsr) > { > - register_t addr = READ_SYSREG(FAR_EL2); > + struct hsr_iabt iabt = hsr.iabt; > + int rc; > + register_t addr; > + vaddr_t gva; > + paddr_t gpa; > + > +#ifdef CONFIG_ARM_32 > + gva = READ_CP32(HIFAR); > +#else > + gva = READ_SYSREG64(FAR_EL2); > +#endif You can just use READ_SYSREG(FAR_EL2) here and it will do the right thing without the ifdef. [...] > + addr = READ_SYSREG(FAR_EL2); Like you do here ;-) > index b844f1d..044de12 100644 > --- a/xen/include/asm-arm/processor.h > +++ b/xen/include/asm-arm/processor.h > @@ -292,6 +292,36 @@ enum dabt_dfsc { > DABT_DFSC_TLB_CONFLICT = 0b110000, > }; > > +/* Instruction abort instruction fault status codes */ > +enum iabt_ifsc { > + IABT_IFSC_ADDR_SIZE_0 = 0b000000, Apart from the related comments on the last patch which mostly apply here too, aren't these mostly common with the DABT codes? > @@ -371,10 +401,18 @@ union hsr { > } sysreg; /* HSR_EC_SYSREG */ > #endif > > + struct hsr_iabt { > + unsigned long ifsc:6; /* Instruction fault status code */ > + unsigned long res0:1; > + unsigned long s1ptw:1; /* Fault during a stage 1 translation table walk */ > + unsigned long res1:1; > + unsigned long ea:1; /* External abort type */ Please use eat for consistency here. You should also include the common len/cc/etc bits and sufficient padding that the whole thing adds up to 32-bits. Ian.