On Tue, Feb 26, 2019 at 01:53:52PM +0000, Dave Martin wrote: > On Wed, Feb 20, 2019 at 07:41:43PM +0000, Marc Zyngier wrote: > > On Wed, 20 Feb 2019 19:14:53 +0000 > > Dave Martin <Dave.Martin@xxxxxxx> wrote: [...] > > > Perhaps an alternative approach would be to nobble the preempt notifier > > > and stick an explicit vcpu_put()...vcpu_load() around the > > > swait_event_interruptible_exclusive() call in vcpu_req_sleep(). This > > > is not fast path. > > > > What does it buy us? The problem we're solving here is a powered-off, > > spuriously woken-up vcpu racing against the reset performed from > > another vcpu. I don't see what adding more put/load would solve. > > Not a lot. Changes in this area would allow some code to move outside > preempt_disable(), but only at the expense of extra cost elsewhere... > > > > Any, with the code as-is, it looks like the SVE regs resetting should > > > go in the preempt_disable() region, after the kvm_arch_vcpu_put() call. > > > > All resets must go there. This is the only safe location. > > OK, that's what I wanted to be sure of. Hmmm, it turns out this is no good because we may need to kzalloc() the vcpu->arch.sve_state storage here in the new-vcpu case. Rather than create additional mess in this code, I will move that allocation to be done more eagerly rather than deferred until the last possible moment. With the current API flow, this may mean krealloc()'ing the buffer if the set of vector lengths for the vcpu is modified from the default before the vcpu is run, via a user write to KVM_REG_ARM64_SVE_VLS -- though we would anyway get away with not doing it, since the default buffer size will be the maximum that the hardware could possibly require anyway. We could waste a few K per vcpu, but that's almost not worth caring about. I'd rather not churn the API at this point, but I'll experiment to see which approach under the hood looks cleanest. Thoughts? Cheers ---Dave
