Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Alex Bennée]

Peter Maydell  writes:

> On 24 February 2017 at 16:43, Alex Bennée  wrote:
>>
>> Peter Maydell  writes:
>>
>>> On 24 February 2017 at 14:40, Alex Bennée  wrote:
>>> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
>>> PRIx32 does it build OK?
>>
>> Well the problem is the non-eabi compilers don't have PRIx32 defined for
>> you. You have to specify yourself.
>
> Oh, right. Or we could just cast these things to unsigned int ;-)
>
> Other than the tests being a bit ropy, are you happy with the
> patches proper?

Yes, I've just looked through the last few and they look good to me.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Peter Maydell]

On 24 February 2017 at 16:43, Alex Bennée  wrote:
>
> Peter Maydell  writes:
>
>> On 24 February 2017 at 14:40, Alex Bennée  wrote:
>> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
>> PRIx32 does it build OK?
>
> Well the problem is the non-eabi compilers don't have PRIx32 defined for
> you. You have to specify yourself.

Oh, right. Or we could just cast these things to unsigned int ;-)

Other than the tests being a bit ropy, are you happy with the
patches proper?

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Alex Bennée]

Peter Maydell  writes:

> On 24 February 2017 at 14:40, Alex Bennée  wrote:
>> The tests currently fail to build:
>>
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
>> test7.c: In function 'usage':
>> test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', 
>> but argument 2 has type 'uint32_t' [-Werror=format=]
>>  testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
>>  ^
>> test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', 
>> but argument 2 has type 'uint32_t' [-Werror=format=]
>>  testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
>>  ^
>> test7.c: In function 'svc':
>> test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', 
>> but argument 2 has type 'uint32_t' [-Werror=format=]
>>  testDiag("New xPSR %x", sframe[7]);
>>  ^
>> test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', 
>> but argument 2 has type 'uint32_t' [-Werror=format=]
>>  testDiag("old xPSR %x", sframe[7]);
>>  ^
>> test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', 
>> but argument 2 has type 'uint32_t' [-Werror=format=]
>>  testDiag("In SVC handler (direct call from main with LR %x)", 
>> old_lr);
>>  ^
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
>> /usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
>> cc1: all warnings being treated as errors
>> Makefile:49: recipe for target 'test7.o' failed
>> make: *** [test7.o] Error 1
>> make: Target 'all' not remade because of errors.
>>
>> I suspect its the same thing as I came across with kvm-unit-tests that
>> compilers targeting the same abi can still disagree about sizes:
>>
>>   
>> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126
>
> Could be. I use arm-linux-gnueabihf-gcc to build. If you use
> PRIx32 does it build OK?

Well the problem is the non-eabi compilers don't have PRIx32 defined for
you. You have to specify yourself.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Peter Maydell]

On 24 February 2017 at 14:40, Alex Bennée  wrote:
> The tests currently fail to build:
>
> /usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
> test7.c: In function 'usage':
> test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', but 
> argument 2 has type 'uint32_t' [-Werror=format=]
>  testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
>  ^
> test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', but 
> argument 2 has type 'uint32_t' [-Werror=format=]
>  testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
>  ^
> test7.c: In function 'svc':
> test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', 
> but argument 2 has type 'uint32_t' [-Werror=format=]
>  testDiag("New xPSR %x", sframe[7]);
>  ^
> test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', 
> but argument 2 has type 'uint32_t' [-Werror=format=]
>  testDiag("old xPSR %x", sframe[7]);
>  ^
> test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', 
> but argument 2 has type 'uint32_t' [-Werror=format=]
>  testDiag("In SVC handler (direct call from main with LR %x)", 
> old_lr);
>  ^
> /usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
> /usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
> cc1: all warnings being treated as errors
> Makefile:49: recipe for target 'test7.o' failed
> make: *** [test7.o] Error 1
> make: Target 'all' not remade because of errors.
>
> I suspect its the same thing as I came across with kvm-unit-tests that
> compilers targeting the same abi can still disagree about sizes:
>
>   
> https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126

Could be. I use arm-linux-gnueabihf-gcc to build. If you use
PRIx32 does it build OK?

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Alex Bennée]

Peter Maydell  writes:

> On 24 February 2017 at 13:55, Alex Bennée  wrote:
>> Even this branch is failing the tests for me:
>
>> === Testing test9-kern.bin ===
>
>> not ok 9 -  == 0800 icsr
>
>> === Testing test10-kern.bin ===
>> not ok 1 -  == 0800 ICSR
>
>> not ok 9 -  == 0800 ICSR
>
>> === Testing test4-kern.bin ===
>> not ok 5 -  == 0800 ICSR
>
>> not ok 9 - 0041 == 00410800 ICSR
>
> Ah, I missed the test failures, but these are all test bugs.

I thought it was likely to be that ;-)

> All of these failures are for reads of ICSR when we're not
> in an exception handler and the mismatch is because the
> expected value of RETTOBASE differs. The bit is architecturally
> UNKNOWN, and we did a late swap from making it be clear to
> making it be set, because that seemed to be more in line
> with the Cortex-M3 documented behaviour.
>
> I didn't notice that the tests needed to be updated to
> mask out the UNKNOWN bit before comparison.

The tests currently fail to build:

/usr/bin/arm-none-eabi-objcopy -S -O binary test6-kern.elf test6-kern.bin
test7.c: In function 'usage':
test7.c:58:9: error: format '%x' expects argument of type 'unsigned int', but 
argument 2 has type 'uint32_t' [-Werror=format=]
 testDiag("Ignoring bogus LR %x, doing return to handler", old_lr);
 ^
test7.c:76:9: error: format '%x' expects argument of type 'unsigned int', but 
argument 2 has type 'uint32_t' [-Werror=format=]
 testDiag("Ignoring bogus LR %x, doing return to thread", old_lr);
 ^
test7.c: In function 'svc':
test7.c:105:9: error: format '%x' expects argument of type 'unsigned int', but 
argument 2 has type 'uint32_t' [-Werror=format=]
 testDiag("New xPSR %x", sframe[7]);
 ^
test7.c:109:9: error: format '%x' expects argument of type 'unsigned int', but 
argument 2 has type 'uint32_t' [-Werror=format=]
 testDiag("old xPSR %x", sframe[7]);
 ^
test7.c:116:9: error: format '%x' expects argument of type 'unsigned int', but 
argument 2 has type 'uint32_t' [-Werror=format=]
 testDiag("In SVC handler (direct call from main with LR %x)", old_lr);
 ^
/usr/bin/arm-none-eabi-objcopy -S -O binary test8-kern.elf test8-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test9-kern.elf test9-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test10-kern.elf test10-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test11-kern.elf test11-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test13-kern.elf test13-kern.bin
/usr/bin/arm-none-eabi-objcopy -S -O binary test14-kern.elf test14-kern.bin
cc1: all warnings being treated as errors
Makefile:49: recipe for target 'test7.o' failed
make: *** [test7.o] Error 1
make: Target 'all' not remade because of errors.

I suspect its the same thing as I came across with kvm-unit-tests that
compilers targeting the same abi can still disagree about sizes:

  
https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git/commit/?id=529046c397059b8c5ef4dc5fb3c258d86fafb126

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Peter Maydell]

On 24 February 2017 at 14:07, Peter Maydell  wrote:
> On 24 February 2017 at 13:55, Alex Bennée  wrote:
>> Even this branch is failing the tests for me:
>
>> === Testing test9-kern.bin ===
>
>> not ok 9 -  == 0800 icsr
>
>> === Testing test10-kern.bin ===
>> not ok 1 -  == 0800 ICSR
>
>> not ok 9 -  == 0800 ICSR
>
>> === Testing test4-kern.bin ===
>> not ok 5 -  == 0800 ICSR
>
>> not ok 9 - 0041 == 00410800 ICSR
>
> Ah, I missed the test failures, but these are all test bugs.

...pushed fixes for the tests to the test suite repo.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Peter Maydell]

On 24 February 2017 at 13:55, Alex Bennée  wrote:
> Even this branch is failing the tests for me:

> === Testing test9-kern.bin ===

> not ok 9 -  == 0800 icsr

> === Testing test10-kern.bin ===
> not ok 1 -  == 0800 ICSR

> not ok 9 -  == 0800 ICSR

> === Testing test4-kern.bin ===
> not ok 5 -  == 0800 ICSR

> not ok 9 - 0041 == 00410800 ICSR

Ah, I missed the test failures, but these are all test bugs.
All of these failures are for reads of ICSR when we're not
in an exception handler and the mismatch is because the
expected value of RETTOBASE differs. The bit is architecturally
UNKNOWN, and we did a late swap from making it be clear to
making it be set, because that seemed to be more in line
with the Cortex-M3 documented behaviour.

I didn't notice that the tests needed to be updated to
mask out the UNKNOWN bit before comparison.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Alex Bennée]

Peter Maydell  writes:

> On 16 February 2017 at 16:35, Peter Maydell  wrote:
>> This patchset is the revamp of the NVIC code from Michael
>> Davidsaver's patchset of a year ago.
>>
>> Despite some superficial similarities of register layout, the
>> M-profile NVIC is really very different from the A-profile GIC.  Our
>> current attempt to reuse the GIC code means that we have significant
>> bugs in our NVIC.  The series pulls the NVIC apart from the GIC code
>> (fixing a few accidental bugs in the process), and then once it has a
>> place to stand, implements a few minor cleanups, a key bugfix
>> (getting priority calculations and masking right) and a missing
>> feature (escalation to HardFault).
>>
>> For testing, I have used the Stellaris image I have to hand:
>> http://people.linaro.org/~peter.maydell/stellaris.tgz
>> and also a set of bare-metal test programs also written by
>> Michael. You can find my slightly tweaked and cleand up
>> version of those here (a README explains how to run them):
>> https://git.linaro.org/people/peter.maydell/m-profile-tests.git
>
> PS: git branch of this v2 patchset at
> https://git.linaro.org/people/peter.maydell/qemu-arm.git nvic-rewrite
> (includes squashed-in fix to patch 3).

Even this branch is failing the tests for me:

QEMU emulator version 2.8.50 (v2.8.0-1289-g63d3eb3ef5-dirty)
Copyright (c) 2003-2016 Fabrice Bellard and the QEMU Project developers
make: Nothing to be done for 'all'.
=== Testing test1-kern.bin ===
1..2
# Starting
# 12345678
# 23456789
# 6789
# 600789
ok 1 -  ==  var1
ok 2 - 87654321 == 87654321 var2
Ran2/2
Passed 2/2
Fail   0/2
=== Testing test9-kern.bin ===
NVIC: Bad read offset 0xd90
NVIC: Bad read offset 0xd94
1..19
ok 1 - deadbeaf == deadbeaf marker
ok 2 -  ==  LR
# XPSR 4000
ok 3 -  ==  PRIMASK
ok 4 -  ==  FAULTMASK
ok 5 -  ==  BASEPRI
ok 6 -  ==  CONTROL
ok 7 - 2460 == 2460 MSP
ok 8 -  ==  PSP
# cpuid 410fc231
not ok 9 -  == 0800 icsr
ok 10 -  ==  vtor
ok 11 - fa05 == fa05 aircr
ok 12 -  ==  scr
ok 13 - 0200 == 0200 ccr
ok 14 -  ==  shpr[0]
ok 15 -  ==  shpr[1]
ok 16 -  ==  shpr[2]
ok 17 -  ==  shcsr
ok 18 -  ==  syst_csr
# ictr 0001
# mpu_type 
ok 19 -  ==  mpu_ctrl
Ran19/19
Passed 18/19
Fail   1/19
=== Testing test10-kern.bin ===
1..10
# BASEPRI mask 00ff
# DEBUG prio 00ff
not ok 1 -  == 0800 ICSR
ok 2 -  ==  SHCSR
# Call SVC
# In SVC
ok 3 - 080b == 080b ICSR
ok 4 - 0080 == 0080 SHCSR
# In PendSV
ok 5 - 000e == 000e ICSR
ok 6 - 0480 == 0480 SHCSR
# Back in SVC
ok 7 - 0003 == 0003 Back in SVC
# Back in main
ok 8 - 0004 == 0004 Back in SVC
not ok 9 -  == 0800 ICSR
ok 10 -  ==  SHCSR
# Done
Ran10/10
Passed 8/10
Fail   2/10
=== Testing test4-kern.bin ===
1..33
ok 1 - 0005 == 0005 PRIGROUP
# Enable IRQ0/1
ok 2 - 0003 == 0003 ENA
ok 3 -  ==  PEND
ok 4 -  ==  ACT
not ok 5 -  == 0800 ICSR
# Pend IRQ0 (shouldn't run)
ok 6 - 0003 == 0003 ENA
ok 7 - 0001 == 0001 PEND
ok 8 -  ==  ACT
not ok 9 - 0041 == 00410800 ICSR
ok 10 - 0001 == 0001 SEQ 1
# Unmask (should run now)
# in IRQ0 SEQ 2
ok 11 - 0003 == 0003 ENA
ok 12 -  ==  PEND
ok 13 - 0001 == 0001 ACT
ok 14 - 0810 == 0810 ICSR
# Back in main
ok 15 - 0003 == 0003 ENA
ok 16 -  ==  PEND
ok 17 -  ==  ACT
ok 18 - 0003 == 0003 SEQ 3
# Give IRQ1 priority over IRQ0
# Pend IRQ0 and IRQ1 (should run now)
# in IRQ1 SEQ 4
ok 19 - 0003 == 0003 ENA
ok 20 - 0001 == 0001 PEND
ok 21 - 0002 == 0002 ACT
ok 22 - 00410811 == 00410811 ICSR
# in IRQ0 SEQ 5
ok 23 - 0003 == 0003 ENA
ok 24 -  ==  PEND
ok 25 - 0001 == 0001 ACT
ok 26 - 0810 == 0810 ICSR
# Back in main
ok 27 - 0006 == 0006 SEQ 6
# Pend IRQ0 (should run now)
# in IRQ0 SEQ 7
# pend IRQ1
# in IRQ1 SEQ 8
# back in IRQ0
ok 28 - 0009 == 0009 SEQ 9
# Back in main
ok 29 - 000a == 000a SEQ 10
# Pend IRQ1 (should run now)
# in IRQ1 SEQ 11
# pend IRQ0
# still in IRQ1
ok 30 - 000c == 000c SEQ 12
# in IRQ0 SEQ 13
# Back in main
ok 31 - 000e == 000e SEQ 14
# equal prio, IRQ1 has lower sub-group
# Pend IRQ0 and IRQ1 (should run now)
# in IRQ1 SEQ 15
# in IRQ0 SEQ 16
# Back in main
ok 32 - 0011 == 0011 SEQ 17
# Pend IRQ0 (should run now) and IRQ1
# in IRQ0 SEQ 18
# in IRQ1 SEQ 19
# Back in main
ok 33 - 0014 == 0014 

Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/13] Rewrite NVIC to not depend on the GIC

[Peter Maydell]

On 16 February 2017 at 16:35, Peter Maydell  wrote:
> This patchset is the revamp of the NVIC code from Michael
> Davidsaver's patchset of a year ago.
>
> Despite some superficial similarities of register layout, the
> M-profile NVIC is really very different from the A-profile GIC.  Our
> current attempt to reuse the GIC code means that we have significant
> bugs in our NVIC.  The series pulls the NVIC apart from the GIC code
> (fixing a few accidental bugs in the process), and then once it has a
> place to stand, implements a few minor cleanups, a key bugfix
> (getting priority calculations and masking right) and a missing
> feature (escalation to HardFault).
>
> For testing, I have used the Stellaris image I have to hand:
> http://people.linaro.org/~peter.maydell/stellaris.tgz
> and also a set of bare-metal test programs also written by
> Michael. You can find my slightly tweaked and cleand up
> version of those here (a README explains how to run them):
> https://git.linaro.org/people/peter.maydell/m-profile-tests.git

PS: git branch of this v2 patchset at
https://git.linaro.org/people/peter.maydell/qemu-arm.git nvic-rewrite
(includes squashed-in fix to patch 3).

thanks
-- PMM