Re: [PATCH RFC 3/8] KVM: SVM: write back corrected CPUID page

[Tom Lendacky <thomas.lendacky@amd.com>]

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;
>   		}
>   	}