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

From: "Jan Beulich" <JBeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Julien Grall <julien.grall@arm.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 5/7] vpci: fix execution of long running operations
Date: Mon, 05 Nov 2018 09:56:13 -0700	[thread overview]
Message-ID: <5BE0762D02000078001F8356@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <20181030154123.4218-6-roger.pau@citrix.com>

>>> On 30.10.18 at 16:41, <roger.pau@citrix.com> 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.

> +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?

Jan

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