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

[Peter Maydell]

On Wed, 13 Feb 2019 at 16:29, 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.

> [snip a long test setup that I didn't really understand]

I'm confused -- I'm not sure if the things you're saying
didn't work:
 (a) didn't work before this patch and still don't
 (b) used to work and are broken by this patch
 (c) used to not work and are fixed by this patch

More generally:
 * this patch claims to fix the code path where QEMU sets
   up the guest to take a breakpoint exception (specifically
   making our change of ESR_EL1 actually take effect) -- so does
   it do that? Or is it impossible for that code path in QEMU
   to be used (if so, can we just remove it) ?

thanks
-- PMM