On 2024/3/28 22:54, Peter Maydell wrote:
> On Mon, 25 Mar 2024 at 08:52, Jinjie Ruan <ruanjin...@huawei.com> wrote:
>>
>> A SPI, PPI or SGI interrupt can have non-maskable property. So maintain
>> non-maskable property in PendingIrq and GICR/GICD. Since add new device
>> state, it also needs to be migrated, so also save NMI info in
>> vmstate_gicv3_cpu and vmstate_gicv3.
>>
>> Signed-off-by: Jinjie Ruan <ruanjin...@huawei.com>
>> Acked-by: Richard Henderson <richard.hender...@linaro.org>
>> ---
>> v10:
>> - superprio -> nmi, gicr_isuperprio -> gicr_inmir0.
>> - Save NMI state in vmstate_gicv3_cpu and vmstate_gicv3.
>> - Update the commit message.
>> v3:
>> - Place this ahead of implement GICR_INMIR.
>> - Add Acked-by.
>> ---
>> hw/intc/arm_gicv3_common.c | 44 ++++++++++++++++++++++++++++++
>> include/hw/intc/arm_gicv3_common.h | 4 +++
>> 2 files changed, 48 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 2d2cea6858..be76ae0be6 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -164,6 +164,24 @@ const VMStateDescription vmstate_gicv3_gicv4 = {
>> }
>> };
>>
>> +static bool nmi_needed(void *opaque)
>> +{
>> + GICv3CPUState *cs = opaque;
>> +
>> + return cs->gic->nmi_support != 0;
>
> nmi_support is already a bool, so you can
> return cs->gic_nmi_support;
>
>
>> +}
>> +
>> +static const VMStateDescription vmstate_gicv3_cpu_nmi = {
>> + .name = "arm_gicv3_cpu/nmi",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .needed = nmi_needed,
>> + .fields = (const VMStateField[]) {
>> + VMSTATE_UINT32(gicr_inmir0, GICv3CPUState),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> static const VMStateDescription vmstate_gicv3_cpu = {
>> .name = "arm_gicv3_cpu",
>> .version_id = 1,
>> @@ -197,6 +215,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>> &vmstate_gicv3_cpu_sre_el1,
>> &vmstate_gicv3_gicv4,
>> NULL
>> + },
>> + .subsections = (const VMStateDescription * const []) {
>> + &vmstate_gicv3_cpu_nmi,
>> + NULL
>
> You add your subsection to the existing .subsections[] list.
> Otherwise this field initializer overwrites the previous one.
>
>> }
>> };
>>
>> @@ -238,6 +260,24 @@ const VMStateDescription
>> vmstate_gicv3_gicd_no_migration_shift_bug = {
>> }
>> };
>>
>> +static bool needed_nmi(void *opaque)
>> +{
>> + GICv3State *cs = opaque;
>> +
>> + return cs->nmi_support != 0;
>> +}
>
> You already have nmi_needed() above, so you can use it
> as the .needed function for both vmstate struct definitions.
The input opaque pointer seems not same, one is "GICv3CPUState *", but
another is "GICv3State *"
>
>> +
>> +const VMStateDescription vmstate_gicv3_gicd_nmi = {
>> + .name = "arm_gicv3/gicd_nmi",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .needed = needed_nmi,
>> + .fields = (const VMStateField[]) {
>> + VMSTATE_UINT32_ARRAY(nmi, GICv3State, GICV3_BMP_SIZE),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> static const VMStateDescription vmstate_gicv3 = {
>> .name = "arm_gicv3",
>> .version_id = 1,
>> @@ -267,6 +307,10 @@ static const VMStateDescription vmstate_gicv3 = {
>> .subsections = (const VMStateDescription * const []) {
>> &vmstate_gicv3_gicd_no_migration_shift_bug,
>> NULL
>> + },
>> + .subsections = (const VMStateDescription * const []) {
>> + &vmstate_gicv3_gicd_nmi,
>> + NULL
>
> Similarly here this must be added to the existing list.
>
>> }
>> };
>>
>> diff --git a/include/hw/intc/arm_gicv3_common.h
>> b/include/hw/intc/arm_gicv3_common.h
>> index 4358c5319c..88533749eb 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 nmi;
>> } PendingIrq;
>>
>> struct GICv3CPUState {
>> @@ -172,6 +173,7 @@ struct GICv3CPUState {
>> uint32_t gicr_ienabler0;
>> uint32_t gicr_ipendr0;
>> uint32_t gicr_iactiver0;
>> + uint32_t gicr_inmir0;
>> uint32_t edge_trigger; /* ICFGR0 and ICFGR1 even bits */
>> uint32_t gicr_igrpmodr0;
>> uint32_t gicr_nsacr;
>> @@ -275,6 +277,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(nmi); /* 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
>> @@ -314,6 +317,7 @@ GICV3_BITMAP_ACCESSORS(pending)
>> GICV3_BITMAP_ACCESSORS(active)
>> GICV3_BITMAP_ACCESSORS(level)
>> GICV3_BITMAP_ACCESSORS(edge_trigger)
>> +GICV3_BITMAP_ACCESSORS(nmi)
>>
>> #define TYPE_ARM_GICV3_COMMON "arm-gicv3-common"
>> typedef struct ARMGICv3CommonClass ARMGICv3CommonClass;
>> --
>> 2.34.1
>
> thanks
> -- PMM