Hi Jan,
On 23/12/2020 15:00, Jan Beulich wrote:
On 23.12.2020 15:56, Julien Grall wrote:
On 23/12/2020 14:12, Jan Beulich wrote:
On 22.12.2020 16:43, Julien Grall wrote:
This is an RFC because it would break AMD IOMMU driver. One option would
be to move the call to the teardown callback earlier on. Any opinions?
We already have
static void amd_iommu_domain_destroy(struct domain *d)
{
dom_iommu(d)->arch.amd.root_table = NULL;
}
and this function is AMD's teardown handler. Hence I suppose
doing the same for VT-d would be quite reasonable. And really
VT-d's iommu_domain_teardown() also already has
hd->arch.vtd.pgd_maddr = 0;
Let me have a look if that works.
I guess what's missing is prevention of the root table
getting re-setup.
This is taken care in the follow-up patch by forbidding page-table
allocation. I can mention it in the commit message.
My expectation is that with that subsequent change the change here
(or any variant of it) would become unnecessary.
I am not be sure. iommu_unmap() would still get called from put_page().
Are you suggesting to gate the code if d->is_dying as well?
Even if this patch is deemed to be unecessary to fix the issue.
This issue was quite hard to chase/reproduce.
I think it would still be good to harden the code by zeroing
hd->arch.vtd.pgd_maddr to avoid anyone else wasting 2 days because the
pointer was still "valid".
Cheers,
--
Julien Grall
