Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1576690 > unrolled thread
| Started by | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| First post | 2017-02-08 17:40 +0100 |
| Last post | 2017-02-08 18:50 +0100 |
| Articles | 7 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-08 17:40 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Timur Tabi <timur@codeaurora.org> - 2017-02-08 17:40 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Mark Rutland <mark.rutland@arm.com> - 2017-02-08 18:10 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Jeffrey Hugo <jhugo@codeaurora.org> - 2017-02-08 18:30 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Mark Rutland <mark.rutland@arm.com> - 2017-02-08 18:40 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-02-08 18:50 +0100
Re: [PATCH v3 06/10] arm64: efi: add EFI stub Jeffrey Hugo <jhugo@codeaurora.org> - 2017-02-08 18:50 +0100
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2017-02-08 17:40 +0100 |
| Subject | Re: [PATCH v3 06/10] arm64: efi: add EFI stub |
| Message-ID | <t8DrH-2DS-7@gated-at.bofh.it> |
> On 8 Feb 2017, at 16:28, Timur Tabi <timur@codeaurora.org> wrote:
>
>> On Fri, Apr 4, 2014 at 1:45 PM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>>
>> +/*
>> + * EFI entry point for the arm/arm64 EFI stubs. This is the entrypoint
>> + * that is described in the PE/COFF header. Most of the code is the same
>> + * for both archictectures, with the arch-specific code provided in the
>> + * handle_kernel_image() function.
>> + */
>> +unsigned long __init efi_entry(void *handle, efi_system_table_t *sys_table,
>> + unsigned long *image_addr)
>> +{
>
> ...
>
>> +
>> + status = handle_cmdline_files(sys_table, image, cmdline_ptr,
>> + "initrd=", dram_base + SZ_512M,
>> + (unsigned long *)&initrd_addr,
>> + (unsigned long *)&initrd_size);
>
> So I know this patch is almost three years old, but why is there a
> 512M limit on the initrd size?
>
How do you reckon this constitutes a limit?
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.
[toc] | [next] | [standalone]
| From | Timur Tabi <timur@codeaurora.org> |
|---|---|
| Date | 2017-02-08 17:40 +0100 |
| Message-ID | <t8DrH-2DS-3@gated-at.bofh.it> |
| In reply to | #1576690 |
On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: >>> >> + status = handle_cmdline_files(sys_table, image, cmdline_ptr, >>> >> + "initrd=", dram_base + SZ_512M, >>> >> + (unsigned long *)&initrd_addr, >>> >> + (unsigned long *)&initrd_size); >> > >> > So I know this patch is almost three years old, but why is there a >> > 512M limit on the initrd size? >> > > How do you reckon this constitutes a limit? handle_cmdline_files() calls efi_high_alloc() with that limit. I'm still trying to understand all the details myself, but apparently our firmware and initrd need to fit within the first 512MB because of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to "~0", everything works. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-02-08 18:10 +0100 |
| Message-ID | <t8DUJ-33r-15@gated-at.bofh.it> |
| In reply to | #1576691 |
On Wed, Feb 08, 2017 at 10:35:02AM -0600, Timur Tabi wrote: > On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: > >>>>> + status = handle_cmdline_files(sys_table, image, cmdline_ptr, > >>>>> + "initrd=", dram_base + SZ_512M, > >>>>> + (unsigned long *)&initrd_addr, > >>>>> + (unsigned long *)&initrd_size); > >>> > >>> So I know this patch is almost three years old, but why is there a > >>> 512M limit on the initrd size? > >>> > >How do you reckon this constitutes a limit? > > handle_cmdline_files() calls efi_high_alloc() with that limit. I'm > still trying to understand all the details myself, but apparently > our firmware and initrd need to fit within the first 512MB because > of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to > "~0", everything works. Just to check, how big is that initrd? I guess it's possible that there simply isn't sufficient contiguous free memory in that range, even if the initrd isn't that large. Can you share the EFI memory map dump from booting with efi=debug? We originally needed to restrict this to ensure that the kernel could map the initrd (and I think the 512M restriction specifically was inherited from the DTB mapping restriction). Since then, we have relaxed things in the kernel, and today Documentation/arm64/booting.txt says: If an initrd/initramfs is passed to the kernel at boot, it must reside entirely within a 1 GB aligned physical memory window of up to 32 GB in size that fully covers the kernel Image as well. ... so I think the EFI stub should be able to take advantage of that relaxation. Ard? Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Jeffrey Hugo <jhugo@codeaurora.org> |
|---|---|
| Date | 2017-02-08 18:30 +0100 |
| Message-ID | <t8Ee7-3aO-57@gated-at.bofh.it> |
| In reply to | #1576724 |
On 2/8/2017 10:03 AM, Mark Rutland wrote: > On Wed, Feb 08, 2017 at 10:35:02AM -0600, Timur Tabi wrote: >> On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: >>>>>>> + status = handle_cmdline_files(sys_table, image, cmdline_ptr, >>>>>>> + "initrd=", dram_base + SZ_512M, >>>>>>> + (unsigned long *)&initrd_addr, >>>>>>> + (unsigned long *)&initrd_size); >>>>> >>>>> So I know this patch is almost three years old, but why is there a >>>>> 512M limit on the initrd size? >>>>> >>> How do you reckon this constitutes a limit? >> >> handle_cmdline_files() calls efi_high_alloc() with that limit. I'm >> still trying to understand all the details myself, but apparently >> our firmware and initrd need to fit within the first 512MB because >> of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to >> "~0", everything works. > > Just to check, how big is that initrd? 120MB right now. Probably could be optimized, but I don't think that's really the issue here. The big problem is we don't have much memory free as the platform requires that all of firmware exists in the first 512MB. On systems with a lot of devices (ie 16+ STAT drives), UEFI ends up doing a lot of allocations, which pushes the platform over the edge due to the limited memory and fragmentation of what is available. > > I guess it's possible that there simply isn't sufficient contiguous free > memory in that range, even if the initrd isn't that large. Can you share > the EFI memory map dump from booting with efi=debug? > > We originally needed to restrict this to ensure that the kernel could > map the initrd (and I think the 512M restriction specifically was > inherited from the DTB mapping restriction). Since then, we have relaxed > things in the kernel, and today Documentation/arm64/booting.txt says: > > If an initrd/initramfs is passed to the kernel at boot, it must > reside entirely within a 1 GB aligned physical memory window of > up to 32 GB in size that fully covers the kernel Image as well. > > ... so I think the EFI stub should be able to take advantage of that > relaxation. > > Ard? > > Thanks, > Mark. > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-02-08 18:40 +0100 |
| Message-ID | <t8EnM-3ey-11@gated-at.bofh.it> |
| In reply to | #1576724 |
On Wed, Feb 08, 2017 at 10:30:37AM -0700, Jeffrey Hugo wrote: > On 2/8/2017 10:03 AM, Mark Rutland wrote: > >On Wed, Feb 08, 2017 at 10:35:02AM -0600, Timur Tabi wrote: > >>On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: > >>>>>>>+ status = handle_cmdline_files(sys_table, image, cmdline_ptr, > >>>>>>>+ "initrd=", dram_base + SZ_512M, > >>>>>>>+ (unsigned long *)&initrd_addr, > >>>>>>>+ (unsigned long *)&initrd_size); > >>>>> > >>>>>So I know this patch is almost three years old, but why is there a > >>>>>512M limit on the initrd size? > >>>>> > >>>How do you reckon this constitutes a limit? > >> > >>handle_cmdline_files() calls efi_high_alloc() with that limit. I'm > >>still trying to understand all the details myself, but apparently > >>our firmware and initrd need to fit within the first 512MB because > >>of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to > >>"~0", everything works. > > > >Just to check, how big is that initrd? > > > >I guess it's possible that there simply isn't sufficient contiguous free > >memory in that range, even if the initrd isn't that large. Can you share > >the EFI memory map dump from booting with efi=debug? > > > >We originally needed to restrict this to ensure that the kernel could > >map the initrd (and I think the 512M restriction specifically was > >inherited from the DTB mapping restriction). Since then, we have relaxed > >things in the kernel, and today Documentation/arm64/booting.txt says: > > > > If an initrd/initramfs is passed to the kernel at boot, it must > > reside entirely within a 1 GB aligned physical memory window of > > up to 32 GB in size that fully covers the kernel Image as well. > > > >... so I think the EFI stub should be able to take advantage of that > >relaxation. > > I agree. The wrinkle I can see in this is it looks like KASLR can > put the kernel anywhere in RAM. How do we ensure initrd is within > 32GB of the kernel on a system with 256 GB of RAM? The EFI stub chose the physical location of the kernel, and should know where it put it. The virtual location of the kernel shouldn't matter. At some point though, this does become best-effort, unless we want a SAT solver in the stub. Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Ard Biesheuvel <ard.biesheuvel@linaro.org> |
|---|---|
| Date | 2017-02-08 18:50 +0100 |
| Message-ID | <t8Exs-3hZ-11@gated-at.bofh.it> |
| In reply to | #1576724 |
> On 8 Feb 2017, at 17:03, Mark Rutland <mark.rutland@arm.com> wrote: > >> On Wed, Feb 08, 2017 at 10:35:02AM -0600, Timur Tabi wrote: >> On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: >>>>>>> + status = handle_cmdline_files(sys_table, image, cmdline_ptr, >>>>>>> + "initrd=", dram_base + SZ_512M, >>>>>>> + (unsigned long *)&initrd_addr, >>>>>>> + (unsigned long *)&initrd_size); >>>>> >>>>> So I know this patch is almost three years old, but why is there a >>>>> 512M limit on the initrd size? >>>>> >>> How do you reckon this constitutes a limit? >> >> handle_cmdline_files() calls efi_high_alloc() with that limit. I'm >> still trying to understand all the details myself, but apparently >> our firmware and initrd need to fit within the first 512MB because >> of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to >> "~0", everything works. > > Just to check, how big is that initrd? > > I guess it's possible that there simply isn't sufficient contiguous free > memory in that range, even if the initrd isn't that large. Can you share > the EFI memory map dump from booting with efi=debug? > > We originally needed to restrict this to ensure that the kernel could > map the initrd (and I think the 512M restriction specifically was > inherited from the DTB mapping restriction). Since then, we have relaxed > things in the kernel, and today Documentation/arm64/booting.txt says: > > If an initrd/initramfs is passed to the kernel at boot, it must > reside entirely within a 1 GB aligned physical memory window of > up to 32 GB in size that fully covers the kernel Image as well. > > ... so I think the EFI stub should be able to take advantage of that > relaxation. > > Ard? > Interestingly enough, this code originates on 32-bit ARM, where the linear mapping is only 800 MB so I suspect that may have something to do with it. I agree the stub should simply follow the rules laid out in booting.txt. I think nobody hit this because it is usually GRUB that loads the initrd not the EFI stub
[toc] | [prev] | [next] | [standalone]
| From | Jeffrey Hugo <jhugo@codeaurora.org> |
|---|---|
| Date | 2017-02-08 18:50 +0100 |
| Message-ID | <t8EnM-3ey-13@gated-at.bofh.it> |
| In reply to | #1576724 |
On 2/8/2017 10:03 AM, Mark Rutland wrote: > On Wed, Feb 08, 2017 at 10:35:02AM -0600, Timur Tabi wrote: >> On 02/08/2017 10:29 AM, Ard Biesheuvel wrote: >>>>>>> + status = handle_cmdline_files(sys_table, image, cmdline_ptr, >>>>>>> + "initrd=", dram_base + SZ_512M, >>>>>>> + (unsigned long *)&initrd_addr, >>>>>>> + (unsigned long *)&initrd_size); >>>>> >>>>> So I know this patch is almost three years old, but why is there a >>>>> 512M limit on the initrd size? >>>>> >>> How do you reckon this constitutes a limit? >> >> handle_cmdline_files() calls efi_high_alloc() with that limit. I'm >> still trying to understand all the details myself, but apparently >> our firmware and initrd need to fit within the first 512MB because >> of dram_base + SZ_512M. When we change "dram_base + SZ_512M" to >> "~0", everything works. > > Just to check, how big is that initrd? > > I guess it's possible that there simply isn't sufficient contiguous free > memory in that range, even if the initrd isn't that large. Can you share > the EFI memory map dump from booting with efi=debug? > > We originally needed to restrict this to ensure that the kernel could > map the initrd (and I think the 512M restriction specifically was > inherited from the DTB mapping restriction). Since then, we have relaxed > things in the kernel, and today Documentation/arm64/booting.txt says: > > If an initrd/initramfs is passed to the kernel at boot, it must > reside entirely within a 1 GB aligned physical memory window of > up to 32 GB in size that fully covers the kernel Image as well. > > ... so I think the EFI stub should be able to take advantage of that > relaxation. I agree. The wrinkle I can see in this is it looks like KASLR can put the kernel anywhere in RAM. How do we ensure initrd is within 32GB of the kernel on a system with 256 GB of RAM? > > Ard? > > Thanks, > Mark. > -- > To unsubscribe from this list: send the line "unsubscribe linux-efi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web