[RFC PATCH] target/i386: SEV: Allow pflash devices with SEV-ES guests

["Naveen N Rao (AMD)" <naveen@kernel.org>]

KVM commit 66155de93bcf ("KVM: x86: Disallow read-only memslots for
SEV-ES and SEV-SNP (and TDX)"), and the subsequent commit d30d9ee94cc0
("KVM: x86: Only advertise KVM_CAP_READONLY_MEM when supported by VM")
stopped advertising KVM_CAP_READONLY_MEM support for encrypted guests
(KVM_X86_SEV_ES_VM and KVM_X86_SNP_VM), but not for KVM_X86_DEFAULT_VM
type SEV-ES guests. As a result of this, it is no longer possible to
start SEV-ES guests with any SEV feature enabled (in particular,
debug-swap) with pflash devices.

This is an issue since SEV-ES guests have historically used pflash
devices for OVMF. Guests on older KVM+Qemu are able to enable debug-swap
while using pflash devices, so work around the KVM limitation by
switching to using a VMA-based write protection. This allows
well-behaved SEV-ES guests to continue to work with pflash devices and
enable debug-swap. Mis-behaving guests trying to write to the protected
OVMF area will be killed.

Enable VMA protection and set the memory to be RO when adding the KVM
memory slot. Because pflash devices support command-mode, change VMA
protection to RW when tearing down the KVM memory slot. KVM
SEV_LAUNCH_UPDATE also requires memory to be RW, so disable the
protection when calling that.

Print a warning when switching to VMA-based protection so that it is
clear that KVM itself isn't supporting readonly memory, and that a
workaround is in place. Users can plan on switching to using '-bios'.

Finally, drop the check rejecting SEV-ES guests with SEV features so 
that debug-swap can be enabled.


Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
Background discussion on this issue:
http://lore.kernel.org/r/aUq9_cUDWeEW_qli@google.com

This series depends on VMSA features support:
http://lore.kernel.org/r/cover.1779281646.git.naveen@kernel.org


- Naveen


 include/system/kvm.h     |  5 ++++
 include/system/kvm_int.h |  1 +
 accel/kvm/kvm-all.c      | 52 +++++++++++++++++++++++++++++++++++++++-
 hw/i386/pc_sysfw.c       | 19 +++++++++------
 target/i386/sev.c        | 21 +++++++++++-----
 5 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/include/system/kvm.h b/include/system/kvm.h
index 5fa33eddda38..585058bd6f1c 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -555,6 +555,8 @@ uint32_t kvm_dirty_ring_size(void);
 
 void kvm_mark_guest_state_protected(void);
 
+void kvm_enable_ro_mem_vma_protection(void);
+
 /**
  * kvm_hwpoisoned_mem - indicate if there is any hwpoisoned page
  * reported for the VM.
@@ -568,6 +570,9 @@ int kvm_set_memory_attributes_shared(hwaddr start, uint64_t size);
 
 int kvm_convert_memory(hwaddr start, hwaddr size, bool to_private);
 
+void kvm_set_memory_readonly(void *addr, size_t len);
+void kvm_set_memory_readwrite(void *addr, size_t len);
+
 /* argument to vmfd change notifier */
 typedef struct VmfdChangeNotifier {
     int vmfd;
diff --git a/include/system/kvm_int.h b/include/system/kvm_int.h
index 0876aac938d3..0e083a56ce2a 100644
--- a/include/system/kvm_int.h
+++ b/include/system/kvm_int.h
@@ -123,6 +123,7 @@ struct KVMState
     OnOffAuto kernel_irqchip_split;
     bool sync_mmu;
     bool guest_state_protected;
+    bool guest_wants_ro_mem_vma_protection;
     uint64_t manual_dirty_log_protect;
     /*
      * Older POSIX says that ioctl numbers are signed int, but in
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 96f90ebb240f..4208df5b25ac 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1629,6 +1629,42 @@ int kvm_set_memory_attributes_shared(hwaddr start, uint64_t size)
     return kvm_set_memory_attributes(start, size, 0);
 }
 
+static void kvm_set_memory_flags(void *addr, size_t len, int flags)
+{
+    if (mprotect(addr, len, flags)) {
+        error_report("failed to apply memory protection "
+                     "(0x%" HWADDR_PRIx "+0x%" PRIx64 ") error '%s'",
+                     (hwaddr)addr, len, strerror(errno));
+        exit(1);
+    }
+}
+
+void kvm_set_memory_readonly(void *addr, size_t len)
+{
+    if (kvm_state->guest_wants_ro_mem_vma_protection) {
+        kvm_set_memory_flags(addr, len, PROT_READ);
+    }
+}
+
+void kvm_set_memory_readwrite(void *addr, size_t len)
+{
+    if (kvm_state->guest_wants_ro_mem_vma_protection) {
+        kvm_set_memory_flags(addr, len, PROT_READ | PROT_WRITE);
+    }
+}
+
+static bool kvm_mem_wants_vma_protection(MemoryRegion *mr)
+{
+    if (!memory_region_is_ram(mr) &&
+        (mr->readonly || mr->rom_device) &&
+        !kvm_readonly_mem_allowed &&
+        kvm_state->guest_wants_ro_mem_vma_protection) {
+            return true;
+    }
+
+    return false;
+}
+
 /* Called with KVMMemoryListener.slots_lock held */
 static void kvm_set_phys_mem(KVMMemoryListener *kml,
                              MemoryRegionSection *section, bool add)
@@ -1642,7 +1678,8 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     void *ram;
 
     if (!memory_region_is_ram(mr)) {
-        if (writable || !kvm_readonly_mem_allowed) {
+        if (writable || (!kvm_readonly_mem_allowed &&
+                         !kvm_state->guest_wants_ro_mem_vma_protection)) {
             return;
         } else if (!mr->romd_mode) {
             /* If the memory device is not in romd_mode, then we actually want
@@ -1697,6 +1734,10 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
                 kvm_slot_sync_dirty_pages(mem);
             }
 
+            if (kvm_mem_wants_vma_protection(mr)) {
+                kvm_set_memory_readwrite(mem->ram, mem->memory_size);
+            }
+
             /* unregister the slot */
             g_free(mem->dirty_bmap);
             mem->dirty_bmap = NULL;
@@ -1746,6 +1787,10 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
             }
         }
 
+        if (kvm_mem_wants_vma_protection(mr)) {
+            kvm_set_memory_readonly(mem->ram, mem->memory_size);
+        }
+
         start_addr += slot_size;
         ram_start_offset += slot_size;
         ram += slot_size;
@@ -4771,6 +4816,11 @@ void kvm_mark_guest_state_protected(void)
     kvm_state->guest_state_protected = true;
 }
 
+void kvm_enable_ro_mem_vma_protection(void)
+{
+    kvm_state->guest_wants_ro_mem_vma_protection = true;
+}
+
 int kvm_create_guest_memfd(uint64_t size, uint64_t flags, Error **errp)
 {
     int fd;
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 1a41a5972bd0..9590458e00c5 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -254,13 +254,18 @@ void pc_system_firmware_init(PCMachineState *pcms,
         }
     } else {
         if (kvm_enabled() && !kvm_readonly_mem_enabled()) {
-            /*
-             * Older KVM cannot execute from device memory. So, flash
-             * memory cannot be used unless the readonly memory kvm
-             * capability is present.
-             */
-            error_report("pflash with kvm requires KVM readonly memory support");
-            exit(1);
+            if (sev_es_enabled() && !sev_snp_enabled()) {
+                warn_report("pflash not supported with SEV-ES guests, "
+                            "attempting VMA based protection");
+            } else {
+                /*
+                 * Older KVM cannot execute from device memory. So, flash
+                 * memory cannot be used unless the readonly memory kvm
+                 * capability is present.
+                 */
+                error_report("pflash with kvm requires KVM readonly memory support");
+                exit(1);
+            }
         }
 
         pc_system_flash_map(pcms, rom_memory);
diff --git a/target/i386/sev.c b/target/i386/sev.c
index f04ae4e91f3e..82cf2c562729 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -550,12 +550,6 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
             return -1;
         }
     } else {
-        if (sev_features && sev_es_enabled()) {
-            error_setg(errp,
-                       "%s: SEV features are not supported with SEV-ES at this time",
-                       __func__);
-            return -1;
-        }
         if (sev_features & SVM_SEV_FEAT_SNP_ACTIVE) {
             error_setg(errp,
                        "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
@@ -2024,6 +2018,15 @@ static int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
         return -1;
     }
 
+    /*
+     * Use VMA-based protection for SEV-ES guests that enable any
+     * SEV feature, since KVM does not advertise readonly memory
+     * support for non-default type SEV guests.
+     */
+    if (sev_es_enabled() && SEV_COMMON(cgs)->sev_features) {
+        kvm_enable_ro_mem_vma_protection();
+    }
+
     if (!cgs->ready) {
         /*
          * SEV uses these notifiers to register/pin pages prior to guest use,
@@ -2111,7 +2114,13 @@ sev_encrypt_flash(hwaddr gpa, uint8_t *ptr, uint64_t len, Error **errp)
     if (sev_check_state(sev_common, SEV_STATE_LAUNCH_UPDATE)) {
         int ret;
 
+        /*
+         * KVM requires these pages to be RW, so remove VMA RO protection
+         * for the duration of SEV_LAUNCH_UPDATE if using SEV features.
+         */
+        kvm_set_memory_readwrite(ptr, len);
         ret = klass->launch_update_data(sev_common, gpa, ptr, len, errp);
+        kvm_set_memory_readonly(ptr, len);
         if (ret < 0) {
             return ret;
         }

base-commit: 5611a9268dae7b7ff99d478ed134052a9fc7e9f7
prerequisite-patch-id: dc27ad6297d47d063b04fa797c1b8203ee97d9c8
prerequisite-patch-id: 603eff49233c4b0483e7c405754b95aa455dd38c
prerequisite-patch-id: d4085e72ecfb0fbf977f7358d1edd29951b93784
prerequisite-patch-id: f6f201825b1e56f89a87a26bc457b3e6018aee49
prerequisite-patch-id: 08d79cec4c3f117178be8f6c866ff1be08e971f3
prerequisite-patch-id: c112bccab9ab9cee2d0227516fc857590b99a75b
prerequisite-patch-id: f42fd829d0ea4909537e722477057a4013a247ab
prerequisite-patch-id: 67136368ed1d2fa0ae55fee4368a7bd1fe394368
prerequisite-patch-id: 08349bb1e0e11ee1518a9041771302c97866b5cd
-- 
2.54.0