Re: [Xen-devel] [PATCH v3 5/7] vpci: fix execution of long running operations

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

On Wed, Nov 07, 2018 at 08:06:00AM -0700, Jan Beulich wrote:
> >>> On 07.11.18 at 12:11, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Nov 05, 2018 at 09:56:13AM -0700, Jan Beulich wrote:
> >> >>> On 30.10.18 at 16:41, <roger.pau@xxxxxxxxxx> wrote:
> >> > BAR map/unmap is a long running operation that needs to be preempted
> >> > in order to avoid overrunning the assigned vCPU time (or even
> >> > triggering the watchdog).
> >> > 
> >> > Current logic for this preemption is wrong, and won't work at all for
> >> > AMD since only Intel makes use of hvm_io_pending (and even in that
> >> > case the current code is wrong).
> >> 
> >> I'm having trouble understanding this, both for the AMD aspect
> >> (it is only vvmx.c which has a function call not mirrored on the
> >> AMD side) and for the supposed general brokenness. Without
> >> some clarification I can't judge whether re-implementing via
> >> tasklet is actually the best approach.
> > 
> > hvm_io_pending itself cannot block the vCPU from executing, it's used
> > by nvmx_switch_guest in order to prevent changing the nested VMCS if
> > there's pending IO emulation work AFAICT.
> > 
> > The only way I could find to actually prevent a vCPU from running
> > while doing some work on it's behalf in a preemptive way is by
> > blocking it and using a tasklet. What's done with IOREQs is not
> > suitable here since Xen needs to do some work instead of just wait on
> > an external event (an event channel from the IOREQ).
> 
> No, there is a second means, I've just confused the functions. The
> question is whether your vpci_process_pending() invocation
> perhaps sits in the wrong function. handle_hvm_io_completion() is
> what hvm_do_resume() calls, and what can prevent a guest from
> resuming execution. The hvm_io_pending() invocation just sits on
> a special case path down from there (through handle_pio()).

Yes, handle_hvm_io_completion is the function that actually blocks the
vCPU and waits for an event channel from the ioreq. This is however
not suitable because it uses the following code (simplified):

set_bit(_VPF_blocked_in_xen, &current->pause_flags);
raise_softirq(SCHEDULE_SOFTIRQ);
do_softirq();

In the vPCI case Xen cannot set the vCPU as blocked_in_xen, Xen needs
the scheduler to schedule the vCPU so the pending work can be
processed. Then if the blocked bit is not set the call to do_softirq
would be recurred, thus probably leading to a stack overflow if
there's enough pending work. ie:

<process work>
        <do_softirq>
                <process work>
                        <do_softirq>
                                <...>

Thanks, Roger.

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