Re: [PATCH] x86/efi: Add missing 1:1 mappings to support buggy firmware

[Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>]

On Tue, 14 Feb, at 07:42:00PM, Sai Praneeth Prakhya wrote:
> 
> It makes sense that "efi_sync_low_kernel_mappings()" would be the best
> place but I added them here for these reasons
> 
> 1. These mappings refer to "direct mapping of all physical memory" and
> thus in "efi_map_regions()" we update *these* mappings to create 1:1
> mappings (to allow physical mode accesses by buggy firmware), so if we
> copy these mappings in "efi_sync_low_kernel_mappings()" which is called
> later than efi_map_regions(), we will overwrite previous 1:1 mappings
> created by efi_map_regions() and hence kernel panics.
> 
> 2. The other reason for adding these mappings in
> "efi_alloc_page_tables()" is because that's the way mappings looked in
> swapper_pgd before we shifted EFI stuff to efi_pgd.
> 
> I have tested this by moving this code to
> "efi_sync_low_kernel_mappings()" and I see that kernel panics in
> set_virtual_address_map().
> 
> Please do correct me if you think otherwise.
> 
> Thanks for the review, Matt

OK, I now realise that I misunderstood this patch when I reviewed it
the first time. Sorry about that.

I see that the problem you're addressing is that some firmware will
access the physical address of EFI_CONVENTIONAL_MEMORY regions while
executing runtime services. And unless we're running in EFI mixed
mode, we don't map EFI_CONVENTIONAL_MEMORY regions into the EFI page
tables.

We already have code to handle the 1:1 mapping of
EFI_CONVENTIONAL_MEMORY because we need to do this for the EFI mixed
mode scenario. The difference for non-mixed mode is that we only want
to create the 1:1 mapping for EFI_CONVENTIONAL_MEMORY, we don't want
the virtual mapping too.

Lemme go and reply to your v2.