Hey Lu,
> On 7 Mar 2019, at 06:31, Lu Baolu <baolu...@linux.intel.com> 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.
>
> Best regards,
> Lu Baolu
Cheers,
James.
