[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Jan,

On 19/02/2021 08:49, Jan Beulich wrote:
On 18.02.2021 14:25, Julien Grall wrote:
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?

I'm implying that failure to split a superpage may have unknown
consequences.

As failure to flush the TLBs (see below).

Since we make no use of superpages when not
sharing page tables, I call this a latent issue which may go
unnoticed for quite some time once no longer latent.

And it would take a lot longer to unnotice an OOM as this should happen less often ;).

But, I think this is wrong to blame the lower layer for a (latent) bug in the upper layer...

Even with your solution to zap page-tables, there is still a risk of failure because the TLB flush may fail (the operation can return an error).

So regardless the solution, we still need to have callers that function correctly.

Anyway, what matters right now is fixing a host crash when running PV guest with passthrough. Neither patch #3 nor the iommu_unmap() part is strictly necessary for 4.15 (as you say this is latent).

So I would suggest to defer this discussion post-4.15.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.