From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 3/3] xen/arm: errata 766422: decode thumb store during data abort Date: Wed, 31 Jul 2013 11:19:53 +0100 Message-ID: <51F8E4C9.5070000@linaro.org> References: <1374765692-31370-1-git-send-email-julien.grall@linaro.org> <1374765692-31370-4-git-send-email-julien.grall@linaro.org> <1375110911.12741.34.camel@kazak.uk.xensource.com> <51F7F9E6.5000704@linaro.org> <1375260446.32691.86.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1375260446.32691.86.camel@kazak.uk.xensource.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 Cc: Stefano.Stabellini@eu.citrix.com, Tim Deegan , patches@linaro.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 07/31/2013 09:47 AM, Ian Campbell wrote: > On Tue, 2013-07-30 at 18:37 +0100, Julien Grall wrote: >> On 07/29/2013 04:15 PM, Ian Campbell wrote: >>> On Thu, 2013-07-25 at 16:21 +0100, Julien Grall wrote: > >>>> + /* Retrieve the transfer register from the instruction */ >>>> + if ( dabt.len ) >>>> + /* With 32-bit store instruction, the register is in [12..15] */ >>>> + info.dabt.reg = (instr & 0xf000) >> 12; >>>> + else >>>> + /* With 16-bit store instruction, the register is in [0..3] */ >>>> + info.dabt.reg = instr & 0x7; >>> >>> Encoding T2 (store via imm8 offset from sp) has it in 8..10. >> >> Right but ... from ARM DDI 0406C.b B3-1432: an instruction is valid if >> it "is not using the PC as its destination register". So this >> instruction is consider as invalid and will go to "bad_data_abort". > > I'm not sure what this has to do with the encoding I pointed to. > > A8.8.203 STR (immediate, Thumb), Encoding T2: > STR , [SP, #] > is: > 1001 0ttt mmmm mmmm (ttt=Rt, mmmm mmmm=imm) > Oh right, I read 'pc' instead of 'sp'. > So Rt is in bits 8..10 which != 0..3 which is all you handle above. I > can't see any reason why you wouldn't need to handle this case, it is > certainly a valid instruction. > > I think it would be safest to explicitly check for known opcode patterns > and handle those while logging any which we don't recognise. This might > be doable with a lookup table but it may be too sparse. If we were doing > a more full featured instruction decoder then it might be worth it, not > sure about this very specific case. As discussed with Ian, I will create a function decode_instruction. It will decode and fill the HSR. -- Julien