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

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

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).

> > +void vpci_init_vcpu(struct vcpu *v)
> > +{
> > +    tasklet_init(&v->vpci.task, vpci_process_pending, (unsigned long)v);
> >  }
> 
> Since there's no respective cleanup code added afaics - what
> if the domain gets cleaned up behind the back of the (long
> running) tasklet? Don't you want to acquire (and then release)
> an extra domain reference somewhere?

Yes, that's correct. Isn't just doing a tasklet_kill at domain
destruction enough?

Thanks, Roger.

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