[PATCH 0/2] "nosnp" sev command line support

[PATCH 0/2] "nosnp" sev command line support

[Pavan Kumar Paluri]

Provide "nosnp" boot option via "sev=nosnp" kernel command line to
prevent SNP-capable host kernel from enabling SEV-SNP and initializing
RMP table.

On providing sev=nosnp via kernel command line:
cat /sys/module/kvm_amd/parameters/sev_snp should be "N".

This patchset is based on tip/master.

Any feedback is much appreciated.

	Pavan
---
Pavan Kumar Paluri (2):
  x86, KVM:SVM: Move sev specific parsing into arch/x86/virt/svm
  x86 KVM:SVM: Provide "nosnp" boot option for sev kernel command line

 .../arch/x86/x86_64/boot-options.rst          |  3 ++
 arch/x86/coco/sev/core.c                      | 44 -------------------
 arch/x86/include/asm/sev-common.h             | 27 ++++++++++++
 arch/x86/virt/svm/Makefile                    |  1 +
 arch/x86/virt/svm/cmdline.c                   | 38 ++++++++++++++++
 5 files changed, 69 insertions(+), 44 deletions(-)
 create mode 100644 arch/x86/virt/svm/cmdline.c


base-commit: 6d9b02543ea85fcef2595e0a7a63a336ea2ac5e1
-- 
2.34.1

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

[Pavan Kumar Paluri]

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>
---
 arch/x86/coco/sev/core.c          | 44 -------------------------------
 arch/x86/include/asm/sev-common.h | 27 +++++++++++++++++++
 arch/x86/virt/svm/Makefile        |  1 +
 arch/x86/virt/svm/cmdline.c       | 32 ++++++++++++++++++++++
 4 files changed, 60 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..dd302fe49f04 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -220,4 +220,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..507549a9c793
--- /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/memblock.h>
+
+#include <asm/sev.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

[PATCH 2/2] x86 KVM:SVM: Provide "nosnp" boot option for sev kernel command line

[Pavan Kumar Paluri]

Provide a "nosnp" kernel command line option to prevent enabling of the
RMP and SEV-SNP features in the host/hypervisor. Not initializing the
RMP removes system overhead associated with RMP checks.

Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>
---
 Documentation/arch/x86/x86_64/boot-options.rst | 3 +++
 arch/x86/virt/svm/cmdline.c                    | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/Documentation/arch/x86/x86_64/boot-options.rst b/Documentation/arch/x86/x86_64/boot-options.rst
index 137432d34109..3d4e9a7dccf2 100644
--- a/Documentation/arch/x86/x86_64/boot-options.rst
+++ b/Documentation/arch/x86/x86_64/boot-options.rst
@@ -317,3 +317,6 @@ The available options are:
 
    debug
      Enable debug messages.
+
+   nosnp
+     Do not enable SEV-SNP (applies to host/hypervisor only).
diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
index 507549a9c793..1e71c75f0201 100644
--- a/arch/x86/virt/svm/cmdline.c
+++ b/arch/x86/virt/svm/cmdline.c
@@ -24,6 +24,12 @@ static int __init init_sev_config(char *str)
 			continue;
 		}
 
+		if (!strcmp(s, "nosnp")) {
+			setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+			cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
+			continue;
+		}
+
 		pr_info("SEV command-line option '%s' was not recognized\n", s);
 	}
 
-- 
2.34.1

Re: [PATCH 0/2] "nosnp" sev command line support

[Matthew Wilcox]

On Thu, Aug 01, 2024 at 03:56:36PM -0500, Pavan Kumar Paluri wrote:
> Provide "nosnp" boot option via "sev=nosnp" kernel command line to
> prevent SNP-capable host kernel from enabling SEV-SNP and initializing
> RMP table.

Congratulations on using three acronyms multiple times and defining none
of them.

Re: [PATCH 0/2] "nosnp" sev command line support

[Paluri, PavanKumar]

Hi Matthew,

On 8/1/2024 3:58 PM, Matthew Wilcox wrote:
> On Thu, Aug 01, 2024 at 03:56:36PM -0500, Pavan Kumar Paluri wrote:
>> Provide "nosnp" boot option via "sev=nosnp" kernel command line to
>> prevent SNP-capable host kernel from enabling SEV-SNP and initializing
>> RMP table.
> 
> Congratulations on using three acronyms multiple times and defining none
> of them.

I apologize for not defining the acronyms, I had wrongly assumed they
were well understood by the reviewers/maintainers. I can fix that up
going forward. In the meantime:

RMP: Reverse Map Table
SEV: Secure Encrypted Virtualization
SNP: Secure Nested Paging

These definitions are also available in AMD-APM vol 2: Chapter 15 [1]

[1]:
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf

Thanks,
Pavan

Re: [PATCH 0/2] "nosnp" sev command line support

[Matthew Wilcox]

On Thu, Aug 01, 2024 at 05:23:29PM -0500, Paluri, PavanKumar wrote:
> Hi Matthew,
> 
> On 8/1/2024 3:58 PM, Matthew Wilcox wrote:
> > On Thu, Aug 01, 2024 at 03:56:36PM -0500, Pavan Kumar Paluri wrote:
> >> Provide "nosnp" boot option via "sev=nosnp" kernel command line to
> >> prevent SNP-capable host kernel from enabling SEV-SNP and initializing
> >> RMP table.
> > 
> > Congratulations on using three acronyms multiple times and defining none
> > of them.
> 
> I apologize for not defining the acronyms, I had wrongly assumed they
> were well understood by the reviewers/maintainers. I can fix that up
> going forward. In the meantime:

You cc'd linux-doc.  That's going to people who have no idea what
specialist part of the kernel you're working on.

> RMP: Reverse Map Table
> SEV: Secure Encrypted Virtualization
> SNP: Secure Nested Paging
> 
> These definitions are also available in AMD-APM vol 2: Chapter 15 [1]
> 
> [1]:
> https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/24593.pdf
> 
> Thanks,
> Pavan

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

[Tom Lendacky]

On 8/1/24 15:56, 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 | 27 +++++++++++++++++++
>  arch/x86/virt/svm/Makefile        |  1 +
>  arch/x86/virt/svm/cmdline.c       | 32 ++++++++++++++++++++++
>  4 files changed, 60 insertions(+), 44 deletions(-)
>  create mode 100644 arch/x86/virt/svm/cmdline.c
> 

Re: [PATCH 2/2] x86 KVM:SVM: Provide "nosnp" boot option for sev kernel command line

[Tom Lendacky]

On 8/1/24 15:56, Pavan Kumar Paluri wrote:
> Provide a "nosnp" kernel command line option to prevent enabling of the
> RMP and SEV-SNP features in the host/hypervisor. Not initializing the
> RMP removes system overhead associated with RMP checks.
> 
> Co-developed-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Eric Van Tassell <Eric.VanTassell@amd.com>
> Signed-off-by: Pavan Kumar Paluri <papaluri@amd.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  Documentation/arch/x86/x86_64/boot-options.rst | 3 +++
>  arch/x86/virt/svm/cmdline.c                    | 6 ++++++
>  2 files changed, 9 insertions(+)

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

[Borislav Petkov]

On Thu, Aug 01, 2024 at 03:56:37PM -0500, Pavan Kumar Paluri wrote:
> +#include <linux/memblock.h>

What's the idea of adding some random include here?

Does this file use memblock?

I don't think so.

You need to resolve include visibility by including the headers where you need
them:

diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
index dd302fe49f04..d3e7f97e2a4a 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -8,6 +8,9 @@
 #ifndef __ASM_X86_SEV_COMMON_H
 #define __ASM_X86_SEV_COMMON_H
 
+#include <asm/cache.h>
+#include <asm/pgtable_types.h>
+
 #define GHCB_MSR_INFO_POS		0
 #define GHCB_DATA_LOW			12
 #define GHCB_MSR_INFO_MASK		(BIT_ULL(GHCB_DATA_LOW) - 1)
diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
index 507549a9c793..f0a532108f49 100644
--- a/arch/x86/virt/svm/cmdline.c
+++ b/arch/x86/virt/svm/cmdline.c
@@ -5,11 +5,8 @@
  * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
  *
  * Author: Michael Roth <michael.roth@amd.com>
- *
  */
 
-#include <linux/memblock.h>
-
 #include <asm/sev.h>
 
 struct sev_config sev_cfg;

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Borislav Petkov]

On Thu, Aug 29, 2024 at 03:24:38PM +0200, Borislav Petkov wrote:
> On Thu, Aug 01, 2024 at 03:56:37PM -0500, Pavan Kumar Paluri wrote:
> > +#include <linux/memblock.h>
> 
> What's the idea of adding some random include here?
> 
> Does this file use memblock?
> 
> I don't think so.
> 
> You need to resolve include visibility by including the headers where you need
> them:

And with this applied, your next patch needs includes too.

Please include only those headers into sev/cmdline.c which supply the
facilities you're using. IOW, include only those headers and only into those
files which need the respective facilities.

This needs to be done right because otherwise we have an include hell and some
poor moron gets to mop up after you in the future.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Paluri, PavanKumar]

Hi Boris,

On 8/29/2024 8:24 AM, Borislav Petkov wrote:
> On Thu, Aug 01, 2024 at 03:56:37PM -0500, Pavan Kumar Paluri wrote:
>> +#include <linux/memblock.h>
> 
> What's the idea of adding some random include here?
> 
> Does this file use memblock?
> 
> I don't think so.
> 
> You need to resolve include visibility by including the headers where you need
> them:
> 

Understood, will include *only* those headers that provide me with the
facilities as you mentioned.

> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> index dd302fe49f04..d3e7f97e2a4a 100644
> --- a/arch/x86/include/asm/sev-common.h
> +++ b/arch/x86/include/asm/sev-common.h
> @@ -8,6 +8,9 @@
>  #ifndef __ASM_X86_SEV_COMMON_H
>  #define __ASM_X86_SEV_COMMON_H
>  
> +#include <asm/cache.h>
> +#include <asm/pgtable_types.h>
> +
>  #define GHCB_MSR_INFO_POS		0
>  #define GHCB_DATA_LOW			12
>  #define GHCB_MSR_INFO_MASK		(BIT_ULL(GHCB_DATA_LOW) - 1)
> diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
> index 507549a9c793..f0a532108f49 100644
> --- a/arch/x86/virt/svm/cmdline.c
> +++ b/arch/x86/virt/svm/cmdline.c
> @@ -5,11 +5,8 @@
>   * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
>   *
>   * Author: Michael Roth <michael.roth@amd.com>
> - *
>   */
>  
> -#include <linux/memblock.h>
> -
>  #include <asm/sev.h>
>  
>  struct sev_config sev_cfg;
> 

With the above diff applied, I was observing the following compilation
errors relating to string header:

arch/x86/virt/svm/cmdline.c: In function ‘init_sev_config’:
arch/x86/virt/svm/cmdline.c:20:21: error: implicit declaration of
function ‘strsep’ [-Werror=implicit-function-declaration]
   20 |         while ((s = strsep(&str, ","))) {
      |                     ^~~~~~
arch/x86/virt/svm/cmdline.c:20:19: warning: assignment to ‘char *’ from
‘int’ makes pointer from integer without a cast [-Wint-conversion]
   20 |         while ((s = strsep(&str, ","))) {
      |                   ^
arch/x86/virt/svm/cmdline.c:21:22: error: implicit declaration of
function ‘strcmp’ [-Werror=implicit-function-declaration]
   21 |                 if (!strcmp(s, "debug")) {
      |                      ^~~~~~
arch/x86/virt/svm/cmdline.c:13:1: note: include ‘<string.h>’ or provide
a declaration of ‘strcmp’
   12 | #include <asm/sev.h>
  +++ |+#include <string.h>
   13 |
arch/x86/virt/svm/cmdline.c:26:17: error: implicit declaration of
function ‘pr_info’ [-Werror=implicit-function-declaration]
   26 |                 pr_info("SEV command-line option '%s' was not
recognized\n", s);
      |                 ^~~~~~~

So here's the updated diff (for patch #1) that is compile-tested:

diff --git a/arch/x86/include/asm/sev-common.h
b/arch/x86/include/asm/sev-common.h
index dd302fe49f04..d3e7f97e2a4a 100644
--- a/arch/x86/include/asm/sev-common.h
+++ b/arch/x86/include/asm/sev-common.h
@@ -8,6 +8,9 @@
 #ifndef __ASM_X86_SEV_COMMON_H
 #define __ASM_X86_SEV_COMMON_H

+#include <asm/cache.h>
+#include <asm/pgtable_types.h>
+
 #define GHCB_MSR_INFO_POS              0
 #define GHCB_DATA_LOW                  12
 #define GHCB_MSR_INFO_MASK             (BIT_ULL(GHCB_DATA_LOW) - 1)
diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
index 507549a9c793..be3504a601c0 100644
--- a/arch/x86/virt/svm/cmdline.c
+++ b/arch/x86/virt/svm/cmdline.c
@@ -5,10 +5,9 @@
  * Copyright (C) 2023 - 2024 Advanced Micro Devices, Inc.
  *
  * Author: Michael Roth <michael.roth@amd.com>
- *
  */

-#include <linux/memblock.h>
+#include <linux/string.h>

 #include <asm/sev.h>


And for Patch #2, here's the diff:

diff --git a/arch/x86/virt/svm/cmdline.c b/arch/x86/virt/svm/cmdline.c
index 9cec2c2fb67c..5880df8027e6 100644
--- a/arch/x86/virt/svm/cmdline.c
+++ b/arch/x86/virt/svm/cmdline.c
@@ -8,6 +8,7 @@
  */

 #include <linux/string.h>
+#include <asm/cpufeature.h>

 #include <asm/sev.h>

If these changes look good to you, I will send a v2 incorporating the
changes.

Thanks for the review,
Pavan

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

[Borislav Petkov]

On Thu, Aug 29, 2024 at 10:29:16AM -0500, Paluri, PavanKumar wrote:
> So here's the updated diff (for patch #1) that is compile-tested:

Make sure you build

"allnoconfig" "defconfig" "allmodconfig" "allyesconfig"

builds, for both 32-bit and 64-bit on each patch before you resend.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

[Paluri, PavanKumar]

On 8/29/2024 10:41 AM, Borislav Petkov wrote:
> On Thu, Aug 29, 2024 at 10:29:16AM -0500, Paluri, PavanKumar wrote:
>> So here's the updated diff (for patch #1) that is compile-tested:
> 
> Make sure you build
> 
> "allnoconfig" "defconfig" "allmodconfig" "allyesconfig"
> 
> builds, for both 32-bit and 64-bit on each patch before you resend.
> 

Sure, will build with all the above configurations and send v2.

> Thx.
> 
Thanks,
Pavan