Re: [Qemu-devel] [PATCH v2 05/26] armv7m: add armv7m_excp_running_prio()

[Michael Davidsaver <mdavidsaver@gmail.com>]

On 12/17/2015 09:36 AM, Peter Maydell wrote:
> On 3 December 2015 at 00:18, Michael Davidsaver <mdavidsaver@gmail.com> wrote:
>> Implements v7m exception priority algorithm
>> using FAULTMASK, PRIMASK, BASEPRI, and the highest
>> priority active exception.
>>
>> The number returned is the current execution priority
>> which may be in the range [-2,0x7f] when an exception is active
>> or 0x100 when no exception is active.
>> ---
>>  hw/intc/armv7m_nvic.c | 25 +++++++++++++++++++++++++
>>  target-arm/cpu.h      |  1 +
>>  2 files changed, 26 insertions(+)
>>
>> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
>> index 6fc167e..0145ca7 100644
>> --- a/hw/intc/armv7m_nvic.c
>> +++ b/hw/intc/armv7m_nvic.c
>> @@ -18,6 +18,8 @@
>>
>>  typedef struct {
>>      GICState gic;
>> +    uint8_t prigroup;
>> +
>>      struct {
>>          uint32_t control;
>>          uint32_t reload;
>> @@ -116,6 +118,29 @@ static void systick_reset(nvic_state *s)
>>      timer_del(s->systick.timer);
>>  }
>>
>> +/* @returns the active (running) exception priority.
>> + *    only a higher (numerically lower) priority can preempt.
>> + */
>> +int armv7m_excp_running_prio(ARMCPU *cpu)
>> +{
>> +    CPUARMState *env = &cpu->env;
>> +    nvic_state *s = env->nvic;
>> +    int running;
>> +
>> +    if (env->daif & PSTATE_F) { /* FAULTMASK */
>> +        running = -1;
>> +    } else if (env->daif & PSTATE_I) { /* PRIMASK */
>> +        running = 0;
>> +    } else if (env->v7m.basepri > 0) {
>> +        /* BASEPRI==1 -> masks [1,255] (not same as PRIMASK==1) */
>> +        running = env->v7m.basepri >> (s->prigroup+1);
> This isn't right -- the effect of PRIGROUP is that we mask
> out the lower (subgroup) bits, but the upper group bits stay
> where they are rather than shifting down.
>
> So you want env->v7m.basepri & ~((1 << (s->prigroup + 1)) - 1);
>
> (the same mask you need to get the group priority for
> an interrupt).

I don't know about "right", but it is consistent with how the
.prio_group field is now handled in the nvic.  So I think the final
behavior is as specified.

There is no functional reason that I do this.  I just think it makes the
DPRINTF messages easier to interpret.  If you feel strongly I can change
this.