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.1On 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
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
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
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
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
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 >
© 2016 - 2024 Red Hat, Inc.