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;
MIME-Version: 1.0
References: <cover.1664298147.git.demi@invisiblethingslab.com>
 <5649176eacda434267f68676f1733d06c572d19e.1664298147.git.demi@invisiblethingslab.com>
 <CAMj1kXEs-o8jvNqRiW+Ue2i52RBgg4iktg8UONCACk8-Gx4XXA@mail.gmail.com>
 <YzczpIYop5olD4hj@itl-email> <CAMj1kXHGPzy9T1LcE8LX+woGtUGTzrDgbjDBJabJ+bwDVPbTag@mail.gmail.com>
 <YzdPv4+fYX3SG9P0@itl-email>
In-Reply-To: <YzdPv4+fYX3SG9P0@itl-email>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Fri, 30 Sep 2022 22:59:49 +0200
Message-ID: <CAMj1kXGTZZmW=QZOL1FvrsBLsvFzN1GkvZCotuZ-C-gBVGY6CQ@mail.gmail.com>
Subject: Re: [PATCH v4 2/2] Support ESRT in Xen dom0
To:     Demi Marie Obenour <demi@invisiblethingslab.com>
Cc:     Peter Jones <pjones@redhat.com>, 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>,
        =?UTF-8?Q?Marek_Marczykowski=2DG=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>, xen-devel@lists.xenproject.org,
        linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 30 Sept 2022 at 22:21, Demi Marie Obenour
<demi@invisiblethingslab.com> wrote:
>
> On Fri, Sep 30, 2022 at 09:11:19PM +0200, Ard Biesheuvel wrote:
> > On Fri, 30 Sept 2022 at 20:21, Demi Marie Obenour
> > <demi@invisiblethingslab.com> wrote:
> > >
> > > On Fri, Sep 30, 2022 at 06:36:11PM +0200, Ard Biesheuvel wrote:
> > > > On Fri, 30 Sept 2022 at 01:02, Demi Marie Obenour
> > > > <demi@invisiblethingslab.com> wrote:
> > > > >
> > > > > fwupd requires access to the EFI System Resource Table (ESRT) to
> > > > > discover which firmware can be updated by the OS.  Currently, Lin=
ux does
> > > > > not expose the ESRT when running as a Xen dom0.  Therefore, it is=
 not
> > > > > possible to use fwupd in a Xen dom0, which is a serious problem f=
or e.g.
> > > > > Qubes OS.
> > > > >
> > > > > Before Xen 4.17, this was not fixable due to hypervisor limitatio=
ns.
> > > > > The UEFI specification requires the ESRT to be in EfiBootServices=
Data
> > > > > memory, which Xen will use for whatever purposes it likes.  There=
fore,
> > > > > Linux cannot safely access the ESRT, as Xen may have overwritten =
it.
> > > > >
> > > > > Starting with Xen 4.17, Xen checks if the ESRT is in EfiBootServi=
cesData
> > > > > or EfiRuntimeServicesData memory.  If the ESRT is in EfiBootServi=
cesData
> > > > > memory, Xen replaces the ESRT with a copy in memory that it has
> > > > > reserved.  Such memory is currently of type EFI_RUNTIME_SERVICES_=
DATA,
> > > > > but in the future it will be of type EFI_ACPI_RECLAIM_MEMORY.  Th=
is
> > > > > ensures that the ESRT can safely be accessed by the OS.
> > > > >
> > > > > When running as a Xen dom0, use the new
> > > > > xen_config_table_memory_region_max() function to determine if Xen=
 has
> > > > > reserved the ESRT and, if so, find the end of the memory region
> > > > > containing it.  This allows programs such as fwupd which require =
the
> > > > > ESRT to run under Xen, and so makes fwupd support in Qubes OS pos=
sible.
> > > > >
> > > > > Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> > > >
> > > > Why do we need this patch? I'd expect esrt_table_exists() to return
> > > > false when patch 1/2 is applied.
> > >
> > > efi_enabled(EFI_MEMMAP) is false under Xen, so there needs to be an
> > > alternative way to get the end of the memory region containing the ES=
RT.
> > > That is what this patch provides.
> >
> > OK. I don't think we need that to be honest. When running under Xen,
> > we should be able to assume that the ESRT does not span multiple
> > memory regions arbitrarily, so we can just omit this check if
> > !efi_enabled(EFI_MEMMAP)
> >
> > IIRC (and Peter would know), we are trying to filter out descriptors
> > that are completely bogus here: zero lenght, zero address, etc etc. I
> > don't think we need that for Xen.
>
> Xen doesn=E2=80=99t uninstall bogus ESRTs, so there is no less reason to =
worry
> under Xen than on bare hardware.

That may be true. But if Xen needs dom0 to be able to cross reference
the EFI memory map, it should provide one (and set EFI_MEMMAP to
enabled).