[PATCH for-9.1 v1 0/3] Add SEV/SEV-ES machine compat options for KVM_SEV_INIT2

[PATCH for-9.1 v1 0/3] Add SEV/SEV-ES machine compat options for KVM_SEV_INIT2

[Michael Roth]

These patches are also available at:

  https://github.com/amdese/qemu/commits/sev-init-legacy-v1

and are based on top Paolo's qemu-coco-queue branch containing the
following patches:

  [PATCH for-9.1 00/26] x86, kvm: common confidential computing subset
  https://lore.kernel.org/all/20240322181116.1228416-1-pbonzini@redhat.com/T/

Overview
--------

With the following patches applied from qemu-coco-queue:

  https://lore.kernel.org/all/20240319140000.1014247-1-pbonzini@redhat.com/

QEMU version 9.1+ will begin automatically making use of the new
KVM_SEV_INIT2 API for initializing SEV and SEV-ES (and eventually, SEV-SNP)
guests verses the older KVM_SEV_INIT/KVM_SEV_ES_INIT interfaces.

However, the older interfaces would silently avoid sync'ing FPU/XSAVE state
set by QEMU to each vCPU's VMSA prior to encryption. With KVM_SEV_INIT2,
this state will now be synced into the VMSA, resulting in measurements
changes and, theoretically, behaviorial changes, though the latter are
unlikely to be seen in practice. The specific VMSA changes are documented
in the section below for reference.

This series implements machine compatibility options for SEV/SEV-ES so that
only VMs created with QEMU 9.1+ will make use of KVM_SEV_INIT2 so that VMSA
differences can be accounted for beforehand, and older machine types will
continue using the older interfaces to avoid unexpected measurement
changes.

Specific VMSA changes
---------------------

With KVM_SEV_INIT2, rather than 0, QEMU/KVM will instead begin setting the
following fields in the VMSA before measurement/encryption:

  VMSA byte offset [1032:1033] = 80 1f (MXCSR, Multimedia Control Status
                                        Register)
  VMSA byte offset [1040:1041] = 7f 03 (FCW, FPU/x86 Control Word)

Setting FCW (FPU/x86 Control Word) to 0x37f is consistent with 11.5.7 of
APM Volume 2. MXCSR reset state is not defined for XSAVE, but QEMU's 0x1f80
value is consistent with machine reset state documented in APM Volume 2
4.2.2. As such, it is reasonable to begin including these in the VMSA
measurement calculations.

NOTE: section 11.5.7 also documents that FTW should be all 1's, whereas
      QEMU currently sets all zeroes. Should that be changed as part of
      this, or are there other reasons for setting 0?

Thanks,

Mike

----------------------------------------------------------------
Michael Roth (3):
      i386/sev: Add 'legacy-vm-type' parameter for SEV guest objects
      hw/i386: Add 9.1 machine types for i440fx/q35
      hw/i386/sev: Use legacy SEV VM types for older machine types

 hw/i386/pc.c         |  5 +++++
 hw/i386/pc_piix.c    | 13 ++++++++++++-
 hw/i386/pc_q35.c     | 12 +++++++++++-
 include/hw/i386/pc.h |  3 +++
 qapi/qom.json        | 11 ++++++++++-
 target/i386/sev.c    | 19 ++++++++++++++++++-
 6 files changed, 59 insertions(+), 4 deletions(-)

[PATCH v1 1/3] i386/sev: Add 'legacy-vm-type' parameter for SEV guest objects

[Michael Roth]

QEMU will currently automatically make use of the KVM_SEV_INIT2 API for
initializing SEV and SEV-ES guests verses the older
KVM_SEV_INIT/KVM_SEV_ES_INIT interfaces.

However, the older interfaces will silently avoid sync'ing FPU/XSAVE
state to the VMSA prior to encryption, thus relying on behavior and
measurements that assume the related fields to be allow zero.

With KVM_SEV_INIT2, this state is now synced into the VMSA, resulting in
measurements changes and, theoretically, behaviorial changes, though the
latter are unlikely to be seen in practice.

To allow a smooth transition to the newer interface, while still
providing a mechanism to maintain backward compatibility with VMs
created using the older interfaces, provide a new command-line
parameter:

  -object sev-guest,legacy-vm-type=true,...

and have it default to false.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qapi/qom.json     | 11 ++++++++++-
 target/i386/sev.c | 18 +++++++++++++++++-
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index baae3a183f..8f2f75bde6 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -898,6 +898,14 @@
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)
 #
+# @legacy-vm-type: Use legacy KVM_SEV_INIT KVM interface for creating the VM.
+#                  The newer KVM_SEV_INIT2 interface syncs additional vCPU
+#                  state when initializing the VMSA structures, which will
+#                  result in a different guest measurement. Set this to
+#                  maintain compatibility with older QEMU or kernel versions
+#                  that rely on legacy KVM_SEV_INIT behavior.
+#                  (default: false) (since 9.1)
+#
 # Since: 2.12
 ##
 { 'struct': 'SevGuestProperties',
@@ -908,7 +916,8 @@
             '*handle': 'uint32',
             '*cbitpos': 'uint32',
             'reduced-phys-bits': 'uint32',
-            '*kernel-hashes': 'bool' } }
+            '*kernel-hashes': 'bool',
+            '*legacy-vm-type': 'bool' } }
 
 ##
 # @ThreadContextProperties:
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9dab4060b8..f4ee317cb0 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -67,6 +67,7 @@ struct SevGuestState {
     uint32_t cbitpos;
     uint32_t reduced_phys_bits;
     bool kernel_hashes;
+    bool legacy_vm_type;
 
     /* runtime state */
     uint32_t handle;
@@ -356,6 +357,16 @@ static void sev_guest_set_kernel_hashes(Object *obj, bool value, Error **errp)
     sev->kernel_hashes = value;
 }
 
+static bool sev_guest_get_legacy_vm_type(Object *obj, Error **errp)
+{
+    return SEV_GUEST(obj)->legacy_vm_type;
+}
+
+static void sev_guest_set_legacy_vm_type(Object *obj, bool value, Error **errp)
+{
+    SEV_GUEST(obj)->legacy_vm_type = value;
+}
+
 bool
 sev_enabled(void)
 {
@@ -863,7 +874,7 @@ static int sev_kvm_type(X86ConfidentialGuest *cg)
     }
 
     kvm_type = (sev->policy & SEV_POLICY_ES) ? KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
-    if (kvm_is_vm_type_supported(kvm_type)) {
+    if (kvm_is_vm_type_supported(kvm_type) && !sev->legacy_vm_type) {
         sev->kvm_type = kvm_type;
     } else {
         sev->kvm_type = KVM_X86_DEFAULT_VM;
@@ -1381,6 +1392,11 @@ sev_guest_class_init(ObjectClass *oc, void *data)
                                    sev_guest_set_kernel_hashes);
     object_class_property_set_description(oc, "kernel-hashes",
             "add kernel hashes to guest firmware for measured Linux boot");
+    object_class_property_add_bool(oc, "legacy-vm-type",
+                                   sev_guest_get_legacy_vm_type,
+                                   sev_guest_set_legacy_vm_type);
+    object_class_property_set_description(oc, "legacy-vm-type",
+            "use legacy VM type to maintain measurement compatibility with older QEMU or kernel versions.");
 }
 
 static void
-- 
2.25.1

[PATCH v1 2/3] hw/i386: Add 9.1 machine types for i440fx/q35

[Michael Roth]

Define the 9.1 machine types and make them identical to 9.0 for now.
This will be needed to add PC compat options for 9.1+ features.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 hw/i386/pc_piix.c | 12 +++++++++++-
 hw/i386/pc_q35.c  | 11 ++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 18ba076609..069414a1ac 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -513,13 +513,23 @@ static void pc_i440fx_machine_options(MachineClass *m)
                                      "Use a different south bridge than PIIX3");
 }
 
-static void pc_i440fx_9_0_machine_options(MachineClass *m)
+static void pc_i440fx_9_1_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = true;
 }
 
+DEFINE_I440FX_MACHINE(v9_1, "pc-i440fx-9.1", NULL,
+                      pc_i440fx_9_1_machine_options);
+
+static void pc_i440fx_9_0_machine_options(MachineClass *m)
+{
+    pc_i440fx_machine_options(m);
+    m->alias = NULL;
+    m->is_default = false;
+}
+
 DEFINE_I440FX_MACHINE(v9_0, "pc-i440fx-9.0", NULL,
                       pc_i440fx_9_0_machine_options);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 7f2d85df75..77d7f700a8 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -367,12 +367,21 @@ static void pc_q35_machine_options(MachineClass *m)
                      pc_q35_compat_defaults, pc_q35_compat_defaults_len);
 }
 
-static void pc_q35_9_0_machine_options(MachineClass *m)
+static void pc_q35_9_1_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v9_1, "pc-q35-9.1", NULL,
+                   pc_q35_9_1_machine_options);
+
+static void pc_q35_9_0_machine_options(MachineClass *m)
+{
+    pc_q35_machine_options(m);
+    m->alias = NULL;
+}
+
 DEFINE_Q35_MACHINE(v9_0, "pc-q35-9.0", NULL,
                    pc_q35_9_0_machine_options);
 
-- 
2.25.1

[PATCH v1 3/3] hw/i386/sev: Use legacy SEV VM types for older machine types

[Michael Roth]

Newer 9.1 machine types will default to using the KVM_SEV_INIT2 API for
creating SEV/SEV-ES going forward. However, this API results in guest
measurement changes which are generally not expected for users of these
older guest types and can cause disruption if they switch to a newer
QEMU/kernel version. Avoid this by continuing to use the older
KVM_SEV_INIT/KVM_SEV_ES_INIT APIs for older machine types.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 hw/i386/pc.c         | 5 +++++
 hw/i386/pc_piix.c    | 1 +
 hw/i386/pc_q35.c     | 1 +
 include/hw/i386/pc.h | 3 +++
 target/i386/sev.c    | 1 +
 5 files changed, 11 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e80f02bef4..96bf90c17e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -78,6 +78,11 @@
     { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
     { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
+GlobalProperty pc_compat_9_0[] = {
+    { "sev-guest", "legacy-vm-type", "true" },
+};
+const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0);
+
 GlobalProperty pc_compat_8_2[] = {};
 const size_t pc_compat_8_2_len = G_N_ELEMENTS(pc_compat_8_2);
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 069414a1ac..0b7a9debab 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -528,6 +528,7 @@ static void pc_i440fx_9_0_machine_options(MachineClass *m)
     pc_i440fx_machine_options(m);
     m->alias = NULL;
     m->is_default = false;
+    compat_props_add(m->compat_props, pc_compat_9_0, pc_compat_9_0_len);
 }
 
 DEFINE_I440FX_MACHINE(v9_0, "pc-i440fx-9.0", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 77d7f700a8..acb55fc787 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -380,6 +380,7 @@ static void pc_q35_9_0_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = NULL;
+    compat_props_add(m->compat_props, pc_compat_9_0, pc_compat_9_0_len);
 }
 
 DEFINE_Q35_MACHINE(v9_0, "pc-q35-9.0", NULL,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fb1d4106e5..e52290916c 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -199,6 +199,9 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
 /* sgx.c */
 void pc_machine_init_sgx_epc(PCMachineState *pcms);
 
+extern GlobalProperty pc_compat_9_0[];
+extern const size_t pc_compat_9_0_len;
+
 extern GlobalProperty pc_compat_8_2[];
 extern const size_t pc_compat_8_2_len;
 
diff --git a/target/i386/sev.c b/target/i386/sev.c
index f4ee317cb0..d30b68c11e 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1417,6 +1417,7 @@ sev_guest_instance_init(Object *obj)
     object_property_add_uint32_ptr(obj, "reduced-phys-bits",
                                    &sev->reduced_phys_bits,
                                    OBJ_PROP_FLAG_READWRITE);
+    object_apply_compat_props(obj);
 }
 
 /* sev guest info */
-- 
2.25.1

Re: [PATCH for-9.1 v1 0/3] Add SEV/SEV-ES machine compat options for KVM_SEV_INIT2

[Paolo Bonzini]

On Wed, Apr 10, 2024 at 1:08 AM Michael Roth <michael.roth@amd.com> wrote:
>
> These patches are also available at:
>
>   https://github.com/amdese/qemu/commits/sev-init-legacy-v1
>
> and are based on top Paolo's qemu-coco-queue branch containing the
> following patches:

A more complete version of patch 2 was already on the list, so I
queued 1 and 3 to qemu-coco-queue.

Thanks!

Paolo

>
>   [PATCH for-9.1 00/26] x86, kvm: common confidential computing subset
>   https://lore.kernel.org/all/20240322181116.1228416-1-pbonzini@redhat.com/T/
>
> Overview
> --------
>
> With the following patches applied from qemu-coco-queue:
>
>   https://lore.kernel.org/all/20240319140000.1014247-1-pbonzini@redhat.com/
>
> QEMU version 9.1+ will begin automatically making use of the new
> KVM_SEV_INIT2 API for initializing SEV and SEV-ES (and eventually, SEV-SNP)
> guests verses the older KVM_SEV_INIT/KVM_SEV_ES_INIT interfaces.
>
> However, the older interfaces would silently avoid sync'ing FPU/XSAVE state
> set by QEMU to each vCPU's VMSA prior to encryption. With KVM_SEV_INIT2,
> this state will now be synced into the VMSA, resulting in measurements
> changes and, theoretically, behaviorial changes, though the latter are
> unlikely to be seen in practice. The specific VMSA changes are documented
> in the section below for reference.
>
> This series implements machine compatibility options for SEV/SEV-ES so that
> only VMs created with QEMU 9.1+ will make use of KVM_SEV_INIT2 so that VMSA
> differences can be accounted for beforehand, and older machine types will
> continue using the older interfaces to avoid unexpected measurement
> changes.
>
> Specific VMSA changes
> ---------------------
>
> With KVM_SEV_INIT2, rather than 0, QEMU/KVM will instead begin setting the
> following fields in the VMSA before measurement/encryption:
>
>   VMSA byte offset [1032:1033] = 80 1f (MXCSR, Multimedia Control Status
>                                         Register)
>   VMSA byte offset [1040:1041] = 7f 03 (FCW, FPU/x86 Control Word)
>
> Setting FCW (FPU/x86 Control Word) to 0x37f is consistent with 11.5.7 of
> APM Volume 2. MXCSR reset state is not defined for XSAVE, but QEMU's 0x1f80
> value is consistent with machine reset state documented in APM Volume 2
> 4.2.2. As such, it is reasonable to begin including these in the VMSA
> measurement calculations.
>
> NOTE: section 11.5.7 also documents that FTW should be all 1's, whereas
>       QEMU currently sets all zeroes. Should that be changed as part of
>       this, or are there other reasons for setting 0?
>
> Thanks,
>
> Mike
>
> ----------------------------------------------------------------
> Michael Roth (3):
>       i386/sev: Add 'legacy-vm-type' parameter for SEV guest objects
>       hw/i386: Add 9.1 machine types for i440fx/q35
>       hw/i386/sev: Use legacy SEV VM types for older machine types
>
>  hw/i386/pc.c         |  5 +++++
>  hw/i386/pc_piix.c    | 13 ++++++++++++-
>  hw/i386/pc_q35.c     | 12 +++++++++++-
>  include/hw/i386/pc.h |  3 +++
>  qapi/qom.json        | 11 ++++++++++-
>  target/i386/sev.c    | 19 ++++++++++++++++++-
>  6 files changed, 59 insertions(+), 4 deletions(-)
>
>
>