Re: [PATCH v1 21/26] crypto: ccp: Add panic notifier for SEV/SNP firmware shutdown on kdump

["Kalra, Ashish" <ashish.kalra@amd.com>]

Hello Boris,

On 1/21/2024 5:49 AM, Borislav Petkov wrote:
> On Sat, Dec 30, 2023 at 10:19:49AM -0600, Michael Roth wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> Add a kdump safe version of sev_firmware_shutdown() registered as a
>> crash_kexec_post_notifier, which is invoked during panic/crash to do
>> SEV/SNP shutdown. This is required for transitioning all IOMMU pages
>> to reclaim/hypervisor state, otherwise re-init of IOMMU pages during
>> crashdump kernel boot fails and panics the crashdump kernel. This
>> panic notifier runs in atomic context, hence it ensures not to
>> acquire any locks/mutexes and polls for PSP command completion
>> instead of depending on PSP command completion interrupt.
>>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> [mdr: remove use of "we" in comments]
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
> Cleanups ontop, see if the below works too. Especially:
>
> * I've zapped the WBINVD before the TMR pages are freed because
> __sev_snp_shutdown_locked() will WBINVD anyway.

This flush is required for TMR, as TMR is encrypted and it needs to be 
flushed from cache before being reclaimed and freed, therefore this 
flush is required.

SNP_SHUTDOWN_EX may additionally require wbinvd + DF_FLUSH, therefore 
there is another WBINVD in __sev_snp_shutdown_locked().

>
> * The mutex_is_locked() check in snp_shutdown_on_panic() is silly
> because the panic notifier runs on one CPU anyway.

But, what if there is an active command on the PSP when panic occurs, 
the mutex will already be acquired in such a case and we can't issue 
another PSP command if there is an active PSP command, so i believe this 
check is required.

Thanks, Ashish