Received-SPF: pass (google.com: domain of linux-kernel+bounces-71736-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240213124143.1484862-13-ardb+git@google.com>
 <20240213124143.1484862-20-ardb+git@google.com> <b2e0b647-b5a7-4c66-bb00-7907a2318f58@amd.com>
In-Reply-To: <b2e0b647-b5a7-4c66-bb00-7907a2318f58@amd.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Mon, 19 Feb 2024 18:06:07 +0100
Message-ID: <CAMj1kXG_sQ-+ULtvE7SqD0DYe3p9=tP8K9VPgfiR-0Z55A9vVw@mail.gmail.com>
Subject: Re: [PATCH v4 07/11] efi/libstub: Add generic support for parsing mem_encrypt=
To: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb+git@google.com>, linux-kernel@vger.kernel.org, 
	Kevin Loughlin <kevinloughlin@google.com>, Dionna Glaze <dionnaglaze@google.com>, 
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, Andy Lutomirski <luto@kernel.org>, 
	Arnd Bergmann <arnd@arndb.de>, Nathan Chancellor <nathan@kernel.org>, 
	Nick Desaulniers <ndesaulniers@google.com>, Justin Stitt <justinstitt@google.com>, 
	Kees Cook <keescook@chromium.org>, Brian Gerst <brgerst@gmail.com>, linux-arch@vger.kernel.org, 
	llvm@lists.linux.dev
Content-Type: text/plain; charset="UTF-8"

On Mon, 19 Feb 2024 at 18:00, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 2/13/24 06:41, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Parse the mem_encrypt= command line parameter from the EFI stub if
> > CONFIG_ARCH_HAS_MEM_ENCRYPT=y, so that it can be passed to the early
> > boot code by the arch code in the stub.
> >
> > This avoids the need for the core kernel to do any string parsing very
> > early in the boot.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   drivers/firmware/efi/libstub/efi-stub-helper.c | 8 ++++++++
> >   drivers/firmware/efi/libstub/efistub.h         | 2 +-
> >   2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index bfa30625f5d0..3dc2f9aaf08d 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -24,6 +24,8 @@ static bool efi_noinitrd;
> >   static bool efi_nosoftreserve;
> >   static bool efi_disable_pci_dma = IS_ENABLED(CONFIG_EFI_DISABLE_PCI_DMA);
> >
> > +int efi_mem_encrypt;
> > +
> >   bool __pure __efi_soft_reserve_enabled(void)
> >   {
> >       return !efi_nosoftreserve;
> > @@ -75,6 +77,12 @@ efi_status_t efi_parse_options(char const *cmdline)
> >                       efi_noinitrd = true;
> >               } else if (IS_ENABLED(CONFIG_X86_64) && !strcmp(param, "no5lvl")) {
> >                       efi_no5lvl = true;
> > +             } else if (IS_ENABLED(CONFIG_ARCH_HAS_MEM_ENCRYPT) &&
> > +                        !strcmp(param, "mem_encrypt") && val) {
> > +                     if (parse_option_str(val, "on"))
> > +                             efi_mem_encrypt = 1;
> > +                     else if (parse_option_str(val, "off"))
> > +                             efi_mem_encrypt = -1;
>
> With CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT having recently been
> removed, I'm not sure what parsing for mem_encrypt=off does.
>
> (Same thing in the next patch.)
>

We have to deal with both mem_encrypt=on and mem_encrypt=off occurring
on the command line. efi_parse_options() may be called more than once,
i.e., when there is a default command line baked into the image that
can be overridden at runtime. So if the baked in one has
mem_encrypt=on, mem_encrypt=off appearing later should counter that.

The same applies to the next patch, although the decompressor appears
to ignore the built-in command line entirely (I made a note of that in
the commit log)