Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions

From: James Sewart
Date: Fri Apr 05 2019 - 14:03:05 EST


Hey Lu,

My bad, did some debugging on my end. The issue was swapping out
find_domain for iommu_get_domain_for_dev. It seems in some situations the
domain is not attached to the group but the device is expected to have the
domain still stored in its archdata.

Iâve attached the final patch with find_domain unremoved which seems to
work in my testing.

Cheers,
James.

Attachment: 0009-iommu-vt-d-Remove-lazy-allocation-of-domains.patch
Description: Binary data




> On 4 Apr 2019, at 07:49, Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> wrote:
>
> Hi James,
>
> I did a sanity test from my end. The test machine fails to boot. I
> haven't seen any valuable kernel log. It results in a purple screen.
>
> Best regards,
> Lu Baolu
>
> On 3/29/19 11:26 PM, James Sewart wrote:
>> Hey Lu,
>> Iâve attached a preliminary v3, if you could take a look and run some tests
>> that would be great.
>> Since v2 iâve added your default domain type patches, the new device_group
>> handler, and incorporated Jacobâs feedback.
>>> On 28 Mar 2019, at 18:37, James Sewart <jamessewart@xxxxxxxxxx> wrote:
>>>
>>> Hey Lu,
>>>
>>>> On 26 Mar 2019, at 01:24, Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> wrote:
>>>>
>>>> Hi James,
>>>>
>>>> On 3/25/19 8:57 PM, James Sewart wrote:
>>>>>>> Theres an issue that if we choose to alloc a new resv_region with type
>>>>>>> IOMMU_RESV_DIRECT, we will need to refactor intel_iommu_put_resv_regions
>>>>>>> to free this entry type which means refactoring the rmrr regions in
>>>>>>> get_resv_regions. Should this work be in this patchset?
>>>>>> Do you mean the rmrr regions are not allocated in get_resv_regions, but
>>>>>> are freed in put_resv_regions? I think we should fix this in this patch
>>>>>> set since this might impact the device passthrough if we don't do it.
>>>>> Theyâre not allocated and not freed currently, only type IOMMU_RESV_MSI is
>>>>> freed in put_resv_regions. If we allocate a new resv_region with type
>>>>> IOMMU_RESV_DIRECT for the isa region, then it wonât be freed. If we modify
>>>>> put_resv_regions to free type IOMMU_RESV_DIRECT, then we will try to free
>>>>> the static RMRR regions.
>>>>> Either the ISA region is static and not freed as with my implementation,
>>>>> or the RMRR regions are converted to be allocated on each call to
>>>>> get_resv_regions and freed in put_resv_regions.
>>>>
>>>> By the way, there's another way in my mind. Let's add a new region type
>>>> for LPC devices, e.x. IOMMU_RESV_LPC, and then handle it in the same way
>>>> as those MSI regions. Just FYI.
>>>
>>> This solution would require adding some extra code to
>>> iommu_group_create_direct_mappings as currently only type
>>> IOMMU_RESV_DIRECT is identity mapped, other types are only reserved.
>>>
>>>
>>>>
>>>> Best regards,
>>>> Lu Baolu
>>>
>>> Cheers,
>>> James.
>> Cheers,
>> James.