[PATCH v4 1/2] x86, KVM:SVM: Move sev specific parsing into arch/x86/virt/svm

Pavan Kumar Paluri posted 2 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v4 1/2] x86, KVM:SVM: Move sev specific parsing into arch/x86/virt/svm
Posted by Pavan Kumar Paluri 2 months, 1 week ago
Move SEV specific kernel command line option parsing support from
arch/x86/coco/sev/core.c to arch/x86/virt/svm/cmdline.c so that both
host and guest related SEV command line options can be supported.

No functional changes intended.

Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/coco/sev/core.c          | 44 -------------------------------
 arch/x86/include/asm/sev-common.h | 29 ++++++++++++++++++++
 arch/x86/virt/svm/Makefile        |  1 +
 arch/x86/virt/svm/cmdline.c       | 32 ++++++++++++++++++++++
 4 files changed, 62 insertions(+), 44 deletions(-)
 create mode 100644 arch/x86/virt/svm/cmdline.c

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index de1df0cb45da..ff19e805e7a1 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -141,33 +141,6 @@ static DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
 static DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
 static DEFINE_PER_CPU(u64, svsm_caa_pa);
 
-struct sev_config {
-	__u64 debug		: 1,
-
-	      /*
-	       * Indicates when the per-CPU GHCB has been created and registered
-	       * and thus can be used by the BSP instead of the early boot GHCB.
-	       *
-	       * For APs, the per-CPU GHCB is created before they are started
-	       * and registered upon startup, so this flag can be used globally
-	       * for the BSP and APs.
-	       */
-	      ghcbs_initialized	: 1,
-
-	      /*
-	       * Indicates when the per-CPU SVSM CA is to be used instead of the
-	       * boot SVSM CA.
-	       *
-	       * For APs, the per-CPU SVSM CA is created as part of the AP
-	       * bringup, so this flag can be used globally for the BSP and APs.
-	       */
-	      use_cas		: 1,
-
-	      __reserved	: 61;
-};
-
-static struct sev_config sev_cfg __read_mostly;
-
 static __always_inline bool on_vc_stack(struct pt_regs *regs)
 {
 	unsigned long sp = regs->sp;
@@ -2374,23 +2347,6 @@ static int __init report_snp_info(void)
 }
 arch_initcall(report_snp_info);
 
-static int __init init_sev_config(char *str)
-{
-	char *s;
-
-	while ((s = strsep(&str, ","))) {
-		if (!strcmp(s, "debug")) {
-			sev_cfg.debug = true;
-			continue;
-		}
-
-		pr_info("SEV command-line option '%s' was not recognized\n", s);
-	}
-
-	return 1;
-}
-__setup("sev=", init_sev_config);
-
 static void update_attest_input(struct svsm_call *call, struct svsm_attest_call *input)
 {
 	/* If (new) lengths have been returned, propagate them up */
diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index 98726c2b04f8..adddcf3edaf0 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -8,6 +8,8 @@
 #ifndef __ASM_X86_SEV_COMMON_H
 #define __ASM_X86_SEV_COMMON_H
 
+#include <asm/cache.h>
+
 #define GHCB_MSR_INFO_POS		0
 #define GHCB_DATA_LOW			12
 #define GHCB_MSR_INFO_MASK		(BIT_ULL(GHCB_DATA_LOW) - 1)
@@ -220,4 +222,31 @@ struct snp_psc_desc {
 #define GHCB_ERR_INVALID_INPUT		5
 #define GHCB_ERR_INVALID_EVENT		6
 
+struct sev_config {
+	__u64 debug		: 1,
+
+	      /*
+	       * Indicates when the per-CPU GHCB has been created and registered
+	       * and thus can be used by the BSP instead of the early boot GHCB.
+	       *
+	       * For APs, the per-CPU GHCB is created before they are started
+	       * and registered upon startup, so this flag can be used globally
+	       * for the BSP and APs.
+	       */
+	      ghcbs_initialized	: 1,
+
+	      /*
+	       * Indicates when the per-CPU SVSM CA is to be used instead of the
+	       * boot SVSM CA.
+	       *
+	       * For APs, the per-CPU SVSM CA is created as part of the AP
+	       * bringup, so this flag can be used globally for the BSP and APs.
+	       */
+	      use_cas		: 1,
+
+	      __reserved	: 61;
+};
+
+extern struct sev_config sev_cfg __read_mostly;
+
 #endif
diff --git a/arch/x86/virt/svm/Makefile b/arch/x86/virt/svm/Makefile
index ef2a31bdcc70..eca6d71355fa 100644
--- a/arch/x86/virt/svm/Makefile
+++ b/arch/x86/virt/svm/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
 obj-$(CONFIG_KVM_AMD_SEV) += sev.o
+obj-$(CONFIG_CPU_SUP_AMD) += cmdline.o
diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
new file mode 100644
index 000000000000..964677ab02d6
--- /dev/null
+++ b/arch/x86/virt/svm/cmdline.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * AMD SVM-SEV command line parsing support
+ *
+ * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
+ *
+ * Author: Michael Roth <michael.roth@amd.com>
+ */
+
+#include <linux/string.h>
+#include <linux/printk.h>
+
+#include <asm/sev-common.h>
+
+struct sev_config sev_cfg;
+
+static int __init init_sev_config(char *str)
+{
+	char *s;
+
+	while ((s = strsep(&str, ","))) {
+		if (!strcmp(s, "debug")) {
+			sev_cfg.debug = true;
+			continue;
+		}
+
+		pr_info("SEV command-line option '%s' was not recognized\n", s);
+	}
+
+	return 1;
+}
+__setup("sev=", init_sev_config);
-- 
2.34.1
Re: [PATCH v4 1/2] x86, KVM:SVM: Move sev specific parsing into arch/x86/virt/svm
Posted by Tom Lendacky 2 months ago
On 9/21/24 22:36, Pavan Kumar Paluri wrote:
> Move SEV specific kernel command line option parsing support from
> arch/x86/coco/sev/core.c to arch/x86/virt/svm/cmdline.c so that both
> host and guest related SEV command line options can be supported.
> 
> No functional changes intended.
> 
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/coco/sev/core.c          | 44 -------------------------------
>  arch/x86/include/asm/sev-common.h | 29 ++++++++++++++++++++
>  arch/x86/virt/svm/Makefile        |  1 +
>  arch/x86/virt/svm/cmdline.c       | 32 ++++++++++++++++++++++
>  4 files changed, 62 insertions(+), 44 deletions(-)
>  create mode 100644 arch/x86/virt/svm/cmdline.c
> 

...

> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index 98726c2b04f8..adddcf3edaf0 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -8,6 +8,8 @@
>  #ifndef __ASM_X86_SEV_COMMON_H
>  #define __ASM_X86_SEV_COMMON_H
>  
> +#include <asm/cache.h>
> +
>  #define GHCB_MSR_INFO_POS		0
>  #define GHCB_DATA_LOW			12
>  #define GHCB_MSR_INFO_MASK		(BIT_ULL(GHCB_DATA_LOW) - 1)
> @@ -220,4 +222,31 @@ struct snp_psc_desc {
>  #define GHCB_ERR_INVALID_INPUT		5
>  #define GHCB_ERR_INVALID_EVENT		6
>  
> +struct sev_config {
> +	__u64 debug		: 1,
> +
> +	      /*
> +	       * Indicates when the per-CPU GHCB has been created and registered
> +	       * and thus can be used by the BSP instead of the early boot GHCB.
> +	       *
> +	       * For APs, the per-CPU GHCB is created before they are started
> +	       * and registered upon startup, so this flag can be used globally
> +	       * for the BSP and APs.
> +	       */
> +	      ghcbs_initialized	: 1,
> +
> +	      /*
> +	       * Indicates when the per-CPU SVSM CA is to be used instead of the
> +	       * boot SVSM CA.
> +	       *
> +	       * For APs, the per-CPU SVSM CA is created as part of the AP
> +	       * bringup, so this flag can be used globally for the BSP and APs.
> +	       */
> +	      use_cas		: 1,
> +
> +	      __reserved	: 61;
> +};
> +
> +extern struct sev_config sev_cfg __read_mostly;

So I believe the "__read_mostly" attribute really needs to be on the
actual declaration of the struct, below, in cmdline.c, right?

You can check and see which section the sev_cfg struct ends up being
placed after a build to verify.

Thanks,
Tom

> +
>  #endif
> diff --git a/arch/x86/virt/svm/Makefile b/arch/x86/virt/svm/Makefile
> index ef2a31bdcc70..eca6d71355fa 100644
> --- a/arch/x86/virt/svm/Makefile
> +++ b/arch/x86/virt/svm/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-$(CONFIG_KVM_AMD_SEV) += sev.o
> +obj-$(CONFIG_CPU_SUP_AMD) += cmdline.o
> diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
> new file mode 100644
> index 000000000000..964677ab02d6
> --- /dev/null
> +++ b/arch/x86/virt/svm/cmdline.c
> @@ -0,0 +1,32 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * AMD SVM-SEV command line parsing support
> + *
> + * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
> + *
> + * Author: Michael Roth <michael.roth@amd.com>
> + */
> +
> +#include <linux/string.h>
> +#include <linux/printk.h>
> +
> +#include <asm/sev-common.h>
> +
> +struct sev_config sev_cfg;
> +
> +static int __init init_sev_config(char *str)
> +{
> +	char *s;
> +
> +	while ((s = strsep(&str, ","))) {
> +		if (!strcmp(s, "debug")) {
> +			sev_cfg.debug = true;
> +			continue;
> +		}
> +
> +		pr_info("SEV command-line option '%s' was not recognized\n", s);
> +	}
> +
> +	return 1;
> +}
> +__setup("sev=", init_sev_config);
Re: [PATCH v4 1/2] x86, KVM:SVM: Move sev specific parsing into arch/x86/virt/svm
Posted by Paluri, PavanKumar 2 months ago
Hello Tom,

On 9/25/2024 10:57 AM, Tom Lendacky wrote:
> On 9/21/24 22:36, Pavan Kumar Paluri wrote:
>> Move SEV specific kernel command line option parsing support from
>> arch/x86/coco/sev/core.c to arch/x86/virt/svm/cmdline.c so that both
>> host and guest related SEV command line options can be supported.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  arch/x86/coco/sev/core.c          | 44 -------------------------------
>>  arch/x86/include/asm/sev-common.h | 29 ++++++++++++++++++++
>>  arch/x86/virt/svm/Makefile        |  1 +
>>  arch/x86/virt/svm/cmdline.c       | 32 ++++++++++++++++++++++
>>  4 files changed, 62 insertions(+), 44 deletions(-)
>>  create mode 100644 arch/x86/virt/svm/cmdline.c
>>
> 

...
>> +extern struct sev_config sev_cfg __read_mostly;
> 
> So I believe the "__read_mostly" attribute really needs to be on the
> actual declaration of the struct, below, in cmdline.c, right?
> 

Yes, I will move this attribute to cmdline.c

> You can check and see which section the sev_cfg struct ends up being
> placed after a build to verify.
>

It is placed in .data.readmostly section.
0000000000000000 g     O .data..read_mostly     0000000000000008 sev_cfg

> Thanks,
> Tom
> 
Thanks for the review,
Pavan

>> +
>>  #endif
>> diff --git a/arch/x86/virt/svm/Makefile b/arch/x86/virt/svm/Makefile
>> index ef2a31bdcc70..eca6d71355fa 100644
>> --- a/arch/x86/virt/svm/Makefile
>> +++ b/arch/x86/virt/svm/Makefile
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  
>>  obj-$(CONFIG_KVM_AMD_SEV) += sev.o
>> +obj-$(CONFIG_CPU_SUP_AMD) += cmdline.o
>> diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
>> new file mode 100644
>> index 000000000000..964677ab02d6
>> --- /dev/null
>> +++ b/arch/x86/virt/svm/cmdline.c
>> @@ -0,0 +1,32 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * AMD SVM-SEV command line parsing support
>> + *
>> + * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
>> + *
>> + * Author: Michael Roth <michael.roth@amd.com>
>> + */
>> +
>> +#include <linux/string.h>
>> +#include <linux/printk.h>
>> +
>> +#include <asm/sev-common.h>
>> +
>> +struct sev_config sev_cfg;
>> +
>> +static int __init init_sev_config(char *str)
>> +{
>> +	char *s;
>> +
>> +	while ((s = strsep(&str, ","))) {
>> +		if (!strcmp(s, "debug")) {
>> +			sev_cfg.debug = true;
>> +			continue;
>> +		}
>> +
>> +		pr_info("SEV command-line option '%s' was not recognized\n", s);
>> +	}
>> +
>> +	return 1;
>> +}
>> +__setup("sev=", init_sev_config);