On Tue, 29 Sep, at 11:12:30AM, Ingo Molnar wrote:

* Ard Biesheuvel [off-list ref] wrote:

quoted

quoted

except that I don't think
the condition on 64-bit makes any sense:

+       if (!efi_enabled(EFI_OLD_MEMMAP) && efi_enabled(EFI_64BIT)) {

I can see us being nervous wrt. backported patches, but is there any strong reason
to not follow this up with a third (non-backported) patch that changes this to:

+       if (!efi_enabled(EFI_OLD_MEMMAP)) {

for v4.4?

The 32-bit side essentially implements the old memmap only, which is the the 
bottom-up version. So old memmap will be implied by 32-bit but not set in the 
EFI flags, resulting in the reverse enumeration being used with the bottom-up 
mapping logic. The net result of that is that we create the same problem for 
32-bit that we are trying to solve for 64-bit, i.e., the regions will end up in 
reverse order in the VA mapping.

To deobfuscate this particular conditional, we could set EFI_OLD_MEMMAP 
unconditionally on 32-bit x86. Or we could reshuffle variables and conditionals 
in various other way.

Setting EFI_OLD_MEMMAP would be fine, if doing that has no bad side effects.

 
Right, I think that's a very good suggestion, because like Ard
mentioned, since EFI_OLD_MEMMAP is implied for 32-bit (there's no
other way to map stuff currently), so it makes sense to force set the
bit.

quoted

[...] I am not convinced that the overall end result will be any better though.

That's not true, we change an obscure, implicit dependency on 32-bit detail to an 
explicit EFI_OLD_MEMMAP flag that shows exactly what's happening. That's a clear 
improvement.

Agreed.

-- 
Matt Fleming, Intel Open Source Technology Center