Re: [PATCH v2 2/4] KVM: arm64: pkvm: Fix missing reset of GICv3 LRs

[Vincent Donnefort <vdonnefort@google.com>]

On Thu, Oct 22, 2026 at 06:36:04PM +0100, Fuad Tabba wrote:
> Hi Vincent,
> 
> On Thu, 22 Oct 2026 15:00:46 +0100, Vincent Donnefort <vdonnefort@google.com> wrote:
> > The vGIC LRs are architecturally UNKNOWN when the CPU boots or resumes.
> > However, in protected mode, only the used ones are reset, because the
> > HVC __vgic_v3_init_lrs() is never called:
> 
> Could you say here what happens to a guest when an LR isn't reset, as I
> asked on v1? Without this series, on QEMU changed to leave a pending
> vINTID 100 in the top LR at CPU reset, the guest warns "Unexpected
> interrupt (irqnr 100)". With only the EOI bit set there, the npVM
> doesn't print anything in 150 seconds, since the maintenance interrupt
> is asserted on every entry. With that in the message, could you also
> bring back the Cc: stable you had on v1?

Sorry I missed that.

Actually I don't know if this should be backported. I believe it was working
until now because all SoCs do reset their LRs even though they don't have to do
it.

-- 
Vincent

> 
> [...]
> > Then, cpu_hyp_init() is only called from init_subsystems and
> > kvm_arch_enable_virtualization_cpu(). Remove the kvm_hyp_initialized
> > gate: when pKVM is enabled, the cpu_hyp_reset() will just return an
> > error while for other modes, we already know kvm_hyp_initialized is
> > false at this point.
> 
> In init_subsystems(), only the CPU that ran do_pkvm_init() returns an
> error here. The others are still on the hyp stub, which returns 0 for
> HVC_RESET_VECTORS, so they go through the full init. Could you reword
> this part?
> 
> [...]
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> [...]
> >  static void cpu_hyp_reinit(void)
> >  {
> > -        cpu_hyp_reset();
> > -        cpu_hyp_init_context();
> > -        cpu_hyp_init_features();
> > +        if (!cpu_hyp_reset()) {
> > +                cpu_hyp_init_context();
> > +                cpu_hyp_init_features();
> > +        } else {
> > +                if (WARN_ON(!__this_cpu_read(kvm_hyp_initialized)))
> > +                        return;
> > +        }
> 
> Could you make this "} else if (WARN_ON(...)) {", as elsewhere in
> arm.c?
> 
> > +
> > +        /*
> > +         * Even if the hypervisor didn't allow resetting the vectors we can
> > +         * reset what we have access to.
> > +         */
> 
> nit: "what we have access to" is the LRs, so I'd name them.
> 
> With the above addressed:
> Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
> Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
> 
> Cheers,
> /fuad