diff mbox series

[2/2] x86/sme: Mark the code as __head in mem_encrypt_identity.c

Message ID b2670a8a79a7b4a5c8993fb916904af7c675b7f8.1697525407.git.houwenlong.hwl@antgroup.com
State New
Headers
Series None |

Commit Message

Hou Wenlong Oct. 17, 2023, 7:08 a.m. UTC 
  The functions sme_enable() and sme_encrypt_kernel() are only called by
the head code which runs in identity virtual address. Therefore, it's
better to mark them as __head as well.

Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 arch/x86/include/asm/mem_encrypt.h |  8 ++++----
 arch/x86/mm/mem_encrypt_identity.c | 27 ++++++++++++++-------------
 2 files changed, 18 insertions(+), 17 deletions(-)

Comments

Ingo Molnar Oct. 17, 2023, 12:52 p.m. UTC | #1 
* Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:

> The functions sme_enable() and sme_encrypt_kernel() are only called by
> the head code which runs in identity virtual address. Therefore, it's
> better to mark them as __head as well.
> 
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> ---
>  arch/x86/include/asm/mem_encrypt.h |  8 ++++----
>  arch/x86/mm/mem_encrypt_identity.c | 27 ++++++++++++++-------------
>  2 files changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> index 359ada486fa9..48469e22a75e 100644
> --- a/arch/x86/include/asm/mem_encrypt.h
> +++ b/arch/x86/include/asm/mem_encrypt.h
> @@ -46,8 +46,8 @@ void __init sme_unmap_bootdata(char *real_mode_data);
>  
>  void __init sme_early_init(void);
>  
> -void __init sme_encrypt_kernel(struct boot_params *bp);
> -void __init sme_enable(struct boot_params *bp);
> +void sme_encrypt_kernel(struct boot_params *bp);
> +void sme_enable(struct boot_params *bp);
>  
>  int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
>  int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> @@ -75,8 +75,8 @@ static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
>  
>  static inline void __init sme_early_init(void) { }
>  
> -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
> -static inline void __init sme_enable(struct boot_params *bp) { }
> +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
> +static inline void sme_enable(struct boot_params *bp) { }

So I think we should preserve the previous convention of marking functions 
__init in the header-declaration and at the definition site as well, and do 
the same with __head as well?

Thanks,

	Ingo
Thomas Gleixner Oct. 17, 2023, 7:50 p.m. UTC | #2 
On Tue, Oct 17 2023 at 14:52, Ingo Molnar wrote:
> * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
>> -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
>> -static inline void __init sme_enable(struct boot_params *bp) { }
>> +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
>> +static inline void sme_enable(struct boot_params *bp) { }
>
> So I think we should preserve the previous convention of marking functions 
> __init in the header-declaration and at the definition site as well, and do 
> the same with __head as well?

I'm not convinced about the value of prototype annotations, but have no
strong preference either.

Thanks,

        tglx
Hou Wenlong Oct. 18, 2023, 7:13 a.m. UTC | #3 
On Tue, Oct 17, 2023 at 08:52:46PM +0800, Ingo Molnar wrote:
> 
> * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
> 
> > The functions sme_enable() and sme_encrypt_kernel() are only called by
> > the head code which runs in identity virtual address. Therefore, it's
> > better to mark them as __head as well.
> > 
> > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> > ---
> >  arch/x86/include/asm/mem_encrypt.h |  8 ++++----
> >  arch/x86/mm/mem_encrypt_identity.c | 27 ++++++++++++++-------------
> >  2 files changed, 18 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> > index 359ada486fa9..48469e22a75e 100644
> > --- a/arch/x86/include/asm/mem_encrypt.h
> > +++ b/arch/x86/include/asm/mem_encrypt.h
> > @@ -46,8 +46,8 @@ void __init sme_unmap_bootdata(char *real_mode_data);
> >  
> >  void __init sme_early_init(void);
> >  
> > -void __init sme_encrypt_kernel(struct boot_params *bp);
> > -void __init sme_enable(struct boot_params *bp);
> > +void sme_encrypt_kernel(struct boot_params *bp);
> > +void sme_enable(struct boot_params *bp);
> >  
> >  int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
> >  int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> > @@ -75,8 +75,8 @@ static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
> >  
> >  static inline void __init sme_early_init(void) { }
> >  
> > -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
> > -static inline void __init sme_enable(struct boot_params *bp) { }
> > +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
> > +static inline void sme_enable(struct boot_params *bp) { }
> 
> So I think we should preserve the previous convention of marking functions 
> __init in the header-declaration and at the definition site as well, and do 
> the same with __head as well?
> 
Hi Ingo,

I tried to include <asm/init.h> into <asm/mem_encrypt.h> and mark the
function declaration as __head, but it resulted in a build failure. This
is because <asm/init.h> is not self-contained; the type "pgd_t" is
defined in <asm/pgtable_types.h>, which includes <asm/mem_encrypt.h>,
leading to mutual inclusion of header files. To avoid the issue of
complicated header file inclusion, I removed the annotation from the
function declaration.

Actually, initially, I noticed that the __init definition is in
<linux/init.h>, so I first placed the __head definition in
<linux/init.h> as well. However, this conflicted with the local variable
in the "list_next_or_null_rcu" macro in <linux/rculist.h>. Then I
realized that __head was only used in x86, so I made the decision to put
it in the architecture-specific header. Considering simplicity, I chose
to put the definition in <asm/init.h>. I also attempted to put the
definition in other headers such as <asm/boot.h> and
<asm/bootparam_utils.h>, and included them in <asm/mem_encrypt.h>, but
the build still failed.

Thanks!

> Thanks,
> 
> 	Ingo
Ingo Molnar Oct. 18, 2023, 10:20 a.m. UTC | #4 
* Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:

> On Tue, Oct 17, 2023 at 08:52:46PM +0800, Ingo Molnar wrote:
> > 
> > * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
> > 
> > > The functions sme_enable() and sme_encrypt_kernel() are only called by
> > > the head code which runs in identity virtual address. Therefore, it's
> > > better to mark them as __head as well.
> > > 
> > > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> > > ---
> > >  arch/x86/include/asm/mem_encrypt.h |  8 ++++----
> > >  arch/x86/mm/mem_encrypt_identity.c | 27 ++++++++++++++-------------
> > >  2 files changed, 18 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> > > index 359ada486fa9..48469e22a75e 100644
> > > --- a/arch/x86/include/asm/mem_encrypt.h
> > > +++ b/arch/x86/include/asm/mem_encrypt.h
> > > @@ -46,8 +46,8 @@ void __init sme_unmap_bootdata(char *real_mode_data);
> > >  
> > >  void __init sme_early_init(void);
> > >  
> > > -void __init sme_encrypt_kernel(struct boot_params *bp);
> > > -void __init sme_enable(struct boot_params *bp);
> > > +void sme_encrypt_kernel(struct boot_params *bp);
> > > +void sme_enable(struct boot_params *bp);
> > >  
> > >  int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
> > >  int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> > > @@ -75,8 +75,8 @@ static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
> > >  
> > >  static inline void __init sme_early_init(void) { }
> > >  
> > > -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
> > > -static inline void __init sme_enable(struct boot_params *bp) { }
> > > +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
> > > +static inline void sme_enable(struct boot_params *bp) { }
> > 
> > So I think we should preserve the previous convention of marking functions 
> > __init in the header-declaration and at the definition site as well, and do 
> > the same with __head as well?
> > 
> Hi Ingo,
> 
> I tried to include <asm/init.h> into <asm/mem_encrypt.h> and mark the
> function declaration as __head, but it resulted in a build failure. This
> is because <asm/init.h> is not self-contained; the type "pgd_t" is
> defined in <asm/pgtable_types.h>, which includes <asm/mem_encrypt.h>,
> leading to mutual inclusion of header files. To avoid the issue of
> complicated header file inclusion, I removed the annotation from the
> function declaration.

The right solution at that point is to make <asm/init.h> self-contained...

> Actually, initially, I noticed that the __init definition is in
> <linux/init.h>, so I first placed the __head definition in
> <linux/init.h> as well. However, this conflicted with the local variable
> in the "list_next_or_null_rcu" macro in <linux/rculist.h>. Then I
> realized that __head was only used in x86, so I made the decision to put
> it in the architecture-specific header. Considering simplicity, I chose
> to put the definition in <asm/init.h>. I also attempted to put the
> definition in other headers such as <asm/boot.h> and
> <asm/bootparam_utils.h>, and included them in <asm/mem_encrypt.h>, but
> the build still failed.

When exporting a localized definition you should consider namespace 
collisions - the name '__head' is way too generic, no wonder it caused 
problems elsewhere.

I'd suggest naming it __init_head or so, but still keep it in a x86-only 
header.

I presume keeping it all in the  separate section and widening its usage has a 
specific purpose? Please outline that in the changelog as well.

Ie. instead of mechanical patches that try to follow existing patterns 
cargo-cult style, this area of x86 code requires well-argued, well thought 
out patches that show background knowledge of the area.

Thanks,

	Ingo
Ingo Molnar Oct. 18, 2023, 10:22 a.m. UTC | #5 
* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, Oct 17 2023 at 14:52, Ingo Molnar wrote:
> > * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
> >> -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
> >> -static inline void __init sme_enable(struct boot_params *bp) { }
> >> +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
> >> +static inline void sme_enable(struct boot_params *bp) { }
> >
> > So I think we should preserve the previous convention of marking functions 
> > __init in the header-declaration and at the definition site as well, and do 
> > the same with __head as well?
> 
> I'm not convinced about the value of prototype annotations, but have no
> strong preference either.

So it has some minor documentation purpose: when someone looks up a 
function via the header only (I do that frequently), __init-alike 
annotations really show the intended boot-only limitations of the API.

But that's a really minor Nth order benefit, I have no strong preference 
either.

Thanks,

	Ingo
Hou Wenlong Oct. 18, 2023, 12:03 p.m. UTC | #6 
On Wed, Oct 18, 2023 at 06:20:15PM +0800, Ingo Molnar wrote:
> 
> * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
> 
> > On Tue, Oct 17, 2023 at 08:52:46PM +0800, Ingo Molnar wrote:
> > > 
> > > * Hou Wenlong <houwenlong.hwl@antgroup.com> wrote:
> > > 
> > > > The functions sme_enable() and sme_encrypt_kernel() are only called by
> > > > the head code which runs in identity virtual address. Therefore, it's
> > > > better to mark them as __head as well.
> > > > 
> > > > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> > > > ---
> > > >  arch/x86/include/asm/mem_encrypt.h |  8 ++++----
> > > >  arch/x86/mm/mem_encrypt_identity.c | 27 ++++++++++++++-------------
> > > >  2 files changed, 18 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
> > > > index 359ada486fa9..48469e22a75e 100644
> > > > --- a/arch/x86/include/asm/mem_encrypt.h
> > > > +++ b/arch/x86/include/asm/mem_encrypt.h
> > > > @@ -46,8 +46,8 @@ void __init sme_unmap_bootdata(char *real_mode_data);
> > > >  
> > > >  void __init sme_early_init(void);
> > > >  
> > > > -void __init sme_encrypt_kernel(struct boot_params *bp);
> > > > -void __init sme_enable(struct boot_params *bp);
> > > > +void sme_encrypt_kernel(struct boot_params *bp);
> > > > +void sme_enable(struct boot_params *bp);
> > > >  
> > > >  int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
> > > >  int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
> > > > @@ -75,8 +75,8 @@ static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
> > > >  
> > > >  static inline void __init sme_early_init(void) { }
> > > >  
> > > > -static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
> > > > -static inline void __init sme_enable(struct boot_params *bp) { }
> > > > +static inline void sme_encrypt_kernel(struct boot_params *bp) { }
> > > > +static inline void sme_enable(struct boot_params *bp) { }
> > > 
> > > So I think we should preserve the previous convention of marking functions 
> > > __init in the header-declaration and at the definition site as well, and do 
> > > the same with __head as well?
> > > 
> > Hi Ingo,
> > 
> > I tried to include <asm/init.h> into <asm/mem_encrypt.h> and mark the
> > function declaration as __head, but it resulted in a build failure. This
> > is because <asm/init.h> is not self-contained; the type "pgd_t" is
> > defined in <asm/pgtable_types.h>, which includes <asm/mem_encrypt.h>,
> > leading to mutual inclusion of header files. To avoid the issue of
> > complicated header file inclusion, I removed the annotation from the
> > function declaration.
> 
> The right solution at that point is to make <asm/init.h> self-contained...
>

The "pgd_t" is a typedef declaration in <asm/pgtable_types.h>, so it
cannot be forward declared. Therefore, I had to include
<asm/pgtable_types.h> into <asm/init.h> to make it self-contained.
However, <asm/pgtable_types.h> includes <asm/mem_encrypt.h>. If I
include <asm/init.h> into <asm/mem_encrypt.h> to mark functions as
__head in the header-declaration, it would result in mutual inclusion of
header files. It appears that <asm/mem_encrypt.h> is a base header that
is included in multiple headers, so adding one more header to it would
complicate things. In reality, if it is acceptable, I could move the
__head definition into <asm/mem_encrypt.h>.
 
> > Actually, initially, I noticed that the __init definition is in
> > <linux/init.h>, so I first placed the __head definition in
> > <linux/init.h> as well. However, this conflicted with the local variable
> > in the "list_next_or_null_rcu" macro in <linux/rculist.h>. Then I
> > realized that __head was only used in x86, so I made the decision to put
> > it in the architecture-specific header. Considering simplicity, I chose
> > to put the definition in <asm/init.h>. I also attempted to put the
> > definition in other headers such as <asm/boot.h> and
> > <asm/bootparam_utils.h>, and included them in <asm/mem_encrypt.h>, but
> > the build still failed.
> 
> When exporting a localized definition you should consider namespace 
> collisions - the name '__head' is way too generic, no wonder it caused 
> problems elsewhere.
> 
> I'd suggest naming it __init_head or so, but still keep it in a x86-only 
> header.
> 
> I presume keeping it all in the  separate section and widening its usage has a 
> specific purpose? Please outline that in the changelog as well.
> 

Based on my understanding, the __head section contains the early boot
code that runs at a low identity address instead of the compile-time
address. Therefore, it must use RIP-relative addressing to access
memory. This makes the __head section special. However, when it comes to
C source code, the compiler may generate absolute addressing, which can
result in boot failure. That's why the fixup_pointer() function is
introduced in head64.c. So maybe we could consider validating the memory
access instructions in this section using objtool to ensure that the
generated instructions are PC-relative. Then we should mark all the
early boot code as __head.

Thanks!

> Ie. instead of mechanical patches that try to follow existing patterns 
> cargo-cult style, this area of x86 code requires well-argued, well thought 
> out patches that show background knowledge of the area.
> 
> Thanks,
> 
> 	Ingo
diff mbox series

Patch

diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h
index 359ada486fa9..48469e22a75e 100644
--- a/arch/x86/include/asm/mem_encrypt.h
+++ b/arch/x86/include/asm/mem_encrypt.h
@@ -46,8 +46,8 @@  void __init sme_unmap_bootdata(char *real_mode_data);
 
 void __init sme_early_init(void);
 
-void __init sme_encrypt_kernel(struct boot_params *bp);
-void __init sme_enable(struct boot_params *bp);
+void sme_encrypt_kernel(struct boot_params *bp);
+void sme_enable(struct boot_params *bp);
 
 int __init early_set_memory_decrypted(unsigned long vaddr, unsigned long size);
 int __init early_set_memory_encrypted(unsigned long vaddr, unsigned long size);
@@ -75,8 +75,8 @@  static inline void __init sme_unmap_bootdata(char *real_mode_data) { }
 
 static inline void __init sme_early_init(void) { }
 
-static inline void __init sme_encrypt_kernel(struct boot_params *bp) { }
-static inline void __init sme_enable(struct boot_params *bp) { }
+static inline void sme_encrypt_kernel(struct boot_params *bp) { }
+static inline void sme_enable(struct boot_params *bp) { }
 
 static inline void sev_es_init_vc_handling(void) { }
 
diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
index d73aeb16417f..72aeb0f3dec6 100644
--- a/arch/x86/mm/mem_encrypt_identity.c
+++ b/arch/x86/mm/mem_encrypt_identity.c
@@ -46,6 +46,7 @@ 
 #include <asm/cmdline.h>
 #include <asm/coco.h>
 #include <asm/sev.h>
+#include <asm/init.h>
 
 #include "mm_internal.h"
 
@@ -99,7 +100,7 @@  static char sme_cmdline_arg[] __initdata = "mem_encrypt";
 static char sme_cmdline_on[]  __initdata = "on";
 static char sme_cmdline_off[] __initdata = "off";
 
-static void __init sme_clear_pgd(struct sme_populate_pgd_data *ppd)
+static void __head sme_clear_pgd(struct sme_populate_pgd_data *ppd)
 {
 	unsigned long pgd_start, pgd_end, pgd_size;
 	pgd_t *pgd_p;
@@ -114,7 +115,7 @@  static void __init sme_clear_pgd(struct sme_populate_pgd_data *ppd)
 	memset(pgd_p, 0, pgd_size);
 }
 
-static pud_t __init *sme_prepare_pgd(struct sme_populate_pgd_data *ppd)
+static pud_t __head *sme_prepare_pgd(struct sme_populate_pgd_data *ppd)
 {
 	pgd_t *pgd;
 	p4d_t *p4d;
@@ -151,7 +152,7 @@  static pud_t __init *sme_prepare_pgd(struct sme_populate_pgd_data *ppd)
 	return pud;
 }
 
-static void __init sme_populate_pgd_large(struct sme_populate_pgd_data *ppd)
+static void __head sme_populate_pgd_large(struct sme_populate_pgd_data *ppd)
 {
 	pud_t *pud;
 	pmd_t *pmd;
@@ -167,7 +168,7 @@  static void __init sme_populate_pgd_large(struct sme_populate_pgd_data *ppd)
 	set_pmd(pmd, __pmd(ppd->paddr | ppd->pmd_flags));
 }
 
-static void __init sme_populate_pgd(struct sme_populate_pgd_data *ppd)
+static void __head sme_populate_pgd(struct sme_populate_pgd_data *ppd)
 {
 	pud_t *pud;
 	pmd_t *pmd;
@@ -193,7 +194,7 @@  static void __init sme_populate_pgd(struct sme_populate_pgd_data *ppd)
 		set_pte(pte, __pte(ppd->paddr | ppd->pte_flags));
 }
 
-static void __init __sme_map_range_pmd(struct sme_populate_pgd_data *ppd)
+static void __head __sme_map_range_pmd(struct sme_populate_pgd_data *ppd)
 {
 	while (ppd->vaddr < ppd->vaddr_end) {
 		sme_populate_pgd_large(ppd);
@@ -203,7 +204,7 @@  static void __init __sme_map_range_pmd(struct sme_populate_pgd_data *ppd)
 	}
 }
 
-static void __init __sme_map_range_pte(struct sme_populate_pgd_data *ppd)
+static void __head __sme_map_range_pte(struct sme_populate_pgd_data *ppd)
 {
 	while (ppd->vaddr < ppd->vaddr_end) {
 		sme_populate_pgd(ppd);
@@ -213,7 +214,7 @@  static void __init __sme_map_range_pte(struct sme_populate_pgd_data *ppd)
 	}
 }
 
-static void __init __sme_map_range(struct sme_populate_pgd_data *ppd,
+static void __head __sme_map_range(struct sme_populate_pgd_data *ppd,
 				   pmdval_t pmd_flags, pteval_t pte_flags)
 {
 	unsigned long vaddr_end;
@@ -237,22 +238,22 @@  static void __init __sme_map_range(struct sme_populate_pgd_data *ppd,
 	__sme_map_range_pte(ppd);
 }
 
-static void __init sme_map_range_encrypted(struct sme_populate_pgd_data *ppd)
+static void __head sme_map_range_encrypted(struct sme_populate_pgd_data *ppd)
 {
 	__sme_map_range(ppd, PMD_FLAGS_ENC, PTE_FLAGS_ENC);
 }
 
-static void __init sme_map_range_decrypted(struct sme_populate_pgd_data *ppd)
+static void __head sme_map_range_decrypted(struct sme_populate_pgd_data *ppd)
 {
 	__sme_map_range(ppd, PMD_FLAGS_DEC, PTE_FLAGS_DEC);
 }
 
-static void __init sme_map_range_decrypted_wp(struct sme_populate_pgd_data *ppd)
+static void __head sme_map_range_decrypted_wp(struct sme_populate_pgd_data *ppd)
 {
 	__sme_map_range(ppd, PMD_FLAGS_DEC_WP, PTE_FLAGS_DEC_WP);
 }
 
-static unsigned long __init sme_pgtable_calc(unsigned long len)
+static unsigned long __head sme_pgtable_calc(unsigned long len)
 {
 	unsigned long entries = 0, tables = 0;
 
@@ -289,7 +290,7 @@  static unsigned long __init sme_pgtable_calc(unsigned long len)
 	return entries + tables;
 }
 
-void __init sme_encrypt_kernel(struct boot_params *bp)
+void __head sme_encrypt_kernel(struct boot_params *bp)
 {
 	unsigned long workarea_start, workarea_end, workarea_len;
 	unsigned long execute_start, execute_end, execute_len;
@@ -502,7 +503,7 @@  void __init sme_encrypt_kernel(struct boot_params *bp)
 	native_write_cr3(__native_read_cr3());
 }
 
-void __init sme_enable(struct boot_params *bp)
+void __head sme_enable(struct boot_params *bp)
 {
 	const char *cmdline_ptr, *cmdline_arg, *cmdline_on, *cmdline_off;
 	unsigned int eax, ebx, ecx, edx;