On Tue, 2017-02-28 at 11:51 +0000, Matt Fleming wrote:
> On Tue, 14 Feb, at 08:03:41PM, Sai Praneeth Prakhya wrote:
> > From: Sai Praneeth <sai.praneeth.prakhya@xxxxxxxxx>
> >
> > There are some machines with buggy firmware that access EFI regions in
> > 1:1 mode (or physical mode) rather than virtual mode even after kernel
> > being booted. On these machines, if we invoke an EFI runtime service
> > (that does these buggy accesses) then it causes a page fault and hence
> > results in kernel hang. The page fault happens because the requested
> > region doesn't have appropriate page attributes set or the mapping for
> > the region might be missing. This issue was introduced by commit
> > 67a9108ed431 ("x86/efi: Build our own page table structures"). Before
> > this commit, 1:1 mappings for EFI regions were in swapper_pgd and were
> > not needed to be synced, but this commit introduced efi_pgd which missed
> > these mappings.
>
> The subject line needs to mention EFI_CONVENTIONAL_MEMORY, because we
> already have 1:1 mappings for some regions, just not for
> EFI_CONVENTIONAL_MEMORY (by default). Those are the missing mappings.
Sorry! for the confusing subject line.
>
> > Below is the edited version of the page fault output that I noticed:
> >
> > BUG: unable to handle kernel paging request at 0000000018847980
> > I have looked at EFI Memory map for the faulted address and found that
> > it belongs to memory region of type "Conventional Memory". So, this
> > firmware bug is not same as accessing EFI_BOOT_SERVICES_* regions after
> > boot, but firmware accessing *illegal regions* in *1:1 mode*.
>
> Can you confirm that the firmware is accessing the corresponding
> physical address of a virtual address that was passed an argument to
> the EFI runtime service? In other words, is the firmware accessing
> memory (via the wrong mapping) that the kernel has passed, or is it
> accessing memory locations it shouldn't be?
>
Sure! Will do that. Will change the patch according to your suggestion
(mapping EFI_CONVENTIONAL_MEMORY only in 1:1 mode) and see if that fixes
the issue. I am sure that it's the buggy firmware causing the issue but
let me just confirm.
> > Below shown are the efi_pgd dumps before and after the bad commit.
> > efi_dump_pagetable() is called before calling efi_merge_regions() in
> > __efi_enter_virtual_mode() and this kernel is booted on qemu to obtain
> > page table dumps.
> > While not having these mappings isn't a bug but we need these mappings
> > to support machines with buggy firmware.
>
> If buggy machines do not boot because we do not have these mappings,
> then yes, it is a bug.
>
Actually, the machine does boot. But after booting when I run some efi
tests (using LUV) and when the tests call some EFI_RUNTIME_SERVICE
(efi_query_capsule_caps(), efi_get_next_variable()), the system hangs.
It hangs because page fault occurs in kernel mode and it is not
resolved.
> > Signed-off-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@xxxxxxxxx>
> > Cc: Lee, Chun-Yi <jlee@xxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Ricardo Neri <ricardo.neri@xxxxxxxxx>
> > Cc: Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx>
>
> I don't think we should be adding yet another place in the EFI code
> where we're modifying the page tables.
>
> We already have the ability to map EFI_CONVENTIONAL_MEMORY regions
> inside of efi_map_regions() via the should_map_region() function.
>
> Currently, unless you're booting in mixed mode that function will
> return 'false' if the region type is EFI_CONVENTIONAL_MEMORY, so to
> get your machine booting you need to do two things,
>
> 1) Modify should_map_region() to allow EFI_CONVENTIONAL_MEMORY to be
> mapped
>
> 2) Modify the 64-bit version of efi_map_region() to *only* create
> 1:1 mapping for EFI_CONVENTIONAL_MEMORY regions.
Thanks for the suggestions! Will try these and will let you know if that
fixes the issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
