Re: [PATCH] x86/efi: Map EFI memmap entries in-order at runtime

[Matt Fleming <matt@codeblueprint.co.uk>]

On Wed, 09 Sep, at 09:37:21AM, Ard Biesheuvel wrote:
> On 8 September 2015 at 22:37, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> >
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index 691b333e0038..a2af35f6093a 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -704,6 +704,44 @@ out:
> >         return ret;
> >  }
> >
> > +static inline void *efi_map_next_entry_reverse(void *entry)
> > +{
> > +       if (!entry)
> > +               return memmap.map_end - memmap.desc_size;
> > +
> > +       entry -= memmap.desc_size;
> > +       if (entry < memmap.map)
> > +               return NULL;
> > +
> > +       return entry;
> > +}
> > +
> > +static void *efi_map_next_entry(void *entry)
> > +{
> > +       bool reverse = false;
> > +
> > +       if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) {
> 
> Here, you could also test whether the
> EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA bit
> (sigh) is set

No, leaving this out was intentional because we're already suffering
from the combinatoral explosion of config options. Introducing more
code paths is very much the wrong thing to do unless absolutely
necessary.

If we can get away with using one mapping scheme here, we should.

When trying to debug this code in the future I do not want to be
thinking "Do you have EFI_PROPERTIES_RUNTIME_OMG_THIS_IS_SILLY bit
set? because that means we're mapping the runtime regions in a
different order".
 
-- 
Matt Fleming, Intel Open Source Technology Center