Re: [for-4.15][PATCH v3 2/3] xen/x86: iommu: Ignore IOMMU mapping requests when a domain is dying

[Julien Grall <julien@xxxxxxx>]

Hi,

On 18/02/2021 13:05, Jan Beulich wrote:
> On 17.02.2021 17:07, Julien Grall wrote:
> > On 17/02/2021 15:01, Jan Beulich wrote:
> > > On 17.02.2021 15:24, Julien Grall wrote:
> > > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > The new x86 IOMMU page-tables allocator will release the pages when
> > > > relinquishing the domain resources. However, this is not sufficient
> > > > when the domain is dying because nothing prevents page-table to be
> > > > allocated.
> > > >
> > > > Currently page-table allocations can only happen from iommu_map(). As
> > > > the domain is dying, there is no good reason to continue to modify the
> > > > IOMMU page-tables.
> > > While I agree this to be the case right now, I'm not sure it is a
> > > good idea to build on it (in that you leave the unmap paths
> > > untouched).
> > I don't build on that assumption. See next patch.
> Yet as said there that patch makes unmapping perhaps more fragile,
> by introducing a latent error source into the path.
I still don't see what latent error my patch will introduce. Allocation 
of page-tables are not guarantee to succeed.
So are you implying that a code may rely on the page allocation to succeed?

> > > Imo there's a fair chance this would be overlooked at
> > > the point super page mappings get introduced (which has been long
> > > overdue), and I thought prior discussion had lead to a possible
> > > approach without risking use-after-free due to squashed unmap
> > > requests.
> > I know you suggested to zap the root page-tables... However, I don't
> > think this is 4.15 material and you agree with this (you were the one
> > pointed out that out).
> Paul - do you have any thoughts here? Outright zapping isn't
> going to work, we'd need to switch to quarantine page tables at
> the very least to prevent the issue with babbling devices. But
> that still seems better to me than the introduction of a latent
> issue on the unmap paths.
I am afraid I am not going to be able to come up with such patch for 
4.15. If you want to go that route for 4.15, then feel free to suggest a 
patch.
[...]

> Btw - "no more IOMMU mapping" is also possibly ambiguous here:
> One might take it to mean both maps and unmaps. How about "no
> new IOMMU mappings can be inserted", as long as the unmap paths
> don't follow suit?
Sure.

Cheers,

--
Julien Grall