Re: [Qemu-devel] [PATCH] nvic: Change NVIC to support ARMv6-M

[Peter Maydell]

On 18 July 2018 at 14:59, Julia Suvorova <address@hidden> wrote:
> On 17.07.2018 15:58, Peter Maydell wrote:
>>
>> On 10 July 2018 at 16:33, Julia Suvorova <address@hidden> wrote:
>>>
>>> The differences from ARMv7-M NVIC are:
>>>    * ARMv6-M only supports up to 32 external interrupts
>>>     (configurable feature already). The ICTR is reserved.
>>>    * Active Bit Register is reserved.
>>>    * ARMv6-M supports 4 priority levels against 256 in ARMv7-M.
>>>
>>> Signed-off-by: Julia Suvorova <address@hidden>
>>> ---
>>>   hw/intc/armv7m_nvic.c | 29 +++++++++++++++++++++++++----
>>>   1 file changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
>>> index 38aaf3dc8e..8545c87caa 100644
>>> --- a/hw/intc/armv7m_nvic.c
>>> +++ b/hw/intc/armv7m_nvic.c
>>> @@ -420,6 +420,10 @@ static void set_prio(NVICState *s, unsigned irq,
>>> bool secure, uint8_t prio)
>>>       assert(irq > ARMV7M_EXCP_NMI); /* only use for configurable prios
>>> */
>>>       assert(irq < s->num_irq);
>>>
>>> +    if (!arm_feature(&s->cpu->env, ARM_FEATURE_V7)) {
>>> +        prio &= 0xc0;
>>
>>
>> Rather than hard-coding this, I think we should have a
>> num_prio_bits field in the NVICState struct which defines
>> how many bits of priority are implemented. This would be
>> 2 for v6M and 8 otherwise, and can be set in
>> armv7m_nvic_realize. Then the mask is
>>    MAKE_64BIT_MASK(8 - num_prio_bits, num_prio_bits);
>> (For v8M the number of priority bits is configurable.)
>
>
> Do I understand correctly that the check in armv7m_nvic_realize is for
> Baseline, not only v6m, because Baseline has only 4 priority levels too?
> And num_prio_bits should be just a field, not a property?

Make it for v6M -- v8M baseline doesn't specify a particular
number of priority levels, you can do anything. You can leave
it as a field and we'll bump it up to a property if we need it
(probably if/when we do a v8M baseline CPU model); though
if you want to make it a full property that's fine too.

thanks
-- PMM