Re: [RFC PATCH 5/6] i386/sev: add support to encrypt BIOS when SEV-SNP is enabled

Dov Murik
2021-07-19T13:01:14Z

On 10/07/2021 0:55, Brijesh Singh wrote:
> The KVM_SEV_SNP_LAUNCH_UPDATE command is used for encrypting the bios
> image used for booting the SEV-SNP guest.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> target/i386/sev.c | 33 ++++++++++++++++++++++++++++++++-
> target/i386/trace-events | 1 +
> 2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 259408a8f1..41dcb084d1 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -883,6 +883,30 @@ out:
> return ret;
> }
>
> +static int
> +sev_snp_launch_update(SevGuestState *sev, uint8_t *addr, uint64_t len, int type)

This seems similar to the SEV LAUNCH_UPDATE_DATA API (with the added
`type` argument). In SEV API these are the limitations (from the SEV
API document):

* PADDR - System physical address of the data to be encrypted.
Must be 16 B aligned.
* LENGTH - Length of the data to be encrypted.
Must be a multiple of 16 B.

But in SNP_LAUNCH_UPDATE it is my understanding that addr must be page
aligned (4KB) and length must be in whole pages (because the underlying
types are PAGE_TYPE_NORMAL, PAGE_TYPE_ZERO, ...).

So what happens if we call sev_encrypt_flash with a non-page-aligned
addr / length?

Or maybe I misunderstood the SNP ABI document?

-Dov

> +{
> + int ret, fw_error;
> + struct kvm_sev_snp_launch_update update = {};
> +
> + if (!addr || !len) {
> + return 1;
> + }
> +
> + update.uaddr = (__u64)(unsigned long)addr;
> + update.len = len;
> + update.page_type = type;
> + trace_kvm_sev_snp_launch_update(addr, len, type);
> + ret = sev_ioctl(sev->sev_fd, KVM_SEV_SNP_LAUNCH_UPDATE,
> + &update, &fw_error);
> + if (ret) {
> + error_report("%s: SNP_LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
> + __func__, ret, fw_error, fw_error_to_str(fw_error));
> + }
> +
> + return ret;
> +}
> +
> static int
> sev_launch_update_data(SevGuestState *sev, uint8_t *addr, uint64_t len)
> {
> @@ -1161,7 +1185,14 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error **errp)
>
> /* if SEV is in update state then encrypt the data else do nothing */
> if (sev_check_state(sev_guest, SEV_STATE_LAUNCH_UPDATE)) {
> - int ret = sev_launch_update_data(sev_guest, ptr, len);
> + int ret;
> +
> + if (sev_snp_enabled()) {
> + ret = sev_snp_launch_update(sev_guest, ptr, len,
> + KVM_SEV_SNP_PAGE_TYPE_NORMAL);
> + } else {
> + ret = sev_launch_update_data(sev_guest, ptr, len);
> + }
> if (ret < 0) {
> error_setg(errp, "failed to encrypt pflash rom");
> return ret;
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 18cc14b956..0c2d250206 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -12,3 +12,4 @@ kvm_sev_launch_finish(void) ""
> kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
> kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s"
> kvm_sev_snp_launch_start(uint64_t policy) "policy 0x%" PRIx64
> +kvm_sev_snp_launch_update(void *addr, uint64_t len, int type) "addr %p len 0x%" PRIx64 " type %d"
>