On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost <ehabk...@redhat.com> wrote: > > > > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: > > > Exactly. It appears that there's a bug in our mechanisms, > > > which is why I'm suggesting that the right thing is > > > to fix that bug rather than marking the CPU as halted > > > earlier in the reset process so that the KVM_RUN happens > > > to do nothing... > > > > I agree this is necessary, but it doesn't seem sufficient. > > > > Having cpu_reset() set halted=0 on spapr (and probably other > > machines) is also a bug, as it could still trigger unwanted > > KVM_RUN when cpu_reset() returns (and before machine code sets > > halted=1). > > The Arm handling of starting-halted sets halted=1 within cpu_reset, > based on whether the CPU object was created with a > "start-powered-off" property.
Making this mechanism generic sounds like a good idea. > > I'm not sure in practice that anything can get in asynchronously > and cause a KVM_RUN in between spapr_reset_vcpu() calling > cpu_reset() and it setting cs->halted (and the other stuff), > though. This function ought to be called with the iothread > lock held, so KVM_RUN will only happen if it calls some > other function which incorrectly lets the CPU run. Yeah, maybe it won't happen in practice. It just seems fragile. The same way ppc_cpu_reset() kicked the CPU by accident, code outside cpu_reset() might one day kick the CPU by accident before setting halted=1. -- Eduardo
