Received-SPF: pass (google.com: domain of linux-crypto+bounces-1607-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <ea2a6e5d-583a-4e11-b9b1-1f6f353d2c02@amd.com>
Date: Thu, 25 Jan 2024 21:03:45 -0600
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 21/26] crypto: ccp: Add panic notifier for SEV/SNP
 firmware shutdown on kdump
Content-Language: en-US
To: Borislav Petkov <bp@alien8.de>, Michael Roth <michael.roth@amd.com>
Cc: x86@kernel.org, kvm@vger.kernel.org, linux-coco@lists.linux.dev,
 linux-mm@kvack.org, linux-crypto@vger.kernel.org,
 linux-kernel@vger.kernel.org, tglx@linutronix.de, mingo@redhat.com,
 jroedel@suse.de, thomas.lendacky@amd.com, hpa@zytor.com, ardb@kernel.org,
 pbonzini@redhat.com, seanjc@google.com, vkuznets@redhat.com,
 jmattson@google.com, luto@kernel.org, dave.hansen@linux.intel.com,
 slp@redhat.com, pgonda@google.com, peterz@infradead.org,
 srinivas.pandruvada@linux.intel.com, rientjes@google.com, tobin@ibm.com,
 vbabka@suse.cz, kirill@shutemov.name, ak@linux.intel.com,
 tony.luck@intel.com, sathyanarayanan.kuppuswamy@linux.intel.com,
 alpergun@google.com, jarkko@kernel.org, nikunj.dadhania@amd.com,
 pankaj.gupta@amd.com, liam.merwick@oracle.com
References: <20231230161954.569267-1-michael.roth@amd.com>
 <20231230161954.569267-22-michael.roth@amd.com>
 <20240121114900.GLZa0ErBHIqvook5zK@fat_crate.local>
From: "Kalra, Ashish" <ashish.kalra@amd.com>
In-Reply-To: <20240121114900.GLZa0ErBHIqvook5zK@fat_crate.local>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?QjBYbDF2YkJpUUl0dk9tRFdUUVl6T0Z2eDhRK2JubUhnR3FJTm1QeStpc3R1?=
 =?utf-8?B?aDlidXAzRVZOSERudHovR0lpVVZSbEhSVzA3UncxZjFQcDRxSFNISjhVcTBI?=
 =?utf-8?B?TjBNQ1lmeERxOUFlb0w4SVNtSEJyM3dTbHJ4RGl4RFJhTHhFRldqT2NYMHZn?=
 =?utf-8?B?S0tkRXNLVXR3K2tSRWtDd1FXcGZ4elRkSWFzcVNRZTdITWNuK0JDVnIxS0Zs?=
 =?utf-8?B?aytjcXFwMlMxRUJET2FZL1hTZDAzV0U2S0VXck82clgyc0NjSDdpajNHT0FZ?=
 =?utf-8?B?Q1MwNE9rSlFsY3Q1RTRKZnNLWGRjajFoaGxWMHR4S1J3UzU1TGRvK2ZJb2ta?=
 =?utf-8?B?M1NGVFBmeFh0MHhRbHlCbDFGT2xITGJEeTVwVW9aKzJWaUk0YS9KakdWRU1v?=
 =?utf-8?B?MlpIcHhVYVNyaUh3bEFtN2JDdkZ6REVTTXFVTEM0ZDhFd3VLUUhIMWxhbUk2?=
 =?utf-8?B?REpTYzRZQ05SWmNHellqeGc2Qm13RDRCUms3VHkrYjNFQWhxWm84SmpBZnFZ?=
 =?utf-8?B?eHNyYURPbHdHa3k4VmppZkZxSFpVVWxIY2RzbmZQeUl1U3lvcWZ0MHI4TlFG?=
 =?utf-8?B?SzkrUlh6dy81UEFZRlFsejVnNEF6ejM5WllYRDBNUTRiWlRNMGo0ZmU1dk1J?=
 =?utf-8?B?VCtWdVZzaE8zYjA0emljT0ZqWlE4cHNiRGZ3YXpDSnBoak9GN0wyTy9qTFZi?=
 =?utf-8?B?TFlBN1FzNXlXT0x1U2k3dnl1WFQ4S0tqOVJ2aVE0OTV2WU1sWTRmc1cwZi94?=
 =?utf-8?B?TDBCY1dhaGY4R2J6amtQQ1Vkcm0zKzFQNExoNkJWUDZRS3c3ZHBIeEFWZm9K?=
 =?utf-8?B?VmdNK00vbW11cVl3QWM2ZVArdGVsNStxcytYZVRqbmxwdlNQa1JqT0FhTkls?=
 =?utf-8?B?eVFvSE5QaXY2Tmt5RGVpVGNkWmpTaVdTVzZDNUtJbEpxTHV5eHZQaERQOUNU?=
 =?utf-8?B?bzREa3NQd0YveUlFeXpBM1o1MGpmY1lUdVo3VVc4UHYyVjlOOUtQSlg3bm5r?=
 =?utf-8?B?NThETmpiL2ZHNjViZ2hZT3d6RGhaaTNWcjNiYS9sS01RQnZpZzFYNE9rUXhY?=
 =?utf-8?B?cCtlSkVOZE9Bcm1IY25OZUNjb05FUm80VUtxM3hrTktrSEpHUkxVRlhnVUNn?=
 =?utf-8?B?VUxpVVIzYTJ6aXlUT1BPZWdraVpic1k4VkFSYXlpT3hoaTJDT1BEd2pRNFhk?=
 =?utf-8?B?WU5DNmtiNGd0MzRlcjlVbmFoT0dNWEhuVFdGWmVsMUEzbGtZRUZWaWR0Z0dz?=
 =?utf-8?B?YnJIVjduUy8rWkhGTWtZMjU3YjNzMUFFZ0k1YnY2MVcrY1BPSTZLMG9qeW80?=
 =?utf-8?B?bDYzSlBPNkkxM0JoMEE4enVkWTVmTjhaNDByMlFaV2IwMExHWmhlakFvRSt0?=
 =?utf-8?B?aFpxc09IWmdUeEVCY0l0Q2pNTWEyTjFQa1JlQzU1cHU4SEZNRWJadWdqMWs1?=
 =?utf-8?B?NmxvejZBM0lSQ0VEZU9kclFoY0swMDhya0JwNWNkb0dTek1pczdWQnZvY25F?=
 =?utf-8?B?cmpYcU9uWnZXdm9nNHdJYlE0NWUrd3JxTGg1Y2VlZGJoWnFxSUNQWnNlSGpz?=
 =?utf-8?B?RkJVZlZMYy9CdnVZbXd6V3E3YlB0a1BZbUF0bzFWd3R6b20wWEZHYnhBbDhB?=
 =?utf-8?B?MkdJUXdGaXBVM1FWT29XUDZ1aUoyRTN3aUM3Tnl6NDFtbnFrcW1DU0U5cFB2?=
 =?utf-8?B?S0YwM1p3dGEyWklGYlZqVmhKNzIxYnRZZHp4QmtWZUxROWV1cm1DbmhxZ1Nv?=
 =?utf-8?B?aU1jSGJOUUdBRlZ6WWJSZ1pSa2dieVl3dVR2VGd2TENRS3h0OWEyRHpzRXp5?=
 =?utf-8?B?MDRxbWM2TWphQTgrRVUxVEpQNGhuOGIvczBtY2tOYk9ZRU43c2Erd01Cam9P?=
 =?utf-8?B?MndhdGRqNFc5Wkk1WWdOTDJvU1hjdWhPQW9weDdnT2YwN09FK0IzUTRraStm?=
 =?utf-8?B?ZG1PV3pvSERIWVVsZnBpTy9kNHlnd0I1WDA5OG9IU0MvVGZCU2xaSjBXYkMw?=
 =?utf-8?B?YklPaGk2bEpENU1ldWJlVjV5MXZJd0RVWSt0VWRydzdkbm9XUENPSE1Pa3Iv?=
 =?utf-8?B?WFpMQ3ZpS1hNNmU3eTFDRnlPcUwyRDJ0V3pQUDhmc2FUQnFlbGVRWkVYaGh3?=
 =?utf-8?Q?z2Q2tVkRfEZBKx01jIE8Ucgjg?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 52d64228-b167-4f8f-2847-08dc1e1b6a5e
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2767.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jan 2024 03:03:47.8519
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 7X/bX7Mf4MDGDXFyo5lU7AnDVZRz0vAM1GnJWBBGHr4tKprifQaGwkEiGwEFSJWgb8OILuTL+iDgNxTILiFaMw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR12MB7998

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