Re: [PATCH 1/1] iommu/vt-d: Fix RID2PASID setup failure

From: Baolu Lu
Date: Tue Jun 21 2022 - 02:15:23 EST


On 2022/6/21 13:48, Tian, Kevin wrote:
From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Tuesday, June 21, 2022 12:28 PM

On 2022/6/21 11:46, Tian, Kevin wrote:
From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Tuesday, June 21, 2022 11:39 AM

On 2022/6/21 10:54, Tian, Kevin wrote:
From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Monday, June 20, 2022 4:17 PM
@@ -2564,7 +2564,7 @@ static int domain_add_dev_info(struct
dmar_domain *domain, struct device *dev)
ret = intel_pasid_setup_second_level(iommu,
domain,
dev, PASID_RID2PASID);
spin_unlock_irqrestore(&iommu->lock, flags);
- if (ret) {
+ if (ret && ret != -EBUSY) {
dev_err(dev, "Setup RID2PASID failed\n");
dmar_remove_one_dev_info(dev);
return ret;
--
2.25.1

It's cleaner to avoid this error at the first place, i.e. only do the
setup when the first device is attached to the pasid table.

The logic that identifies the first device might introduce additional
unnecessary complexity. Devices that share a pasid table are rare. I
even prefer to give up sharing tables so that the code can be
simpler.:-)


It's not that complex if you simply move device_attach_pasid_table()
out of intel_pasid_alloc_table(). Then do the setup if
list_empty(&pasid_table->dev) and then attach device to the
pasid table in domain_add_dev_info().

The pasid table is part of the device, hence a better place to
allocate/free the pasid table is in the device probe/release paths.
Things will become more complicated if we change relationship between
device and it's pasid table when attaching/detaching a domain. That's
the reason why I thought it was additional complexity.


If you do want to follow current route it’s still cleaner to check
whether the pasid entry has pointed to the domain in the individual
setup function instead of blindly returning -EBUSY and then ignoring
it even if a real busy condition occurs. The setup functions can
just return zero for this benign alias case.

Fair enough. Let me improve it.

--
Best regards,
baolu