RE: [PATCH] hw/intc/arm_gicv3_kvm: Avoid reading ICC_CTLR_EL1 from kernel in cpuif reset

[Salil Mehta via <qemu-devel@nongnu.org>]

Hi Peter,

> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Tuesday, October 14, 2025 3:29 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> 
> On Tue, 14 Oct 2025 at 15:22, Salil Mehta <salil.mehta@huawei.com> wrote:
> >
> > Hi Peter,
> >
> > > From: Peter Maydell <peter.maydell@linaro.org>
> > > Sent: Tuesday, October 14, 2025 2:50 PM
> > > To: Salil Mehta <salil.mehta@huawei.com>
> > >
> > > On Tue, 14 Oct 2025 at 14:41, Salil Mehta <salil.mehta@huawei.com>
> wrote:
> > > > I thought you asked me to validate the fix by replacing below:
> > > >
> > > > https://lore.kernel.org/qemu-devel/20251001010127.3092631-22-salil
> > > > .meh
> > > > ta@opnsrc.net/
> > > >
> > > >
> > > > Yes, I'm using the recent RFC V6 vCPU Hotplug patches branch I've
> > > > pushed to the community.
> > > >
> > > > https://lore.kernel.org/qemu-devel/20251001010127.3092631-1-salil.
> > > > meht
> > > > a@opnsrc.net/
> > >
> > > That's the one with the "lazy realize" hack, right? I imagine what's
> > > happening is that we realize the GIC, and the code in this patch
> > > assumes that all the CPUs are already realized at that point. When
> > > we try to get the register value for a not-yet-realized CPU the kernel
> complains.
> >
> >
> > Even if we realize all of the vCPUs the problem will not go away. This
> > problem is happening because we have recently started to Exit Hypercalls
> to userspace.
> > This means we are now accessing the system register in a non-atomic
> context.
> 
> The point of this patch is that it moves the read of ICC_CTLR_EL1 out of the
> reset path and into the GIC realize method, at which point no vCPUs should
> have started running. But it does assume that you don't have half-created
> VCPUs connected to the GIC.


This Is not true. Actually, inner cpu_exec() (in kvm-all..c)  loop keeps on dipping
into the KVM_RUN IOCTL and exiting back with INTR continuously as the realized
vCPUs are in RUNNABLE state initially. The actual "start-powered-off" policy only
gets applied after first system-reset happens.

Hence, this is the state of *transient* lock contention within the KVM and is 
probabilistic. It also explains why it does not happens always.

If we increase the number of realized vCPU threads, the probability of this
transient lock contention becomes even higher and you will tend to see this
condition most of the times.



> 
> > The observation you are seeing has got nothing to do with lazy realization.
> > The problem happens even after threads are realized and then we try to
> > access the ICC_CTLR_EL1 register during cpu_reset()
> 
> With this patch, we should not be accessing ICC_CTLR_EL1 during CPU reset.
> The backtrace you posted does not have CPU reset in it, so whatever is going
> wrong there must be something else.

Yes, but its crashing in the realization of the GIC i.e. in context of machvirt_init()
First reset of the vCPUs happens much later than this. Hence, the reason of this
contention is different than the one you are trying to solve using this patch.
We can get -EBUSY at many different paths but the reason is always vCPU lock
contention. It could be transient or because guest are actually running inside
the KVM_RUN IOCTL (which means vCPU mutex will be held - a more permanent
condition)

Many thanks!

Best regards
Salil.