From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 3/3] xen/arm: errata 766422: decode thumb store during data abort Date: Mon, 29 Jul 2013 16:04:48 +0100 Message-ID: <51F68490.8080503@linaro.org> References: <1374602713-716-1-git-send-email-julien.grall@linaro.org> <1374602713-716-4-git-send-email-julien.grall@linaro.org> <1374603497.6623.149.camel@hastur.hellion.org.uk> <1374617800.6623.173.camel@hastur.hellion.org.uk> <1374677740.6623.192.camel@hastur.hellion.org.uk> <51EFF081.8050408@linaro.org> <20130729144626.GI37169@ocelot.phlegethon.org> <51F682B6.7090904@linaro.org> <1375110032.12741.23.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: <1375110032.12741.23.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: "xen-devel@lists.xen.org" , Patch Tracking , Tim Deegan , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 07/29/2013 04:00 PM, Ian Campbell wrote: > On Mon, 2013-07-29 at 15:56 +0100, Julien Grall wrote: >> On 07/29/2013 03:46 PM, Tim Deegan wrote: >>> Hi, >>> >>> At 16:19 +0100 on 24 Jul (1374682769), Julien Grall wrote: >>>> On 07/24/2013 03:55 PM, Ian Campbell wrote: >>>>> On Tue, 2013-07-23 at 23:43 +0100, Julien Grall wrote: >>>>>> On 23 July 2013 23:16, Ian Campbell wrote: >>>>>>> Were does this the idea of swapping them come from though? The ARM ARM >>>>>>> seems (see e.g. section A6.3) to describe things in the order they are >>>>>>> in memory -- is doing otherwise not going to end up confusing us? >>>>>> >>>>>> In THUMB set, a 32-bit instruction consisting of two consecutive >>>>>> halfwords (see section A6.1). >>>>>> So the instruction is in "big endian", but Xen will read the word as a >>>>>> "little endian" things. >>>>> >>>>> Are you saying that the 16-bit sub-words are in the opposite order to >>>>> what I would have expected and what seems most natural from a decode >>>>> PoV? >>>> >>>>> Consider the T3 encoding of STR (imm, thumb) from A8.8.203, which is: >>>>> 1 1 1 1 1 0 0 0 1 1 0 0 Rn | Rt imm12 >>>>> >>>>> (where Rn == bits 0..3 of the first halfword, Rt is 15..12 of the second >>>>> and imm12 is the remainder of the second halfword). >>>>> >>>>> I would have expected that the halfword with the "11111" pattern (which >>>>> identifies this as a 32-bit thumb instruction) would have to come first, >>>>> so the decode knows to look for the second. IOW the halfword with 11111 >>>>> should be at PC and the Rt/imm12 halfword should be at PC+2. >>> >>> Yes. This seems to be pretty much explicit in section A6.1 -- you can >>> read 16 bits and decide from those whether you need to read another 16. >>> >>> "If bits [15:11] of the halfword being decoded take any of the following >>> values, the halfword is the first halfword of a 32-bit instruction" >>> >>>>> So if we >>>>> copy 4 bytes from guest PC we should end up with things in the order >>>>> given above (and in the manual) and swapping shouldn't be needed. >>> >>> Sadly, no. Instruction memory is always little-endian, but: >>> >>> "The Thumb instruction stream is a sequence of halfword-aligned >>> halfwords. Each Thumb instruction is either a single 16-bit halfword >>> in that stream, or a 32-bit instruction consisting of two consecutive >>> halfwords in that stream." >>> >>> So a 32-bit thumb instruction with bytes ABCD (where byte A is the one >>> with the magic 5-bit pattern) will be stored in memory as a mixed-endian >>> monstrosity: >>> >>> PC: B >>> PC+1: A >>> PC+2: D >>> PC+3: C >>> >>> A little-endian halfword read of PC gives you AB; another halfword read >>> at PC+2 gives CD, and a 32-bit little-endian read gives CDAB. >>> >>> We don't necessarily have to do the bit-shuffling explicitly: we could >>> do thumb32 decode with the shuffle implicit in the layout used for >>> decoding. >> >> I think we need to keep the bit-shuffling explicitly. It's less >> confusing than having "non-coherent" shift during the decoding. > > Whatever we do it needs a big comment along the lines of what Tim wrote > above. I will add the comment in the third version of this patch series. -- Julien