On Thu, Oct 05, 2023, Nicolas Saenz Julienne wrote: > Hi Sean, > Thanks for taking the time to look at this. > > On Wed Oct 4, 2023 at 11:47 PM UTC, Sean Christopherson wrote: > > This does not look remotely safe on multiple fronts. For starters, I don't see > > anything in the .poll() infrastructure that provides serialization, e.g. if there > > are multiple tasks polling then this will be "interesting". > > Would allowing only one poller be acceptable? As a last resort, but I think we should first try to support multiple pollers. > > And there is zero chance this is race-free, e.g. nothing prevents the vCPU task > > itself from changing vcpu->mode from POLLING_FOR_EVENTS to something else. > > > > Why on earth is this mucking with vcpu->mode? Ignoring for the moment that using > > vcpu->requests as the poll source is never going to happen, there's zero reason > > IIUC accessing vcpu->requests in the kvm_vcpu_poll() is out of the > question? Aren't we're forced to do so in order to avoid the race I > mention above. Reading vcpu->requests is fine, though I suspect it will be easier to use a dedicated field. Requests aren't single bit values and most of them are arch specific, which will make it annoying to key off of requests directly. I'm guessing it will be impossible to completely avoid arch specific polling logic, but I'd rather not jump straight to that. > > @@ -285,6 +293,9 @@ static void kvm_make_vcpu_request(struct kvm_vcpu *vcpu, unsigned int req, > > if (cpu != -1 && cpu != current_cpu) > > __cpumask_set_cpu(cpu, tmp); > > } > > + > > + if (kvm_request_is_being_polled(vcpu, req)) > > + wake_up_interruptible(...); > > } > > > > bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req, > > I'll use this approach. > > So since we have to provide a proper uAPI, do you have anything against > having user-space set the polling mask through an ioctl? What exactly do you mean? The mask of what poll "events" userspace cares about? I doubt that will work well if KVM supports more than one poller, as preventing them from stomping over one another would be all but impossible. > Also any suggestions on how kvm_request_to_poll_mask() should look like. For > ex. VSM mostly cares for regular interrupts/timers, so mapping > > KVM_REQ_UNBLOCK, KVM_REQ_HV_STIMER, KVM_REQ_EVENT, KVM_REQ_SMI, > KVM_REQ_NMI > > to a KVM_POLL_INTERRUPTS_FLAG would work. We can then have ad-hoc flags > for async-pf, kvmclock updates, dirty logging, etc... What all does your use case need/want to poll on? Mapping out exactly what all you want/need to poll is probably the best way to answer this question.
