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

Tian, Kevin
2022-08-18T08:33:00Z

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Wednesday, August 17, 2022 9:11 AM
>
> The translation table copying code for kdump kernels is currently based
> on the extended root/context entry formats of ECS mode defined in older
> VT-d v2.5, and doesn't handle the scalable mode formats. This causes
> the kexec capture kernel boot failure with DMAR faults if the IOMMU was
> enabled in scalable mode by the previous kernel.
>
> The ECS mode has already been deprecated by the VT-d spec since v3.0 and
> Intel IOMMU driver doesn't support this mode as there's no real hardware
> implementation. Hence this converts ECS checking in copying table code
> into scalable mode.
>
> The existing copying code consumes a bit in the context entry as a mark
> of copied entry. This marker needs to work for the old format as well as
> for extended context entries. It's hard to find such a bit for both

The 2nd sentence "This marker..." is misleading. better removed.

> legacy and scalable mode context entries. This replaces it with a per-
> IOMMU bitmap.
>
> Fixes: 7373a8cc38197 ("iommu/vt-d: Setup context and enable RID2PASID
> support")
> Cc: stable@vger.kernel.org
> Reported-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Tested-by: Wen Jin <wen.jin@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
...
> @@ -2735,8 +2693,8 @@ static int copy_translation_tables(struct
> intel_iommu *iommu)
> bool new_ext, ext;
>
> 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()

>
> /*
> * 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?