Re: [PATCH 2/2] xen: arm: traps: correct cond

[Peng Fan <b51431@freescale.com>]

On Mon, Sep 21, 2015 at 11:33:10AM +0100, Ian Campbell wrote:
>On Mon, 2015-09-21 at 17:08 +0800, Peng Fan wrote:
>> On Mon, Sep 21, 2015 at 11:10:11AM +0100, Julien Grall wrote:
>> > Hi Peng,
>> > 
>> > On 21/09/15 08:07, Peng Fan wrote:
>> > > From "G6.2.29 CPSR, Current Program Status Register" of Aarch64 ARM
>> > > and "B1.3.3 Program Status Registers (PSRs)" of ARMv7-A ARM:
>> > > "
>> > 
>> > The section number may change between the different version of the
>> > spec.
>> > Can you also precise the spec version?
>> > 
>> > For instance on my ARM64 spec (ARM DDI 0497A.d) the section G6.2.29
>> > points to "CSSELR" and not "CPSR".
>> > 
>> > > IT[7:5] holds the base condition for the IT block. The base condition
>> > > is
>> > >         the top 3 bits of the condition code specified by the first
>> > > 	condition field of the IT instruction.
>> > > IT[4:0] encodes the size of the IT block, which is the number of
>> > >         instructions that are to be conditionally executed, by the
>> > > 	position of the least significant 1 in this field. It also
>> > > 	encodes the value of the least significant bit of the condition
>> > > 	code for each instruction in the block.
>> > > "
>> > > So should be "cond = ( it >> 5 );" but not "cond = ( it >> 4 );"
>> > 
>> > IT[7:5] encodes the top 3 bits of the condition code and one bit of
>> > IT[4:0] will contain the least significant bit of the condition code.
>> > 
>> > In order to get the full condition code you have to use IT[7:4] which
>> > the current code does (see A2.5.2 ARM DDI 0406C.b).
>> > 
>> > So the current code looks valid to me. Did I miss something?
>> 
>> No, you are correct.
>
>Were these two patches motivated by an actual issue you were seeing? Or
>just from code inspection?

Just code inspection, not an actual issue.

Regards,
Peng.

>

--