Re: [PATCH for-4.15 2/4] xen/iommu: x86: Free the IOMMU page-tables with the pgtables.lock held

[Julien Grall <julien@xxxxxxx>]

On 23/12/2020 14:01, Julien Grall wrote:
> Hi Jan,
>
> On 23/12/2020 13:48, Jan Beulich wrote:
> > On 22.12.2020 16:43, Julien Grall wrote:
> > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > >
> > > The pgtables.lock is protecting access to the page list pgtables.list.
> > > However, iommu_free_pgtables() will not held it. I guess it was assumed
> > > that page-tables cannot be allocated while the domain is dying.
> > >
> > > Unfortunately, there is no guarantee that iommu_map() will not be
> > > called while a domain is dying (it looks like to be possible from
> > > XEN_DOMCTL_memory_mapping).
> > I'd rather disallow any new allocations for a dying domain, not
> > the least because ...
> Patch #4 will disallow such allocation. However...
It turns out that I can't disallow it because there is at least one call 
of iommu_map() while an HVM domain is destroyed:
    (XEN)    [<ffff82d04026e399>] R iommu_map+0xf2/0x171
    (XEN)    [<ffff82d04026e43e>] F iommu_legacy_map+0x26/0x63
    (XEN)    [<ffff82d040302a42>] F 
arch/x86/mm/p2m-ept.c#ept_set_entry+0x6b2/0x730
    (XEN)    [<ffff82d0402f761d>] F p2m_set_entry+0x91/0x128
    (XEN)    [<ffff82d0402f8643>] F guest_physmap_add_entry+0x3d7/0x4e0
    (XEN)    [<ffff82d0402f87cb>] F guest_physmap_add_page+0x7f/0xfe
    (XEN)    [<ffff82d0402ba0a2>] F 
arch/x86/hvm/ioreq.c#hvm_add_ioreq_gfn+0x6f/0x8d
    (XEN)    [<ffff82d0402ba0f9>] F 
arch/x86/hvm/ioreq.c#hvm_ioreq_server_disable+0x39/0x4f
    (XEN)    [<ffff82d0402bb50e>] F hvm_destroy_all_ioreq_servers+0x6f/0x9b
    (XEN)    [<ffff82d0402afc15>] F 
hvm_domain_relinquish_resources+0x3e/0x92
    (XEN)    [<ffff82d04031f278>] F domain_relinquish_resources+0x355/0x372
    (XEN)    [<ffff82d040205dd4>] F domain_kill+0xc7/0x150
    (XEN)    [<ffff82d04023baf0>] F do_domctl+0xba7/0x1a09
    (XEN)    [<ffff82d040312c8f>] F pv_hypercall+0x2f0/0x55f
    (XEN)    [<ffff82d040391432>] F lstar_enter+0x112/0x120

This one resulted to a domain crash rather than a clean destruction.

I would still like to disallow page-table allocation, so I am think to 
ignore any request in iommu_map() if the domain is dying.
Cheers,

--
Julien Grall