Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: iac594737:Fastmail
Date:   Tue, 4 Oct 2022 11:46:59 -0400
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 =?utf-8?Q?Marczykowski-G=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>, Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v4 1/2] Avoid using EFI tables Xen may have clobbered
Message-ID: <YzxxXuovwQt3NskE@itl-email>
References: <cover.1664298147.git.demi@invisiblethingslab.com>
 <f3b624e99adfdbbfc1976a60a73a6b5950e1840d.1664298147.git.demi@invisiblethingslab.com>
 <282a225d-8782-0321-6f0e-19dd4510dc42@suse.com>
 <CAMj1kXFQNqsW5RfHGac-eGbosJHBybu6+-Fap_bi_kVxWNpGeg@mail.gmail.com>
 <YzcjeiOW8+i2Zxsd@itl-email>
 <CAMj1kXHBBbCNV3CLesqZi7ttmmi8y4tZ1KO5vievy_CJrU2o3Q@mail.gmail.com>
 <YzeaKjmls1YI/3ox@itl-email>
 <01d22092-8292-8ed7-ece7-9ca32d15bbce@suse.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="lgxbz4vBgheHS+D8"
Content-Disposition: inline
In-Reply-To: <01d22092-8292-8ed7-ece7-9ca32d15bbce@suse.com>
Precedence: bulk


--lgxbz4vBgheHS+D8
Content-Type: text/plain; protected-headers=v1; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Date: Tue, 4 Oct 2022 11:46:59 -0400
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 =?utf-8?Q?Marczykowski-G=C3=B3recki?= <marmarek@invisiblethingslab.com>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH v4 1/2] Avoid using EFI tables Xen may have clobbered

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 tabl=
es
> >>>> 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 c=
an
> >>> 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.
> >=20
> > 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.
>=20
> 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=E2=80=99t wind up using.  That allows dom0=E2=80=99s memo=
ry reservation
> > code to work properly, which it currently does not.
>=20
> 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=E2=80=99s memory reservat=
ion
> 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.
--=20
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

--lgxbz4vBgheHS+D8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmM8VY8ACgkQsoi1X/+c
IsHLdA/+LM4HOMK+UqW8ug41g4/DBfgbjzO9EIOgBAXcDsotBsE2hgv+29Nd8k2/
x08K8YAFtt6tgef77/ZH2pFmtCbY3v9AbO+Y0AUn6TD8ownHkXRvQZcH4f+/q1qu
I/eMEvBHn2+Tcl5NLk8JbIj+iufNcZsgvuPxO5Bn6L6cs3uLKey68T+iBN1djFNc
LrhecxdhCSa1096qkvxAn6d7RRQcplv1//S4LXrCjt3q77T4Dg7y8sPcqDQw98W1
QW+6l9GDwVr0EDWse00UkigGhQK+GP3bEv3ucJoxiDFYCAQy24sOTBL/m4wfcZmv
t/43wgXv9010fUACW0Fl0kgKPWMKCMfUM1d7VLEj4GkWLaN41C8eZc5EBj5p37Df
AR1xqfFhR3X50gACUUyoUVfLw2LbohUdzmqncPIAhFOYnrZ4/EfaQjHEq1EMHsPi
X0ocJZhndjM12OXOgLpYdijTZ/1fl6LtYmEULzGdDP1ANinE85ihHsBmLOfUMF+R
MOsy9XDSKzYpi1naZ0Pt+hJzEsEmzoxV7JytqBSxCKihGlnsFf6PPF+eKa3UnFyp
ji4rdgWVtUqhrPa8SWHaxiNhTnNJSZ7VDYbj7hkwejS6dk08ZXmcfHmKDRo3FflR
Y4PVJCITmoxzY63hLDFPN5bDwiEYqRAVlHpsix40rjxGDMDq7EM=
=u0CQ
-----END PGP SIGNATURE-----

--lgxbz4vBgheHS+D8--