lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4281e562-0dfa-41a0-bcff-02b7bb1d67ca@amd.com>
Date: Thu, 21 Aug 2025 17:15:27 +0530
From: Vasant Hegde <vasant.hegde@....com>
To: Ashish Kalra <Ashish.Kalra@....com>, joro@...tes.org,
 suravee.suthikulpanit@....com, thomas.lendacky@....com,
 Sairaj.ArunKodilkar@....com, herbert@...dor.apana.org.au
Cc: seanjc@...gle.com, pbonzini@...hat.com, will@...nel.org,
 robin.murphy@....com, john.allen@....com, davem@...emloft.net,
 michael.roth@....com, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
 linux-crypto@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH v5 2/4] iommu/amd: Reuse device table for kdump



On 7/31/2025 3:26 AM, Ashish Kalra wrote:
> From: Ashish Kalra <ashish.kalra@....com>
> 
> After a panic if SNP is enabled in the previous kernel then the kdump
> kernel boots with IOMMU SNP enforcement still enabled.
> 
> IOMMU device table register is locked and exclusive to the previous
> kernel. Attempts to copy old device table from the previous kernel
> fails in kdump kernel as hardware ignores writes to the locked device
> table base address register as per AMD IOMMU spec Section 2.12.2.1.
> 

Can you please expand and add something like below so that actually issue is clear.

This causes the IOMMU driver (OS) and the hardware to reference different memory
locations. As a result, the IOMMU hardware cannot process the command.




> This results in repeated "Completion-Wait loop timed out" errors and a
> second kernel panic: "Kernel panic - not syncing: timer doesn't work
> through Interrupt-remapped IO-APIC".
> 
> Reuse device table instead of copying device table in case of kdump
> boot and remove all copying device table code.
>  
> Tested-by: Sairaj Kodilkar <sarunkod@....com>
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>


Few minor nits. Otherwise patch looks ok.

Reviewed-by: Vasant Hegde <vasant.hegde@....com>


> ---
>  drivers/iommu/amd/init.c | 106 +++++++++++++--------------------------
>  1 file changed, 36 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index aae1aa7723a5..05d9c1764883 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -406,6 +406,9 @@ static void iommu_set_device_table(struct amd_iommu *iommu)
>  
>  	BUG_ON(iommu->mmio_base == NULL);
>  
> +	if (is_kdump_kernel())
> +		return;
> +
>  	entry = iommu_virt_to_phys(dev_table);
>  	entry |= (dev_table_size >> 12) - 1;
>  	memcpy_toio(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET,
> @@ -646,7 +649,10 @@ static inline int __init alloc_dev_table(struct amd_iommu_pci_seg *pci_seg)
>  
>  static inline void free_dev_table(struct amd_iommu_pci_seg *pci_seg)
>  {
> -	iommu_free_pages(pci_seg->dev_table);
> +	if (is_kdump_kernel())
> +		memunmap((void *)pci_seg->dev_table);
> +	else
> +		iommu_free_pages(pci_seg->dev_table);
>  	pci_seg->dev_table = NULL;
>  }
>  
> @@ -1129,15 +1135,12 @@ static void set_dte_bit(struct dev_table_entry *dte, u8 bit)
>  	dte->data[i] |= (1UL << _bit);
>  }
>  
> -static bool __copy_device_table(struct amd_iommu *iommu)
> +static bool __reuse_device_table(struct amd_iommu *iommu)
>  {
> -	u64 int_ctl, int_tab_len, entry = 0;
>  	struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
> -	struct dev_table_entry *old_devtb = NULL;
> -	u32 lo, hi, devid, old_devtb_size;
> +	u32 lo, hi, old_devtb_size;
>  	phys_addr_t old_devtb_phys;
> -	u16 dom_id, dte_v, irq_v;
> -	u64 tmp;
> +	u64 entry;
>  
>  	/* Each IOMMU use separate device table with the same size */
>  	lo = readl(iommu->mmio_base + MMIO_DEV_TABLE_OFFSET);
> @@ -1162,66 +1165,22 @@ static bool __copy_device_table(struct amd_iommu *iommu)
>  		pr_err("The address of old device table is above 4G, not trustworthy!\n");
>  		return false;
>  	}
> -	old_devtb = (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT) && is_kdump_kernel())
> -		    ? (__force void *)ioremap_encrypted(old_devtb_phys,
> -							pci_seg->dev_table_size)
> -		    : memremap(old_devtb_phys, pci_seg->dev_table_size, MEMREMAP_WB);
> -
> -	if (!old_devtb)
> -		return false;
>  
> -	pci_seg->old_dev_tbl_cpy = iommu_alloc_pages_sz(
> -		GFP_KERNEL | GFP_DMA32, pci_seg->dev_table_size);
> +	/*
> +	 * IOMMU Device Table Base Address MMIO register is locked
> +	 * if SNP is enabled during kdump, reuse the previous kernel's
> +	 * device table.

Can you please reword as its reusing crash kernel device table in all scenarios ?

-Vasant


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ