On Wed, Jun 29, 2016 at 01:03:34PM +0200, Ard Biesheuvel wrote: > On 29 June 2016 at 12:50, Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > > On Wed, Jun 29, 2016 at 12:03:16PM +0200, Ard Biesheuvel wrote: > >> On 29 June 2016 at 11:39, Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > >> > On Tue, Jun 28, 2016 at 06:12:22PM +0200, Ard Biesheuvel wrote: > >> >> On 28 June 2016 at 18:05, Catalin Marinas <catalin.marinas@xxxxxxx> wrote: > >> >> > On Mon, Jun 06, 2016 at 11:18:14PM +0200, Ard Biesheuvel wrote: > >> >> >> 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. > >> >> > >> >> >> 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. > > > >> 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). > > 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 -- 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
