Re: [Xen-devel] [PATCH 3/3] xen/arm: errata 766422: decode thumb store during data abort

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-07-29 at 16:04 +0100, Julien Grall wrote:
> 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 <ian.campbell@xxxxxxxxxx> 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.

FYI I am in the process of applying the first 2 patches of this series,
so you might want to wait an hour or so.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel