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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Jan 04, 2018 at 05:10:52AM -0700, Jan Beulich wrote:
> >>> 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).

Sorry, I misunderstood the context of that sentence. Regarding your
point, yes, that's right, drivers should be able to cope with spurious
interrupts. I would be very surprised if an event channel driver is not
able to cope with spurious interrupts.

Roger.

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