[PATCH 1/2] hw/intc/arm_gicv3_kvm: Remove writes to ICPENDR registers

Zenghui Yu posted 2 patches 6 months, 1 week ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
[PATCH 1/2] hw/intc/arm_gicv3_kvm: Remove writes to ICPENDR registers
Posted by Zenghui Yu 6 months, 1 week ago
As per the arm-vgic-v3 kernel doc [1]:

    Accesses to GICD_ICPENDR register region and GICR_ICPENDR0 registers
    have RAZ/WI semantics, meaning that reads always return 0 and writes
    are always ignored.

Remove the useless writes to ICPENDR registers in kvm_arm_gicv3_put().

[1] https://docs.kernel.org/virt/kvm/devices/arm-vgic-v3.html

Signed-off-by: Zenghui Yu <zenghui.yu@linux.dev>
---
 hw/intc/arm_gicv3_kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 8ed88e7429..f798a6e28c 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -387,8 +387,6 @@ static void kvm_arm_gicv3_put(GICv3State *s)
         reg = c->level;
         kvm_gic_line_level_access(s, 0, ncpu, &reg, true);
 
-        reg = ~0;
-        kvm_gicr_access(s, GICR_ICPENDR0, ncpu, &reg, true);
         reg = c->gicr_ipendr0;
         kvm_gicr_access(s, GICR_ISPENDR0, ncpu, &reg, true);
 
@@ -445,7 +443,7 @@ static void kvm_arm_gicv3_put(GICv3State *s)
     kvm_gic_put_line_level_bmp(s, s->level);
 
     /* s->pending bitmap -> GICD_ISPENDRn */
-    kvm_dist_putbmp(s, GICD_ISPENDR, GICD_ICPENDR, s->pending);
+    kvm_dist_putbmp(s, GICD_ISPENDR, 0, s->pending);
 
     /* s->active bitmap -> GICD_ISACTIVERn */
     kvm_dist_putbmp(s, GICD_ISACTIVER, GICD_ICACTIVER, s->active);
-- 
2.34.1
Re: [PATCH 1/2] hw/intc/arm_gicv3_kvm: Remove writes to ICPENDR registers
Posted by Peter Maydell 6 months, 1 week ago
On Tue, 29 Jul 2025 at 17:17, Zenghui Yu <zenghui.yu@linux.dev> wrote:
>
> As per the arm-vgic-v3 kernel doc [1]:
>
>     Accesses to GICD_ICPENDR register region and GICR_ICPENDR0 registers
>     have RAZ/WI semantics, meaning that reads always return 0 and writes
>     are always ignored.
>
> Remove the useless writes to ICPENDR registers in kvm_arm_gicv3_put().
>
> [1] https://docs.kernel.org/virt/kvm/devices/arm-vgic-v3.html

The kernel doesn't implement any state behind these
registers today, but that doesn't inherently mean it
will never do so or that it never did do so.
Since we have the state fields in the GICv3State struct (for the
benefit of TCG) and the kernel defines the accessors for it,
I prefer to leave this code in QEMU, and leave it up to the
kernel whether it provides any state behind them.

thanks
-- PMM
Re: [PATCH 1/2] hw/intc/arm_gicv3_kvm: Remove writes to ICPENDR registers
Posted by Peter Maydell 6 months, 1 week ago
On Thu, 31 Jul 2025 at 17:42, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 29 Jul 2025 at 17:17, Zenghui Yu <zenghui.yu@linux.dev> wrote:
> >
> > As per the arm-vgic-v3 kernel doc [1]:
> >
> >     Accesses to GICD_ICPENDR register region and GICR_ICPENDR0 registers
> >     have RAZ/WI semantics, meaning that reads always return 0 and writes
> >     are always ignored.
> >
> > Remove the useless writes to ICPENDR registers in kvm_arm_gicv3_put().
> >
> > [1] https://docs.kernel.org/virt/kvm/devices/arm-vgic-v3.html
>
> The kernel doesn't implement any state behind these
> registers today, but that doesn't inherently mean it
> will never do so or that it never did do so.
> Since we have the state fields in the GICv3State struct (for the
> benefit of TCG) and the kernel defines the accessors for it,
> I prefer to leave this code in QEMU, and leave it up to the
> kernel whether it provides any state behind them.

Ah, having read the second patch I realise I misunderstood
this one. Yes, the ISPENDR registers are special because
the kernel implements them as "just write the state" rather
than with "first clear them via the C register and then write
the set bits via the S register". So this is correct.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM