Hi Ashish, On 7/31/2025 3:25 AM, Ashish Kalra wrote: > From: Ashish Kalra <ashish.kalra@xxxxxxx> > > 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@xxxxxxx> > Signed-off-by: Ashish Kalra <ashish.kalra@xxxxxxx> 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@xxxxxxx> > --- > 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
