In preparation for SEV firmware hotloading support, introduce a new way
to create, activate, and decommission GCTX pages such that ccp is has
all GCTX pages available to update as needed.

Compliance with SEV-SNP API section 3.3 Firmware Updates and 4.1.1
Live Update: before a firmware is committed, all active GCTX pages
should be updated with SNP_GUEST_STATUS to ensure their data structure
remains consistent for the new firmware version.
There can only be CPUID 0x8000001f_EDX-1 many SEV-SNP asids in use at
one time, so this map associates asid to gctx in order to track which
addresses are active gctx pages that need updating. When an asid and
gctx page are decommissioned, the page is removed from tracking for
update-purposes.

CC: Sean Christopherson <seanjc@google.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: Ashish Kalra <ashish.kalra@amd.com>
CC: Tom Lendacky <thomas.lendacky@amd.com>
CC: John Allen <john.allen@amd.com>
CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Michael Roth <michael.roth@amd.com>
CC: Luis Chamberlain <mcgrof@kernel.org>
CC: Russ Weight <russ.weight@linux.dev>
CC: Danilo Krummrich <dakr@redhat.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: "Rafael J. Wysocki" <rafael@kernel.org>
CC: Tianfei zhang <tianfei.zhang@intel.com>
CC: Alexey Kardashevskiy <aik@amd.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 drivers/crypto/ccp/sev-dev.c | 159 ++++++++++++++++++++++++++++++++++-
 drivers/crypto/ccp/sev-dev.h |   4 +
 include/linux/psp-sev.h      |  55 ++++++++++++
 3 files changed, 217 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index af018afd9cd7f..d8c35b8478ff5 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/file.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/kthread.h>
@@ -109,6 +110,10 @@ static void *sev_init_ex_buffer;
  */
 static struct sev_data_range_list *snp_range_list;
 
+/* SEV ASID data tracks resources associated with an ASID to safely manage operations. */
+struct sev_asid_data *sev_asid_data;
+u32 nr_asids, sev_min_asid, sev_es_max_asid;
+
 static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
 {
 	struct sev_device *sev = psp_master->sev_data;
@@ -1093,6 +1098,109 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
 	return 0;
 }
 
+static bool sev_check_external_user(int fd);
+void *sev_snp_create_context(int fd, int asid, int *psp_ret)
+{
+	struct sev_data_snp_addr data = {};
+	void *context;
+	int rc, error;
+
+	if (!sev_check_external_user(fd))
+		return ERR_PTR(-EBADF);
+
+	if (!sev_asid_data)
+		return ERR_PTR(-ENODEV);
+
+	if (asid < 0 || asid >= nr_asids)
+		return ERR_PTR(-EINVAL);
+
+	/* Can't create a context for a used ASID. */
+	if (WARN_ON_ONCE(sev_asid_data[asid].snp_context))
+		return ERR_PTR(-EBUSY);
+
+	/* Allocate memory for context page */
+	context = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT);
+	if (!context)
+		return ERR_PTR(-ENOMEM);
+
+	data.address = __psp_pa(context);
+	rc = sev_do_cmd(SEV_CMD_SNP_GCTX_CREATE, &data, &error);
+	if (rc) {
+		pr_warn("Failed to create SEV-SNP context, rc=%d fw_error=0x%x",
+			rc, error);
+		if (psp_ret)
+			*psp_ret = error;
+		snp_free_firmware_page(context);
+		return ERR_PTR(-EIO);
+	}
+
+	sev_asid_data[asid].snp_context = context;
+
+	return context;
+}
+EXPORT_SYMBOL_GPL(sev_snp_create_context);
+
+int sev_snp_activate_asid(int fd, int asid, int *psp_ret)
+{
+	struct sev_data_snp_activate data = {0};
+	void *context;
+
+	if (!sev_check_external_user(fd))
+		return -EBADF;
+
+	if (!sev_asid_data)
+		return -ENODEV;
+
+	if (asid < 0 || asid >= nr_asids)
+		return -EINVAL;
+
+	context = sev_asid_data[asid].snp_context;
+	if (WARN_ON_ONCE(!context))
+		return -EINVAL;
+
+	data.gctx_paddr = __psp_pa(context);
+	data.asid = asid;
+	return sev_do_cmd(SEV_CMD_SNP_ACTIVATE, &data, psp_ret);
+}
+EXPORT_SYMBOL_GPL(sev_snp_activate_asid);
+
+int sev_snp_guest_decommission(int fd, int asid, int *psp_ret)
+{
+	struct sev_data_snp_addr addr = {};
+	struct sev_asid_data *data;
+	int ret, error;
+
+	if (!sev_check_external_user(fd))
+		return -EBADF;
+
+	if (!sev_asid_data)
+		return -ENODEV;
+
+	if (asid < 0 || asid >= nr_asids)
+		return -EINVAL;
+
+	data = &sev_asid_data[asid];
+	/* If context is not created then do nothing */
+	if (!data->snp_context)
+		return 0;
+
+	/* Do the decommission, which will unbind the ASID from the SNP context */
+	addr.address = __sme_pa(data->snp_context);
+	ret = sev_do_cmd(SEV_CMD_SNP_DECOMMISSION, &addr, &error);
+
+	if (WARN_ONCE(ret, "Failed to release guest context, rc=%d, fw_error=0x%x", ret, error)) {
+		if (psp_ret)
+			*psp_ret = error;
+		return ret;
+	}
+
+	snp_free_firmware_page(data->snp_context);
+	data->snp_context = NULL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(sev_snp_guest_decommission);
+
 static int __sev_snp_init_locked(int *error)
 {
 	struct psp_device *psp = psp_master;
@@ -1306,6 +1414,27 @@ static int __sev_platform_init_locked(int *error)
 	return 0;
 }
 
+static int sev_asid_data_init(void)
+{
+	u32 eax, ebx, ecx;
+
+	if (sev_asid_data)
+		return 0;
+
+	cpuid(0x8000001f, &eax, &ebx, &ecx, &sev_min_asid);
+	if (!ecx)
+		return -ENODEV;
+
+	nr_asids = ecx + 1;
+	sev_es_max_asid = sev_min_asid - 1;
+
+	sev_asid_data = kcalloc(nr_asids, sizeof(*sev_asid_data), GFP_KERNEL);
+	if (!sev_asid_data)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int _sev_platform_init_locked(struct sev_platform_init_args *args)
 {
 	struct sev_device *sev;
@@ -1319,6 +1448,10 @@ static int _sev_platform_init_locked(struct sev_platform_init_args *args)
 	if (sev->state == SEV_STATE_INIT)
 		return 0;
 
+	rc = sev_asid_data_init();
+	if (rc)
+		return rc;
+
 	/*
 	 * Legacy guests cannot be running while SNP_INIT(_EX) is executing,
 	 * so perform SEV-SNP initialization at probe time.
@@ -2329,6 +2462,9 @@ static void __sev_firmware_shutdown(struct sev_device *sev, bool panic)
 		snp_range_list = NULL;
 	}
 
+	kfree(sev_asid_data);
+	sev_asid_data = NULL;
+
 	__sev_snp_shutdown_locked(&error, panic);
 }
 
@@ -2377,10 +2513,31 @@ static struct notifier_block snp_panic_notifier = {
 	.notifier_call = snp_shutdown_on_panic,
 };
 
+static bool file_is_sev(struct file *filep)
+{
+	return filep && filep->f_op == &sev_fops;
+}
+
+static bool sev_check_external_user(int fd)
+{
+	struct fd f;
+	bool ret = true;
+
+	f = fdget(fd);
+	if (!fd_file(f))
+		return false;
+
+	if (!file_is_sev(fd_file(f)))
+		ret = false;
+
+	fdput(f);
+	return ret;
+}
+
 int sev_issue_cmd_external_user(struct file *filep, unsigned int cmd,
 				void *data, int *error)
 {
-	if (!filep || filep->f_op != &sev_fops)
+	if (!file_is_sev(filep))
 		return -EBADF;
 
 	return sev_do_cmd(cmd, data, error);
diff --git a/drivers/crypto/ccp/sev-dev.h b/drivers/crypto/ccp/sev-dev.h
index 3e4e5574e88a3..ccf3ba78d8332 100644
--- a/drivers/crypto/ccp/sev-dev.h
+++ b/drivers/crypto/ccp/sev-dev.h
@@ -65,4 +65,8 @@ void sev_dev_destroy(struct psp_device *psp);
 void sev_pci_init(void);
 void sev_pci_exit(void);
 
+struct sev_asid_data {
+	void *snp_context;
+};
+
 #endif /* __SEV_DEV_H */
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 903ddfea85850..0b3b7707ccb21 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -942,6 +942,61 @@ int sev_guest_decommission(struct sev_data_decommission *data, int *error);
  */
 int sev_do_cmd(int cmd, void *data, int *psp_ret);
 
+/**
+ * sev_snp_create_context - allocates an SNP context firmware page
+ *
+ * Associates the created context with the ASID that an activation
+ * call after SNP_LAUNCH_START will commit. The association is needed
+ * to track active guest context pages to refresh during firmware hotload.
+ *
+ * @fd:      A file descriptor for the SEV device
+ * @asid:    The ASID allocated to the caller that will be used in a subsequent SNP_ACTIVATE.
+ * @psp_ret: sev command return code.
+ *
+ * Returns:
+ * A pointer to the SNP context page, or an ERR_PTR of
+ * -%ENODEV    if the PSP device is not available
+ * -%ENOTSUPP  if PSP device does not support SEV
+ * -%ETIMEDOUT if the SEV command timed out
+ * -%EIO       if PSP device returned a non-zero return code
+ */
+void *sev_snp_create_context(int fd, int asid, int *psp_ret);
+
+/**
+ * sev_snp_activate_asid - issues SNP_ACTIVATE for the ASID and associated guest context page.
+ *
+ * @fd:      A file descriptor for the SEV device
+ * @asid:    The ASID to activate.
+ * @psp_ret: sev command return code.
+ *
+ * Returns:
+ * 0 if the SEV device successfully processed the command
+ * -%ENODEV    if the PSP device is not available
+ * -%ENOTSUPP  if PSP device does not support SEV
+ * -%ETIMEDOUT if the SEV command timed out
+ * -%EIO       if PSP device returned a non-zero return code
+ */
+int sev_snp_activate_asid(int fd, int asid, int *psp_ret);
+
+/**
+ * sev_snp_guest_decommission - issues SNP_DECOMMISSION for an ASID's guest context page, and frees
+ * it.
+ *
+ * The caller must ensure mutual exclusion with any process that may deactivate ASIDs.
+ *
+ * @fd:      A file descriptor for the SEV device
+ * @asid:    The ASID to activate.
+ * @psp_ret: sev command return code.
+ *
+ * Returns:
+ * 0 if the SEV device successfully processed the command
+ * -%ENODEV    if the PSP device is not available
+ * -%ENOTSUPP  if PSP device does not support SEV
+ * -%ETIMEDOUT if the SEV command timed out
+ * -%EIO       if PSP device returned a non-zero return code
+ */
+int sev_snp_guest_decommission(int fd, int asid, int *psp_ret);
+
 void *psp_copy_user_blob(u64 uaddr, u32 len);
 void *snp_alloc_firmware_page(gfp_t mask);
 void snp_free_firmware_page(void *addr);
-- 
2.47.0.277.g8800431eea-goog

On Tue, Nov 12, 2024, Dionna Glaze wrote:
> @@ -109,6 +110,10 @@ static void *sev_init_ex_buffer;
>   */
>  static struct sev_data_range_list *snp_range_list;
>  
> +/* SEV ASID data tracks resources associated with an ASID to safely manage operations. */
> +struct sev_asid_data *sev_asid_data;
> +u32 nr_asids, sev_min_asid, sev_es_max_asid;
> +
>  static inline bool sev_version_greater_or_equal(u8 maj, u8 min)
>  {
>  	struct sev_device *sev = psp_master->sev_data;
> @@ -1093,6 +1098,109 @@ static int snp_filter_reserved_mem_regions(struct resource *rs, void *arg)
>  	return 0;
>  }
>  
> +static bool sev_check_external_user(int fd);
> +void *sev_snp_create_context(int fd, int asid, int *psp_ret)
> +{
> +	struct sev_data_snp_addr data = {};
> +	void *context;
> +	int rc, error;
> +
> +	if (!sev_check_external_user(fd))
> +		return ERR_PTR(-EBADF);
> +
> +	if (!sev_asid_data)
> +		return ERR_PTR(-ENODEV);
> +
> +	if (asid < 0 || asid >= nr_asids)
> +		return ERR_PTR(-EINVAL);
> +
> +	/* Can't create a context for a used ASID. */
> +	if (WARN_ON_ONCE(sev_asid_data[asid].snp_context))
> +		return ERR_PTR(-EBUSY);

Tracking contexts in an array that's indexed per ASID is unsafe and unnecessarily
splits ASID management across KVM and the PSP driver.  There is zero reason the
PSP driver needs to care about ASIDs.  Attempting to police KVM is futile, and
leads to bloated, convoluted code.

AFAICT, there is nothing to guard against a use-after-free in 
snp_update_guest_contexts().  The need to handle SEV_RET_INVALID_GUEST is a pretty
big clue that there are races between KVM and firmware updates.

		if (!sev_asid_data[i].snp_context)
			continue;

		status_data.gctx_paddr = __psp_pa(sev_asid_data[i].snp_context);
		status_data.address = __psp_pa(snp_guest_status);
		rc = sev_do_cmd(SEV_CMD_SNP_GUEST_STATUS, &status_data, &error);
		if (!rc)
			continue;

		/*
		 * Handle race with SNP VM being destroyed/decommissoned,
		 * if guest context page invalid error is returned,
		 * assume guest has been destroyed.
		 */
		if (error == SEV_RET_INVALID_GUEST)
			continue;

Using an array is also inefficient, as it requires iterating over all possible
ASIDs, many of which may be unused.

Furthermore, handling this in the PSP driver (correctly) leads to unnecessary
locking.  KVM already protects SNP ASID allocations with sev_deactivate_lock, I
see zero reason to complicate things with another lock.

The "rollback" mechanism is also broken.  If SEV_CMD_SNP_GUEST_STATUS fails,
synthetic_restore_required is set and never cleared, and impacts *all* SEV PSP
commands.  I.e. failure to update one guest context comletely cripples the entire
system.  Not to mention synthetic_restore_required also lacks any form of SMP
synchronication.

I also don't see how a rollback is possible if an error occurs after one or more
guest contexts have been updated.  Presumably trying to rollback in that state
will leave the updated guests in a bad state.  Of course, I don't see any rollback
code as nothing ever clears synthetic_restore_required, so what's intented to
happen is entirely unclear.

I also don't see anything in this series that explains why a SEV_CMD_SNP_GUEST_STATUS
failure shouldn't be treated as a fatal error.  Of the error codes listed in the
SNP ABI, everything except UPDATE_FAILED is clearly a software bug.  And I can't
find anything that explains when UPDATE_FAILED will be returned.

  Table 80. Status Codes for SNP_GUEST_STATUS
  Status                          Condition
  SUCCESS                         Successful completion.
  INVALID_PLATFORM_STATE          The platform is not in the INIT state.
  INVALID_ADDRESS                 The address is invalid for use by the firmware.
  INVALID_PARAM                   MBZ fields are not zero.
  INVALID_GUEST                   The guest context page was invalid.
  INVALID_PAGE_STATE              The guest status page was not in the correct state.
  INVALID_PAGE_SIZE               The guest status page was not the correct size.
  UPDATE_FAILED                   Update of the firmware internal state or a guest context page has failed.

Somewhat off the cuff, I think the only sane way to approach this is to call into
KVM when doing a firmware update, and let KVM react accordingly.   E.g. let KVM
walk its list of VMs in order to update SNP VMs, taking kvm_lock and the somewhat
misnamed sev_deactivate_lock() as needed.  Then if updating a guest context fails,
terminate _that_ VM, and move on to the next VM.

Side topic, I don't see any code that ensures no SEV or SEV-ES VMs are running.
Is the idea to let userspace throw noodles at the PSP and see what sticks?

+        Provide support for AMD Platform Security Processor firmware.
+        The PSP firmware can be updated while no SEV or SEV-ES VMs are active.
                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+        Users of this feature should be aware of the error modes that indicate
+        required manual rollback or reset due to instablity.