On Wed, 13 Feb 2019 at 16:29, Alex Bennée <[email protected]> wrote: > > > Peter Maydell <[email protected]> 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
