Re: [PATCH v2 1/1] iommu/vt-d: Fix kdump kernels boot failure with scalable mode

[Baolu Lu <baolu.lu@linux.intel.com>]

On 2022/8/22 12:42, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Thursday, August 18, 2022 7:13 PM
>>>>    	rtaddr_reg = dmar_readq(iommu->reg + DMAR_RTADDR_REG);
>>>> -	ext        = !!(rtaddr_reg & DMA_RTADDR_RTT);
>>>> -	new_ext    = !!ecap_ecs(iommu->ecap);
>>>> +	ext        = !!(rtaddr_reg & DMA_RTADDR_SMT);
>>>> +	new_ext    = !!ecap_smts(iommu->ecap);
>>>
>>> should be !!sm_supported()
>>
>> Not really. The IOMMU was setup by the previous kernel. Here we just
>> check whether the scalable mode was enabled there.
> 
> You want to compare whether old kernel and new kernel enable
> the same mode. ecap_smts is only about the capability. only
> sm_supported() can tell the mode which is actually used by the
> new kernel.

Oh, yes! You are right. I will update this.

> 
>>
>>>
>>>>
>>>>    	/*
>>>>    	 * The RTT bit can only be changed when translation is disabled,
>>>> @@ -2747,6 +2705,10 @@ static int copy_translation_tables(struct
>>>> intel_iommu *iommu)
>>>>    	if (new_ext != ext)
>>>>    		return -EINVAL;
>>>>
>>>> +	iommu->copied_tables = bitmap_zalloc(BIT_ULL(16), GFP_KERNEL);
>>>> +	if (!iommu->copied_tables)
>>>> +		return -ENOMEM;
>>>> +
>>>>    	old_rt_phys = rtaddr_reg & VTD_PAGE_MASK;
>>>>    	if (!old_rt_phys)
>>>>    		return -EINVAL;
>>>
>>> Out of curiosity. What is the rationale that we copy root table and
>>> context tables but not pasid tables?
>>
>> We only copy the context table and reconstruct it when the default
>> domain is attached. Before that, there's no need to reconstruct the
>> pasid table, hence it's safe to use the previous pasid tables.
>>
> 
> I still didn't get why context table must be reconstructed but not
> pasid table...

The pasid table is also reconstructed. The context table entry and the
pasid tables are reconstructed together, hence there's no need to copy
the pasid table.

Best regards,
baolu