[PATCH v3 0/3] SEV-SNP: Add support for SNP certificate fetching

[PATCH v3 0/3] SEV-SNP: Add support for SNP certificate fetching

[Michael Roth]

This patchset is also available at:

  https://github.com/amdese/qemu/commits/snp-certs-v3

and is based on top of qemu master (d731492832)

v1: https://lore.kernel.org/qemu-devel/20241218154939.1114831-1-michael.roth@amd.com/
v2: https://lore.kernel.org/qemu-devel/20250120213116.521519-1-huibo.wang@amd.com/
v3:
- add QEMU documentation for how to work with certificate files (Markus)
- address QAPI scheme formatting issues (Markus)
- partially re-use async_safe_run_on_cpu() infrastructure for handling
  immediate exit callbacks so that multiple vCPUs waiting to release
  read lock on the certificate file are each handled correctly
- refactor certificate file handling to thoroughly log unexpected errors
  without flooding host logs
- propagate all unexpected errors to guest rather than terminating QEMU
- additional comments/cleanups and other fixes


Overview
--------

The GHCB 2.0 specification defines 2 GHCB request types to allow SNP guests
to send encrypted messages/requests to firmware: SNP Guest Requests and SNP
Extended Guest Requests. These encrypted messages are used for things like
servicing attestation requests issued by the guest. Implementing support for
these is required to be fully GHCB-compliant.

For the most part, KVM only needs to handle forwarding these requests to
firmware (to be issued via the SNP_GUEST_REQUEST firmware command defined
in the SEV-SNP Firmware ABI), and then forwarding the encrypted response to
the guest.

However, in the case of SNP Extended Guest Requests, the host is also
able to provide the certificate data corresponding to the endorsement key
used by firmware to sign attestation report requests. This certificate data
is provided by userspace because:

  1) It allows for different keys/key types to be used for each particular
     guest with requiring any sort of KVM API to configure the certificate
     table in advance on a per-guest basis.

  2) It provides additional flexibility with how attestation requests might
     be handled during live migration where the certificate data for
     source/dest might be different.

  3) It allows all synchronization between certificates and firmware/signing
     key updates to be handled purely by userspace rather than requiring
     some in-kernel mechanism to facilitate it. [1]

To support fetching certificate data from userspace, a new KVM
KVM_EXIT_SNP_REQ_CERTS exit type is used to fetch the data similarly to
KVM_EXIT_MMIO/etc, with an associate KVM capability to detect/enable the
exits depending on whether userspace has been configured to provide
certificate data.

Add support for this exit in QEMU. Additionally, because some of the locking
behavior relies on kvm_immediate_exit, add an some infrastructure to allow
for callbacks to be registered and executed upon a kvm_immediate_exit-induced
exit from KVM.

See the documentation included in the patches for more details on the
expected usage and kvm_immediate_exit handling:

[1] https://lore.kernel.org/kvm/ZS614OSoritrE1d2@google.com/


Testing
-------

For testing this, use the following host kernel:

  https://github.com/amdese/linux/commits/snp-certs-v3

For the guest, any 5.19 or newer kernel will do.

A basic command-line invocation for an SNP guest with certificate data
supplied would be:

 qemu-system-x86_64 -smp 32,maxcpus=255 -cpu EPYC-Milan-v2
  -machine q35,confidential-guest-support=sev0,memory-backend=ram1
  -object memory-backend-memfd,id=ram1,size=4G,share=true,reserve=false
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,id-auth=,certs-path=/home/mroth/cert.blob
  -bios OVMF.fd

Something like the following simple example can be used to simulate an
exclusive lock being held on the certificate by management tools performing an
update:

  #include <stdlib.h>
  #include <stdio.h>
  #define __USE_GNU
  #include <fcntl.h>
  #include <unistd.h>
  #include <errno.h>
  #include <stdbool.h>
  #include <sys/types.h>
  #include <sys/stat.h>
  
  int main(int argc, void **argv)
  {
      int ret, fd, i = 0;
      char *path = argv[1];
  
      struct flock fl = {
          .l_whence = SEEK_SET,
          .l_start = 0,
          .l_len = 0,
          .l_type = F_WRLCK
      };
  
      fd = open(path, O_RDWR);
      ret = fcntl(fd, F_OFD_SETLK, &fl);
      if (ret) {
          printf("error locking file, ret %d errno %d\n", ret, errno);
          return ret;
      }
  
      while (true) {
          i++;
          printf("now holding lock (%d seconds elapsed)...\n", i);
          usleep(1000 * 1000);
      }
  
      return 0;
  }

For requesting attestation with certificate data attached, you can use
something like the following tool/workflow:

  https://github.com/virtee/snpguest?tab=readme-ov-file#extended-attestation-workflow

The format of the certificate blob is defined in the GHCB 2.0 specification,
but if it's not being parsed on the guest-side then random data will suffice
for testing the KVM bits.

Any feedback/review is appreciated.

Thanks!

-Mike


----------------------------------------------------------------
Michael Roth (3):
      hw/cpu: Add async_run_on_cpu_after_accel()
      accel/kvm: Add kvm_immediate_exit callback infrastructure
      i386/sev: Add KVM_EXIT_SNP_REQ_CERTS support for certificate-fetching

 accel/kvm/kvm-all.c                        |  20 ++
 cpu-common.c                               |  66 ++++++
 docs/system/i386/amd-memory-encryption.rst |  77 +++++++
 hw/core/cpu-common.c                       |   2 +
 include/hw/core/cpu.h                      |  33 +++
 include/system/kvm.h                       |  11 +
 qapi/qom.json                              |  23 +-
 target/i386/kvm/kvm.c                      |  10 +
 target/i386/sev-system-stub.c              |   5 +
 target/i386/sev.c                          | 338 +++++++++++++++++++++++++++++
 target/i386/sev.h                          |   3 +-
 11 files changed, 586 insertions(+), 2 deletions(-)

[PATCH v3 1/3] hw/cpu: Add async_run_on_cpu_after_accel()

[Michael Roth]

For some operations, hypervisors/accelerators like KVM might requiring
calling back into the accelerator code prior to userspace being able to
complete handling of certains operations. One common example is I/O
emulation where the accelerator call may be necessary to complete
pending work on the accelerator side and confirming completion to
userspace via another exit, which (for this particular case) already has
handling in KVM-specific code.

In some cases however, userspace may need to perform additional work on
behalf of the vCPU after the accelerator exists, requiring some kind of
per-vCPU callback mechanism. async_run_on_cpu() already provides
something very similar, but it forces an exit from the inner loop, and
is processed at the start of the outer loop, so those callbacks might
be issued before the accelerator has a chance to get called again.

Introduce a new variant, async_run_on_cpu_after_accel(), that is
designed to queue up work to be processed after the next accelerator
exit, and without unnecessarily forcing an exit from the inner loop. The
handling is via process_queued_cpu_accel_work(), which is left to
accelerator/arch-specific code to implement in their inner loops where
appropriate.

KVM will use this general per-vCPU infrastructure to handle callbacks
for these cases. Other accelerators maybe have other uses in the future.

Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 cpu-common.c          | 66 +++++++++++++++++++++++++++++++++++++++++++
 hw/core/cpu-common.c  |  2 ++
 include/hw/core/cpu.h | 33 ++++++++++++++++++++++
 3 files changed, 101 insertions(+)

diff --git a/cpu-common.c b/cpu-common.c
index adb76b3a78..0e735da673 100644
--- a/cpu-common.c
+++ b/cpu-common.c
@@ -141,6 +141,23 @@ static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
     cpu_exit(cpu);
 }
 
+/*
+ * Queue work on a CPU to run after the next accelerator exit.
+ *
+ * This helper intentionally leaves the inner loop running rather than forcing
+ * an exit via cpu_exit(), since the intention is to immediately call back into
+ * the accelerator to process pending events, and then upon exiting issue
+ * callbacks/notifications to any CPUs that need to complete any work that is
+ * pending on that accelerator call/exit.
+ */
+static void queue_accel_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
+{
+    assert(wi->free);
+    qemu_mutex_lock(&cpu->work_mutex);
+    QSIMPLEQ_INSERT_TAIL(&cpu->accel_work_list, wi, node);
+    qemu_mutex_unlock(&cpu->work_mutex);
+}
+
 void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data,
                    QemuMutex *mutex)
 {
@@ -178,6 +195,24 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data)
     queue_work_on_cpu(cpu, wi);
 }
 
+void async_run_on_cpu_after_accel(CPUState *cpu, run_on_cpu_func func,
+                                  run_on_cpu_data data)
+{
+    struct qemu_work_item *wi;
+
+    wi = g_new0(struct qemu_work_item, 1);
+
+    /*
+     * Only asynchronous accelerator work items are allowed. I.e. wi->free
+     * must be true. Subsequent code will assert this.
+     */
+    wi->func = func;
+    wi->data = data;
+    wi->free = true;
+
+    queue_accel_work_on_cpu(cpu, wi);
+}
+
 /* Wait for pending exclusive operations to complete.  The CPU list lock
    must be held.  */
 static inline void exclusive_idle(void)
@@ -349,6 +384,37 @@ void free_queued_cpu_work(CPUState *cpu)
     }
 }
 
+void free_queued_cpu_accel_work(CPUState *cpu)
+{
+    while (!QSIMPLEQ_EMPTY(&cpu->accel_work_list)) {
+        struct qemu_work_item *wi = QSIMPLEQ_FIRST(&cpu->accel_work_list);
+        QSIMPLEQ_REMOVE_HEAD(&cpu->accel_work_list, node);
+        assert(wi->free);
+        g_free(wi);
+    }
+}
+
+void process_queued_cpu_accel_work(CPUState *cpu)
+{
+    struct qemu_work_item *wi;
+
+    qemu_mutex_lock(&cpu->work_mutex);
+
+    while (!QSIMPLEQ_EMPTY(&cpu->accel_work_list)) {
+        wi = QSIMPLEQ_FIRST(&cpu->accel_work_list);
+        QSIMPLEQ_REMOVE_HEAD(&cpu->accel_work_list, node);
+
+        qemu_mutex_unlock(&cpu->work_mutex);
+        wi->func(cpu, wi->data);
+        qemu_mutex_lock(&cpu->work_mutex);
+
+        assert(wi->free);
+        g_free(wi);
+    }
+
+    qemu_mutex_unlock(&cpu->work_mutex);
+}
+
 void process_queued_cpu_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 4f4c87f33a..599963a85d 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -304,6 +304,7 @@ static void cpu_common_initfn(Object *obj)
     qemu_mutex_init(&cpu->work_mutex);
     qemu_lockcnt_init(&cpu->in_ioctl_lock);
     QSIMPLEQ_INIT(&cpu->work_list);
+    QSIMPLEQ_INIT(&cpu->accel_work_list);
     QTAILQ_INIT(&cpu->breakpoints);
     QTAILQ_INIT(&cpu->watchpoints);
 
@@ -332,6 +333,7 @@ static void cpu_common_finalize(Object *obj)
     }
 #endif
     free_queued_cpu_work(cpu);
+    free_queued_cpu_accel_work(cpu);
     /* If cleanup didn't happen in context to gdb_unregister_coprocessor_all */
     if (cpu->gdb_regs) {
         g_array_free(cpu->gdb_regs, TRUE);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5b7c330047..8c55269074 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -523,6 +523,7 @@ struct CPUState {
 
     QemuMutex work_mutex;
     QSIMPLEQ_HEAD(, qemu_work_item) work_list;
+    QSIMPLEQ_HEAD(, qemu_work_item) accel_work_list;
 
     struct CPUAddressSpace *cpu_ases;
     AddressSpace *as;
@@ -968,6 +969,24 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data);
  */
 void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data);
 
+/**
+ * async_run_on_cpu_after_accel:
+ * @cpu: The vCPU to run on.
+ * @func: The function to be executed.
+ * @data: Data to pass to the function.
+ *
+ * Schedules the function @func for execution on the vCPU @cpu asynchronously,
+ * to be handled only after the next invocation of the configured accelerator.
+ * Unlike async_run_on_cpu(), @func will not be run with the BQL held and is
+ * instead left to @func itself to take if needed.
+ *
+ * This does not guarantee any sort of expedited exiting from the accelerator
+ * or the inner loop; this aspect is instead left up to
+ * accelerator/arch-specific code.
+ */
+void async_run_on_cpu_after_accel(CPUState *cpu, run_on_cpu_func func,
+                                  run_on_cpu_data data);
+
 /**
  * async_safe_run_on_cpu:
  * @cpu: The vCPU to run on.
@@ -1115,12 +1134,26 @@ void cpu_remove_sync(CPUState *cpu);
  */
 void free_queued_cpu_work(CPUState *cpu);
 
+/**
+ * free_queued_cpu_accel_work() - free all items on CPU accel work queue
+ * @cpu: The CPU which accel work queue to free.
+ */
+void free_queued_cpu_accel_work(CPUState *cpu);
+
 /**
  * process_queued_cpu_work() - process all items on CPU work queue
  * @cpu: The CPU which work queue to process.
  */
 void process_queued_cpu_work(CPUState *cpu);
 
+/**
+ * process_queued_cpu_accel_work() - process all items on CPU accel work queue
+ * @cpu: The CPU which work queue to process.
+ *
+ * Must be called without the BQL held.
+ */
+void process_queued_cpu_accel_work(CPUState *cpu);
+
 /**
  * cpu_exec_start:
  * @cpu: The CPU for the current thread.
-- 
2.43.0

[PATCH v3 2/3] accel/kvm: Add kvm_immediate_exit callback infrastructure

[Michael Roth]

Provide a generic mechanism so that users of struct kvm_run's
'immediate_exit' flag can register a callback that can be issued after
returning from KVM. Utilize the recently-added
async_run_on_cpu_after_accel() interface, with a small wrapper to set
the 'immediate_exit' to arm any new callbacks.

As with async_run_on_cpu_after_accel(), these callbacks are not run with
the BQL held; that is left to individual callback implementations when
needed.

Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Melody Wang <huibo.wang@amd.com>
---
 accel/kvm/kvm-all.c  | 20 ++++++++++++++++++++
 include/system/kvm.h | 11 +++++++++++
 2 files changed, 31 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index db8fba53f3..a6bc0b3bed 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3449,6 +3449,19 @@ out_unref:
     return ret;
 }
 
+/*
+ * Queue up CPU work to run after the next call to KVM_RUN.
+ *
+ * Requires KVM_CAP_IMMEDIATE_EXIT.
+ */
+void kvm_add_immediate_exit_callback(CPUState *cpu, run_on_cpu_func cb,
+                                     run_on_cpu_data data)
+{
+    assert(kvm_immediate_exit);
+    async_run_on_cpu_after_accel(cpu, cb, data);
+    kvm_cpu_kick(cpu);
+}
+
 int kvm_cpu_exec(CPUState *cpu)
 {
     struct kvm_run *run = cpu->kvm_run;
@@ -3511,6 +3524,13 @@ int kvm_cpu_exec(CPUState *cpu)
             if (run_ret == -EINTR || run_ret == -EAGAIN) {
                 trace_kvm_io_window_exit();
                 kvm_eat_signals(cpu);
+                /*
+                 * This will process any events queued by
+                 * kvm_add_immediate_exit_callback(). This is intentionally run
+                 * after 'immediate_exit' field is cleared by kvm_eat_signal()
+                 * so that queued accel work can requeue itself if needed.
+                 */
+                process_queued_cpu_accel_work(cpu);
                 ret = EXCP_INTERRUPT;
                 break;
             }
diff --git a/include/system/kvm.h b/include/system/kvm.h
index d29624034c..2c08992bce 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -21,6 +21,7 @@
 #include "qemu/accel.h"
 #include "accel/accel-route.h"
 #include "qom/object.h"
+#include "hw/core/cpu.h"
 
 #ifdef COMPILING_PER_TARGET
 # ifdef CONFIG_KVM
@@ -594,4 +595,14 @@ void kvm_vcpufd_add_change_notifier(NotifierWithReturn *n);
  */
 void kvm_vcpufd_remove_change_notifier(NotifierWithReturn *n);
 
+/**
+ * kvm_add_immediate_exit_callback - register a callback to be performed after
+ * the next call to KVM_RUN (with immediate_exit set via kvm_cpu_kick()).
+ * @cpu: CPU to run the callback on.
+ * @cb: The callback to run.
+ * @data: The data to pass to the callback.
+ */
+void kvm_add_immediate_exit_callback(CPUState *cpu, run_on_cpu_func cb,
+                                     run_on_cpu_data data);
+
 #endif
-- 
2.43.0

[PATCH v3 3/3] i386/sev: Add KVM_EXIT_SNP_REQ_CERTS support for certificate-fetching

[Michael Roth]

The GHCB specification[1] defines a VMGEXIT-based Guest Request
hypercall to allow an SNP guest to issue encrypted requests directly to
SNP firmware to do things like query the attestation report for the
guest. These are generally handled purely in the kernel.

In some cases, it's useful for the host to be able to additionally
supply the certificate chain for the signing key that SNP firmware uses
to sign these attestation reports. To allow for this, the GHCB
specification defines an Extended Guest Request where this certificate
data can be provided in a special format described in the GHCB spec.
This certificate data may be global or guest-specific depending on how
the guest was configured. Rather than providing interfaces to manage
these within the kernel, KVM provides a new KVM_EXIT_SNP_REQ_CERTS exit
to request the certificate contents from userspace. Implement support
for that in QEMU.

To synchronize delivery of the certificates to the guest in a way where
they will not be rendered invalid by updates to SNP firmware or
attestation singing/endorsement keys by management tools outside the
purview of QEMU, it is expected by users of KVM_EXIT_SNP_REQ_CERTS to
obtain a shared/read lock on the certificate file prior to delivering
them back to KVM. Only after this will the attestation report be
retrieved from firmware and bundled with the certificate data, so QEMU
must continue to hold the file lock until KVM confirms that the
attestation report has been retrieved/bundled.

This confirmation is done by way of the kvm_immediate_exit callback
infrastructure that was introduced in a prior commit. Once the callback
is issued (after the next KVM_RUN), the attestation report will have
been fetched, so the report and certificate data fetched by QEMU should
be consistent with one another, and the certificate lock can then be
released. The included documentation updates have more details on how
this is handled.

[1] "Guest Hypervisor Communication Block (GHCB) Standardization",
    https://www.amd.com/en/developer/sev.html

Co-developed-by: Melody Wang <huibo.wang@amd.com>
Signed-off-by: Melody Wang <huibo.wang@amd.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 docs/system/i386/amd-memory-encryption.rst |  77 +++++
 qapi/qom.json                              |  23 +-
 target/i386/kvm/kvm.c                      |  10 +
 target/i386/sev-system-stub.c              |   5 +
 target/i386/sev.c                          | 338 +++++++++++++++++++++
 target/i386/sev.h                          |   3 +-
 6 files changed, 454 insertions(+), 2 deletions(-)

diff --git a/docs/system/i386/amd-memory-encryption.rst b/docs/system/i386/amd-memory-encryption.rst
index 6c23f3535f..da6f1a3693 100644
--- a/docs/system/i386/amd-memory-encryption.rst
+++ b/docs/system/i386/amd-memory-encryption.rst
@@ -228,6 +228,83 @@ schema for the 'sev-snp-guest' object)::
     -machine ...,confidential-guest-support=sev0 \
     -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1
 
+Certificate-fetching (SEV-SNP)
+------------------------------
+
+When an SEV-SNP guest requests an attestation report from the host, it may
+optionally request that the host also deliver the certificates (and
+potentially other accompanying data) as part of the response.
+
+QEMU can be configured to provide this certificate data in the following
+manner::
+
+  # ${QEMU} \
+    -machine ...,confidential-guest-support=sev0 \
+    -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,certs-filename=<certs>
+
+where the format of the file is defined in the SEV-ES Guest-Hypervisor
+Communication Block Standardization (GHCB) v2.04, section 4.1.8.1, as::
+
+  Returned certificates are identified in a table starting at offset 0x0 of
+  [the file]. Each entry consists of a 16-byte GUID, a 4-byte offset from
+  [the file] to where the certificate data starts and a 4-byte length
+  representing the length of the certificate data. The table is terminated
+  with an entry containing all zeroes for the GUID, offset and length.
+
+  struct cert_table {
+      struct {
+          unsigned char guid[16];
+          uint32 offset;
+          uint32 length;
+      } cert_table_entry[];
+  };
+
+Because QEMU supplies the certificate to the hypervisor prior to the
+hypervisor actually issuing an attestation request, there is potential for
+the certificate data to becoming stale due to a firmware upgrade or other
+host management activities. The ioctl which enables certificate-fetching
+support via the KVM hypervisor, KVM_SEV_SNP_ENABLE_REQ_CERTS, documents
+the following suggested scheme as part of its linux documentation (v7.3
+at the time of this writing), and it is highly recommended that all software
+interacting with these certificates, or performing operations that may
+affect their validity, adopt this same scheme for interoperability::
+
+  The recommended scheme to do this is to use file locking (e.g. via
+  fcntl()'s F_OFD_SETLK) in the following manner:
+
+    - Prior to obtaining/providing certificate data as part of servicing an
+      exit type of ``KVM_EXIT_SNP_REQ_CERTS``, the VMM should obtain a
+      shared/read or exclusive/write lock on the certificate blob file before
+      reading it and returning it to KVM, and continue to hold the lock until
+      the attestation request is actually sent to firmware. To facilitate
+      this, the VMM can set the ``immediate_exit`` flag of kvm_run just after
+      supplying the certificate data, and just before resuming the vCPU.
+      This will ensure the vCPU will exit again to userspace with ``-EINTR``
+      after it finishes fetching the attestation request from firmware, at
+      which point the VMM can safely drop the file lock.
+
+    - Tools/libraries that perform updates to SNP firmware TCB values or
+      endorsement keys (e.g. via /dev/sev interfaces such as ``SNP_COMMIT``,
+      ``SNP_SET_CONFIG``, or ``SNP_VLEK_LOAD``, see
+      Documentation/virt/coco/sev-guest.rst for more details) in such a way
+      that the certificate blob needs to be updated, should similarly take an
+      exclusive lock on the certificate blob for the duration of any updates
+      to endorsement keys or the certificate blob contents to ensure that
+      VMMs using the above scheme will not return certificate blob data that
+      is out of sync with the endorsement key used by firmware at the time
+      the attestation request is actually issued.
+
+  This scheme is recommended so that tools can use a fairly generic/natural
+  approach to synchronizing firmware/certificate updates via file-locking,
+  which should make it easier to maintain interoperability across
+  tools/VMMs/vendors.
+
+As such, QEMU implements this same scheme by waiting for a read-lock on the
+file containing the certificate data before supplying any data it contains.
+In order to ensure that the lock is not released until after the hypervisor
+is finished fetching the attestation report for the guest, the lock is held
+until a subsequent exit from the hypervisor that indicates the attestation
+report has already been fetched, as which point QEMU will drop the lock.
 
 Debugging
 ---------
diff --git a/qapi/qom.json b/qapi/qom.json
index 4810d53e58..8884a1eb66 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1104,6 +1104,25 @@
 # @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
 #     is enabled (default: zero) (since 11.1)
 #
+# @certs-filename: Certificate data that can be passed to guests as
+#     part of an attestion request.  See the
+#     :doc:`/system/i386/amd-memory-encryption` documentation for more
+#     details on the usage and file format.
+#     (default: none) (since: 11.2)
+#
+# @certs-timeout: Maximum time in milliseconds to wait to obtain a
+#     read lock on the certificate file specified by @certs-filename.
+#     This is not a cumulative value and only affects how long QEMU
+#     waits before returning execution to the vCPU and informing the
+#     guest of the timeout, so the guest can still continue retrying
+#     for as long as it likes, which for linux guests at the time of
+#     this writing will be about 60 seconds before the guest gives
+#     up and disables further requests.  If this guest-side timeout is
+#     insufficient, set this higher to allow more time to fetch the
+#     certificate.  If the guest-side timeout is sufficient,
+#     set this lower to reduce the likelihood of soft lockups in the
+#     guest.  (default: 100) (since: 11.2)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1117,7 +1136,9 @@
             '*host-data': 'str',
             '*vcek-disabled': 'bool',
             '*secure-tsc': 'bool',
-            '*tsc-frequency': 'uint32' } }
+            '*tsc-frequency': 'uint32',
+            '*certs-filename': 'str',
+            '*certs-timeout': 'uint32' } }
 
 ##
 # @TdxGuestProperties:
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index da922f72d7..9f7bdf9618 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -6651,6 +6651,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
             break;
         }
         ret = 0;
+        break;
+    case KVM_EXIT_SNP_REQ_CERTS:
+        if (!sev_snp_enabled()) {
+            error_report("KVM: Encountered a certificate request exit for a "
+                         "non-SEV-SNP guest.");
+            ret = -1;
+        } else {
+            ret = kvm_handle_snp_req_certs(cs, run);
+        }
+
         break;
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
diff --git a/target/i386/sev-system-stub.c b/target/i386/sev-system-stub.c
index d97536cca1..e53f1bbefc 100644
--- a/target/i386/sev-system-stub.c
+++ b/target/i386/sev-system-stub.c
@@ -34,3 +34,8 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
 void pc_system_parse_sev_metadata(uint8_t *flash_ptr, size_t flash_size)
 {
 }
+
+int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run)
+{
+    g_assert_not_reached();
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 11072b00dc..3cb2e9cd0c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -186,6 +186,8 @@ struct SevSnpGuestState {
     uint8_t *id_auth;
     char *host_data;
     uint32_t tsc_khz;
+    char *certs_filename;
+    uint32_t certs_timeout;
 
     struct kvm_sev_snp_launch_start kvm_start_conf;
     struct kvm_sev_snp_launch_finish kvm_finish_conf;
@@ -1701,6 +1703,278 @@ sev_snp_launch_finish(SevCommonState *sev_common)
     sev_set_guest_state(sev_common, SEV_STATE_RUNNING);
 }
 
+/*
+ * Cases where -EAGAIN is returned on an error path are expected to terminate
+ * after the next call (e.g. via a qemu_open() error) if they are not transient
+ * issues. These potentially-transient issues are still logged however, since
+ * they are not expected on a well-behaved system.
+ */
+static int open_certs_locked(SevSnpGuestState *sev_snp_guest)
+{
+    struct stat locked_file, current_file;
+    int fd, ret;
+
+    fd = qemu_open(sev_snp_guest->certs_filename, O_RDONLY, NULL);
+    if (fd == -1) {
+        error_report_once("Unable to open certificate file: %s",
+                          strerror(errno));
+        return -errno;
+    }
+
+    if (fstat(fd, &locked_file) == -1) {
+        error_report_once("Unable to check status of certificate file: %s",
+                          strerror(errno));
+        ret = -errno;
+        goto out_close;
+    }
+
+    ret = qemu_lock_fd(fd, 0, 0, false);
+    if (ret == -EACCES) {
+        /*
+         * F_SETLK is documented to potentially return either EACCES or EAGAIN
+         * for contended file locks.
+         */
+        ret = -EAGAIN;
+    } else if (ret == -EDEADLK) {
+        error_report_once("Potential deadlock encountered while trying "
+                          "to lock the certificate file");
+        ret = -EAGAIN;
+    }
+
+    if (ret) {
+        if (ret != -EAGAIN) {
+            error_report_once("Error while trying to lock certificate file: %s",
+                              strerror(-ret));
+        }
+        goto out_close;
+    }
+
+    if (stat(sev_snp_guest->certs_filename, &current_file) == -1) {
+        error_report_once("Certificate file no longer present: %s",
+                          strerror(errno));
+        ret = -EAGAIN;
+        goto out_close;
+    }
+
+    /*
+     * Locked files should not be changing; this violates the locking
+     * protocol assumed by this implementation. Indicate the need to
+     * retry to the caller, but make sure to log it first.
+     */
+    if (locked_file.st_ino != current_file.st_ino ||
+        locked_file.st_dev != current_file.st_dev) {
+        error_report_once("Locked certificate file was recreated");
+        ret = -EAGAIN;
+        goto out_close;
+    }
+
+    /*
+     * The locked file is intact and as-expected, and it has not been replaced
+     * since then, so the fd should be okay to use.
+     */
+    return fd;
+
+out_close:
+    close(fd);
+    return ret;
+}
+
+static void close_certs(int fd)
+{
+    qemu_unlock_fd(fd, 0, 0);
+    close(fd);
+}
+
+static ssize_t get_certs_size(int fd)
+{
+    ssize_t size;
+
+    size = lseek(fd, 0, SEEK_END);
+    if (size == -1) {
+        error_report_once("Error while determining the size of the certificate file: %s",
+                          strerror(errno));
+        return -errno;
+    }
+
+    return size;
+}
+
+static int read_certs(int fd, void *buf, size_t buf_len)
+{
+    ssize_t n;
+
+    n = lseek(fd, 0, SEEK_SET);
+    if (n == -1) {
+        error_report_once("Error while setting certificate file position: %s",
+                          strerror(errno));
+        return -errno;
+    }
+
+    while ((n = read(fd, buf, buf_len)) != 0) {
+        if (n == -1) {
+            if (errno == EINTR) {
+                continue;
+            }
+
+            error_report_once("Error reading the certificate file: %s",
+                              strerror(errno));
+            return -errno;
+        }
+        buf += n;
+        buf_len -= n;
+    }
+
+    return 0;
+}
+
+static void snp_release_certs_lock(CPUState *cpu, run_on_cpu_data data)
+{
+    close_certs(data.host_int);
+}
+
+/*
+ * NOTE: It is not expected that the guest will call this in a tight loop
+ * where error reporting would flood logs. A failed certificate fetch results
+ * in a failed attestation request, which results in a disabled attestation
+ * driver in the guest usually.
+ *
+ * However, one instance of such an error is usually enough of a clue to
+ * to identify the component of the system that should be investigated.
+ *
+ * So, to guard against a malicious guest trying to flood host logs, while
+ * still providing reasonable debug/error information to the user, use
+ * error_report_once() throughout this function and its supporting helpers.
+ */
+int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run)
+{
+    struct kvm_exit_snp_req_certs *req_certs = &run->snp_req_certs;
+    SevSnpGuestState *sev_snp_guest;
+    ssize_t certs_size;
+    uint32_t npages;
+    void *guest_buf;
+    int64_t t0_ms;
+    hwaddr buf_sz;
+    int ret, fd;
+    hwaddr gpa;
+
+    gpa = req_certs->gpa;
+    npages = req_certs->npages;
+
+    sev_snp_guest = SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
+    if (!sev_snp_guest->certs_filename) {
+        req_certs->ret = 0;
+        return 0;
+    }
+
+    /*
+     * By default, any errors encountered below will be forwarded to the guest
+     * as an EIO. This will likely result in the guest disabling its driver, but
+     * it's better than crashing QEMU.
+     *
+     * Leave the special cases, SUCCESS/ENOSPC/EAGAIN, to re-set the guest
+     * return code as-needed.
+     *
+     * Errors that are useful for debugging QEMU will still be logged, with care
+     * to avoid busy/EAGAIN paths/etc.
+     */
+    req_certs->ret = EIO;
+
+    /*
+     * -EAGAIN from open_certs_locked() indicates that a lock could not be
+     * obtained on the certificate file. It would be possible to set
+     * req_certs->ret = EAGAIN to immediately inform the requesting vCPU of
+     * this condition so that it can retry again later, however, linux guests
+     * are currently hard-limited to a 60 second timeout, at which point they
+     * will assume misbehavior and refuse to continue issuing attestation
+     * requests from that point forward.
+     *
+     * Avoid risking triggering this by instead busy-waiting on the QEMU side,
+     * which allows more time for QEMU to wait on potentially long-running
+     * tasks like SNP firmware updates/etc. which may be holding an exclusive
+     * lock on the certificate file.
+     *
+     * The down-side to doing things this way is that too long of a busy-wait
+     * will result in soft-lockups in the guest, but the guest will otherwise
+     * continue normally afterward.
+     *
+     * If the QEMU-side timeout is reached, then just go ahead and indicate
+     * EAGAIN to the guest, at which point the above-mentioned guest-side
+     * timeout will trigger if QEMU's timeout exceeds that of the guest. But
+     * that's better than killing the vCPU, which is the only viable
+     * alternative at that point.
+     */
+    t0_ms = g_get_monotonic_time() / 1000;
+
+    do {
+        fd = open_certs_locked(sev_snp_guest);
+
+        if (fd == -EAGAIN) {
+            int64_t t1_ms = g_get_monotonic_time() / 1000;
+
+            if (t1_ms - t0_ms > sev_snp_guest->certs_timeout) {
+                req_certs->ret = EAGAIN;
+                return 0;
+            }
+
+            /*
+             * If some other process (QEMU only ever takes a read lock) has an
+             * exclusive lock on the certificate file, then it is likely because
+             * a firmware update or other long-running operation is taking place;
+             * it's a waste to re-try at a high rate.
+             */
+            g_usleep(1000);
+        } else if (fd < 0) {
+            return 0;
+        }
+    } while (fd == -EAGAIN);
+
+    certs_size = get_certs_size(fd);
+    if (certs_size < 0) {
+        goto err_close;
+    }
+
+    buf_sz = (hwaddr)npages * TARGET_PAGE_SIZE;
+    if (buf_sz < certs_size) {
+        /* Communicate to guest that more pages are needed. */
+        req_certs->ret = ENOSPC;
+        req_certs->npages = DIV_ROUND_UP(certs_size, TARGET_PAGE_SIZE);
+        goto err_close;
+    }
+
+    guest_buf = address_space_map(&address_space_memory, gpa, &buf_sz, true,
+                                  MEMTXATTRS_UNSPECIFIED);
+    if (!guest_buf) {
+        error_report_once("Unable to map guest buffer to provide "
+                          "certificate data.");
+        goto err_close;
+    }
+
+    if (buf_sz < (hwaddr)npages * TARGET_PAGE_SIZE) {
+        error_report_once("Unable to map the full guest buffer to provide "
+                          "certificate data.");
+        address_space_unmap(&address_space_memory, guest_buf, buf_sz, true, 0);
+        goto err_close;
+    }
+
+    ret = read_certs(fd, guest_buf, buf_sz);
+    address_space_unmap(&address_space_memory, guest_buf, buf_sz, true, buf_sz);
+
+    if (!ret) {
+        /*
+         * Looks good, return the certificate to the guest and prepare to
+         * release the file lock once KVM signals completion.
+         */
+        kvm_add_immediate_exit_callback(cpu, snp_release_certs_lock,
+                                        RUN_ON_CPU_HOST_INT(fd));
+
+        req_certs->ret = 0;
+        return 0;
+    }
+
+err_close:
+    close_certs(fd);
+    return 0;
+}
 
 static void
 sev_vm_state_change(void *opaque, bool running, RunState state)
@@ -2051,6 +2325,41 @@ static int sev_snp_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
     g_free(sev_snp_guest->kernel_hashes_data);
     sev_snp_guest->kernel_hashes_data = NULL;
 
+    if (sev_snp_guest->certs_filename) {
+        unsigned long certs_attr;
+        struct kvm_device_attr attr = {
+            .group = KVM_X86_GRP_SEV,
+            .attr = KVM_X86_SEV_SNP_REQ_CERTS,
+            .addr = (unsigned long) &certs_attr
+        };
+        int ret;
+
+        if (!kvm_check_extension(kvm_state, KVM_CAP_SYS_ATTRIBUTES) ||
+            !kvm_check_extension(kvm_state, KVM_CAP_IMMEDIATE_EXIT)) {
+            return -EINVAL;
+        }
+
+        ret = kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
+        if (ret < 0) {
+            error_setg(errp, "Certificate-fetching attribute not present.");
+            return ret;
+        }
+
+        if (!certs_attr) {
+            error_setg(errp, "Certificate-fetching attribute not set.");
+            return -EINVAL;
+        }
+
+        ret = sev_ioctl(SEV_COMMON(sev_snp_guest)->sev_fd,
+                        KVM_SEV_SNP_ENABLE_REQ_CERTS,
+                        NULL, NULL);
+        if (ret) {
+            error_setg(errp, "Failed to enable support for SEV-SNP "
+                             "certificate-fetching requests.");
+            return ret;
+        }
+    }
+
     return 0;
 }
 
@@ -3320,6 +3629,26 @@ sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
     SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
 }
 
+static char *
+sev_snp_guest_get_certs_filename(Object *obj, Error **errp)
+{
+    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
+
+    return g_strdup(sev_snp_guest->certs_filename);
+}
+
+static void
+sev_snp_guest_set_certs_filename(Object *obj, const char *value, Error **errp)
+{
+    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
+
+    if (sev_snp_guest->certs_filename) {
+        g_free(sev_snp_guest->certs_filename);
+    }
+
+    sev_snp_guest->certs_filename = value ? g_strdup(value) : NULL;
+}
+
 static void
 sev_snp_guest_class_init(ObjectClass *oc, const void *data)
 {
@@ -3361,6 +3690,12 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
     object_class_property_add(oc, "tsc-frequency", "uint32",
                               sev_snp_guest_get_tsc_frequency,
                               sev_snp_guest_set_tsc_frequency, NULL, NULL);
+    object_class_property_add_str(oc, "certs-filename",
+                                  sev_snp_guest_get_certs_filename,
+                                  sev_snp_guest_set_certs_filename);
+    object_class_property_add_uint32_ptr(oc, "certs-timeout",
+                                   offsetof(SevSnpGuestState, certs_timeout),
+                                   OBJ_PROP_FLAG_READWRITE);
 }
 
 static void
@@ -3374,6 +3709,8 @@ sev_snp_guest_instance_init(Object *obj)
     /* default init/start/finish params for kvm */
     sev_snp_guest->kvm_start_conf.policy = DEFAULT_SEV_SNP_POLICY;
     sev_set_feature(SEV_COMMON(sev_snp_guest), SVM_SEV_FEAT_SNP_ACTIVE, true);
+
+    sev_snp_guest->certs_timeout = 100;
 }
 
 static void
@@ -3387,6 +3724,7 @@ sev_snp_guest_finalize(Object *obj)
     g_free(sev_snp_guest->id_auth_base64);
     g_free(sev_snp_guest->id_auth);
     g_free(sev_snp_guest->host_data);
+    g_free(sev_snp_guest->certs_filename);
 }
 
 /* guest info specific to sev-snp */
diff --git a/target/i386/sev.h b/target/i386/sev.h
index 7725f92e19..4d9e9b44b4 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -15,6 +15,7 @@
 #define I386_SEV_H
 
 #include "target/i386/svm.h"
+#include "system/kvm.h"
 
 #ifndef CONFIG_USER_ONLY
 #include CONFIG_DEVICES /* CONFIG_SEV */
@@ -198,5 +199,5 @@ void pc_system_parse_sev_metadata(uint8_t *flash_ptr, size_t flash_size);
 
 uint32_t sev_get_cbit_position(void);
 uint32_t sev_get_reduced_phys_bits(void);
-
+int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run);
 #endif
-- 
2.43.0

Re: [PATCH v3 3/3] i386/sev: Add KVM_EXIT_SNP_REQ_CERTS support for certificate-fetching

[Markus Armbruster]

Michael Roth <michael.roth@amd.com> writes:

> The GHCB specification[1] defines a VMGEXIT-based Guest Request
> hypercall to allow an SNP guest to issue encrypted requests directly to
> SNP firmware to do things like query the attestation report for the
> guest. These are generally handled purely in the kernel.
>
> In some cases, it's useful for the host to be able to additionally
> supply the certificate chain for the signing key that SNP firmware uses
> to sign these attestation reports. To allow for this, the GHCB
> specification defines an Extended Guest Request where this certificate
> data can be provided in a special format described in the GHCB spec.
> This certificate data may be global or guest-specific depending on how
> the guest was configured. Rather than providing interfaces to manage
> these within the kernel, KVM provides a new KVM_EXIT_SNP_REQ_CERTS exit
> to request the certificate contents from userspace. Implement support
> for that in QEMU.
>
> To synchronize delivery of the certificates to the guest in a way where
> they will not be rendered invalid by updates to SNP firmware or
> attestation singing/endorsement keys by management tools outside the
> purview of QEMU, it is expected by users of KVM_EXIT_SNP_REQ_CERTS to
> obtain a shared/read lock on the certificate file prior to delivering
> them back to KVM. Only after this will the attestation report be
> retrieved from firmware and bundled with the certificate data, so QEMU
> must continue to hold the file lock until KVM confirms that the
> attestation report has been retrieved/bundled.
>
> This confirmation is done by way of the kvm_immediate_exit callback
> infrastructure that was introduced in a prior commit. Once the callback
> is issued (after the next KVM_RUN), the attestation report will have
> been fetched, so the report and certificate data fetched by QEMU should
> be consistent with one another, and the certificate lock can then be
> released. The included documentation updates have more details on how
> this is handled.
>
> [1] "Guest Hypervisor Communication Block (GHCB) Standardization",
>     https://www.amd.com/en/developer/sev.html
>
> Co-developed-by: Melody Wang <huibo.wang@amd.com>
> Signed-off-by: Melody Wang <huibo.wang@amd.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  docs/system/i386/amd-memory-encryption.rst |  77 +++++
>  qapi/qom.json                              |  23 +-
>  target/i386/kvm/kvm.c                      |  10 +
>  target/i386/sev-system-stub.c              |   5 +
>  target/i386/sev.c                          | 338 +++++++++++++++++++++
>  target/i386/sev.h                          |   3 +-
>  6 files changed, 454 insertions(+), 2 deletions(-)
>
> diff --git a/docs/system/i386/amd-memory-encryption.rst b/docs/system/i386/amd-memory-encryption.rst
> index 6c23f3535f..da6f1a3693 100644
> --- a/docs/system/i386/amd-memory-encryption.rst
> +++ b/docs/system/i386/amd-memory-encryption.rst
> @@ -228,6 +228,83 @@ schema for the 'sev-snp-guest' object)::
>      -machine ...,confidential-guest-support=sev0 \
>      -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1
>  
> +Certificate-fetching (SEV-SNP)
> +------------------------------
> +
> +When an SEV-SNP guest requests an attestation report from the host, it may
> +optionally request that the host also deliver the certificates (and
> +potentially other accompanying data) as part of the response.
> +
> +QEMU can be configured to provide this certificate data in the following
> +manner::
> +
> +  # ${QEMU} \
> +    -machine ...,confidential-guest-support=sev0 \
> +    -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,certs-filename=<certs>
> +
> +where the format of the file is defined in the SEV-ES Guest-Hypervisor
> +Communication Block Standardization (GHCB) v2.04, section 4.1.8.1, as::
> +
> +  Returned certificates are identified in a table starting at offset 0x0 of
> +  [the file]. Each entry consists of a 16-byte GUID, a 4-byte offset from
> +  [the file] to where the certificate data starts and a 4-byte length
> +  representing the length of the certificate data. The table is terminated
> +  with an entry containing all zeroes for the GUID, offset and length.
> +
> +  struct cert_table {
> +      struct {
> +          unsigned char guid[16];
> +          uint32 offset;
> +          uint32 length;
> +      } cert_table_entry[];
> +  };
> +
> +Because QEMU supplies the certificate to the hypervisor prior to the
> +hypervisor actually issuing an attestation request, there is potential for
> +the certificate data to becoming stale due to a firmware upgrade or other
> +host management activities. The ioctl which enables certificate-fetching
> +support via the KVM hypervisor, KVM_SEV_SNP_ENABLE_REQ_CERTS, documents
> +the following suggested scheme as part of its linux documentation (v7.3
> +at the time of this writing), and it is highly recommended that all software
> +interacting with these certificates, or performing operations that may
> +affect their validity, adopt this same scheme for interoperability::
> +
> +  The recommended scheme to do this is to use file locking (e.g. via
> +  fcntl()'s F_OFD_SETLK) in the following manner:
> +
> +    - Prior to obtaining/providing certificate data as part of servicing an
> +      exit type of ``KVM_EXIT_SNP_REQ_CERTS``, the VMM should obtain a
> +      shared/read or exclusive/write lock on the certificate blob file before
> +      reading it and returning it to KVM, and continue to hold the lock until
> +      the attestation request is actually sent to firmware. To facilitate
> +      this, the VMM can set the ``immediate_exit`` flag of kvm_run just after
> +      supplying the certificate data, and just before resuming the vCPU.
> +      This will ensure the vCPU will exit again to userspace with ``-EINTR``
> +      after it finishes fetching the attestation request from firmware, at
> +      which point the VMM can safely drop the file lock.
> +
> +    - Tools/libraries that perform updates to SNP firmware TCB values or
> +      endorsement keys (e.g. via /dev/sev interfaces such as ``SNP_COMMIT``,
> +      ``SNP_SET_CONFIG``, or ``SNP_VLEK_LOAD``, see
> +      Documentation/virt/coco/sev-guest.rst for more details) in such a way
> +      that the certificate blob needs to be updated, should similarly take an
> +      exclusive lock on the certificate blob for the duration of any updates
> +      to endorsement keys or the certificate blob contents to ensure that
> +      VMMs using the above scheme will not return certificate blob data that
> +      is out of sync with the endorsement key used by firmware at the time
> +      the attestation request is actually issued.
> +
> +  This scheme is recommended so that tools can use a fairly generic/natural
> +  approach to synchronizing firmware/certificate updates via file-locking,
> +  which should make it easier to maintain interoperability across
> +  tools/VMMs/vendors.
> +
> +As such, QEMU implements this same scheme by waiting for a read-lock on the
> +file containing the certificate data before supplying any data it contains.
> +In order to ensure that the lock is not released until after the hypervisor
> +is finished fetching the attestation report for the guest, the lock is held
> +until a subsequent exit from the hypervisor that indicates the attestation
> +report has already been fetched, as which point QEMU will drop the lock.
>  
>  Debugging
>  ---------
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 4810d53e58..8884a1eb66 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1104,6 +1104,25 @@
>  # @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
>  #     is enabled (default: zero) (since 11.1)
>  #
> +# @certs-filename: Certificate data that can be passed to guests as
> +#     part of an attestion request.  See the
> +#     :doc:`/system/i386/amd-memory-encryption` documentation for more
> +#     details on the usage and file format.
> +#     (default: none) (since: 11.2)
> +#
> +# @certs-timeout: Maximum time in milliseconds to wait to obtain a
> +#     read lock on the certificate file specified by @certs-filename.
> +#     This is not a cumulative value and only affects how long QEMU
> +#     waits before returning execution to the vCPU and informing the
> +#     guest of the timeout, so the guest can still continue retrying
> +#     for as long as it likes, which for linux guests at the time of
> +#     this writing will be about 60 seconds before the guest gives
> +#     up and disables further requests.  If this guest-side timeout is
> +#     insufficient, set this higher to allow more time to fetch the
> +#     certificate.  If the guest-side timeout is sufficient,
> +#     set this lower to reduce the likelihood of soft lockups in the
> +#     guest.  (default: 100) (since: 11.2)
> +#
>  # Since: 9.1
>  ##
>  { 'struct': 'SevSnpGuestProperties',
> @@ -1117,7 +1136,9 @@
>              '*host-data': 'str',
>              '*vcek-disabled': 'bool',
>              '*secure-tsc': 'bool',
> -            '*tsc-frequency': 'uint32' } }
> +            '*tsc-frequency': 'uint32',
> +            '*certs-filename': 'str',
> +            '*certs-timeout': 'uint32' } }

Why uint32 for time?  We commonly use 'int'.

Hmm, see [*] below.

>  
>  ##
>  # @TdxGuestProperties:
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index da922f72d7..9f7bdf9618 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -6651,6 +6651,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
>              break;
>          }
>          ret = 0;
> +        break;
> +    case KVM_EXIT_SNP_REQ_CERTS:
> +        if (!sev_snp_enabled()) {
> +            error_report("KVM: Encountered a certificate request exit for a "
> +                         "non-SEV-SNP guest.");
> +            ret = -1;
> +        } else {
> +            ret = kvm_handle_snp_req_certs(cs, run);
> +        }
> +
>          break;
>      default:
>          fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
> diff --git a/target/i386/sev-system-stub.c b/target/i386/sev-system-stub.c
> index d97536cca1..e53f1bbefc 100644
> --- a/target/i386/sev-system-stub.c
> +++ b/target/i386/sev-system-stub.c
> @@ -34,3 +34,8 @@ int sev_es_save_reset_vector(void *flash_ptr, uint64_t flash_size)
>  void pc_system_parse_sev_metadata(uint8_t *flash_ptr, size_t flash_size)
>  {
>  }
> +
> +int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run)
> +{
> +    g_assert_not_reached();
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 11072b00dc..3cb2e9cd0c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -186,6 +186,8 @@ struct SevSnpGuestState {
>      uint8_t *id_auth;
>      char *host_data;
>      uint32_t tsc_khz;
> +    char *certs_filename;
> +    uint32_t certs_timeout;
>  
>      struct kvm_sev_snp_launch_start kvm_start_conf;
>      struct kvm_sev_snp_launch_finish kvm_finish_conf;
> @@ -1701,6 +1703,278 @@ sev_snp_launch_finish(SevCommonState *sev_common)
>      sev_set_guest_state(sev_common, SEV_STATE_RUNNING);
>  }
>  
> +/*
> + * Cases where -EAGAIN is returned on an error path are expected to terminate
> + * after the next call (e.g. via a qemu_open() error) if they are not transient
> + * issues. These potentially-transient issues are still logged however, since
> + * they are not expected on a well-behaved system.
> + */
> +static int open_certs_locked(SevSnpGuestState *sev_snp_guest)
> +{
> +    struct stat locked_file, current_file;
> +    int fd, ret;
> +
> +    fd = qemu_open(sev_snp_guest->certs_filename, O_RDONLY, NULL);
> +    if (fd == -1) {
> +        error_report_once("Unable to open certificate file: %s",
> +                          strerror(errno));
> +        return -errno;
> +    }
> +
> +    if (fstat(fd, &locked_file) == -1) {
> +        error_report_once("Unable to check status of certificate file: %s",
> +                          strerror(errno));
> +        ret = -errno;
> +        goto out_close;
> +    }
> +
> +    ret = qemu_lock_fd(fd, 0, 0, false);
> +    if (ret == -EACCES) {
> +        /*
> +         * F_SETLK is documented to potentially return either EACCES or EAGAIN
> +         * for contended file locks.
> +         */
> +        ret = -EAGAIN;
> +    } else if (ret == -EDEADLK) {
> +        error_report_once("Potential deadlock encountered while trying "
> +                          "to lock the certificate file");
> +        ret = -EAGAIN;
> +    }
> +
> +    if (ret) {
> +        if (ret != -EAGAIN) {
> +            error_report_once("Error while trying to lock certificate file: %s",
> +                              strerror(-ret));
> +        }
> +        goto out_close;
> +    }
> +
> +    if (stat(sev_snp_guest->certs_filename, &current_file) == -1) {
> +        error_report_once("Certificate file no longer present: %s",
> +                          strerror(errno));
> +        ret = -EAGAIN;
> +        goto out_close;
> +    }
> +
> +    /*
> +     * Locked files should not be changing; this violates the locking
> +     * protocol assumed by this implementation. Indicate the need to
> +     * retry to the caller, but make sure to log it first.
> +     */
> +    if (locked_file.st_ino != current_file.st_ino ||
> +        locked_file.st_dev != current_file.st_dev) {
> +        error_report_once("Locked certificate file was recreated");
> +        ret = -EAGAIN;
> +        goto out_close;
> +    }
> +
> +    /*
> +     * The locked file is intact and as-expected, and it has not been replaced
> +     * since then, so the fd should be okay to use.
> +     */
> +    return fd;
> +
> +out_close:
> +    close(fd);
> +    return ret;
> +}
> +
> +static void close_certs(int fd)
> +{
> +    qemu_unlock_fd(fd, 0, 0);
> +    close(fd);
> +}
> +
> +static ssize_t get_certs_size(int fd)
> +{
> +    ssize_t size;
> +
> +    size = lseek(fd, 0, SEEK_END);
> +    if (size == -1) {
> +        error_report_once("Error while determining the size of the certificate file: %s",
> +                          strerror(errno));
> +        return -errno;
> +    }
> +
> +    return size;
> +}
> +
> +static int read_certs(int fd, void *buf, size_t buf_len)
> +{
> +    ssize_t n;
> +
> +    n = lseek(fd, 0, SEEK_SET);
> +    if (n == -1) {
> +        error_report_once("Error while setting certificate file position: %s",
> +                          strerror(errno));
> +        return -errno;
> +    }
> +
> +    while ((n = read(fd, buf, buf_len)) != 0) {
> +        if (n == -1) {
> +            if (errno == EINTR) {
> +                continue;
> +            }
> +
> +            error_report_once("Error reading the certificate file: %s",
> +                              strerror(errno));
> +            return -errno;
> +        }
> +        buf += n;
> +        buf_len -= n;
> +    }
> +
> +    return 0;
> +}
> +
> +static void snp_release_certs_lock(CPUState *cpu, run_on_cpu_data data)
> +{
> +    close_certs(data.host_int);
> +}
> +
> +/*
> + * NOTE: It is not expected that the guest will call this in a tight loop
> + * where error reporting would flood logs. A failed certificate fetch results
> + * in a failed attestation request, which results in a disabled attestation
> + * driver in the guest usually.
> + *
> + * However, one instance of such an error is usually enough of a clue to
> + * to identify the component of the system that should be investigated.
> + *
> + * So, to guard against a malicious guest trying to flood host logs, while
> + * still providing reasonable debug/error information to the user, use
> + * error_report_once() throughout this function and its supporting helpers.
> + */
> +int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run)
> +{
> +    struct kvm_exit_snp_req_certs *req_certs = &run->snp_req_certs;
> +    SevSnpGuestState *sev_snp_guest;
> +    ssize_t certs_size;
> +    uint32_t npages;
> +    void *guest_buf;
> +    int64_t t0_ms;
> +    hwaddr buf_sz;
> +    int ret, fd;
> +    hwaddr gpa;
> +
> +    gpa = req_certs->gpa;
> +    npages = req_certs->npages;
> +
> +    sev_snp_guest = SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
> +    if (!sev_snp_guest->certs_filename) {
> +        req_certs->ret = 0;
> +        return 0;
> +    }
> +
> +    /*
> +     * By default, any errors encountered below will be forwarded to the guest
> +     * as an EIO. This will likely result in the guest disabling its driver, but
> +     * it's better than crashing QEMU.
> +     *
> +     * Leave the special cases, SUCCESS/ENOSPC/EAGAIN, to re-set the guest
> +     * return code as-needed.
> +     *
> +     * Errors that are useful for debugging QEMU will still be logged, with care
> +     * to avoid busy/EAGAIN paths/etc.
> +     */
> +    req_certs->ret = EIO;
> +
> +    /*
> +     * -EAGAIN from open_certs_locked() indicates that a lock could not be
> +     * obtained on the certificate file. It would be possible to set
> +     * req_certs->ret = EAGAIN to immediately inform the requesting vCPU of
> +     * this condition so that it can retry again later, however, linux guests
> +     * are currently hard-limited to a 60 second timeout, at which point they
> +     * will assume misbehavior and refuse to continue issuing attestation
> +     * requests from that point forward.
> +     *
> +     * Avoid risking triggering this by instead busy-waiting on the QEMU side,
> +     * which allows more time for QEMU to wait on potentially long-running
> +     * tasks like SNP firmware updates/etc. which may be holding an exclusive
> +     * lock on the certificate file.
> +     *
> +     * The down-side to doing things this way is that too long of a busy-wait
> +     * will result in soft-lockups in the guest, but the guest will otherwise
> +     * continue normally afterward.
> +     *
> +     * If the QEMU-side timeout is reached, then just go ahead and indicate
> +     * EAGAIN to the guest, at which point the above-mentioned guest-side
> +     * timeout will trigger if QEMU's timeout exceeds that of the guest. But
> +     * that's better than killing the vCPU, which is the only viable
> +     * alternative at that point.
> +     */
> +    t0_ms = g_get_monotonic_time() / 1000;
> +
> +    do {
> +        fd = open_certs_locked(sev_snp_guest);
> +
> +        if (fd == -EAGAIN) {
> +            int64_t t1_ms = g_get_monotonic_time() / 1000;
> +
> +            if (t1_ms - t0_ms > sev_snp_guest->certs_timeout) {

[*] Left operand is int64_t, and the right operand is uint32_t.  The
right operand is converted to int64_t, which is safe, but to see that,
you need to know the "usual arithmetic conversions (C99 § 6.3.1.8)
intimately.  Suggest to make the right operand int64_t to avoid such
brain burn.

> +                req_certs->ret = EAGAIN;
> +                return 0;
> +            }
> +
> +            /*
> +             * If some other process (QEMU only ever takes a read lock) has an
> +             * exclusive lock on the certificate file, then it is likely because
> +             * a firmware update or other long-running operation is taking place;
> +             * it's a waste to re-try at a high rate.
> +             */
> +            g_usleep(1000);
> +        } else if (fd < 0) {
> +            return 0;
> +        }
> +    } while (fd == -EAGAIN);
> +
> +    certs_size = get_certs_size(fd);
> +    if (certs_size < 0) {
> +        goto err_close;
> +    }
> +
> +    buf_sz = (hwaddr)npages * TARGET_PAGE_SIZE;
> +    if (buf_sz < certs_size) {
> +        /* Communicate to guest that more pages are needed. */
> +        req_certs->ret = ENOSPC;
> +        req_certs->npages = DIV_ROUND_UP(certs_size, TARGET_PAGE_SIZE);
> +        goto err_close;
> +    }
> +
> +    guest_buf = address_space_map(&address_space_memory, gpa, &buf_sz, true,
> +                                  MEMTXATTRS_UNSPECIFIED);
> +    if (!guest_buf) {
> +        error_report_once("Unable to map guest buffer to provide "
> +                          "certificate data.");
> +        goto err_close;
> +    }
> +
> +    if (buf_sz < (hwaddr)npages * TARGET_PAGE_SIZE) {
> +        error_report_once("Unable to map the full guest buffer to provide "
> +                          "certificate data.");
> +        address_space_unmap(&address_space_memory, guest_buf, buf_sz, true, 0);
> +        goto err_close;
> +    }
> +
> +    ret = read_certs(fd, guest_buf, buf_sz);
> +    address_space_unmap(&address_space_memory, guest_buf, buf_sz, true, buf_sz);
> +
> +    if (!ret) {
> +        /*
> +         * Looks good, return the certificate to the guest and prepare to
> +         * release the file lock once KVM signals completion.
> +         */
> +        kvm_add_immediate_exit_callback(cpu, snp_release_certs_lock,
> +                                        RUN_ON_CPU_HOST_INT(fd));
> +
> +        req_certs->ret = 0;
> +        return 0;
> +    }
> +
> +err_close:
> +    close_certs(fd);
> +    return 0;
> +}
>  
>  static void
>  sev_vm_state_change(void *opaque, bool running, RunState state)
> @@ -2051,6 +2325,41 @@ static int sev_snp_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>      g_free(sev_snp_guest->kernel_hashes_data);
>      sev_snp_guest->kernel_hashes_data = NULL;
>  
> +    if (sev_snp_guest->certs_filename) {
> +        unsigned long certs_attr;
> +        struct kvm_device_attr attr = {
> +            .group = KVM_X86_GRP_SEV,
> +            .attr = KVM_X86_SEV_SNP_REQ_CERTS,
> +            .addr = (unsigned long) &certs_attr
> +        };
> +        int ret;
> +
> +        if (!kvm_check_extension(kvm_state, KVM_CAP_SYS_ATTRIBUTES) ||
> +            !kvm_check_extension(kvm_state, KVM_CAP_IMMEDIATE_EXIT)) {
> +            return -EINVAL;
> +        }
> +
> +        ret = kvm_ioctl(kvm_state, KVM_GET_DEVICE_ATTR, &attr);
> +        if (ret < 0) {
> +            error_setg(errp, "Certificate-fetching attribute not present.");
> +            return ret;
> +        }
> +
> +        if (!certs_attr) {
> +            error_setg(errp, "Certificate-fetching attribute not set.");
> +            return -EINVAL;
> +        }
> +
> +        ret = sev_ioctl(SEV_COMMON(sev_snp_guest)->sev_fd,
> +                        KVM_SEV_SNP_ENABLE_REQ_CERTS,
> +                        NULL, NULL);
> +        if (ret) {
> +            error_setg(errp, "Failed to enable support for SEV-SNP "
> +                             "certificate-fetching requests.");
> +            return ret;
> +        }
> +    }
> +
>      return 0;
>  }
>  
> @@ -3320,6 +3629,26 @@ sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
>      SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
>  }
>  
> +static char *
> +sev_snp_guest_get_certs_filename(Object *obj, Error **errp)
> +{
> +    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
> +
> +    return g_strdup(sev_snp_guest->certs_filename);
> +}
> +
> +static void
> +sev_snp_guest_set_certs_filename(Object *obj, const char *value, Error **errp)
> +{
> +    SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
> +
> +    if (sev_snp_guest->certs_filename) {
> +        g_free(sev_snp_guest->certs_filename);
> +    }
> +
> +    sev_snp_guest->certs_filename = value ? g_strdup(value) : NULL;
> +}
> +
>  static void
>  sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>  {
> @@ -3361,6 +3690,12 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>      object_class_property_add(oc, "tsc-frequency", "uint32",
>                                sev_snp_guest_get_tsc_frequency,
>                                sev_snp_guest_set_tsc_frequency, NULL, NULL);
> +    object_class_property_add_str(oc, "certs-filename",
> +                                  sev_snp_guest_get_certs_filename,
> +                                  sev_snp_guest_set_certs_filename);
> +    object_class_property_add_uint32_ptr(oc, "certs-timeout",
> +                                   offsetof(SevSnpGuestState, certs_timeout),
> +                                   OBJ_PROP_FLAG_READWRITE);
>  }
>  
>  static void
> @@ -3374,6 +3709,8 @@ sev_snp_guest_instance_init(Object *obj)
>      /* default init/start/finish params for kvm */
>      sev_snp_guest->kvm_start_conf.policy = DEFAULT_SEV_SNP_POLICY;
>      sev_set_feature(SEV_COMMON(sev_snp_guest), SVM_SEV_FEAT_SNP_ACTIVE, true);
> +
> +    sev_snp_guest->certs_timeout = 100;
>  }
>  
>  static void
> @@ -3387,6 +3724,7 @@ sev_snp_guest_finalize(Object *obj)
>      g_free(sev_snp_guest->id_auth_base64);
>      g_free(sev_snp_guest->id_auth);
>      g_free(sev_snp_guest->host_data);
> +    g_free(sev_snp_guest->certs_filename);
>  }
>  
>  /* guest info specific to sev-snp */
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 7725f92e19..4d9e9b44b4 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -15,6 +15,7 @@
>  #define I386_SEV_H
>  
>  #include "target/i386/svm.h"
> +#include "system/kvm.h"
>  
>  #ifndef CONFIG_USER_ONLY
>  #include CONFIG_DEVICES /* CONFIG_SEV */
> @@ -198,5 +199,5 @@ void pc_system_parse_sev_metadata(uint8_t *flash_ptr, size_t flash_size);
>  
>  uint32_t sev_get_cbit_position(void);
>  uint32_t sev_get_reduced_phys_bits(void);
> -
> +int kvm_handle_snp_req_certs(CPUState *cpu, struct kvm_run *run);
>  #endif