On Mon, 22 Feb, at 04:56:57PM, Ard Biesheuvel wrote:

On 22 February 2016 at 16:43, Mark Rutland [off-list ref] wrote:

quoted

On Mon, Feb 22, 2016 at 03:59:39PM +0100, Ard Biesheuvel wrote:

quoted

diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 848ede1587dc..6ce13d6b7122 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c

@@ -64,6 +64,16 @@ static bool __init efi_virtmap_init(void)
                              &phys, ret);
                      return false;
              }
+             /*
+              * If this entry covers the address of the UEFI system table,
+              * calculate and record its virtual address.
+              */
+             if (efi_system_table >= phys &&
+                 efi_system_table < phys + (md->num_pages * EFI_PAGE_SIZE)) {
+                     efi.systab = (void *)(efi_system_table - phys +
+                                           md->virt_addr);
+                     set_bit(EFI_SYSTEM_TABLES, &efi.flags);
+             }

It seems very odd to me to set this given it's currently unused, and we
don't have permanent access to the table. That sounds like we're just
setting ourselves up for future fragility as users appear.

I wondered about the purpose as well. It is only ever set, and never
tested (until this patch)

@Matt: any thoughts?

Good question. EFI_SYSTEM_TABLES was introduced over 4 years ago, but
it looks like it has never had any users. I don't remember why I added
it, probably just be to be "complete" since we had bits for all the
other tables.

If somebody wants to rip it out, I wouldn't object.

To be fair, we don't have permanent access to any of the tables - you
always have to switch to the dedicated EFI page tables before
accessing regions.