[PATCH v2 0/3] hw/intc: handle GICD_TYPER2 for KVM GICv3

[PATCH v2 0/3] hw/intc: handle GICD_TYPER2 for KVM GICv3

[Peter Maydell]

The GICD_TYPER2 GICv3 distributor register is one that is added
for GICv4.1; previously this was architected as a RES0 location.
Our TCG GIC doesn't implement GICv4.1, but for KVM the kernel
might support it.

This patchset:
 * makes GICD_TYPER0 reads not trigger a bad-read trace
   event on the TCG GICv3, for the benefit of GICv4.1-aware
   guest code
 * migrates the GICD_TYPER2 register value on a KVM GIC,
   so that a mismatch between source and destination
   can be caught by the destination kernel

Note that I have only very lightly tested this, on a
host which (I believe) doesn't have a GICv4.1.

Changes v1->v2:
 * fix comment missing bracket
 * fix reset handling so this works on GICv4.1 hosts
 * move get/put code to be with the other GICD regs
 * new patch 3 to drop a barely-used debug printf macro

thanks
-- PMM

Peter Maydell (3):
  hw/intc/arm_gicv3_dist: Implement GICD_TYPER2 as 0
  hw/intc/arm_gicv3_kvm: Migrate GICD_TYPER2
  hw/intc/arm_gicv3_kvm: Drop DPRINTF macro

 hw/intc/gicv3_internal.h           |  1 +
 include/hw/intc/arm_gicv3_common.h |  6 +++++
 hw/intc/arm_gicv3_common.c         | 24 +++++++++++++++++++
 hw/intc/arm_gicv3_dist.c           |  9 +++++++
 hw/intc/arm_gicv3_kvm.c            | 38 ++++++++++++++++++------------
 5 files changed, 63 insertions(+), 15 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] hw/intc/arm_gicv3_dist: Implement GICD_TYPER2 as 0

[Peter Maydell]

The GIC distributor registers GICD_TYPER2 is present when the
GICv4.1 is implemented, and RES0 otherwise. QEMU's TCG implementation
is only GICv4.0, so this register is RES0. However, since it's
reasonable for GICv4.1-aware software to read the register, expecting
the zero for GICv3 and GICv4.0, implement the case to avoid it being
logged as an invalid guest read.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
---
 hw/intc/gicv3_internal.h | 1 +
 hw/intc/arm_gicv3_dist.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index bc9f518fe86..fc586524f56 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -31,6 +31,7 @@
 #define GICD_CTLR            0x0000
 #define GICD_TYPER           0x0004
 #define GICD_IIDR            0x0008
+#define GICD_TYPER2          0x000C
 #define GICD_STATUSR         0x0010
 #define GICD_SETSPI_NSR      0x0040
 #define GICD_CLRSPI_NSR      0x0048
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index d8207acb22c..a7d10ed9493 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -431,6 +431,15 @@ static bool gicd_readl(GICv3State *s, hwaddr offset,
             (0xf << 19) | itlinesnumber;
         return true;
     }
+    case GICD_TYPER2:
+        /*
+         * This register only exists for GICv4.1, which QEMU doesn't
+         * currently emulate. On GICv3 and GICv4 it's defined to be RES0.
+         * We implement as read-zero here to avoid tracing a bad-register-read
+         * if GICv4.1-aware software reads this ID register.
+         */
+        *data = 0;
+        return true;
     case GICD_IIDR:
         /* We claim to be an ARM r0p0 with a zero ProductID.
          * This is the same as an r0p0 GIC-500.
-- 
2.43.0

[PATCH v2 2/3] hw/intc/arm_gicv3_kvm: Migrate GICD_TYPER2

[Peter Maydell]

The GICD_TYPER2 register is new for GICv4.1.  As an ID register, we
migrate it so that on the destination the kernel can check that the
destination supports the same configuration that the source system
had.  This avoids confusing behaviour if the user tries to migrate a
VM from a GICv3 system to a GICv4.1 system or vice-versa.  (In
practice the inability to migrate between different CPU types
probably already prevented this.)

Note that older kernels pre-dating GICv4.1 support in the KVM GIC
will just ignore whatever userspace writes to GICD_TYPER2, so
migration where the destination is one of those kernels won't have
this safety-check.

(The reporting of a mismatch will be a bit clunky, because this
file currently uses error_abort for all failures to write the
data to the kernel. Ideally we would fix this by making them
propagate the failure information back up to the post_load hook
return value, to cause a migration failure.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2:
 * fix comment missing bracket
 * fix reset handling so this works on GICv4.1 hosts
 * move get/put code to be with the other GICD regs
---
 include/hw/intc/arm_gicv3_common.h |  6 ++++++
 hw/intc/arm_gicv3_common.c         | 24 ++++++++++++++++++++++++
 hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++--
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index a3d6a0e5077..bcbcae1fbe7 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -267,6 +267,12 @@ struct GICv3State {
     GICv3CPUState *gicd_irouter_target[GICV3_MAXIRQ];
     uint32_t gicd_nsacr[DIV_ROUND_UP(GICV3_MAXIRQ, 16)];
 
+    /*
+     * GICv4.1 extended ID information. This is currently only needed
+     * for migration of a KVM GIC.
+     */
+    uint32_t gicd_typer2;
+
     GICv3CPUState *cpu;
     /* List of all ITSes connected to this GIC */
     GPtrArray *itslist;
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 1cee68193ca..7b09771310e 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -275,6 +275,29 @@ const VMStateDescription vmstate_gicv3_gicd_nmi = {
     }
 };
 
+static bool gicv3_typer2_needed(void *opaque)
+{
+    /*
+     * GICD_TYPER2 ID register for GICv4.1. Was RES0 for
+     * GICv3 and GICv4.0; don't migrate if zero for backwards
+     * compatibility.
+     */
+    GICv3State *cs = opaque;
+
+    return cs->gicd_typer2 != 0;
+}
+
+const VMStateDescription vmstate_gicv3_gicd_typer2 = {
+    .name = "arm_gicv3/gicd_typer2",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = gicv3_typer2_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT32(gicd_typer2, GICv3State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription vmstate_gicv3 = {
     .name = "arm_gicv3",
     .version_id = 1,
@@ -304,6 +327,7 @@ static const VMStateDescription vmstate_gicv3 = {
     .subsections = (const VMStateDescription * const []) {
         &vmstate_gicv3_gicd_no_migration_shift_bug,
         &vmstate_gicv3_gicd_nmi,
+        &vmstate_gicv3_gicd_typer2,
         NULL
     }
 };
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 3be3bf6c28d..8e34d08b415 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -405,7 +405,16 @@ static void kvm_arm_gicv3_put(GICv3State *s)
         }
     }
 
-    /* Distributor state (shared between all CPUs */
+    /* Distributor state (shared between all CPUs) */
+
+    /*
+     * This ID register is restored so that the kernel can fail
+     * the write if the migration source is GICv4.1 but the
+     * destination is not.
+     */
+    reg = s->gicd_typer2;
+    kvm_gicd_access(s, GICD_TYPER2, &reg, true);
+
     reg = s->gicd_statusr[GICV3_NS];
     kvm_gicd_access(s, GICD_STATUSR, &reg, true);
 
@@ -572,7 +581,10 @@ static void kvm_arm_gicv3_get(GICv3State *s)
         }
     }
 
-    /* Distributor state (shared between all CPUs */
+    /* Distributor state (shared between all CPUs) */
+
+    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
+    s->gicd_typer2 = reg;
 
     kvm_gicd_access(s, GICD_STATUSR, &reg, false);
     s->gicd_statusr[GICV3_NS] = reg;
@@ -707,6 +719,7 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
 {
     GICv3State *s = ARM_GICV3_COMMON(obj);
     KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
+    uint32_t reg;
 
     DPRINTF("Reset\n");
 
@@ -719,6 +732,14 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
         return;
     }
 
+    /*
+     * The reset value of the GICD_TYPER2 ID register should be whatever
+     * the kernel says it is; otherwise the attempt to put the value
+     * in kvm_arm_gicv3_put() will fail.
+     */
+    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
+    s->gicd_typer2 = reg;
+
     kvm_arm_gicv3_put(s);
 }
 
-- 
2.43.0

[PATCH v2 3/3] hw/intc/arm_gicv3_kvm: Drop DPRINTF macro

[Peter Maydell]

We don't generally like DPRINTF debug macros, preferring tracepoints.
In this case the macro is used in only three places (reset, realize,
and in the unlikely event the host kernel doesn't have GICv3 register
access support). These don't seem worth converting to tracepoints,
so simply delete the macro and its uses.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
If anybody needs to debug this code they can add useful tracepoints
at that point. I don't think these DPRINTFs will help at all.
---
 hw/intc/arm_gicv3_kvm.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 8e34d08b415..e118c1b1f04 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -34,14 +34,6 @@
 #include "target/arm/cpregs.h"
 
 
-#ifdef DEBUG_GICV3_KVM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, "kvm_gicv3: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
 #define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3"
 typedef struct KVMARMGICv3Class KVMARMGICv3Class;
 /* This is reusing the GICv3State typedef from ARM_GICV3_ITS_COMMON */
@@ -721,14 +713,11 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
     KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
     uint32_t reg;
 
-    DPRINTF("Reset\n");
-
     if (kgc->parent_phases.hold) {
         kgc->parent_phases.hold(obj, type);
     }
 
     if (s->migration_blocker) {
-        DPRINTF("Cannot put kernel gic state, no kernel interface\n");
         return;
     }
 
@@ -807,8 +796,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
     Error *local_err = NULL;
     int i;
 
-    DPRINTF("kvm_arm_gicv3_realize\n");
-
     kgc->parent_realize(dev, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-- 
2.43.0

Re: [PATCH v2 2/3] hw/intc/arm_gicv3_kvm: Migrate GICD_TYPER2

[Eric Auger]

Hi Peter,

On 7/8/25 1:52 PM, Peter Maydell wrote:
> The GICD_TYPER2 register is new for GICv4.1.  As an ID register, we
> migrate it so that on the destination the kernel can check that the
> destination supports the same configuration that the source system
> had.  This avoids confusing behaviour if the user tries to migrate a
> VM from a GICv3 system to a GICv4.1 system or vice-versa.  (In
> practice the inability to migrate between different CPU types
> probably already prevented this.)
>
> Note that older kernels pre-dating GICv4.1 support in the KVM GIC
> will just ignore whatever userspace writes to GICD_TYPER2, so
> migration where the destination is one of those kernels won't have
> this safety-check.
>
> (The reporting of a mismatch will be a bit clunky, because this
> file currently uses error_abort for all failures to write the
> data to the kernel. Ideally we would fix this by making them
> propagate the failure information back up to the post_load hook
> return value, to cause a migration failure.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Looks good to me
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> v1->v2:
>  * fix comment missing bracket
>  * fix reset handling so this works on GICv4.1 hosts
>  * move get/put code to be with the other GICD regs
> ---
>  include/hw/intc/arm_gicv3_common.h |  6 ++++++
>  hw/intc/arm_gicv3_common.c         | 24 ++++++++++++++++++++++++
>  hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++--
>  3 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index a3d6a0e5077..bcbcae1fbe7 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -267,6 +267,12 @@ struct GICv3State {
>      GICv3CPUState *gicd_irouter_target[GICV3_MAXIRQ];
>      uint32_t gicd_nsacr[DIV_ROUND_UP(GICV3_MAXIRQ, 16)];
>  
> +    /*
> +     * GICv4.1 extended ID information. This is currently only needed
> +     * for migration of a KVM GIC.
> +     */
> +    uint32_t gicd_typer2;
> +
>      GICv3CPUState *cpu;
>      /* List of all ITSes connected to this GIC */
>      GPtrArray *itslist;
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 1cee68193ca..7b09771310e 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -275,6 +275,29 @@ const VMStateDescription vmstate_gicv3_gicd_nmi = {
>      }
>  };
>  
> +static bool gicv3_typer2_needed(void *opaque)
> +{
> +    /*
> +     * GICD_TYPER2 ID register for GICv4.1. Was RES0 for
> +     * GICv3 and GICv4.0; don't migrate if zero for backwards
> +     * compatibility.
> +     */
> +    GICv3State *cs = opaque;
> +
> +    return cs->gicd_typer2 != 0;
> +}
> +
> +const VMStateDescription vmstate_gicv3_gicd_typer2 = {
> +    .name = "arm_gicv3/gicd_typer2",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = gicv3_typer2_needed,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINT32(gicd_typer2, GICv3State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3 = {
>      .name = "arm_gicv3",
>      .version_id = 1,
> @@ -304,6 +327,7 @@ static const VMStateDescription vmstate_gicv3 = {
>      .subsections = (const VMStateDescription * const []) {
>          &vmstate_gicv3_gicd_no_migration_shift_bug,
>          &vmstate_gicv3_gicd_nmi,
> +        &vmstate_gicv3_gicd_typer2,
>          NULL
>      }
>  };
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 3be3bf6c28d..8e34d08b415 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -405,7 +405,16 @@ static void kvm_arm_gicv3_put(GICv3State *s)
>          }
>      }
>  
> -    /* Distributor state (shared between all CPUs */
> +    /* Distributor state (shared between all CPUs) */
> +
> +    /*
> +     * This ID register is restored so that the kernel can fail
> +     * the write if the migration source is GICv4.1 but the
> +     * destination is not.
> +     */
> +    reg = s->gicd_typer2;
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, true);
> +
>      reg = s->gicd_statusr[GICV3_NS];
>      kvm_gicd_access(s, GICD_STATUSR, &reg, true);
>  
> @@ -572,7 +581,10 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>          }
>      }
>  
> -    /* Distributor state (shared between all CPUs */
> +    /* Distributor state (shared between all CPUs) */
> +
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
> +    s->gicd_typer2 = reg;
>  
>      kvm_gicd_access(s, GICD_STATUSR, &reg, false);
>      s->gicd_statusr[GICV3_NS] = reg;
> @@ -707,6 +719,7 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
>  {
>      GICv3State *s = ARM_GICV3_COMMON(obj);
>      KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> +    uint32_t reg;
>  
>      DPRINTF("Reset\n");
>  
> @@ -719,6 +732,14 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
>          return;
>      }
>  
> +    /*
> +     * The reset value of the GICD_TYPER2 ID register should be whatever
> +     * the kernel says it is; otherwise the attempt to put the value
> +     * in kvm_arm_gicv3_put() will fail.
> +     */
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
> +    s->gicd_typer2 = reg;
> +
>      kvm_arm_gicv3_put(s);
>  }
>  

Re: [PATCH v2 3/3] hw/intc/arm_gicv3_kvm: Drop DPRINTF macro

[Eric Auger]

On 7/8/25 1:52 PM, Peter Maydell wrote:
> We don't generally like DPRINTF debug macros, preferring tracepoints.
> In this case the macro is used in only three places (reset, realize,
> and in the unlikely event the host kernel doesn't have GICv3 register
> access support). These don't seem worth converting to tracepoints,
> so simply delete the macro and its uses.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
> If anybody needs to debug this code they can add useful tracepoints
> at that point. I don't think these DPRINTFs will help at all.
> ---
>  hw/intc/arm_gicv3_kvm.c | 13 -------------
>  1 file changed, 13 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 8e34d08b415..e118c1b1f04 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -34,14 +34,6 @@
>  #include "target/arm/cpregs.h"
>  
>  
> -#ifdef DEBUG_GICV3_KVM
> -#define DPRINTF(fmt, ...) \
> -    do { fprintf(stderr, "kvm_gicv3: " fmt, ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) \
> -    do { } while (0)
> -#endif
> -
>  #define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3"
>  typedef struct KVMARMGICv3Class KVMARMGICv3Class;
>  /* This is reusing the GICv3State typedef from ARM_GICV3_ITS_COMMON */
> @@ -721,14 +713,11 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
>      KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
>      uint32_t reg;
>  
> -    DPRINTF("Reset\n");
> -
>      if (kgc->parent_phases.hold) {
>          kgc->parent_phases.hold(obj, type);
>      }
>  
>      if (s->migration_blocker) {
> -        DPRINTF("Cannot put kernel gic state, no kernel interface\n");
>          return;
>      }
>  
> @@ -807,8 +796,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
>      Error *local_err = NULL;
>      int i;
>  
> -    DPRINTF("kvm_arm_gicv3_realize\n");
> -
>      kgc->parent_realize(dev, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);

Re: [PATCH v2 2/3] hw/intc/arm_gicv3_kvm: Migrate GICD_TYPER2

[Eric Auger]

Hi Peter,

On 7/8/25 1:52 PM, Peter Maydell wrote:
> The GICD_TYPER2 register is new for GICv4.1.  As an ID register, we
> migrate it so that on the destination the kernel can check that the
> destination supports the same configuration that the source system
> had.  This avoids confusing behaviour if the user tries to migrate a
> VM from a GICv3 system to a GICv4.1 system or vice-versa.  (In
> practice the inability to migrate between different CPU types
> probably already prevented this.)
>
> Note that older kernels pre-dating GICv4.1 support in the KVM GIC
> will just ignore whatever userspace writes to GICD_TYPER2, so
> migration where the destination is one of those kernels won't have
> this safety-check.
>
> (The reporting of a mismatch will be a bit clunky, because this
> file currently uses error_abort for all failures to write the
> data to the kernel. Ideally we would fix this by making them
> propagate the failure information back up to the post_load hook
> return value, to cause a migration failure.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Looks good to me
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> v1->v2:
>  * fix comment missing bracket
>  * fix reset handling so this works on GICv4.1 hosts
>  * move get/put code to be with the other GICD regs
> ---
>  include/hw/intc/arm_gicv3_common.h |  6 ++++++
>  hw/intc/arm_gicv3_common.c         | 24 ++++++++++++++++++++++++
>  hw/intc/arm_gicv3_kvm.c            | 25 +++++++++++++++++++++++--
>  3 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index a3d6a0e5077..bcbcae1fbe7 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -267,6 +267,12 @@ struct GICv3State {
>      GICv3CPUState *gicd_irouter_target[GICV3_MAXIRQ];
>      uint32_t gicd_nsacr[DIV_ROUND_UP(GICV3_MAXIRQ, 16)];
>  
> +    /*
> +     * GICv4.1 extended ID information. This is currently only needed
> +     * for migration of a KVM GIC.
> +     */
> +    uint32_t gicd_typer2;
> +
>      GICv3CPUState *cpu;
>      /* List of all ITSes connected to this GIC */
>      GPtrArray *itslist;
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 1cee68193ca..7b09771310e 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -275,6 +275,29 @@ const VMStateDescription vmstate_gicv3_gicd_nmi = {
>      }
>  };
>  
> +static bool gicv3_typer2_needed(void *opaque)
> +{
> +    /*
> +     * GICD_TYPER2 ID register for GICv4.1. Was RES0 for
> +     * GICv3 and GICv4.0; don't migrate if zero for backwards
> +     * compatibility.
> +     */
> +    GICv3State *cs = opaque;
> +
> +    return cs->gicd_typer2 != 0;
> +}
> +
> +const VMStateDescription vmstate_gicv3_gicd_typer2 = {
> +    .name = "arm_gicv3/gicd_typer2",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = gicv3_typer2_needed,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINT32(gicd_typer2, GICv3State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3 = {
>      .name = "arm_gicv3",
>      .version_id = 1,
> @@ -304,6 +327,7 @@ static const VMStateDescription vmstate_gicv3 = {
>      .subsections = (const VMStateDescription * const []) {
>          &vmstate_gicv3_gicd_no_migration_shift_bug,
>          &vmstate_gicv3_gicd_nmi,
> +        &vmstate_gicv3_gicd_typer2,
>          NULL
>      }
>  };
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 3be3bf6c28d..8e34d08b415 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -405,7 +405,16 @@ static void kvm_arm_gicv3_put(GICv3State *s)
>          }
>      }
>  
> -    /* Distributor state (shared between all CPUs */
> +    /* Distributor state (shared between all CPUs) */
> +
> +    /*
> +     * This ID register is restored so that the kernel can fail
> +     * the write if the migration source is GICv4.1 but the
> +     * destination is not.
> +     */
> +    reg = s->gicd_typer2;
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, true);
> +
>      reg = s->gicd_statusr[GICV3_NS];
>      kvm_gicd_access(s, GICD_STATUSR, &reg, true);
>  
> @@ -572,7 +581,10 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>          }
>      }
>  
> -    /* Distributor state (shared between all CPUs */
> +    /* Distributor state (shared between all CPUs) */
> +
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
> +    s->gicd_typer2 = reg;
>  
>      kvm_gicd_access(s, GICD_STATUSR, &reg, false);
>      s->gicd_statusr[GICV3_NS] = reg;
> @@ -707,6 +719,7 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
>  {
>      GICv3State *s = ARM_GICV3_COMMON(obj);
>      KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
> +    uint32_t reg;
>  
>      DPRINTF("Reset\n");
>  
> @@ -719,6 +732,14 @@ static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type)
>          return;
>      }
>  
> +    /*
> +     * The reset value of the GICD_TYPER2 ID register should be whatever
> +     * the kernel says it is; otherwise the attempt to put the value
> +     * in kvm_arm_gicv3_put() will fail.
> +     */
> +    kvm_gicd_access(s, GICD_TYPER2, &reg, false);
> +    s->gicd_typer2 = reg;
> +
>      kvm_arm_gicv3_put(s);
>  }
>  

Re: [PATCH v2 1/3] hw/intc/arm_gicv3_dist: Implement GICD_TYPER2 as 0

[Richard Henderson]

On 7/8/25 05:52, Peter Maydell wrote:
> The GIC distributor registers GICD_TYPER2 is present when the
> GICv4.1 is implemented, and RES0 otherwise. QEMU's TCG implementation
> is only GICv4.0, so this register is RES0. However, since it's
> reasonable for GICv4.1-aware software to read the register, expecting
> the zero for GICv3 and GICv4.0, implement the case to avoid it being
> logged as an invalid guest read.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> ---
>   hw/intc/gicv3_internal.h | 1 +
>   hw/intc/arm_gicv3_dist.c | 9 +++++++++
>   2 files changed, 10 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 3/3] hw/intc/arm_gicv3_kvm: Drop DPRINTF macro

[Richard Henderson]

On 7/8/25 05:52, Peter Maydell wrote:
> We don't generally like DPRINTF debug macros, preferring tracepoints.
> In this case the macro is used in only three places (reset, realize,
> and in the unlikely event the host kernel doesn't have GICv3 register
> access support). These don't seem worth converting to tracepoints,
> so simply delete the macro and its uses.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> If anybody needs to debug this code they can add useful tracepoints
> at that point. I don't think these DPRINTFs will help at all.
> ---
>   hw/intc/arm_gicv3_kvm.c | 13 -------------
>   1 file changed, 13 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~