Re: [Xen-devel] [PATCH] x86/upcall: inject a spurious event after setting upcall vector

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 12:37, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Jan 04, 2018 at 03:53:39AM -0700, Jan Beulich wrote:
>> >>> On 04.01.18 at 10:13, <roger.pau@xxxxxxxxxx> wrote:
>> > On Tue, Jan 02, 2018 at 09:47:40AM -0700, Jan Beulich wrote:
>> >> >>> On 28.12.17 at 13:57, <roger.pau@xxxxxxxxxx> wrote:
>> >> > In case the vCPU has pending events to inject. This fixes a bug that
>> >> > happened if the guest mapped the vcpu info area using
>> >> > VCPUOP_register_vcpu_info without having setup the event channel
>> >> > upcall, and then setup the upcall vector.
>> >> > 
>> >> > In this scenario the guest would not receive any upcalls, because the
>> >> > call to VCPUOP_register_vcpu_info would have marked the vCPU as having
>> >> > pending events, but the vector could not be injected because it was
>> >> > not yet setup.
>> >> > 
>> >> > This has not caused issues so far because all the consumers first
>> >> > setup the vector callback and then map the vcpu info page, but there's
>> >> > no limitation that prevents doing it in the inverse order.
>> >> 
>> >> Hmm, yes, okay, I can see that we may indeed want to do this for
>> >> symmetry reasons. There is a small theoretical risk of this causing
>> >> races, though, for not entirely well written guest drivers.
>> > 
>> > Not exactly. In the scenario described above not injecting this event
>> > will cause further events to not be injected also. This is because
>> > VCPUOP_register_vcpu_info sets evtchn_upcall_pending and tries to
>> > inject an event using arch_evtchn_inject. If the vector is not set at
>> > this point, arch_evtchn_inject will do nothing, but
>> > evtchn_upcall_pending will be left set.
>> > 
>> > Further calls to vcpu_mark_events_pending will not call into
>> > hvm_assert_evtchn_irq because the pending bit is already set, thus
>> > preventing the delivery of any event channel interrupts.
>> 
>> I understand that, but I don't understand how this relates to my
>> comment.
> 
> I don't think this applies to "not entirely well written drivers". I
> think it's perfectly fine for a guest to map the vcpu_info page first
> and then setup the vector callback. With the current code doing this
> will result in no interrupts being injected.

My meaning of "not entirely well written drivers" is unrelated to
the scenario you want to fix: When setting the upcall vector, an
event would now be received at a time that the drivers may not
expect (iirc there's no requirement to relocate the vcpu info out
of the shared info page, and even if there was the issue described
would still arise if the vector was changed later on).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel