On Wed, Jun 29, 2016 at 01:03:34PM +0200, Ard Biesheuvel wrote:

On 29 June 2016 at 12:50, Catalin Marinas [off-list ref] wrote:

quoted

On Wed, Jun 29, 2016 at 12:03:16PM +0200, Ard Biesheuvel wrote:

quoted

On 29 June 2016 at 11:39, Catalin Marinas [off-list ref] wrote:

quoted

On Tue, Jun 28, 2016 at 06:12:22PM +0200, Ard Biesheuvel wrote:

quoted

On 28 June 2016 at 18:05, Catalin Marinas [off-list ref] wrote:

quoted

On Mon, Jun 06, 2016 at 11:18:14PM +0200, Ard Biesheuvel wrote:

quoted

Another thing I failed to mention is that the new Memory Attributes
table support may map all of the RuntimeServicesCode regions a second
time, but with a higher granularity, using RO for .text and .rodata
and NX for .data and .bss (and the PE/COFF header).

Can this not be done in a single go without multiple passes? That's what
we did for the core arm64 code, the only one left being EFI run-time
mappings.

Well, we probably could, but it is far from trivial.

quoted

quoted

Due to the higher
granularity, regions that were mapped using the contiguous bit the
first time around may be split into smaller regions. Your current code
does not address that case.

If the above doesn't work, the only solution would be to permanently map
these ranges as individual pages, no large blocks.

That is not unreasonable, since regions >2MB are unusual.

We'll have the contiguous bit supported at some point and we won't be
able to use it for EFI run-time mappings. But I don't think that's
essential, minor improvement on a non-critical path.

I'll post some patches to always use PAGE_SIZE granularity for EFI
run-time mappings.

Given that contiguous bit mappings only affect the TLB footprint, I'd
be more concerned about not using block mappings for EfiMemoryMappedIo
regions (since they may cover fairly sizable NOR flashes like the 64
MB one QEMU mach-virt exposes).

Good point.

quoted

So I would recommend to only use PAGE_SIZE granularity for
EfiRuntimeServicesCode and EfiRuntimeServicesData regions, since those
are the only ones that can be expected to appear in the Memory
Attributes table, and all other regions will only be mapped a single
time.

Is there a possibility that EfiMemoryMappedIo share the same 64K page
with EfiRuntimeServicesCode? If it does, it won't help much with
avoiding splitting.

The spec does not allow it, and it would also imply that memory and
!memory share a 64 KB page frame in the hardware, which seems highly
unlikely as well.

I assume there isn't even a workaround if the EFI maps are broken in
this respect. But we still need to gracefully handle it and avoid a
potential kernel panic (like some BUG_ON in the arm64 page table
creation code).

quoted

Unless I keep a combination of these series
(checking the end/start overlap) with a forced page-only mapping for
EfiRuntimeServicesCode/Data.

If we get rid of the splitting, the only 'issue' that remains is that
the page frame shared between two adjacent unaligned regions is mapped
twice (but the current code will always map them with the same
attribute)

So back to my question I posed a couple of posts ago: if the UEFI page
tables were live at this time (which they are not), could it ever be a
problem that a page table entry is rewritten with the exact same value
it had before (but without bbm?) If not, I think we could educate the
debug routines to allow this case (since it needs to read the entry to
check the valid bit anyway, if it needs to be strict about break
before make)

There wouldn't be any issue, we already do this in other cases like
mark_rodata_ro().

-- 
Catalin