public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Dionna Glaze" <dionnaglaze@google.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	Gerd Hoffmann <kraxel@redhat.com>,
	 "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	linux-kernel@vger.kernel.org,  linux-efi@vger.kernel.org,
	x86@kernel.org, jiewen.yao@intel.com,  devel@edk2.groups.io,
	"Min M. Xu" <min.m.xu@intel.org>,
	 James Bottomley <jejb@linux.ibm.com>,
	Tom Lendacky <Thomas.Lendacky@amd.com>,
	 Erdem Aktas <erdemaktas@google.com>,
	Dave Hansen <dave.hansen@linux.intel.com>
Subject: Re: [PATCH v2] x86/efi: Safely enable unaccepted memory in UEFI
Date: Tue, 17 Jan 2023 08:45:55 -0800	[thread overview]
Message-ID: <CAAH4kHbhSfeDeBCLCO4Bc2MK8Ds-kjXxCnrkMEP1j_GO5sh18w@mail.gmail.com> (raw)
In-Reply-To: <20230116231711.cudsnxvnfg6aef3w@box.shutemov.name>

>
> Why do you call boot with a bootloader a legacy feature?
>

Gerd answered this about EBS called from the bootloader.

> > they'll only get a safe view of the memory map. I don't think it's right
> > to choose unsafe behavior for a legacy setup.
>
> Present memory map with unaccepted memory to OS that doesn't about it is
> perfectly safe. This portion of the memory will be ignored. It is "feature
> not [yet] implemented" case.
>

SNP guest support is already in Linux, and it gets a full view of the
memory given to the VM. If the firmware ever introduces unaccepted
memory, then the kernel's behavior is retroactively broken without the
"accept all if AllowUnacceptedMemory() not called" behavior of the
UEFI.
The memory that existed before becomes ignored. This is not the right
approach IMO.

> > > This patch adds complexity, breaks what works and the only upside will
> > > turn into a dead weight soon.
> > >
> > > There's alternative to add option to instruct firmware to accept all
> > > memory from VMM side. It will serve legacy OS that doesn't know about
> > > unaccepted memory and it is also can be use by latency-sensitive users
> > > later on (analog of qemu -mem-prealloc).
> > >
> >
> > This means that users of a distro that has not enabled unaccepted
> > memory support cannot simply start a VM with the usual command, but
> > instead have to know a baroque extra flag to get access to all the
> > memory that they configured the machine (and for a CSP customer, paid
> > for). That's not a good experience.
>
> New features require enabling. It is not something new.
>

What I'm saying is that you're suggesting a feature _dis_abling
requirement, which is an antipattern. Any SNP user right now would
need to add a "don't use an unimplemented feature" flag to get access
to all its memory again.

> > With GCE at least, you can't (shouldn't) associate the boot feature
> > flag with a disk image because disks are mutable. If a customer
> > upgrades their kernel after initially starting their VM, they can't
> > remove the flag due to the way image annotations work.
>
> I guess a new VM has to be created, right? Doesn't sound like a big deal
> to me.
>

Usually it's not, but the retroactive need to create a new VM once the
firmware adds UEFI v2.9 support with unaccepted memory is a big deal.

> The old will not break with upgraded kernel. Just not get benefit of the
> feature.
>

A user buys access to a high memory VM: 768GiB. They then shut down
and bring it back up on a new firmware that uses unaccepted memory.

That VM goes from 785GiB free memory to 3GiB free memory at boot.

This is because all memory above 4GiB (and nothing there for the
3-4GiB MMIO hole) would be the unknown unaccepted memory type. We need
the accept-all-if-support-not-acked semantics with the protocol.

> > All of this headache goes away by adopting a small patch to the kernel
> > that calls a 0-ary protocol interface and keeping safe acceptance
> > behavior in the firmware. I think Gerd is right here that we should
> > treat it as a transition feature that we can remove later.
>
> Removing a feature is harder than adding one. How do you define that
> "later" has come?
>

Gerd's response of after 6.1-lts EOL is reasonable to me. At the same
time, both SEV-SNP and TDX's Kconfig would need to strictly require
unaccepted memory.

The semantics of the UEFI under the proposed protocol is allowed to
change the default behavior when the protocol is not exposed to the
OS. The default would then be to always introduce unaccepted memory
for TDX and SEV-SNP guests.

To Gerd's point, removing "first in edk2, later in linux too" I think
is backwards. We need all users of the protocol to agree that SEV-SNP
and TDX strictly imply unaccepted memory support. Only then can we
remove the protocol from EDK2.

> Anyway, I think we walk in a circle. I consider it a misfeature. If you
> want still go this path, please add my
>
> Nacked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>

Thanks for your time discussing.


-- 
-Dionna Glaze, PhD (she/her)

  parent reply	other threads:[~2023-01-17 16:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 21:29 [PATCH v2] x86/efi: Safely enable unaccepted memory in UEFI Dionna Glaze
2023-01-13 22:20 ` Kirill A. Shutemov
2023-01-16 10:56   ` Gerd Hoffmann
2023-01-16 12:30     ` Kirill A. Shutemov
2023-01-16 13:11       ` Ard Biesheuvel
2023-01-16 13:42         ` Kirill A. Shutemov
2023-01-16 19:43           ` Dionna Glaze
2023-01-16 23:17             ` Kirill A. Shutemov
2023-01-17 10:24               ` Gerd Hoffmann
2023-01-17 16:45               ` Dionna Glaze [this message]
2023-01-18  7:51                 ` [edk2-devel] " Gerd Hoffmann
2023-01-16 21:22     ` Dave Hansen
2023-01-16 22:46       ` Lendacky, Thomas
2023-01-18 15:09         ` Ard Biesheuvel
2023-01-18 15:40           ` Dave Hansen
2023-01-18 15:46             ` Ard Biesheuvel
2023-01-17 10:34       ` Gerd Hoffmann

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-list 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=CAAH4kHbhSfeDeBCLCO4Bc2MK8Ds-kjXxCnrkMEP1j_GO5sh18w@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /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