Re: [PATCH v3 13/18] iommu/vt-d: preserve PASID table of preserved device

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, Jun 22, 2026 at 02:01:09PM +0800, Baolu Lu wrote:
>On 6/15/26 07:37, Samiullah Khawaja wrote:
>>diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
>>index 89541b74ab8c..5cac8e95f73b 100644
>>--- a/drivers/iommu/intel/pasid.c
>>+++ b/drivers/iommu/intel/pasid.c
>>@@ -60,8 +60,11 @@ int intel_pasid_alloc_table(struct device *dev)
>>  	size = max_pasid >> (PASID_PDE_SHIFT - 3);
>>  	order = size ? get_order(size) : 0;
>>-	dir = iommu_alloc_pages_node_sz(info->iommu->node, GFP_KERNEL,
>>-					1 << (order + PAGE_SHIFT));
>>+
>>+	dir = intel_pasid_try_restore_table(dev, 1 << (order + PAGE_SHIFT + 3));
>>+	if (!dir)
>>+		dir = iommu_alloc_pages_node_sz(info->iommu->node, GFP_KERNEL,
>>+						1 << (order + PAGE_SHIFT));
>>  	if (!dir) {
>>  		kfree(pasid_table);
>>  		return -ENOMEM;
>
>This reads as if PASID table restoration fails, it tries to allocate a
>native PASID table instead. This doesn't match my understanding. It
>would be a disaster, and the system should panic if the table
>restoration fails. Or is there something I'm overlooking?

Your understanding is correct, and that is exactly what
intel_pasid_try_restore_table() does if the device state was preserved.
If the restoration fails, it fails hard with a panic.

The reason I have _try_ in the name is to account for devices that
simply have no preserved state to begin with. In that case, a new PASID
table needs to be allocated.

I agree the naming makes it sound like we ignore restoration failures. I
will rename the function to make this less confusing in v4.
>
>Thanks,
>baolu