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 <ruanjinjie@huawei.com>
Acked-by: Richard Henderson <richard.henderson@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;
+}
+
+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
}
};
@@ -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;
+}
+
+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
}
};
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.1On Mon, 25 Mar 2024 at 08:52, Jinjie Ruan <ruanjinjie@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 <ruanjinjie@huawei.com>
> Acked-by: Richard Henderson <richard.henderson@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.
> +
> +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
On 2024/3/28 22:54, Peter Maydell wrote:
> On Mon, 25 Mar 2024 at 08:52, Jinjie Ruan <ruanjinjie@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 <ruanjinjie@huawei.com>
>> Acked-by: Richard Henderson <richard.henderson@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
On Sat, 30 Mar 2024 at 01:42, Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
>
>
> On 2024/3/28 22:54, Peter Maydell wrote:
> > On Mon, 25 Mar 2024 at 08:52, Jinjie Ruan <ruanjinjie@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 <ruanjinjie@huawei.com>
> >> Acked-by: Richard Henderson <richard.henderson@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;
> >
> >
> >> +}
> >> @@ -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 *"
Oops, you're right. In that case let's give these two
functions names that hopefully guide the reader towards
the difference, like gic_cpu_state_nmi_needed() and
gic_state_nmi_needed().
thanks
-- PMM
© 2016 - 2026 Red Hat, Inc.