Re: [PATCH 2/2] hw/intc/arm_gicv3_kvm: Write all 1's to clear enable/active

[Zenghui Yu <zenghui.yu@linux.dev>]

On 2025/8/1 00:59, Peter Maydell wrote:
> On Tue, 29 Jul 2025 at 17:17, Zenghui Yu <zenghui.yu@linux.dev> wrote:
> >
> > Writing 0 to GICD_IC{ENABLE,ACTIVE}R architecturally has no effect on
> > interrupt status (all writes are simply ignored by KVM) and doesn't comply
> > with the intention of "first write to the clear-reg to clear all bits".
> >
> > Write all 1's to actually clear the enable/active status.
> >
> > Signed-off-by: Zenghui Yu <zenghui.yu@linux.dev>
> > ---
> >  hw/intc/arm_gicv3_kvm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> > index f798a6e28c..6166283cd1 100644
> > --- a/hw/intc/arm_gicv3_kvm.c
> > +++ b/hw/intc/arm_gicv3_kvm.c
> > @@ -295,7 +295,7 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset,
> >           * the 1 bits.
> >           */
> >          if (clroffset != 0) {
> > -            reg = 0;
> > +            reg = ~0;
> >              kvm_gicd_access(s, clroffset, &reg, true);
> >              clroffset += 4;
> >          }
> 
> I guess given what the kernel has implemented that this
> is the correct change, so on that basis
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> I don't understand what the kernel ABI is trying to do here,
> though...
> 
> My expectation for user access for all these registers
> where there's a "set" and a "clear" register pair would
> be that they behave the same way. But looking at the
> implementation, GICD_ICPENDR seems to be implemented
> as "reads zero, writes ignored", whereas GICD_ICACTIVER
> implements the "write-1-to-clear" semantics.
> 
> This seems to match what is documented, but I don't
> understand why we implemented and documented that:
> https://docs.kernel.org/virt/kvm/devices/arm-vgic-v3.html

I think these 2 paragraphs exactly explain the reason:

    "This is identical to the value returned by a guest read from
     ISPENDR for an edge triggered interrupt, but may differ for level

     [...]

     cannot be deduced from purely the line level and the value of the
     ISPENDR registers)."

Does it help?

Thanks,
Zenghui