From: Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> To: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org Cc: leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org Subject: Re: [PATCH 1/2] arm64/efi: base UEFI mapping permissions on region attributes Date: Tue, 30 Jun 2015 10:50:48 -0400 [thread overview] Message-ID: <1435675848.21009.10.camel@redhat.com> (raw) In-Reply-To: <1435659443-17625-2-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> On Tue, 2015-06-30 at 12:17 +0200, Ard Biesheuvel wrote: > Currently, we infer the UEFI memory region mapping permissions > from the memory region type (i.e., runtime services code are > mapped RWX and runtime services data mapped RW-). This appears to > work fine but is not entirely UEFI spec compliant. So instead, use > the designated permission attributes to decide how these regions > should be mapped. > > Since UEFIv2.5 introduces a new EFI_MEMORY_RO permission attribute, > and redefines EFI_MEMORY_WP as a cacheability attribute, use only > the former as a read-only attribute. For setting the PXN bit, the > corresponding EFI_MEMORY_XP attribute is used. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > arch/arm64/kernel/efi.c | 32 +++++++++++++------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index ab21e0d58278..5dcab58d5d30 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -247,20 +247,30 @@ static bool __init efi_virtmap_init(void) > memrange_efi_to_native(&paddr, &npages); > size = npages << PAGE_SHIFT; > > - pr_info(" EFI remap 0x%016llx => %p\n", > - md->phys_addr, (void *)md->virt_addr); > - > - /* > - * Only regions of type EFI_RUNTIME_SERVICES_CODE need to be > - * executable, everything else can be mapped with the XN bits > - * set. > - */ > if (!is_normal_ram(md)) > prot = __pgprot(PROT_DEVICE_nGnRE); > - else if (md->type == EFI_RUNTIME_SERVICES_CODE) > - prot = PAGE_KERNEL_EXEC; > else > - prot = PAGE_KERNEL; > + prot = PAGE_KERNEL_EXEC; > + > + /* > + * On 64 KB granule kernels, only use strict permissions when > + * the region does not share a 64 KB page frame with another > + * region at either end. > + */ > + if (!IS_ENABLED(CONFIG_ARM64_64K_PAGES) || > + !(md->virt_addr % PAGE_SIZE || > + (md->phys_addr + md->num_pages * EFI_PAGE_SIZE) % PAGE_SIZE)) { I think this would read easier with: (PAGE_ALIGNED(md->virt_addr) && PAGE_ALIGNED(md->phys_addr + md->num_pages * EFI_PAGE_SIZE))) { > + > + if (md->attribute & EFI_MEMORY_RO) > + prot |= __pgprot(PTE_RDONLY); > + if (md->attribute & EFI_MEMORY_XP) > + prot |= __pgprot(PTE_PXN); > + } > + > + pr_info(" EFI remap 0x%016llx => %p (R%c%c)\n", > + md->phys_addr, (void *)md->virt_addr, > + prot & __pgprot(PTE_RDONLY) ? '-' : 'W', > + prot & __pgprot(PTE_PXN) ? '-' : 'X'); You can't maninulate pgprot_t directly like that. It will break STRICT_MM_TYPECHECKS. You need to use __pgprot_modify() and/or pgprot_val(). arch/arm64/kernel/efi.c: In function ‘efi_virtmap_init’: arch/arm64/kernel/efi.c:266:10: error: invalid operands to binary | (have ‘pgprot_t’ and ‘pgprot_t’) prot |= __pgprot(PTE_RDONLY); ^ ... (In trying that, I see there are a number of other places which need some STRICT_MM_TYPECHECKS fixing) > > create_pgd_mapping(&efi_mm, paddr, md->virt_addr, size, prot); > }
WARNING: multiple messages have this Message-ID (diff)
From: msalter@redhat.com (Mark Salter) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 1/2] arm64/efi: base UEFI mapping permissions on region attributes Date: Tue, 30 Jun 2015 10:50:48 -0400 [thread overview] Message-ID: <1435675848.21009.10.camel@redhat.com> (raw) In-Reply-To: <1435659443-17625-2-git-send-email-ard.biesheuvel@linaro.org> On Tue, 2015-06-30 at 12:17 +0200, Ard Biesheuvel wrote: > Currently, we infer the UEFI memory region mapping permissions > from the memory region type (i.e., runtime services code are > mapped RWX and runtime services data mapped RW-). This appears to > work fine but is not entirely UEFI spec compliant. So instead, use > the designated permission attributes to decide how these regions > should be mapped. > > Since UEFIv2.5 introduces a new EFI_MEMORY_RO permission attribute, > and redefines EFI_MEMORY_WP as a cacheability attribute, use only > the former as a read-only attribute. For setting the PXN bit, the > corresponding EFI_MEMORY_XP attribute is used. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > arch/arm64/kernel/efi.c | 32 +++++++++++++------- > 1 file changed, 21 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c > index ab21e0d58278..5dcab58d5d30 100644 > --- a/arch/arm64/kernel/efi.c > +++ b/arch/arm64/kernel/efi.c > @@ -247,20 +247,30 @@ static bool __init efi_virtmap_init(void) > memrange_efi_to_native(&paddr, &npages); > size = npages << PAGE_SHIFT; > > - pr_info(" EFI remap 0x%016llx => %p\n", > - md->phys_addr, (void *)md->virt_addr); > - > - /* > - * Only regions of type EFI_RUNTIME_SERVICES_CODE need to be > - * executable, everything else can be mapped with the XN bits > - * set. > - */ > if (!is_normal_ram(md)) > prot = __pgprot(PROT_DEVICE_nGnRE); > - else if (md->type == EFI_RUNTIME_SERVICES_CODE) > - prot = PAGE_KERNEL_EXEC; > else > - prot = PAGE_KERNEL; > + prot = PAGE_KERNEL_EXEC; > + > + /* > + * On 64 KB granule kernels, only use strict permissions when > + * the region does not share a 64 KB page frame with another > + * region at either end. > + */ > + if (!IS_ENABLED(CONFIG_ARM64_64K_PAGES) || > + !(md->virt_addr % PAGE_SIZE || > + (md->phys_addr + md->num_pages * EFI_PAGE_SIZE) % PAGE_SIZE)) { I think this would read easier with: (PAGE_ALIGNED(md->virt_addr) && PAGE_ALIGNED(md->phys_addr + md->num_pages * EFI_PAGE_SIZE))) { > + > + if (md->attribute & EFI_MEMORY_RO) > + prot |= __pgprot(PTE_RDONLY); > + if (md->attribute & EFI_MEMORY_XP) > + prot |= __pgprot(PTE_PXN); > + } > + > + pr_info(" EFI remap 0x%016llx => %p (R%c%c)\n", > + md->phys_addr, (void *)md->virt_addr, > + prot & __pgprot(PTE_RDONLY) ? '-' : 'W', > + prot & __pgprot(PTE_PXN) ? '-' : 'X'); You can't maninulate pgprot_t directly like that. It will break STRICT_MM_TYPECHECKS. You need to use __pgprot_modify() and/or pgprot_val(). arch/arm64/kernel/efi.c: In function ?efi_virtmap_init?: arch/arm64/kernel/efi.c:266:10: error: invalid operands to binary | (have ?pgprot_t? and ?pgprot_t?) prot |= __pgprot(PTE_RDONLY); ^ ... (In trying that, I see there are a number of other places which need some STRICT_MM_TYPECHECKS fixing) > > create_pgd_mapping(&efi_mm, paddr, md->virt_addr, size, prot); > }
next prev parent reply other threads:[~2015-06-30 14:50 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-06-30 10:17 [PATCH 0/2] arm64/efi: adapt to UEFI 2.5 properties table changes Ard Biesheuvel 2015-06-30 10:17 ` Ard Biesheuvel [not found] ` <1435659443-17625-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2015-06-30 10:17 ` [PATCH 1/2] arm64/efi: base UEFI mapping permissions on region attributes Ard Biesheuvel 2015-06-30 10:17 ` Ard Biesheuvel [not found] ` <1435659443-17625-2-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2015-06-30 14:50 ` Mark Salter [this message] 2015-06-30 14:50 ` Mark Salter [not found] ` <1435675848.21009.10.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2015-06-30 14:53 ` Ard Biesheuvel 2015-06-30 14:53 ` Ard Biesheuvel 2015-06-30 10:17 ` [PATCH 2/2] arm64/efi: don't pad between EFI_MEMORY_RUNTIME regions Ard Biesheuvel 2015-06-30 10:17 ` Ard Biesheuvel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1435675848.21009.10.camel@redhat.com \ --to=msalter-h+wxahxf7alqt0dzr+alfa@public.gmane.org \ --cc=ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ --cc=lersek-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \ --cc=roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.