Hi Drew, On Tue, 29 Jan 2019 15:48:58 +0000, Andrew Jones <drjones@xxxxxxxxxx> wrote: > > Hi Christoffer, > > On Fri, Jan 25, 2019 at 10:46:52AM +0100, Christoffer Dall wrote: > > Resetting the VCPU state modifies the system register state in memory, > > but this may interact with vcpu_load/vcpu_put if running with preemption > > disabled, which in turn may lead to corrupted system register state. > ^ enabled > > > > Address this by disabling preemption and doing put/load if required > > around the reset logic. > > I'm having trouble understanding how disabling preemption helps here. > There shouldn't be an issue with the KVM_ARM_VCPU_INIT case, since the > target vcpu is guaranteed not to be loaded and therefore it doesn't > have preempt notifiers registered either. Also, KVM_ARM_VCPU_INIT holds > the vcpu mutex, so there's no chance for a load to occur until it's > complete. > > For the PSCI case it makes sense to force a vcpu load after the reset, > otherwise the sleeping target vcpu won't have the correct state loaded. > The initial put also makes sense in case we're not resetting everything. > I don't understand how we're ensuring the target vcpu thread's preemption > is disabled though. This modified kvm_reset_vcpu would need to be run > from the target vcpu thread to work, but that's not how the PSCI code > currently does it. And that's exactly where we're going with the following patch in the series. Ultimately, we need a vcpu to reset itself, as we otherwise have a window where a vcpu can be spuriously loaded whilst being reset. Thanks, M. -- Jazz is not dead, it just smell funny.
