diff mbox series

[v10,06/24] virt: sev-guest: Simplify VMPCK and sequence number assignments

Message ID 20240621123903.2411843-7-nikunj@amd.com (mailing list archive)
State New
Headers show
Series Add Secure TSC support for SNP guests | expand

Commit Message

Nikunj A. Dadhania June 21, 2024, 12:38 p.m. UTC
Preparatory patch to remove direct usage of VMPCK and message sequence
number in the SEV guest driver. Use arrays for the VM platform
communication key and message sequence number to simplify the function and
usage.

Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/include/asm/sev.h              | 12 ++++-------
 drivers/virt/coco/sev-guest/sev-guest.c | 27 ++++---------------------
 2 files changed, 8 insertions(+), 31 deletions(-)

Comments

Borislav Petkov June 25, 2024, 5:04 p.m. UTC | #1
On Fri, Jun 21, 2024 at 06:08:45PM +0530, Nikunj A Dadhania wrote:
> Preparatory patch to remove direct usage of VMPCK and message sequence

"Prepare the code for removing... "

From Documentation/process/submitting-patches.rst:

"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour."

> number in the SEV guest driver.

remove, because...?

> Use arrays for the VM platform communication key and message sequence number
> to simplify the function and usage.
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/include/asm/sev.h              | 12 ++++-------
>  drivers/virt/coco/sev-guest/sev-guest.c | 27 ++++---------------------
>  2 files changed, 8 insertions(+), 31 deletions(-)
> 
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 2ac899adcbf6..473760208764 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -118,6 +118,8 @@ struct sev_guest_platform_data {
>  	u64 secrets_gpa;
>  };
>  
> +#define VMPCK_MAX_NUM		4
> +
>  /*
>   * The secrets page contains 96-bytes of reserved field that can be used by
>   * the guest OS. The guest OS uses the area to save the message sequence
> @@ -126,10 +128,7 @@ struct sev_guest_platform_data {
>   * See the GHCB spec section Secret page layout for the format for this area.
>   */
>  struct secrets_os_area {
> -	u32 msg_seqno_0;
> -	u32 msg_seqno_1;
> -	u32 msg_seqno_2;
> -	u32 msg_seqno_3;
> +	u32 msg_seqno[VMPCK_MAX_NUM];
>  	u64 ap_jump_table_pa;
>  	u8 rsvd[40];
>  	u8 guest_usage[32];
> @@ -214,10 +213,7 @@ struct snp_secrets_page {
>  	u32 fms;
>  	u32 rsvd2;
>  	u8 gosvw[16];
> -	u8 vmpck0[VMPCK_KEY_LEN];
> -	u8 vmpck1[VMPCK_KEY_LEN];
> -	u8 vmpck2[VMPCK_KEY_LEN];
> -	u8 vmpck3[VMPCK_KEY_LEN];
> +	u8 vmpck[VMPCK_MAX_NUM][VMPCK_KEY_LEN];
>  	struct secrets_os_area os_area;
>  
>  	u8 vmsa_tweak_bitmap[64];
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index 61e190ecfa3a..a5602c84769f 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -678,30 +678,11 @@ static const struct file_operations snp_guest_fops = {
>  
>  static u8 *get_vmpck(int id, struct snp_secrets_page *secrets, u32 **seqno)

Why is this a separate function when it is used only once?

>  {
> -	u8 *key = NULL;
> -
> -	switch (id) {
> -	case 0:
> -		*seqno = &secrets->os_area.msg_seqno_0;
> -		key = secrets->vmpck0;
> -		break;
> -	case 1:
> -		*seqno = &secrets->os_area.msg_seqno_1;
> -		key = secrets->vmpck1;
> -		break;
> -	case 2:
> -		*seqno = &secrets->os_area.msg_seqno_2;
> -		key = secrets->vmpck2;
> -		break;
> -	case 3:
> -		*seqno = &secrets->os_area.msg_seqno_3;
> -		key = secrets->vmpck3;
> -		break;
> -	default:
> -		break;
> -	}
> +	if (!(id < VMPCK_MAX_NUM))
> +		return NULL;

Or

	if (id >= VMPCK_MAX_NUM)
		return NULL;

?

Also that id needs to be unsigned as it is an array index.
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 2ac899adcbf6..473760208764 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -118,6 +118,8 @@  struct sev_guest_platform_data {
 	u64 secrets_gpa;
 };
 
+#define VMPCK_MAX_NUM		4
+
 /*
  * The secrets page contains 96-bytes of reserved field that can be used by
  * the guest OS. The guest OS uses the area to save the message sequence
@@ -126,10 +128,7 @@  struct sev_guest_platform_data {
  * See the GHCB spec section Secret page layout for the format for this area.
  */
 struct secrets_os_area {
-	u32 msg_seqno_0;
-	u32 msg_seqno_1;
-	u32 msg_seqno_2;
-	u32 msg_seqno_3;
+	u32 msg_seqno[VMPCK_MAX_NUM];
 	u64 ap_jump_table_pa;
 	u8 rsvd[40];
 	u8 guest_usage[32];
@@ -214,10 +213,7 @@  struct snp_secrets_page {
 	u32 fms;
 	u32 rsvd2;
 	u8 gosvw[16];
-	u8 vmpck0[VMPCK_KEY_LEN];
-	u8 vmpck1[VMPCK_KEY_LEN];
-	u8 vmpck2[VMPCK_KEY_LEN];
-	u8 vmpck3[VMPCK_KEY_LEN];
+	u8 vmpck[VMPCK_MAX_NUM][VMPCK_KEY_LEN];
 	struct secrets_os_area os_area;
 
 	u8 vmsa_tweak_bitmap[64];
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 61e190ecfa3a..a5602c84769f 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -678,30 +678,11 @@  static const struct file_operations snp_guest_fops = {
 
 static u8 *get_vmpck(int id, struct snp_secrets_page *secrets, u32 **seqno)
 {
-	u8 *key = NULL;
-
-	switch (id) {
-	case 0:
-		*seqno = &secrets->os_area.msg_seqno_0;
-		key = secrets->vmpck0;
-		break;
-	case 1:
-		*seqno = &secrets->os_area.msg_seqno_1;
-		key = secrets->vmpck1;
-		break;
-	case 2:
-		*seqno = &secrets->os_area.msg_seqno_2;
-		key = secrets->vmpck2;
-		break;
-	case 3:
-		*seqno = &secrets->os_area.msg_seqno_3;
-		key = secrets->vmpck3;
-		break;
-	default:
-		break;
-	}
+	if (!(id < VMPCK_MAX_NUM))
+		return NULL;
 
-	return key;
+	*seqno = &secrets->os_area.msg_seqno[id];
+	return secrets->vmpck[id];
 }
 
 struct snp_msg_report_resp_hdr {