Re: [Qemu-arm] [PATCH for-4.0?] arm: Allow system registers for KVM guests to be changed by QEMU code

[Peter Maydell]

On Mon, 18 Mar 2019 at 15:59, Alex Bennée <address@hidden> wrote:
>
>
> Peter Maydell <address@hidden> writes:
>
> > At the moment the Arm implementations of kvm_arch_{get,put}_registers()
> > don't support having QEMU change the values of system registers
> > (aka coprocessor registers for AArch32). This is because although
> > kvm_arch_get_registers() calls write_list_to_cpustate() to
> > update the CPU state struct fields (so QEMU code can read the
> > values in the usual way), kvm_arch_put_registers() does not
> > call write_cpustate_to_list(), meaning that any changes to
> > the CPU state struct fields will not be passed back to KVM.
> >
> > The rationale for this design is documented in a comment in the
> > AArch32 kvm_arch_put_registers() -- writing the values in the
> > cpregs list into the CPU state struct is "lossy" because the
> > write of a register might not succeed, and so if we blindly
> > copy the CPU state values back again we will incorrectly
> > change register values for the guest. The assumption was that
> > no QEMU code would need to write to the registers.
> >
> > However, when we implemented debug support for KVM guests, we
> > broke that assumption: the code to handle "set the guest up
> > to take a breakpoint exception" does so by updating various
> > guest registers including ESR_EL1.
> >
> > Support this by making kvm_arch_put_registers() synchronize
> > CPU state back into the list. We sync only those registers
> > where the initial write succeeds, which should be sufficient.
> >
> > This commit is the same as commit 823e1b3818f9b10b824ddc which we
> > had to revert in commit 942f99c825fc94c8b1a4, except that the bug
> > which was preventing EDK2 guest firmware running has been fixed:
> > kvm_arm_reset_vcpu() now calls write_list_to_cpustate().
> >
> > Signed-off-by: Peter Maydell <address@hidden>
>
>
>
> > ---
> > Should we try to put this in for rc1? Not sure... Testing
> > definitely appreciated.

> Hmm so running my testcase:
>
>  * gdbstub enabled with an active sw or hw breakpoint
>  * run userspace program in guest:
>    - sw breakpoint works fine
>    - hw breakpoint never triggers because guest segs

Further testing from Alex suggests this is some unrelated
bug or regression (ie not caused by this patch), but:
since the only in-tree use for this patch is to get nested
debugging working and it would be broken for this other
reason even with this patch, I'm going to postpone applying
this patch until the start of the 4.1 cycle.

thanks
-- PMM