[PATCH v2 6/6] efi/arm*: add support to dump the EFI page tables

On 22 April 2016 at 19:25, Mark Rutland [off-list ref] wrote:

On Fri, Apr 22, 2016 at 07:20:38PM +0200, Ard Biesheuvel wrote:

quoted

On 22 April 2016 at 19:01, Mark Rutland [off-list ref] wrote:

quoted

On Fri, Apr 22, 2016 at 06:48:08PM +0200, Ard Biesheuvel wrote:

quoted

quoted

quoted

+static struct ptdump_info efi_ptdump_info = {
+     .mm             = &efi_mm,
+     .markers        = efi_addr_markers,
+     .base_addr      = 0,
+     .max_addr       = SZ_1G,
+};

I see that max_addr isn't used for arm64, and for ARM it's only used in
one place. It doesn't seem great to have that on arm64 given it's
unused.

Do we actually need max_addr? Is there any reason not to always dump
whole tables?

I guess you're trying to avoid dumping the kernel VA range on 32-bit?

Indeed. On ARM, the efi_page_tables dumps its copy of
kernel_page_tables. If that is OK (the information could potentially
be useful, I suppose) we can drop the max

I'd argue it's always worth dumping the full tables (dropipng the max).

For instance, we could have some bug that leaves the kernel VA range in
the EFI tables inconsistent with what we expect/require, and having that
exposed would make that clear.

Well, the problem here is that we only sync the EFI page tables with
the kernel page tables in switch_mm() (if they turn out to be out of
sync). This means we should either sync them in this code path as
well, or remove the kernel VA range from the output, since we'll be
dumping known stale data otherwise.

Regardless, arbitrarily limiting the VA range of the arm64 dump doesn't
seem great.

True. [0 .. TASK_SIZE) looks like a useful and not arbitrary range to
limit the EFI page table output to. Unfortunately, TASK_SIZE resolves
to a value that depends on TIF_32BIT, so will still need some
preprocessor conditional to select between TASK_SIZE_64 (on arm64) and
TASK_SIZE (on ARM)