Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20231011192528.262425-1-nik.borisov@suse.com> <20231012101456.goamenepqlte65jv@box.shutemov.name>
In-Reply-To: <20231012101456.goamenepqlte65jv@box.shutemov.name>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 13 Oct 2023 12:28:23 +0200
Message-ID: <CAMj1kXE3+grHM-ay5J5sHD55rs_UXU061AOrukDABRGpbUE4nA@mail.gmail.com>
Subject: Re: [PATCH] x86/efistub: Don't try to print after ExitBootService()
To:     kirill.shutemov@linux.intel.com
Cc:     Nikolay Borisov <nik.borisov@suse.com>, linux-efi@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 12 Oct 2023 at 12:15, <kirill.shutemov@linux.intel.com> wrote:
>
> On Wed, Oct 11, 2023 at 10:25:28PM +0300, Nikolay Borisov wrote:
> > setup_e820() is executed after UEFI's ExitBootService has been called.
> > This causes the firmware to throw an exception because Console IO
> > protocol handler is supposed to work only during boot service
> > environment. As per UEFI 2.9, section 12.1:
> >
> >  "This protocol isused to handle input and output of text-based
> >  information intended for the system user during the operation of code
> >  in the boot services environment."
> >

Thanks. I've queued this up as a fix.

> > Running a TDX guest with TDVF with unaccepted memory disabled results in
> > the following output:
>
> Oh. My bad.
>
> But there's other codepath that does the same. If setup_e820() fails with
> EFI_BUFFER_TOO_SMALL, efi_stub_entry() would try to print "exit_boot()
> failed\n".
>
> I wouldner if it is feasible to hook up earlyprintk console into
> efi_printk() machinery for after ExitBootService() case? Silent boot
> failure is not the best UX.
>

I don't disagree with that in principle, but wiring this up seems
non-trivial, and will be x86-only.

The EFI output is not recorded in the kernel log, and this particular
issue is something we can warn about later on, when it is much more
likely that someone will notice.

So if we want to keep this functionality, I'd prefer it if we could
add something to the generic EFI memmap code that warns_once about
unaccepted memory being present and CONFIG_UNACCEPTED_MEMORY being
disabled.