| 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
| ||
|
Message-ID: <8613b5a8-aef5-4457-a1ca-646443da8d5b@amd.com>
Date: Thu, 21 Aug 2025 16:59:01 +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 1/4] iommu/amd: Add support to remap/unmap IOMMU
buffers for kdump
Hi Ashish,
On 7/31/2025 3:25 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 completion wait buffers (CWBs), command buffers and event buffer
> registers remain locked and exclusive to the previous kernel. Attempts
> to allocate and use new buffers in the kdump kernel fail, as hardware
> ignores writes to the locked MMIO registers as per AMD IOMMU spec
> Section 2.12.2.1.
>
> 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"
>
> The list of MMIO registers locked and which ignore writes after failed
> SNP shutdown are mentioned in the AMD IOMMU specifications below:
>
> Section 2.12.2.1.
> https://docs.amd.com/v/u/en-US/48882_3.10_PUB
>
> Reuse the pages of the previous kernel for completion wait buffers,
> command buffers, event buffers and memremap them during kdump boot
> and essentially work with an already enabled IOMMU configuration and
> re-using the previous kernel’s data structures.
>
> Reusing of command buffers and event buffers is now done for kdump boot
> irrespective of SNP being enabled during kdump.
>
> Re-use of completion wait buffers is only done when SNP is enabled as
> the exclusion base register is used for the completion wait buffer
> (CWB) address only when SNP is enabled.
>
> Tested-by: Sairaj Kodilkar <sarunkod@....com>
> Signed-off-by: Ashish Kalra <ashish.kalra@....com>
I'd still think introducing ops for various callback makes more sense than
having explicit is_kdump_kernel check everywhere. I am fine with having follow
up patch to fix that. With that and few minor nits below :
Reviewed-by: Vasant Hegde <vasant.hegde@....com>
> ---
> drivers/iommu/amd/amd_iommu_types.h | 5 +
> drivers/iommu/amd/init.c | 164 ++++++++++++++++++++++++++--
> drivers/iommu/amd/iommu.c | 2 +-
> 3 files changed, 158 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index 5219d7ddfdaa..8a863cae99db 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -791,6 +791,11 @@ struct amd_iommu {
> u32 flags;
> volatile u64 *cmd_sem;
> atomic64_t cmd_sem_val;
> + /*
> + * Track physical address to directly use it in build_completion_wait()
> + * and avoid adding any special checks and handling for kdump.
> + */
> + u64 cmd_sem_paddr;
>
> #ifdef CONFIG_AMD_IOMMU_DEBUGFS
> /* DebugFS Info */
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 7b5af6176de9..aae1aa7723a5 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -710,6 +710,26 @@ static void __init free_alias_table(struct amd_iommu_pci_seg *pci_seg)
> pci_seg->alias_table = NULL;
> }
>
> +static inline void *iommu_memremap(unsigned long paddr, size_t size)
> +{
> + phys_addr_t phys;
> +
> + if (!paddr)
> + return NULL;
> +
> + /*
> + * Obtain true physical address in kdump kernel when SME is enabled.
> + * Currently, previous kernel with SME enabled and kdump kernel
> + * with SME support disabled is not supported.
> + */> + phys = __sme_clr(paddr);
> +
> + if (cc_platform_has(CC_ATTR_HOST_MEM_ENCRYPT))
> + return (__force void *)ioremap_encrypted(phys, size);
> + else
> + return memremap(phys, size, MEMREMAP_WB);
> +}
> +
> /*
> * Allocates the command buffer. This buffer is per AMD IOMMU. We can
> * write commands to that buffer later and the IOMMU will execute them
> @@ -942,8 +962,103 @@ static int iommu_init_ga_log(struct amd_iommu *iommu)
> static int __init alloc_cwwb_sem(struct amd_iommu *iommu)
> {
> iommu->cmd_sem = iommu_alloc_4k_pages(iommu, GFP_KERNEL, 1);
> + if (!iommu->cmd_sem)
> + return -ENOMEM;
> + iommu->cmd_sem_paddr = iommu_virt_to_phys((void *)iommu->cmd_sem);
> + return 0;
> +}
> +
> +static int __init remap_event_buffer(struct amd_iommu *iommu)
> +{
> + u64 paddr;
> +
> + pr_info_once("Re-using event buffer from the previous kernel\n");
> + /*
> + * Read-back the event log base address register and apply
> + * PM_ADDR_MASK to obtain the event log base address.
IMO this comment is redundant. Its implicit that we have to apply the addr_mask
to get the actual address.
> + */
> + paddr = readq(iommu->mmio_base + MMIO_EVT_BUF_OFFSET) & PM_ADDR_MASK;
> + iommu->evt_buf = iommu_memremap(paddr, EVT_BUFFER_SIZE);
>
> - return iommu->cmd_sem ? 0 : -ENOMEM;
> + return iommu->evt_buf ? 0 : -ENOMEM;
> +}
> +
> +static int __init remap_command_buffer(struct amd_iommu *iommu)
> +{
> + u64 paddr;
> +
> + pr_info_once("Re-using command buffer from the previous kernel\n");
> + /*
> + * Read-back the command buffer base address register and apply
> + * PM_ADDR_MASK to obtain the command buffer base address.
ditto.
-Vasant
Powered by blists - more mailing lists