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

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

> From: Salil Mehta
> Sent: Tuesday, October 14, 2025 4:33 PM
> To: 'Peter Maydell' <peter.maydell@linaro.org>
> 
> > From: Peter Maydell <peter.maydell@linaro.org>
> > Sent: Tuesday, October 14, 2025 4:24 PM
> > To: Salil Mehta <salil.mehta@huawei.com>
> >
> > On Tue, 14 Oct 2025 at 16:13, Salil Mehta <salil.mehta@huawei.com>
> wrote:
> > >
> > > > From: Peter Maydell <peter.maydell@linaro.org> In what situation
> > > > do we ever start running a VCPU before the *GIC* has been
> > > > realized? The GIC should get realized as part of creating the virt
> > > > board, which must complete before we do anything like running a vcpu.
> > >
> > >
> > > Just after realization of vCPU in the machvirt_init() you can see
> > > the default power_state is PSCI CPU_ON, which means
> > KVM_MP_STATE_RUNNABLE.
> > > Since, the thread is up and not doing IO wait in userspace it gets
> > > into
> > > cpu_exec() loop and actually run KVM_RUN IOCTL. Inside the KVM it
> > > momentarily takes the vCPU mutex but later exit and releases. This
> > > keeps going on for all of the vCPU threads realized early.
> >
> > Yikes. We definitely should fix that : letting the vcpu run before we
> > get to
> > qemu_machine_creation_done() seems like it would be a massive source
> > of race conditions.
> 
> I've already proposed fix for this by parking such threads in userspace. Please
> check functions virt_(un)park_cpu_in_userspace(). But need to check if we
> can use this trick can be used at the very early stages of the VM initialization.
> 
> https://lore.kernel.org/qemu-devel/20251001010127.3092631-18-
> salil.mehta@opnsrc.net/
> 
> 
> I also think we need 1:1 mapping between the ARMCPU power-state and the
> KVM MP_STATE.  Right now,
> 
> KVM_MP_STATE_RUNNABLE = {PSCI CPU_OFF, PSCI CPU_ON}
> KVM_MP_STATE_STOPPED    = {PSCI CPU_OFF}
> 
> KVM MP State RUNNABLE seems ambiguous. Should we use PSCI
> CPU_PENDING at early stages instead?

There is one more issue.

/*
 * Update KVM's MP_STATE based on what QEMU thinks it is
 */
static int kvm_arm_sync_mpstate_to_kvm(ARMCPU *cpu)
{
    if (cap_has_mp_state) {
        struct kvm_mp_state mp_state = {
            .mp_state = (cpu->power_state == PSCI_OFF) ?
            KVM_MP_STATE_STOPPED : KVM_MP_STATE_RUNNABLE
        };
        return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, &mp_state);
    }
    return 0;
}


value of PSCI_OFF = 1 and we do not initialize the default state of power_state.
This means KVM_MP_STATE state will be configured as RUNNABLE at the first
instance till the cpu_reset() happens. This is not correct either.

But then what should be the vCPUs default state, KVM_MP_STATE _STOPPED?
Stopped is implemented as sleep inside the KVM - a blocking call.  An invitation
to the vCPU lock contention? or should be sleep in userspace?

static void kvm_vcpu_sleep(struct kvm_vcpu *vcpu)
{
	struct rcuwait *wait = kvm_arch_vcpu_get_wait(vcpu);

	rcuwait_wait_event(wait,
			   (!kvm_arm_vcpu_stopped(vcpu)) && (!vcpu->arch.pause),
			   TASK_INTERRUPTIBLE);

	if (kvm_arm_vcpu_stopped(vcpu) || vcpu->arch.pause) {
		/* Awaken to handle a signal, request we sleep again later. */
		kvm_make_request(KVM_REQ_SLEEP, vcpu);
	}

	/*
	 * Make sure we will observe a potential reset request if we've
	 * observed a change to the power state. Pairs with the smp_wmb() in
	 * kvm_psci_vcpu_on().
	 */
	smp_rmb();
}