Re: [Qemu-devel] [PULL 11/27] arm: Allow system registers for KVM guests to be changed by QEMU code

[Peter Maydell]

On Thu, 21 Feb 2019 at 14:20, Auger Eric <address@hidden> wrote:
> This commit introduces a regression when running with EDK2 FW:

I believe I've now tracked down what was going wrong here.
The problem was with the guest CPU reset path -- in
kvm_arm_reset_vcpu() we copy the kernel's reset state
into the cpreg_indexes/cpreg_values arrays, because the
next thing we do after a reset is a kvm_arch_put_registers()
which will copy from the list values back to the kernel.
But now we've updated kvm_arch_put_registers() to do
a two-step CPUState -> cpreg* arrays -> kernel sync,
we need kvm_arm_reset_vcpu() to also sync cpreg arrays to
the CPUState. Otherwise the first kvm_arch_put_registers()
will write incorrect values back to the kernel, which for
some guests which are more trusting of reset register values
than Linux causes them to fail.

The fix is just this, on top of reverting the revert:

--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -507,6 +507,14 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu)
         fprintf(stderr, "write_kvmstate_to_list failed\n");
         abort();
     }
+    /*
+     * Sync the reset values also into the CPUState. This is necessary
+     * because the next thing we do will be a kvm_arch_put_registers()
+     * which will update the list values from the CPUState before copying
+     * the list values back to KVM. It's OK to ignore failure returns here
+     * for the same reason we do so in kvm_arch_get_registers().
+     */
+    write_list_to_cpustate(cpu);
 }

 /*

I'll send out a proper patch in a bit.

thanks
-- PMM