| Message ID | 1467204690-10790-4-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 74c102c988cd48fff8055a0bfb84234fd3509419 |
| Headers | show |
On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: > + if (!PAGE_ALIGNED(md->phys_addr) || > + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { > + /* > + * If the end address of this region is not aligned to page > + * size, the mapping is rounded up, and may end up sharing a > + * page frame with the next UEFI memory region. If we create > + * a block entry now, we may need to split it again when mapping > + * the next region, and support for that is going to be removed > + * from the MMU routines. So avoid block mappings altogether in > + * that case. > + */ > + allow_block_mappings = false; > + } How common is it for large areas to have unaligned start/end? I wonder whether it's worth implementing my approach to look ahead and explicitly check the overlap with the next section instead of disabling block mappings altogether for this region. -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: >> + if (!PAGE_ALIGNED(md->phys_addr) || >> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { >> + /* >> + * If the end address of this region is not aligned to page >> + * size, the mapping is rounded up, and may end up sharing a >> + * page frame with the next UEFI memory region. If we create >> + * a block entry now, we may need to split it again when mapping >> + * the next region, and support for that is going to be removed >> + * from the MMU routines. So avoid block mappings altogether in >> + * that case. >> + */ >> + allow_block_mappings = false; >> + } > > How common is it for large areas to have unaligned start/end? I wonder > whether it's worth implementing my approach to look ahead and explicitly > check the overlap with the next section instead of disabling block > mappings altogether for this region. > Very uncommon. Typically, only MMIO regions that represent NOR flash are larger than a couple of pages. Taken from QEMU: _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: >>> + if (!PAGE_ALIGNED(md->phys_addr) || >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { >>> + /* >>> + * If the end address of this region is not aligned to page >>> + * size, the mapping is rounded up, and may end up sharing a >>> + * page frame with the next UEFI memory region. If we create >>> + * a block entry now, we may need to split it again when mapping >>> + * the next region, and support for that is going to be removed >>> + * from the MMU routines. So avoid block mappings altogether in >>> + * that case. >>> + */ >>> + allow_block_mappings = false; >>> + } >> >> How common is it for large areas to have unaligned start/end? I wonder >> whether it's worth implementing my approach to look ahead and explicitly >> check the overlap with the next section instead of disabling block >> mappings altogether for this region. >> > > Very uncommon. Typically, only MMIO regions that represent NOR flash > are larger than a couple of pages. Taken from QEMU: RT_Code : 640 Pages (2,621,440 Bytes) RT_Data : 880 Pages (3,604,480 Bytes) so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data regions 3.5 MB. Ideally, they are grouped together, but in reality, there are always a couple of regions of each type, so there is little to gain here from using block mappings _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 29, 2016 at 06:53:18PM +0200, Ard Biesheuvel wrote: > On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: > >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: > >>> + if (!PAGE_ALIGNED(md->phys_addr) || > >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { > >>> + /* > >>> + * If the end address of this region is not aligned to page > >>> + * size, the mapping is rounded up, and may end up sharing a > >>> + * page frame with the next UEFI memory region. If we create > >>> + * a block entry now, we may need to split it again when mapping > >>> + * the next region, and support for that is going to be removed > >>> + * from the MMU routines. So avoid block mappings altogether in > >>> + * that case. > >>> + */ > >>> + allow_block_mappings = false; > >>> + } > >> > >> How common is it for large areas to have unaligned start/end? I wonder > >> whether it's worth implementing my approach to look ahead and explicitly > >> check the overlap with the next section instead of disabling block > >> mappings altogether for this region. > >> > > > > Very uncommon. Typically, only MMIO regions that represent NOR flash > > are larger than a couple of pages. Taken from QEMU: > > RT_Code : 640 Pages (2,621,440 Bytes) > RT_Data : 880 Pages (3,604,480 Bytes) > > so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data > regions 3.5 MB. Ideally, they are grouped together, but in reality, > there are always a couple of regions of each type, so there is little > to gain here from using block mappings Is this representative for real platforms? What about efifb and reserved regions? My (x86) Lenovo workstation has one 64MB and one 16MB Runtime/Uncached MMIO region. As well as a 3MB and a 4MB RT_Data one. / Leif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 19:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Wed, Jun 29, 2016 at 06:53:18PM +0200, Ard Biesheuvel wrote: >> On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: >> >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: >> >>> + if (!PAGE_ALIGNED(md->phys_addr) || >> >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { >> >>> + /* >> >>> + * If the end address of this region is not aligned to page >> >>> + * size, the mapping is rounded up, and may end up sharing a >> >>> + * page frame with the next UEFI memory region. If we create >> >>> + * a block entry now, we may need to split it again when mapping >> >>> + * the next region, and support for that is going to be removed >> >>> + * from the MMU routines. So avoid block mappings altogether in >> >>> + * that case. >> >>> + */ >> >>> + allow_block_mappings = false; >> >>> + } >> >> >> >> How common is it for large areas to have unaligned start/end? I wonder >> >> whether it's worth implementing my approach to look ahead and explicitly >> >> check the overlap with the next section instead of disabling block >> >> mappings altogether for this region. >> >> >> > >> > Very uncommon. Typically, only MMIO regions that represent NOR flash >> > are larger than a couple of pages. Taken from QEMU: >> >> RT_Code : 640 Pages (2,621,440 Bytes) >> RT_Data : 880 Pages (3,604,480 Bytes) >> >> so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data >> regions 3.5 MB. Ideally, they are grouped together, but in reality, >> there are always a couple of regions of each type, so there is little >> to gain here from using block mappings > > Is this representative for real platforms? > I think it is a reasonable ballpark figure > What about efifb and reserved regions? > Those are not tagged as EFI_MEMORY_RUNTIME so they are not covered by the UEFI runtime mappings, and not relevant to this discussion. > My (x86) Lenovo workstation has one 64MB and one 16MB Runtime/Uncached > MMIO region. As well as a 3MB and a 4MB RT_Data one. > Are those MMIO regions naturally aligned? And how about the RT_Data ones? _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 19:04, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 29 June 2016 at 19:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> On Wed, Jun 29, 2016 at 06:53:18PM +0200, Ard Biesheuvel wrote: >>> On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>> > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: >>> >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: >>> >>> + if (!PAGE_ALIGNED(md->phys_addr) || >>> >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { >>> >>> + /* >>> >>> + * If the end address of this region is not aligned to page >>> >>> + * size, the mapping is rounded up, and may end up sharing a >>> >>> + * page frame with the next UEFI memory region. If we create >>> >>> + * a block entry now, we may need to split it again when mapping >>> >>> + * the next region, and support for that is going to be removed >>> >>> + * from the MMU routines. So avoid block mappings altogether in >>> >>> + * that case. >>> >>> + */ >>> >>> + allow_block_mappings = false; >>> >>> + } >>> >> >>> >> How common is it for large areas to have unaligned start/end? I wonder >>> >> whether it's worth implementing my approach to look ahead and explicitly >>> >> check the overlap with the next section instead of disabling block >>> >> mappings altogether for this region. >>> >> >>> > >>> > Very uncommon. Typically, only MMIO regions that represent NOR flash >>> > are larger than a couple of pages. Taken from QEMU: >>> >>> RT_Code : 640 Pages (2,621,440 Bytes) >>> RT_Data : 880 Pages (3,604,480 Bytes) >>> >>> so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data >>> regions 3.5 MB. Ideally, they are grouped together, but in reality, >>> there are always a couple of regions of each type, so there is little >>> to gain here from using block mappings >> >> Is this representative for real platforms? >> > > I think it is a reasonable ballpark figure > >> What about efifb and reserved regions? >> > > Those are not tagged as EFI_MEMORY_RUNTIME so they are not covered by > the UEFI runtime mappings, and not relevant to this discussion. > >> My (x86) Lenovo workstation has one 64MB and one 16MB Runtime/Uncached >> MMIO region. As well as a 3MB and a 4MB RT_Data one. >> > > Are those MMIO regions naturally aligned? And how about the RT_Data ones? Just to be clear: we are talking about regions that a) are larger than 2 MB b) whose naturally aligned 2 MB subregions are relatively aligned with their physical mapping c) share a 16 KB or 64 KB page frame at either end with an adjacent region So first of all, the virtual mapping code in the stub does take the 2 MB alignment into account, but only if the physical base of the region is aligned to 2 MB and the size of the region is at least 2 MB. This is intended for things like the NOR flash mapping (or the MMIO regions that Leif refers to). But a region that happens to exceed 2 MB is not mapped with the relative physical alignment in mind, and usually does not end up aligned correctly for block mappings to be usable. On top of that, the v2.6 Memory Attributes table feature (which I think should be recommended in all cases, but is optional in the spec) breaks RT_Code regions into read-only and non-exec slices, so even if a RT_Code region existed whose size and alignment would happen to allow a block mapping to be used, it will subsequently be split anyway. The bottom line is that I don't think it makes a huge amount of sense to go out of our way to deal with large regions with unaligned boundaries until we encounter any in real life. -- Ard. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 29, 2016 at 07:04:57PM +0200, Ard Biesheuvel wrote: > On 29 June 2016 at 19:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: > > On Wed, Jun 29, 2016 at 06:53:18PM +0200, Ard Biesheuvel wrote: > >> On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: > >> >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: > >> >>> + if (!PAGE_ALIGNED(md->phys_addr) || > >> >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { > >> >>> + /* > >> >>> + * If the end address of this region is not aligned to page > >> >>> + * size, the mapping is rounded up, and may end up sharing a > >> >>> + * page frame with the next UEFI memory region. If we create > >> >>> + * a block entry now, we may need to split it again when mapping > >> >>> + * the next region, and support for that is going to be removed > >> >>> + * from the MMU routines. So avoid block mappings altogether in > >> >>> + * that case. > >> >>> + */ > >> >>> + allow_block_mappings = false; > >> >>> + } > >> >> > >> >> How common is it for large areas to have unaligned start/end? I wonder > >> >> whether it's worth implementing my approach to look ahead and explicitly > >> >> check the overlap with the next section instead of disabling block > >> >> mappings altogether for this region. > >> >> > >> > > >> > Very uncommon. Typically, only MMIO regions that represent NOR flash > >> > are larger than a couple of pages. Taken from QEMU: > >> > >> RT_Code : 640 Pages (2,621,440 Bytes) > >> RT_Data : 880 Pages (3,604,480 Bytes) > >> > >> so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data > >> regions 3.5 MB. Ideally, they are grouped together, but in reality, > >> there are always a couple of regions of each type, so there is little > >> to gain here from using block mappings > > > > Is this representative for real platforms? > > I think it is a reasonable ballpark figure > > > What about efifb and reserved regions? > > Those are not tagged as EFI_MEMORY_RUNTIME so they are not covered by > the UEFI runtime mappings, and not relevant to this discussion. OK. > > My (x86) Lenovo workstation has one 64MB and one 16MB Runtime/Uncached > > MMIO region. As well as a 3MB and a 4MB RT_Data one. > > Are those MMIO regions naturally aligned? And how about the RT_Data ones? So, I've now gone home and don't have access to the Lenovo, however I have a machine at home also with an AMI UEFI implementation, and identical MMIO regions. And they do look naturally aligned. The RT_Data ones are not naturally aligned. / Leif _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 29 June 2016 at 19:54, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Wed, Jun 29, 2016 at 07:04:57PM +0200, Ard Biesheuvel wrote: >> On 29 June 2016 at 19:00, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> > On Wed, Jun 29, 2016 at 06:53:18PM +0200, Ard Biesheuvel wrote: >> >> On 29 June 2016 at 18:50, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >> >> > On 29 June 2016 at 18:45, Catalin Marinas <catalin.marinas@arm.com> wrote: >> >> >> On Wed, Jun 29, 2016 at 02:51:28PM +0200, Ard Biesheuvel wrote: >> >> >>> + if (!PAGE_ALIGNED(md->phys_addr) || >> >> >>> + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { >> >> >>> + /* >> >> >>> + * If the end address of this region is not aligned to page >> >> >>> + * size, the mapping is rounded up, and may end up sharing a >> >> >>> + * page frame with the next UEFI memory region. If we create >> >> >>> + * a block entry now, we may need to split it again when mapping >> >> >>> + * the next region, and support for that is going to be removed >> >> >>> + * from the MMU routines. So avoid block mappings altogether in >> >> >>> + * that case. >> >> >>> + */ >> >> >>> + allow_block_mappings = false; >> >> >>> + } >> >> >> >> >> >> How common is it for large areas to have unaligned start/end? I wonder >> >> >> whether it's worth implementing my approach to look ahead and explicitly >> >> >> check the overlap with the next section instead of disabling block >> >> >> mappings altogether for this region. >> >> >> >> >> > >> >> > Very uncommon. Typically, only MMIO regions that represent NOR flash >> >> > are larger than a couple of pages. Taken from QEMU: >> >> >> >> RT_Code : 640 Pages (2,621,440 Bytes) >> >> RT_Data : 880 Pages (3,604,480 Bytes) >> >> >> >> so all RT_Code regions *combined* are 2.5 MB in total, and all RT_Data >> >> regions 3.5 MB. Ideally, they are grouped together, but in reality, >> >> there are always a couple of regions of each type, so there is little >> >> to gain here from using block mappings >> > >> > Is this representative for real platforms? >> >> I think it is a reasonable ballpark figure >> >> > What about efifb and reserved regions? >> >> Those are not tagged as EFI_MEMORY_RUNTIME so they are not covered by >> the UEFI runtime mappings, and not relevant to this discussion. > > OK. > >> > My (x86) Lenovo workstation has one 64MB and one 16MB Runtime/Uncached >> > MMIO region. As well as a 3MB and a 4MB RT_Data one. >> >> Are those MMIO regions naturally aligned? And how about the RT_Data ones? > > So, I've now gone home and don't have access to the Lenovo, however I > have a machine at home also with an AMI UEFI implementation, and > identical MMIO regions. And they do look naturally aligned. > > The RT_Data ones are not naturally aligned. > OK, that confirms my suspicion. See other email _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c index 4aef89f37049..ba9bee389fd5 100644 --- a/arch/arm64/kernel/efi.c +++ b/arch/arm64/kernel/efi.c @@ -65,6 +65,20 @@ int __init efi_create_mapping(struct mm_struct *mm, efi_memory_desc_t *md) bool allow_block_mappings = (md->type != EFI_RUNTIME_SERVICES_CODE && md->type != EFI_RUNTIME_SERVICES_DATA); + if (!PAGE_ALIGNED(md->phys_addr) || + !PAGE_ALIGNED(md->num_pages << EFI_PAGE_SHIFT)) { + /* + * If the end address of this region is not aligned to page + * size, the mapping is rounded up, and may end up sharing a + * page frame with the next UEFI memory region. If we create + * a block entry now, we may need to split it again when mapping + * the next region, and support for that is going to be removed + * from the MMU routines. So avoid block mappings altogether in + * that case. + */ + allow_block_mappings = false; + } + create_pgd_mapping(mm, md->phys_addr, md->virt_addr, md->num_pages << EFI_PAGE_SHIFT, __pgprot(prot_val | PTE_NG), allow_block_mappings);
When running the OS with a page size > 4 KB, we need to round up mappings for regions that are not aligned to the OS's page size. We already avoid block mappings for EfiRuntimeServicesCode/Data regions for other reasons, but in the unlikely event that other unaliged regions exists that have the EFI_MEMORY_RUNTIME attribute set, ensure that unaligned regions are always mapped down to pages. This way, the overlapping page is guaranteed not to be covered by a block mapping that needs to be split. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm64/kernel/efi.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.7.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel