On Fri, Oct 06, 2017 at 08:05:59PM -0500, Brijesh Singh wrote:
> AMD's new Secure Encrypted Virtualization (SEV) feature allows the
> memory contents of virtual machines to be transparently encrypted with a
> key unique to the VM. The programming and management of the encryption
> keys are handled by the AMD Secure Processor (AMD-SP) which exposes the
> commands for these tasks. The complete spec is available at:
> 
> http://support.amd.com/TechDocs/55766_SEV-KM%20API_Specification.pdf
> 
> Extend the AMD-SP driver to provide the following support:
> 
>  - an in-kernel API to communicate with the SEV firmware. The API can be
>    used by the hypervisor to create encryption context for a SEV guest.
> 
>  - a userspace IOCTL to manage the platform certificates.
> 
> Cc: Paolo Bonzini <pbonz...@redhat.com>
> Cc: "Radim Krčmář" <rkrc...@redhat.com>
> Cc: Borislav Petkov <b...@suse.de>
> Cc: Herbert Xu <herb...@gondor.apana.org.au>
> Cc: Gary Hook <gary.h...@amd.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: linux-crypto@vger.kernel.org
> Cc: k...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Improvements-by: Borislav Petkov <b...@suse.de>
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> 
> Based on Boris feedback split this patch in 9 logical patches, they are
> numbers from 12.1 to 12.9.
> 
>  drivers/crypto/ccp/psp-dev.c | 244 
> +++++++++++++++++++++++++++++++++++++++++++
>  drivers/crypto/ccp/psp-dev.h |  17 +++
>  include/linux/psp-sev.h      | 159 ++++++++++++++++++++++++++++
>  3 files changed, 420 insertions(+)

A bunch of fixes ontop:

* sev_fops_registered is superfluous if you can use psp->has_sev_fops

* s/sev_handle_cmd/sev_do_cmd/g - it really is sev_do_cmd(). "handle" is
something else.

* Flip misc dev reg logic in sev_ops_init()

* PSP_P2CMSG needs arg eval

* text streamlining

---
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index e9b776c3acb2..f0e0fc1fb512 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -31,7 +31,6 @@
 #define DEVICE_NAME    "sev"
 
 static DEFINE_MUTEX(sev_cmd_mutex);
-static bool sev_fops_registered;
 
 static struct psp_device *psp_alloc_struct(struct sp_device *sp)
 {
@@ -121,7 +120,7 @@ static int sev_cmd_buffer_len(int cmd)
        return 0;
 }
 
-static int sev_handle_cmd(int cmd, void *data, int *psp_ret)
+static int sev_do_cmd(int cmd, void *data, int *psp_ret)
 {
        unsigned int phys_lsb, phys_msb;
        struct psp_device *psp;
@@ -182,26 +181,26 @@ static long sev_ioctl(struct file *file, unsigned int 
ioctl, unsigned long arg)
        return -ENOTTY;
 }
 
-const struct file_operations sev_fops = {
+static const struct file_operations sev_fops = {
        .owner  = THIS_MODULE,
        .unlocked_ioctl = sev_ioctl,
 };
 
 int sev_platform_init(struct sev_data_init *data, int *error)
 {
-       return sev_handle_cmd(SEV_CMD_INIT, data, error);
+       return sev_do_cmd(SEV_CMD_INIT, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_platform_init);
 
 int sev_platform_shutdown(int *error)
 {
-       return sev_handle_cmd(SEV_CMD_SHUTDOWN, 0, error);
+       return sev_do_cmd(SEV_CMD_SHUTDOWN, 0, error);
 }
 EXPORT_SYMBOL_GPL(sev_platform_shutdown);
 
 int sev_platform_status(struct sev_data_status *data, int *error)
 {
-       return sev_handle_cmd(SEV_CMD_PLATFORM_STATUS, data, error);
+       return sev_do_cmd(SEV_CMD_PLATFORM_STATUS, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_platform_status);
 
@@ -211,58 +210,58 @@ int sev_issue_cmd_external_user(struct file *filep, 
unsigned int cmd,
        if (!filep || filep->f_op != &sev_fops)
                return -EBADF;
 
-       return sev_handle_cmd(cmd, data, error);
+       return sev_do_cmd(cmd, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_issue_cmd_external_user);
 
 int sev_guest_deactivate(struct sev_data_deactivate *data, int *error)
 {
-       return sev_handle_cmd(SEV_CMD_DEACTIVATE, data, error);
+       return sev_do_cmd(SEV_CMD_DEACTIVATE, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_guest_deactivate);
 
 int sev_guest_activate(struct sev_data_activate *data, int *error)
 {
-       return sev_handle_cmd(SEV_CMD_ACTIVATE, data, error);
+       return sev_do_cmd(SEV_CMD_ACTIVATE, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_guest_activate);
 
 int sev_guest_decommission(struct sev_data_decommission *data, int *error)
 {
-       return sev_handle_cmd(SEV_CMD_DECOMMISSION, data, error);
+       return sev_do_cmd(SEV_CMD_DECOMMISSION, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_guest_decommission);
 
 int sev_guest_df_flush(int *error)
 {
-       return sev_handle_cmd(SEV_CMD_DF_FLUSH, 0, error);
+       return sev_do_cmd(SEV_CMD_DF_FLUSH, 0, error);
 }
 EXPORT_SYMBOL_GPL(sev_guest_df_flush);
 
 static int sev_ops_init(struct psp_device *psp)
 {
        struct miscdevice *misc = &psp->sev_misc;
-       int ret = 0;
+       int ret;
 
        /*
-        * SEV feature support can be detected on the multiple devices but the
-        * SEV FW commands must be issued on the master. During probe time we
-        * do not know the master hence we create /dev/sev on the first device
-        * probe. sev_handle_cmd() finds the right master device to when issuing
-        * the command to the firmware.
+        * SEV feature support can be detected on multiple devices but the SEV
+        * FW commands must be issued on the master. During probe, we do not
+        * know the master hence we create /dev/sev on the first device probe.
+        * sev_do_cmd() finds the right master device to which to issue the
+        * command to the firmware.
         */
-       if (!sev_fops_registered) {
-               misc->minor = MISC_DYNAMIC_MINOR;
-               misc->name = DEVICE_NAME;
-               misc->fops = &sev_fops;
-
-               ret = misc_register(misc);
-               if (!ret) {
-                       sev_fops_registered = true;
-                       psp->has_sev_fops = true;
-                       init_waitqueue_head(&psp->sev_int_queue);
-                       dev_info(psp->dev, "registered SEV device\n");
-               }
+       if (psp->has_sev_fops)
+               return 0;
+
+       misc->minor = MISC_DYNAMIC_MINOR;
+       misc->name = DEVICE_NAME;
+       misc->fops = &sev_fops;
+
+       ret = misc_register(misc);
+       if (!ret) {
+               psp->has_sev_fops = true;
+               init_waitqueue_head(&psp->sev_int_queue);
+               dev_info(psp->dev, "registered SEV device\n");
        }
 
        return ret;
diff --git a/drivers/crypto/ccp/psp-dev.h b/drivers/crypto/ccp/psp-dev.h
index 6e8f83b41521..60a33f5ff79f 100644
--- a/drivers/crypto/ccp/psp-dev.h
+++ b/drivers/crypto/ccp/psp-dev.h
@@ -36,7 +36,7 @@
 #define PSP_CMDBUFF_ADDR_HI             PSP_C2PMSG(57)
 #define PSP_FEATURE_REG                        PSP_C2PMSG(63)
 
-#define PSP_P2CMSG(_num)               (_num << 2)
+#define PSP_P2CMSG(_num)               ((_num) << 2)
 #define PSP_CMD_COMPLETE_REG           1
 #define PSP_CMD_COMPLETE               PSP_P2CMSG(PSP_CMD_COMPLETE_REG)
 
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 2b334fd853c9..10b843cce75f 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -512,8 +512,7 @@ struct sev_data_dbg {
        u32 len;                                /* In */
 } __packed;
 
-#if defined(CONFIG_CRYPTO_DEV_SP_PSP)
-
+#ifdef CONFIG_CRYPTO_DEV_SP_PSP
 /**
  * sev_platform_init - perform SEV INIT command
  *
@@ -562,9 +561,9 @@ int sev_platform_status(struct sev_data_status *status, int 
*error);
  * sev_issue_cmd_external_user - issue SEV command by other driver with a file
  * handle.
  *
- * The function can be used by other drivers to issue a SEV command on
- * behalf by userspace. The caller must pass a valid SEV file descriptor
- * so that we know that caller has access to SEV device.
+ * This function can be used by other drivers to issue a SEV command on
+ * behalf of userspace. The caller must pass a valid SEV file descriptor
+ * so that we know that it has access to SEV device.
  *
  * @filep - SEV device file pointer
  * @cmd - command to issue

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Reply via email to