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 15:46 +0100, 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.

Good lord. No wonder I didn't get what Julien was trying to explain to
me, he was trying to explain an insanity!

> 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.

Since we already know the instruction length in this context that would
seem OK. Although it does make it harder to reason about code which
handles either 16 or 32 bit instructions -- which half contains "bit 0"
becomes different.

If we wanted to be more general we would do a 16-bit read and then check
for the magic pattern before doing a shift and a second 16-bit read,
which mimic the hardware and avoid my tiny brain having to think to hard
about mixed endianness.

> > uint16_t a[2];
> > rc = raw_copy_from_guest(instr, (void * __user)regs->pc, len);
> 
> You definitely should read exactly the correct number of bytes -- if we
> are decoding a 16-bit instruction at the end of a page we don't want to
> trigger a fault by reading 32 bits and crossing a page boundary.

Yes.

> Oh, and one other comment that I've already lost the context for: can
> you please rename the instruction fetch-and-shuffle function to have
> 'thumb' in its name somewhere?

ACK!

Ian.


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