Linux EFI development
 help / color / mirror / Atom feed
From: Demi Marie Obenour <demi@invisiblethingslab.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
	linux-efi@vger.kernel.org, "Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Anton Vorontsov" <anton@enomsg.org>,
	"Colin Cross" <ccross@android.com>,
	"Tony Luck" <tony.luck@intel.com>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Ard Biesheuvel" <ardb@kernel.org>
Subject: Re: [PATCH v4 1/2] Avoid using EFI tables Xen may have clobbered
Date: Tue, 4 Oct 2022 11:46:59 -0400	[thread overview]
Message-ID: <YzxxXuovwQt3NskE@itl-email> (raw)
In-Reply-To: <01d22092-8292-8ed7-ece7-9ca32d15bbce@suse.com>

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

On Tue, Oct 04, 2022 at 10:22:13AM +0200, Jan Beulich wrote:
> On 01.10.2022 02:30, Demi Marie Obenour wrote:
> > On Fri, Sep 30, 2022 at 08:27:09PM +0200, Ard Biesheuvel wrote:
> >> On Fri, 30 Sept 2022 at 19:12, Demi Marie Obenour wrote:
> >>> On Fri, Sep 30, 2022 at 06:30:57PM +0200, Ard Biesheuvel wrote:
> >>>> I know very little about Xen, but based on the context you provided in
> >>>> this thread, I'd say that the best approach from the Xen side is to
> >>>> convert all EfiBootServicesData regions that have configuration tables
> >>>> pointing into them into EfiAcpiReclaimMemory.
> >>>
> >>> Should Xen convert the entire region, or should it try to reserve only
> >>> the memory it needs?  The latter would require it to parse the
> >>> configuration tables.  Is there a list of configuration tables that can
> >>> legitimately be in EfiBootServicesData regions?
> >>>
> >>
> >> Not really, no. So you would have to convert the entire region
> >> /unless/ Xen knows the GUID, and therefore knows how to derive the
> >> size of the table, allowing it to reserve memory more conservatively.
> >> However, I doubt whether this is worth it: splitting entries implies
> >> rewriting the memory map, which is a thing I'd rather avoid if I were
> >> in your shoes.
> > 
> > I actually wonder if Xen needs to reserve *all* of EfiBootServicesData.
> > The reason is that some (probably buggy) firmware may store ACPI tables
> > there, and Xen does not have an ACPI implementation.
> 
> We already have the -mapbs option as a workaround in such situations.

The problem is that there is no way for the user to know when it is
needed.  One option would be for dom0 to print a warning in the kernel
log if it needs to ignore a table due to it being in EfiBootServicesData.

> >  From my
> > perspective, a much safer approach would be to pass all of
> > EfiBootServicesData memory directly to dom0, and have dom0 give Xen back
> > what it doesn’t wind up using.  That allows dom0’s memory reservation
> > code to work properly, which it currently does not.
> 
> As said already on a different thread: Giving memory to domains (incl
> Dom0) isn't related to their original memory type (neither EFI's nor
> E820's); the needed memory is taken from the general page allocator
> (with one exception for initrd, to avoid unnecessary copying around of
> data). Hence what you propose would end up as an (imo) awful hack in
> Xen. I also don't see how this relates to "dom0’s memory reservation
> code", but I'm sure you can clarify that for me.

Linux has a function called efi_mem_reserve() that is used to reserve
EfiBootServicesData memory that contains e.g. EFI configuration tables.
This function does not work under Xen because Xen could have already
clobbered the memory.  efi_mem_reserve() not working is the whole reason
for this thread, as it prevents EFI tables that are in
EfiBootServicesData from being used under Xen.

A much nicer approach would be for Xen to reserve boot services memory
unconditionally, but provide a hypercall that dom0 could used to free
the parts of EfiBootServicesData memory that are no longer needed.  This
would allow efi_mem_reserve() to work normally.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-10-04 17:46 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 23:02 [PATCH v4 0/2] EFI improvements for Xen dom0 Demi Marie Obenour
2022-09-29 23:02 ` [PATCH v4 1/2] Avoid using EFI tables Xen may have clobbered Demi Marie Obenour
2022-09-30  6:44   ` Jan Beulich
2022-09-30 16:30     ` Ard Biesheuvel
2022-09-30 17:11       ` Demi Marie Obenour
2022-09-30 18:27         ` Ard Biesheuvel
2022-09-30 18:50           ` Demi Marie Obenour
2022-10-01  0:30           ` Demi Marie Obenour
2022-10-04  8:22             ` Jan Beulich
2022-10-04 15:46               ` Demi Marie Obenour [this message]
2022-10-05  6:15                 ` Jan Beulich
2022-10-05 18:11                   ` Demi Marie Obenour
2022-10-05 21:28                     ` Ard Biesheuvel
2022-10-06  1:40                       ` Demi Marie Obenour
2022-10-06  7:31                         ` Ard Biesheuvel
2022-10-06 14:43                           ` Demi Marie Obenour
2022-10-06 16:19                             ` Ard Biesheuvel
2022-10-06 17:22                               ` Demi Marie Obenour
2022-10-06 17:56                                 ` Ard Biesheuvel
2022-10-06  9:22                     ` Jan Beulich
2022-09-30 16:38     ` Demi Marie Obenour
2022-09-30 16:25   ` Ard Biesheuvel
2022-09-30 18:15     ` Demi Marie Obenour
2022-09-30 18:42       ` Ard Biesheuvel
2022-09-30 19:00         ` Demi Marie Obenour
2022-09-29 23:02 ` [PATCH v4 2/2] Support ESRT in Xen dom0 Demi Marie Obenour
2022-09-30 16:36   ` Ard Biesheuvel
2022-09-30 18:21     ` Demi Marie Obenour
2022-09-30 19:11       ` Ard Biesheuvel
2022-09-30 20:20         ` Demi Marie Obenour
2022-09-30 20:59           ` Ard Biesheuvel
2022-09-30 21:24             ` Ard Biesheuvel
2022-09-30 22:22               ` Demi Marie Obenour
2022-09-30 22:25             ` Demi Marie Obenour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YzxxXuovwQt3NskE@itl-email \
    --to=demi@invisiblethingslab.com \
    --cc=anton@enomsg.org \
    --cc=ardb@kernel.org \
    --cc=ccross@android.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marmarek@invisiblethingslab.com \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=sstabellini@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox