Re: [PATCH v2] x86/sev: Fix host kdump support for SNP

Ashish Kalra
2024-09-09T23:34:23Z

Hello Sean,

On 9/4/2024 5:23 PM, Sean Christopherson wrote:
>> On Wed, Sep 04, 2024, Ashish Kalra wrote:
>>> On 9/4/2024 2:54 PM, Michael Roth wrote:
>>>> - Sean inquired about making the target kdump kernel more agnostic to
>>>> whether or not SNP_SHUTDOWN was done properly, since that might
>>>> allow for capturing state even for edge cases where we can't go
>>>> through the normal cleanup path. I mentioned we'd tried this to some
>>>> degree but hit issues with the IOMMU, and when working around that
>>>> there was another issue but I don't quite recall the specifics.
>>>> Can you post a quick recap of what the issues are with that approach
>>>> so we can determine whether or not this is still an option?
>>>
>>> Yes, i believe without SNP_SHUTDOWN, early_enable_iommus() configure the
>>> IOMMUs into an IRQ remapping configuration causing the crash in
>>> io_apic.c::check_timer().
>>>
>>> It looks like in this case, we enable IRQ remapping configuration *earlier*
>>> than when it needs to be enabled and which causes the panic as indicated:
>>>
>>> EMERGENCY [ 1.376701] Kernel panic - not syncing: timer doesn't work
>>> through Interrupt-remapped IO-APIC
>>
>> I assume the problem is that IOMMU setup fails in the kdump kernel, not that it
>> does the setup earlier. That's that part I want to understand.

>Here is a deeper understanding of this issue:

>It looks like this is happening: when we do SNP_SHUTDOWN without IOMMU_SNP_SHUTDOWN during panic, kdump boot runs with iommu snp
>enforcement still enabled and IOMMU completion wait buffers (cwb) still locked and exclusivity still setup on those, and then in
>kdump boot, we allocate new iommu completion wait buffers and try to use them, but we get a iommu command completion wait time-out,
>due to the locked in (prev) completion wait buffers, the newly allocated completion wait buffers are not getting used for iommu
>command execution and completion indication :

>[ 1.711588] AMD-Vi: early_amd_iommu_init: irq remaping enabled
>[ 1.718972] AMD-Vi: in early_enable_iommus
>[ 1.723543] AMD-Vi: Translation is already enabled - trying to copy translation structures
>[ 1.733333] AMD-Vi: Copied DEV table from previous kernel.
>[ 1.739566] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.11.0-rc6-next-20240903-snp-host-f2a41ff576cc+ #78
>[ 1.750920] Hardware name: AMD Corporation ETHANOL_X/ETHANOL_X, BIOS RXM100AB 10/17/2022
>[ 1.759950] Call Trace:
>[ 1.762677] <TASK>
>[ 1.765018] dump_stack_lvl+0x70/0x90
>[ 1.769109] dump_stack+0x14/0x20
>[ 1.772809] iommu_completion_wait.part.0.isra.0+0x38/0x140
>[ 1.779035] amd_iommu_flush_all_caches+0xa3/0x240
>[ 1.784383] ? memcpy_toio+0x25/0xc0
>[ 1.788372] early_enable_iommus+0x151/0x880
>[ 1.793140] state_next+0xe67/0x22b0
>[ 1.797130] ? __raw_callee_save___native_queued_spin_unlock+0x19/0x30
>[ 1.804421] amd_iommu_enable+0x24/0x60
>[ 1.808702] irq_remapping_enable+0x1f/0x50
>[ 1.813371] enable_IR_x2apic+0x155/0x260
>[ 1.817848] x86_64_probe_apic+0x13/0x70
>[ 1.822226] apic_intr_mode_init+0x39/0xf0
>[ 1.826799] x86_late_time_init+0x28/0x40
>[ 1.831266] start_kernel+0x6ad/0xb50
>[ 1.835436] x86_64_start_reservations+0x1c/0x30
>[ 1.840591] x86_64_start_kernel+0xbf/0x110
>[ 1.845256] ? setup_ghcb+0x12/0x130
>[ 1.849247] common_startup_64+0x13e/0x141
>[ 1.853821] </TASK>
>[ 2.077901] AMD-Vi: Completion-Wait loop timed out
>...

>And because of this the iommu command, in this case which is for enabling irq remapping does not succeed and that eventually causes
>timer to fail without irq remapping support enabled.

>Once IOMMU SNP support is enabled, to enforce RMP enforcement the IOMMU completion wait buffers are setup as read-only and
>exclusivity set on these and additionally the IOMMU registers used to mark the exclusivity on the store addresses associated with
>these CWB is also locked. This enforcement of SNP in the IOMMU is only disabled with the IOMMU_SNP_SHUTDOWN parameter with
>SNP_SHUTDOWN_EX command.

>From the AMD IOMMU specifications:

>2.12.2.2 SEV-SNP COMPLETION_WAIT Store Restrictions On systems that are SNP-enabled, the store address associated with any host
>COMPLETION_WAIT command (s=1) is restricted. The Store Address must fall within the address range specified by the Completion Store
>Base and Completion Store Limit registers. When the system is SNP-enabled, the memory within this range will be marked in the RMP
>using a special immutable state by the PSP. This memory region will be readable by the CPU but not writable.

>2.12.2.3 SEV-SNP Exclusion Range Restrictions The exclusion range feature is not supported on systems that are SNP-enabled.
>Additionally, the Exclusion Base and Exclusion Range Limit registers are re-purposed to act as the Completion Store Base and Limit
>registers.

>Therefore, we need to disable IOMMU SNP enforcement with SNP_SHUTDOWN_EX command before the kdump kernel starts booting as we can't
>setup IOMMU CWB again in kdump as SEV-SNP exclusion base and range limit registers are locked as IOMMU SNP support is still enabled.

>I tried to use the previous kernel's CWB (cmd_sem) as below:

>static int __init alloc_cwwb_sem(struct amd_iommu *iommu)
>{
> if (!is_kdump_kernel())
> iommu->cmd_sem = iommu_alloc_4k_pages(iommu, GFP_KERNEL, 1);
> else {
> if (check_feature(FEATURE_SNP)) {
> u64 cwwb_sem_paddr;
>
> cwwb_sem_paddr = readq(iommu->mmio_base + MMIO_EXCL_BASE_OFFSET);
> iommu->cmd_sem = iommu_phys_to_virt(cwwb_sem_paddr);
> return iommu->cmd_sem ? 0 : -ENOMEM;
> }
> }
>
> return iommu->cmd_sem ? 0 : -ENOMEM;
>}

>I tried this, but this fails as i believe the kdump kernel will not have these previous kernel's allocated IOMMU CWB in the kernel
>direct map :

>[ 1.708959] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.714327] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x100805000, cmd_sem_vaddr 0xffff9f5340805000
>[ 1.726309] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.731676] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x1050051000, cmd_sem_vaddr 0xffff9f6290051000
>[ 1.743742] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.749109] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x1050052000, cmd_sem_vaddr 0xffff9f6290052000
>[ 1.761177] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.766542] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x100808000, cmd_sem_vaddr 0xffff9f5340808000
>[ 1.778509] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.783877] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x1050053000, cmd_sem_vaddr 0xffff9f6290053000
>[ 1.795942] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.801300] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x100809000, cmd_sem_vaddr 0xffff9f5340809000
>[ 1.813268] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.818636] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x1050054000, cmd_sem_vaddr 0xffff9f6290054000
>[ 1.830701] AMD-Vi: in alloc_cwwb_sem kdump kernel
>[ 1.836069] AMD-Vi: in alloc_cwwb_sem SNP feature enabled, cmd_sem_paddr 0x10080a000, cmd_sem_vaddr 0xffff9f534080a000
>[ 1.848039] AMD-Vi: early_amd_iommu_init: irq remaping enabled
>[ 1.855431] AMD-Vi: in early_enable_iommus
>[ 1.860032] AMD-Vi: Translation is already enabled - trying to copy translation structures
>[ 1.869812] AMD-Vi: Copied DEV table from previous kernel.
>[ 1.875958] AMD-Vi: in build_completion_wait, paddr = 0x100805000
>[ 1.882766] BUG: unable to handle page fault for address: ffff9f5340805000
>[ 1.890441] #PF: supervisor read access in kernel mode
>[ 1.896177] #PF: error_code(0x0000) - not-present page

>....

>I think that memremap(..,..,MEMREMAP_WB) will also fail for the same reason as memremap(.., MEMREMAP_WB) for the RAM region will
>again use the kernel directmap.

To follow up on this:

I am able to use memremap() to map the previous kernel's allocated CWB buffers and try to reuse the same CWB buffers in the
kdump kernel, obviously, memremap() does not return a direct pointer to kernel directmap as the previous kernel's CWB buffers
will be in a RAM address which is not directly mapped into kdump kernel's directmap.

And these memremap() mappings seem to be correct, because if i do a memset(0) on these, i get a RMP #PF violation due
to these buffers being setup as RO in the RMP table, so that means that memremap() seems to have done the mapping correctly.

I am getting inconsistent IOMMU command completion wait timeout's with these reused CWB buffers (which are used as
semaphores to indicate IOMMU command completions) and i am still debugging those issues.

Thanks,
Ashish