[PATCH 05/37] KVM: Record the executing ioctl number on the vcpu struct

[Christoffer Dall]

On Fri, Oct 13, 2017 at 8:38 PM, Radim Krčmář <rkrcmar at redhat.com> wrote:
> 2017-10-13 19:31+0200, Christoffer Dall:
>> On Fri, Oct 13, 2017 at 07:13:07PM +0200, Radim Krčmář wrote:
>> > I think that other (special) callsites of vcpu_load()/vcpu_put() have a
>> > well defined IOCTL that can be used instead of vcpu->ioctl, so we could
>> > just pass the ioctl value all the way to arch code and never save it
>> > anywhere,
>>
>> I don't think that works; what would you do with preempt notifier calls?
>
> Right, BUG :), I didn't consider them before and they need to know.
>
>> One solution is to add a parameter to vcpu_put, lie for vcpu_load, which
>> also sets the ioctl, and other callers than the final vcpu_put in
>> kvm_vcpu_ioctl() just pass the existing value, where the kvm_vcpu_ioctl
>> call can pass 0 which gets set before releasing the mutex.
>>
>> Can you think of a more elegant solution?
>
> Not really, only thought of touching preempt notifiers and it seems to
> be more complicated.
>
> I think we shouldn't restore ioctl on vcpu_put() at all -- the value
> isn't well defined outside of the mutex, so there is no point in looking
> and we can just zero the ioctl.

I agree, that fits with the semantics of vcpu_put() quit nicely actually.

>
> Actually, I wouldn't rely on the existing value at all because that.
> The need for load/put depends on the current code path, not on the one
> we race with.
>
> x86 seems to be the only user of vcpu_load() outside of kvm_vcpu_ioctl()
> and the callers are either under a VM ioctl or under a VM destruction
> paths (invalid IOCTL) and we can just hardcode that.

Yes, we should.  I honestly just wanted feedback on this before
tracing through all call paths to the vcpu_load() calls on the x86
side.

>
> Passing 0 to all other vcpu_load()s and unconditionally zeroing ioctl
> before mutex_unlock() should work.


Agreed.  I'll do that for the next version.

Thanks for having a look!

-Christoffer