Re: [PATCH v4 04/13] iommu/vt-d: Add pasid replace helpers

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

On 2024/11/6 18:05, Tian, Kevin wrote:
>> From: Liu, Yi L<yi.l.liu@intel.com>
>> Sent: Wednesday, November 6, 2024 6:02 PM
>>
>> On 2024/11/6 17:51, Tian, Kevin wrote:
>>>> From: Liu, Yi L<yi.l.liu@intel.com>
>>>> Sent: Wednesday, November 6, 2024 5:31 PM
>>>>
>>>> On 2024/11/6 15:31, Tian, Kevin wrote:
>>>>>> From: Liu, Yi L<yi.l.liu@intel.com>
>>>>>> Sent: Monday, November 4, 2024 9:19 PM
>>>>>>
>>>>>> +
>>>>>> +	spin_lock(&iommu->lock);
>>>>>> +	pte = intel_pasid_get_entry(dev, pasid);
>>>>>> +	if (!pte) {
>>>>>> +		spin_unlock(&iommu->lock);
>>>>>> +		return -ENODEV;
>>>>>> +	}
>>>>>> +
>>>>>> +	if (!pasid_pte_is_present(pte)) {
>>>>>> +		spin_unlock(&iommu->lock);
>>>>>> +		return -EINVAL;
>>>>>> +	}
>>>>>> +
>>>>>> +	old_did = pasid_get_domain_id(pte);
>>>>> probably should pass the old domain in and check whether the
>>>>> domain->did is same as the one in the pasid entry and warn otherwise.
>>>> this would be a sw bug. 🙂 Do we really want to catch every bug by warn? 🙂
>>>>
>>> this one should not happen. If it does, something severe jumps out...
>> yes. that's why I doubt if it's valuable to do it. It should be a vital
>> bug that bring us this warn. or instead of passing id old domain, how
>> about just old_did? We use the passed in did instead of using the did
>> from pte.
>>
> My personal feeling - it's worth as such rare bug once happening
> would be very difficult to debug. the warning provides useful hint.

Agreed!