On Sun, 2 Oct 2022 at 23:43, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > > On Sun, 2 Oct 2022 at 18:28, Demi Marie Obenour > <demi@xxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Sun, Oct 02, 2022 at 11:56:26AM +0200, Ard Biesheuvel wrote: > > > In order to permit the ESRT to be used when doing pseudo-EFI boot > > > without a EFI memory map, e.g., when booting inside a Xen dom0 on x86, > > > make the sanity checks optional based on whether the memory map is > > > available. > > > > > > If additional validation is needed, it is up to the Xen EFI glue code to > > > implement this in its xen_efi_config_table_is_valid() helper, or provide > > > a EFI memory map like it does on other architectures. > > > > I don’t like this. It is easy to use a hypercall to get the end of the > > memory region containing the config table, which is what my one of my > > previous patches actually does. Skipping all of the validation could > > easily lead to a regression. > > I don't like putting Xen specific hacks left and right because Xen on > x86 cannot be bothered to provide an EFI memory map. And as for > regressions, ESRT does not work at all under Xen (given the lack of a > memory map) and so I fail to see how this could break a currently > working case. > > > I understand wanting to get Xen-specific > > code out of esrt.c, but this isn’t the answer. Some sort of abstraction > > over both cases would be a much better solution. > > > > We have such an abstraction already, it is called the EFI memory map. > > So there are two options here: > - expose a EFI memory map > - add a ESRT specific check to xen_efi_config_table_is_valid() so that > the ESRT is withheld from dom0 if there is something wrong with it. > Actually, the obvious answer here is to implement efi_mem_desc_lookup() for the EFI_PARAVIRT / !EFI_MEMMAP case. I'll have a go at that and send a v2 shortly.
