Received-SPF: pass (google.com: domain of linux-crypto+bounces-1610-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C
Date: Thu, 25 Jan 2024 21:32:22 -0600
From: Michael Roth <michael.roth@amd.com>
To: Borislav Petkov <bp@alien8.de>
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>, <ashish.kalra@amd.com>, <nikunj.dadhania@amd.com>,
	<pankaj.gupta@amd.com>, "liam.merwick@oracle.com Brijesh Singh"
	<brijesh.singh@amd.com>
Subject: Re: [PATCH v1 24/26] crypto: ccp: Add the SNP_PLATFORM_STATUS command
Message-ID: <20240126033222.roi6j6pqv7s6mk2c@amd.com>
References: <20231230161954.569267-1-michael.roth@amd.com>
 <20231230161954.569267-25-michael.roth@amd.com>
 <20240121122903.GNZa0OD21W0UxLmOAm@fat_crate.local>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240121122903.GNZa0OD21W0UxLmOAm@fat_crate.local>
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jan 2024 04:13:39.5897
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 231d92b8-9de6-4aa3-0c67-08dc1e252cff
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com]
X-MS-Exchange-CrossTenant-AuthSource:
	SJ1PEPF00001CE3.namprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR12MB8205

On Sun, Jan 21, 2024 at 01:29:20PM +0100, Borislav Petkov wrote:
> On Sat, Dec 30, 2023 at 10:19:52AM -0600, Michael Roth wrote:
> > +	/* Change the page state before accessing it */
> > +	if (snp_reclaim_pages(__pa(data), 1, true)) {
> > +		snp_leak_pages(__pa(data) >> PAGE_SHIFT, 1);
> > +		return -EFAULT;
> > +	}
> 
> This looks weird and it doesn't explain why this needs to happen.
> SNP_PLATFORM_STATUS text doesn't explain either.
> 
> So, what's up?

I've adding some clarifying comment in v2, but the page that firmware
writes needs to first be switched to the Firmware-owned state, and
after successful completion it will be put in Reclaim state. But it's
possible a failure might occur before that transition is made by
firmware, maybe the command fails somewhere in the callstack before it
even reaches firmware.

If that happens the page might still be in firmware-owned state, and
need to go through snp_reclaim_pages()/SNP_PAGE_RECLAIM before it can
be switched back to Default state.

Rather than trying to special-case all these possibilities, it's simpler
to just always use snp_reclaim_pages(), which will handle both Reclaim
and Firmware-owned pages.

However, snp_reclaim_pages() will already leak the page when necessary,
so I've dropped that bit.

-Mike

> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette