Message-ID: <cb551b5a-6a80-37bf-0179-a623861a6a7c@amd.com>
Date:   Fri, 27 Jan 2023 16:46:31 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH RFC 3/8] KVM: SVM: write back corrected CPUID page
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@profian.com>,
        Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "H. Peter Anvin" <hpa@zytor.com>
Cc:     Harald Hoyer <harald@profian.com>, Tom Dohrmann <erbse.13@gmx.de>,
        Ashish Kalra <ashish.kalra@amd.com>,
        Michael Roth <michael.roth@amd.com>,
        "open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)" 
        <kvm@vger.kernel.org>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
References: <20230127025237.269680-1-jarkko@profian.com>
 <20230127025237.269680-4-jarkko@profian.com>
From:   Tom Lendacky <thomas.lendacky@amd.com>
In-Reply-To: <20230127025237.269680-4-jarkko@profian.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d3BuZkFBV2htT0thQ2E4bk5lc0o4a3BZTWVkaVRMK3ovS3ZiRHd1cFVKOU0r?=
 =?utf-8?B?b3E3MVZ5ay9EL0lONHhXUE9QM01uMjlzMjY4SWw4Q3pvcDM2OWpDL0o1dEtV?=
 =?utf-8?B?K1lvRzhQVHYxUzVsZWFGZExZNE01M2Z2blJPanpldU9pR0dXUDU1SFlIbmF5?=
 =?utf-8?B?S2h6T1VIYWxSaER6cFJiekMzVVBIaEJLL3lWcmRlVTRRWXh3MlAvUlFTOWti?=
 =?utf-8?B?MXhZN3RycW5CTmJhYXNRdlpzMEZEZldaOU1OUlJPbFRubDltd1d2OVErejVY?=
 =?utf-8?B?Q3crN3RMZk1BYkpsNS9ldUtDT1BaR1N0ZVdWdXAzNENYRkRuVHpvTGQxUTBC?=
 =?utf-8?B?aDkvYUkxRXYwS3pzSHVvV3UzUVd4cXRQNjYxdlpRcFgxeThuT2kvVnBsUmN3?=
 =?utf-8?B?VDRzSVppcEJDSXhTRjN3RmlMOEk3WmFEZ3cxS2FKSXA3Nm53by94OGxoT2hS?=
 =?utf-8?B?VGNOOHV1WnBiN1FSU3FKZWZteWxTekh5ZCt6V2NmL05XUXFaanB1NkVSVXEx?=
 =?utf-8?B?SEhWd2N5Y1ZsYnhKR0VrV0J4R0Q5MkV5U0ZqUmRhV1czL2U0MUMxV2k1aVND?=
 =?utf-8?B?dXdmc0NqdkxzWWdaWjliQWg0b3FHSHNESFZKUFhTZ04zUjdzZTRFbk5EWlBU?=
 =?utf-8?B?bklDdWh5eTA0THVJTW5iN0N6SUsxcG5CZGNPU1lDY29SOGRTMUU1M3p6c2I1?=
 =?utf-8?B?WmcvajRFQXNudlV3U0E3RjNwNHpmSUtodyttT3pWNkx5NitNTE1tYmRjTE9W?=
 =?utf-8?B?U0ZqQ1lHTEhOeUkxdXVTUStVU0dlbld6NUZ0RTRRcWVvRmhPbVZHaHk5Umpl?=
 =?utf-8?B?V0pOR2dWSVRSdUxualhkVFJ4OVFjamhuMEpjeURITGdCUUV0cjZCQ3duR1g2?=
 =?utf-8?B?aTBvb3lvQW9HVGtqWkQzcUFpL0x6VmFMdlZNTU9LVm9xWG1jZ1pKL0x2OXJx?=
 =?utf-8?B?NjNkcGpLWmV3R2ZtZmRPbXJOdG9rbG85cndTQmc0Tm5jS0hPWUJMWitONDlI?=
 =?utf-8?B?SFBlTkhkNWJnT01YYXRoeFE4V0FzZkgwT0Z5a1lZa2M5eDRGWEtaUzlaT0NZ?=
 =?utf-8?B?T24wUEJNRncxV1dFVUVlc1BzNHJYcFByd05zUDNOSmY2dEFjdHYrbHlTeXVN?=
 =?utf-8?B?bTdET2c5Ti9JTmFXYStOTnd0OFROMDhDTmE0RS9Rd2RzSmI4SkRHcm4zQjhu?=
 =?utf-8?B?QkhRUm1kQnN6dWQrU2JyS2VUSkcvd3podHNwblQ0QlMvbldHRHVUU2VVd1pK?=
 =?utf-8?B?N1g4SFA0eGZsWmZ2UXB1UGdjUXh4Y290bmZ2U1V6ZC9TbnR5cGZSZEw5UmN6?=
 =?utf-8?B?ZWJWdmFlTlkySW9VaEMwLzAxWTgvWW9MRS9JaVRWaUlYNHgvSUhpNCs4bm1S?=
 =?utf-8?B?c3BLNksrQUozWVk2TzYvNGJPNlI0T0lTZk1ORUphaDhHb2c0TWduQ0h3Vysy?=
 =?utf-8?B?VXpnQmNtaWVZZnFrMmYwVXE0WUw2bkhWQTloelIvYjZwdVZibW0rRlhGZzJS?=
 =?utf-8?B?UkhGUHpsS3I5Z0M1SkliTzNncUVlejlSSXFSZDk3bjBoTUROWlE2OG1BMzdK?=
 =?utf-8?B?eFhrTGpteDBaeFdaVHc5SUlQL013K0xSOUVPaDZrQmViQVg4cHl6TC9MSEp0?=
 =?utf-8?B?UzZjUnRnUUtjdEl5TWY4elZHMEJZN1dTOUlKa1JGdGhKalQreGc2a3Q0Z3Vo?=
 =?utf-8?B?Umo2c2VVenZFTFlIL3ZvQlV2TnFRdGt4WDRRRmFzcW5SNHNWWDVPMzdPSVZD?=
 =?utf-8?B?T2M1SFBjUDZVd29hT01nM2FwUXU5SkE3dXJnWjgrYm1aVnVnYk12cGNtMklK?=
 =?utf-8?B?YUcycW10V1BjRXRwU2dkQXNPdVBScWZPbXhnbGpNaTRtN2NINFgzNVVTbEtY?=
 =?utf-8?B?Y3NvZm5BUXRSOWlkNmthcWI0NlF1T2hNZVBDNFphTExjN3ZmQ1NwRU5VTTNI?=
 =?utf-8?B?T3dnVFdTazRpR2liOTAyblAxUndXMU9mOS9EbVJJVVJsSU9qQ1BPU3NRWW1t?=
 =?utf-8?B?Rm1XeFNFYk5SOHpsN2dVNWFlYk52bzg2a2FtSFZ2eVkzQ2lsNlVOZklYNVVh?=
 =?utf-8?B?d3NRdkN6QW8wM0NDQXJjQ2NmR0Q0SDIwVVc3YmlUUm1HdG5xUzhEcDEycWh5?=
 =?utf-8?Q?uyMgNyXQGjhrGzm5CrGOgaCaD?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e876b198-1033-4aa6-f949-08db00b85756
X-MS-Exchange-CrossTenant-AuthSource: DM4PR12MB5229.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2023 22:46:34.4387
 (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: hZY5yn/ND7VNiEm9AkTlFQ+jSfl+mtMYqJOiiYDMWvbiyqowpVZwT2ktTGRndbO8wgf3bHv+GVN+5ol9XwE2pw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7492
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/26/23 20:52, Jarkko Sakkinen wrote:
> From: Tom Dohrmann <erbse.13@gmx.de>
> 
> When doing a launch update for a CPUID page the firmware checks that the values
> conform to the policy laid out in the processor programming manual. If the
> values don't conform, the firmware will return SEV_RET_INVALID_PARAM.
> In addition to returning an error the firmware will choose some acceptable
> values and write them back to the page that was used for the launch update, so
> that the VMM can inspect the changes and try again with the corrected values.
> This is specified in section 8.17.2.6 in the SEV-SNP Firmware ABI spec.
> Because launch updates are always done on the private UPM mappings, the pages
> are first copied from the shared mappings to the private mappings. When the
> firmware corrects the values, the corrected values are in the private mappings,
> inaccessible to userspace. In order to make the corrected values accessible to
> userspace, the page containing them must be copied from the private mappings
> back to the shared mappings.
> 
> [jarkko@profian.com: fixed checkpatch.pl errors]
> Link: https://lore.kernel.org/lkml/Y76%2FI6Nrh7xEAAwv@notebook/
> Signed-off-by: Tom Dohrmann <erbse.13@gmx.de>
> Signed-off-by: Jarkko Sakkinen <jarkko@profian.com>
> ---
>   arch/x86/kvm/svm/sev.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6d3162853c33..4a8e552d8cfe 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2230,6 +2230,23 @@ static int snp_launch_update_gfn_handler(struct kvm *kvm,
>   			pr_err("SEV-SNP launch update failed, ret: 0x%x, fw_error: 0x%x\n",
>   			       ret, *error);
>   			snp_page_reclaim(pfns[i]);
> +
> +			/*
> +			 * When invalid CPUID function entries are detected, the firmware
> +			 * corrects these entries.  In that case write the page back to
> +			 * userspace.

I would additionally add that the firmware does not encrypt the page, 
which allows the hypervisor to copy the page back to userspace.

Thanks,
Tom

> +			 */
> +			if (params.page_type == SNP_PAGE_TYPE_CPUID &&
> +			    *error == SEV_RET_INVALID_PARAM) {
> +				int ret;
> +
> +				host_rmp_make_shared(pfns[i], PG_LEVEL_4K, true);
> +
> +				ret = kvm_write_guest_page(kvm, gfn, kvaddr, 0, PAGE_SIZE);
> +				if (ret)
> +					pr_err("Guest write failed, ret: 0x%x\n", ret);
> +			}
> +
>   			goto e_release;
>   		}
>   	}