Re: [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains

From: Lu Baolu
Date: Thu Mar 07 2019 - 20:14:34 EST


Hi,

On 3/7/19 6:21 PM, James Sewart wrote:
Hey Lu,

On 7 Mar 2019, at 06:31, Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> wrote:

Hi James,

On 3/7/19 2:08 AM, James Sewart wrote:
- /*
- * For each rmrr
- * for each dev attached to rmrr
- * do
- * locate drhd for dev, alloc domain for dev
- * allocate free domain
- * allocate page table entries for rmrr
- * if context not allocated for bus
- * allocate and init context
- * set present in root table for this bus
- * init context with domain, translation etc
- * endfor
- * endfor
- */
- pr_info("Setting RMRR:\n");
- for_each_rmrr_units(rmrr) {
- /* some BIOS lists non-exist devices in DMAR table. */
- for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
- i, dev) {
- ret = iommu_prepare_rmrr_dev(rmrr, dev);
- if (ret)
- pr_err("Mapping reserved region failed\n");
- }
- }
-
- iommu_prepare_isa();
Why do you want to remove this segment of code?
This will only work if the lazy allocation of domains exists, these
mappings will disappear once a default domain is attached to a device and
then remade by iommu_group_create_direct_mappings. This code is redundant
and removing it allows us to remove all the lazy allocation logic.
No exactly.

We need to setup the rmrr mapping before enabling dma remapping,
otherwise, there will be a window after dma remapping enabling and
rmrr getting mapped, during which people might see dma faults.
Do you think this patch instead should be a refactoring to simplify this initial domain setup before the default domain takes over? It seems like duplicated effort to have both lazy allocated domains and externally managed domains. We could allocate a domain here for any devices with RMRR and call get_resv_regions to avoid duplicating RMRR loop.

Agree. We should replace the lazy allocated domains with default domain
in a clean way. Actually, your patches look great to me. But I do think
we need further cleanups. The rmrr code is one example, and the identity
domain (si_domain) is another.

Do you mind if I work on top of your patches for further cleanups and
sign off a v2 together with you?

Sure, sounds good. Iâll fixup patch 3 and have a go at integrating
iommu_prepare_isa into get_resv_regions. This should make the initial
domain logic here quite concise.

Very appreciated! Thank you!

Best regards,
Lu Baolu