Re: [RFC PATCH v8 13/23] hw/intc/arm_gicv3: Add irq superpriority information

[Jinjie Ruan]

On 2024/3/21 21:17, Peter Maydell wrote:
> On Mon, 18 Mar 2024 at 09:38, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>>
>> A SPI, PPI or SGI interrupt can have a superpriority property. So
>> maintain superpriority information in PendingIrq and GICR/GICD.
>>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> Acked-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>> v3:
>> - Place this ahead of implement GICR_INMIR.
>> - Add Acked-by.
>> ---
>>  include/hw/intc/arm_gicv3_common.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/hw/intc/arm_gicv3_common.h 
>> b/include/hw/intc/arm_gicv3_common.h
>> index 7324c7d983..df4380141d 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -146,6 +146,7 @@ typedef struct {
>>      int irq;
>>      uint8_t prio;
>>      int grp;
>> +    bool superprio;
>>  } PendingIrq;
>>
>>  struct GICv3CPUState {
>> @@ -172,6 +173,7 @@ struct GICv3CPUState {
>>      uint32_t gicr_ienabler0;
>>      uint32_t gicr_ipendr0;
>>      uint32_t gicr_iactiver0;
>> +    uint32_t gicr_isuperprio;
> 
> This field stores the state that is in the GICR_INMIR0
> register, so please name it that way: gicr_inmir0.
> 
>>      uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
>>      uint32_t gicr_igrpmodr0;
>>      uint32_t gicr_nsacr;
>> @@ -274,6 +276,7 @@ struct GICv3State {
>>      GIC_DECLARE_BITMAP(active);       /* GICD_ISACTIVER */
>>      GIC_DECLARE_BITMAP(level);        /* Current level */
>>      GIC_DECLARE_BITMAP(edge_trigger); /* GICD_ICFGR even bits */
>> +    GIC_DECLARE_BITMAP(superprio);    /* GICD_INMIR */
>>      uint8_t gicd_ipriority[GICV3_MAXIRQ];
>>      uint64_t gicd_irouter[GICV3_MAXIRQ];
>>      /* Cached information: pointer to the cpu i/f for the CPUs specified
>> @@ -313,6 +316,7 @@ GICV3_BITMAP_ACCESSORS(pending)
>>  GICV3_BITMAP_ACCESSORS(active)
>>  GICV3_BITMAP_ACCESSORS(level)
>>  GICV3_BITMAP_ACCESSORS(edge_trigger)
>> +GICV3_BITMAP_ACCESSORS(superprio)
> 
> This is the state behind the GICD_INMIR<n> registers, and
> the GIC spec calls the bits in those registers NMI<x>,
> so I would call this bitmap nmi, not superprio.
> 
> This commit adds new device state, so it also needs to be migrated.
> You'll want to add a new subsection to vmstate_gicv3_cpu which
> is present if the GIC implements NMIs, and which has an entry
> for the gicr_inmir0 field. Similarly, you want a new subsection
> in vmstate_gicv3 which is present if NMIs are implemented and which
> has a field for the nmi array.

OK, I'll add it.

> 
> thanks
> -- PMM