Re: [PATCH 4/4] xen/arm: Add some useful debug in coprocessor trapping

[Julien Grall <julien.grall@linaro.org>]

On 05/02/2014 12:12 PM, Ian Campbell wrote:
> On Thu, 2014-04-24 at 23:45 +0100, Julien Grall wrote:
>> XSA-93 adds a couple of new functions to trap coprocessor registers. They
>> unconditonally inject an undefined instruction to guest.
> 
> "unconditionally"
> 
>> When debugging an OS at early stage, it may be hard to know why the guest
>> received an UNDEFINED. Add some debug message to help the developper when Xen
> 
> "developer"

I will fix both typo on the next version.

>> is built in debug mode.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/arch/arm/traps.c            |   18 ++++++++++++++++++
>>  xen/include/asm-arm/processor.h |   15 +++++++++++++--
>>  2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 1f61e6e..c04f53f 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -1539,23 +1539,41 @@ bad_cp:
>>  
>>  static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr)
>>  {
>> +    struct hsr_cp64 cp64 = hsr.cp64;
> 
> Won't this be unused in debug=n builds and therefore not build?

Right. I will add #ifndef NDEBUG

>> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
>> index 9267c1b..bc29de1 100644
>> --- a/xen/include/asm-arm/processor.h
>> +++ b/xen/include/asm-arm/processor.h
>> @@ -289,12 +289,23 @@ union hsr {
>>          unsigned long reg2:5;   /* Rt2 */
>>          unsigned long sbzp2:1;
>>          unsigned long op1:4;    /* Op1 */
>> -        unsigned long cc:4;     /* Condition Code */
>> -        unsigned long ccvalid:1;/* CC Valid */
>> +        unsigned long cc:4;     /* condition code */
>> +        unsigned long ccvalid:1;/* cc valid */
> 
> This seems a bit gratuitous, especially given it appears 3 times and you
> only change one. I'd prefer if you just made the new version match the
> existing ones than change everything.

This change has been added by mistake. I will remove it in next version.

Regards,

-- 
Julien Grall