[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH RFC V3 25/29] target/arm/kvm: Write CPU state back to KVM on
From: |
Salil Mehta |
Subject: |
RE: [PATCH RFC V3 25/29] target/arm/kvm: Write CPU state back to KVM on reset |
Date: |
Thu, 4 Jul 2024 12:27:20 +0000 |
Hi Nick,
> From: Nicholas Piggin <npiggin@gmail.com>
> Sent: Thursday, July 4, 2024 4:28 AM
> To: Salil Mehta <salil.mehta@huawei.com>; qemu-devel@nongnu.org;
> qemu-arm@nongnu.org; mst@redhat.com
>
> On Fri Jun 14, 2024 at 9:36 AM AEST, Salil Mehta wrote:
> > From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> >
> > When a KVM vCPU is reset following a PSCI CPU_ON call, its power state
> > is not synchronized with KVM at the moment. Because the vCPU is not
> > marked dirty, we miss the call to kvm_arch_put_registers() that writes
> > to KVM's MP_STATE. Force mp_state synchronization.
>
> Hmm. Is this a bug fix for upstream? arm does respond to CPU_ON calls by
> the look, but maybe it's not doing KVM parking until your series?
Yes, this is required we now park and un-park the vCPUs. We must ensure the
KVM resets the KVM VCPU state as well. Hence, not a fix but a change which
is required in context to this patch-set.
> Maybe just a slight change to say "When KVM parking is implemented for
> ARM..." if so.
Sure.
>
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > target/arm/kvm.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/target/arm/kvm.c b/target/arm/kvm.c index
> > 1121771c4a..7acd83ce64 100644
> > --- a/target/arm/kvm.c
> > +++ b/target/arm/kvm.c
> > @@ -980,6 +980,7 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu)
> void
> > kvm_arm_reset_vcpu(ARMCPU *cpu) {
> > int ret;
> > + CPUState *cs = CPU(cpu);
> >
> > /* Re-init VCPU so that all registers are set to
> > * their respective reset values.
> > @@ -1001,6 +1002,12 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu)
> > * for the same reason we do so in kvm_arch_get_registers().
> > */
> > write_list_to_cpustate(cpu);
> > +
> > + /*
> > + * Ensure we call kvm_arch_put_registers(). The vCPU isn't marked
> dirty if
> > + * it was parked in KVM and is now booting from a PSCI CPU_ON call.
> > + */
> > + cs->vcpu_dirty = true;
> > }
> >
> > void kvm_arm_create_host_vcpu(ARMCPU *cpu)
>
> Also above my pay grade, but arm_set_cpu_on_async_work() which seems
> to be what calls the CPU reset you refer to does a bunch of CPU register and
> state setting including the power state setting that you mention.
> Would the vcpu_dirty be better to go there?
Maybe we can. Let me cross verify this.
Thanks
Salil.
>
> Thanks,
> Nick