Re: [PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI
On 17 November 2015 at 12:37, Mark Rutland wrote:
> On Tue, Nov 17, 2015 at 12:25:58PM +0100, Ard Biesheuvel wrote:
>> On 17 November 2015 at 10:57, wrote:
>> > From: Shannon Zhao
>> >
>> > Add a new function to parse DT parameters for Xen specific UEFI just
>> > like the way for normal UEFI. Then it could reuse the existing codes.
>> >
>> > Signed-off-by: Shannon Zhao
>> > ---
>> > drivers/firmware/efi/efi.c | 67
>> > ++
>> > 1 file changed, 62 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
>> > index d6144e3..629bd06 100644
>> > --- a/drivers/firmware/efi/efi.c
>> > +++ b/drivers/firmware/efi/efi.c
>> > @@ -24,6 +24,7 @@
>> > #include
>> > #include
>> > #include
>> > +#include
>> >
>> > struct efi __read_mostly efi = {
>> > .mps= EFI_INVALID_TABLE_ADDR,
>> > @@ -488,12 +489,60 @@ static __initdata struct {
>> > UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver",
>> > desc_ver)
>> > };
>> >
>> > +static __initdata struct {
>> > + const char name[32];
>> > + const char propname[32];
>> > + int offset;
>> > + int size;
>> > +} xen_dt_params[] = {
>> > + UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
>> > + UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
>> > + UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
>> > + UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size",
>> > desc_size),
>> > + UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver",
>> > desc_ver)
>> > +};
>> > +
>>
>> We discussed (and agreed afair) that dropping the "linux," prefix from
>> the DT properties was an acceptable change. If we do that, and reuse
>> the same names in the xen version (i.e., drop the "xen," prefix), we
>> could make this change a *lot* simpler.
>
> Regardless of if we drop the "linux," prefix from the existing strings,
> I think we need the "xen," prefix here.
>
> The xen EFI interface comes with additional caveats, and we need to
> treat it separately.
>
> Unless you're suggesting that /hypervisor/uefi-* is handled differently
> to /chosen/uefi-*?
>
> I think I'd still prefer the "xen," prefix regardless.
>
Well, we should still be able to reuse more of the existing code
rather than duplicating a bunch of code in fdt_find_xen_uefi_params()
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/13] ARM: Xen: Initialize Xen specific UEFI runtime services
On 17 November 2015 at 10:57, wrote:
> From: Shannon Zhao
>
> When running on Xen hypervisor, runtime services are supported through
> hypercall. So call Xen specific function to initialize runtime services.
>
> Signed-off-by: Shannon Zhao
> ---
> arch/arm/include/asm/xen/hypercall.h | 1 +
> arch/arm/xen/enlighten.c | 1 +
> arch/arm/xen/hypercall.S | 1 +
> arch/arm64/kernel/efi.c | 20 ++--
> arch/arm64/xen/hypercall.S | 1 +
> drivers/xen/Kconfig | 2 +-
> drivers/xen/efi.c| 22 ++
> include/xen/xen-ops.h| 10 ++
> 8 files changed, 51 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/hypercall.h
> b/arch/arm/include/asm/xen/hypercall.h
> index 712b50e..0de6074 100644
> --- a/arch/arm/include/asm/xen/hypercall.h
> +++ b/arch/arm/include/asm/xen/hypercall.h
> @@ -50,6 +50,7 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
> int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
> int HYPERVISOR_tmem_op(void *arg);
> int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
> +int HYPERVISOR_dom0_op(void *arg);
>
> static inline int
> HYPERVISOR_suspend(unsigned long start_info_mfn)
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 7cb82f7..1373d6d 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -289,3 +289,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
> EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
> EXPORT_SYMBOL_GPL(HYPERVISOR_multicall);
> EXPORT_SYMBOL_GPL(privcmd_call);
> +EXPORT_SYMBOL_GPL(HYPERVISOR_dom0_op);
> diff --git a/arch/arm/xen/hypercall.S b/arch/arm/xen/hypercall.S
> index 10fd99c..16fc153 100644
> --- a/arch/arm/xen/hypercall.S
> +++ b/arch/arm/xen/hypercall.S
> @@ -90,6 +90,7 @@ HYPERCALL2(physdev_op);
> HYPERCALL3(vcpu_op);
> HYPERCALL1(tmem_op);
> HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>
> ENTRY(privcmd_call)
> stmdb sp!, {r4}
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index 13671a9..ab1c9e9 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -34,6 +34,9 @@
> #include
> #include
>
> +#include
> +#include
> +
> struct efi_memory_map memmap;
>
> static u64 efi_system_table;
> @@ -308,13 +311,18 @@ static int __init arm64_enable_runtime_services(void)
> }
> set_bit(EFI_SYSTEM_TABLES, &efi.flags);
>
> - if (!efi_virtmap_init()) {
> - pr_err("No UEFI virtual mapping was installed -- runtime
> services will not be available\n");
> - return -1;
> - }
> + if (!xen_initial_domain()) {
> + if (!efi_virtmap_init()) {
> + pr_err("No UEFI virtual mapping was installed --
> runtime services will not be available\n");
> + return -1;
> + }
>
I'd prefer it if we could separate the code logically, rather than
putting xen_initial_domain() tests in all the code paths.
For instance, we could re-use the EFI_PARAVIRT flag, and set if from
the Xen init code. Then, we could simply test it here, and bail early.
That way, you can have a Xen specific alternative (which does not use
the virtmap etc anyway) in a xen source file.
> - /* Set up runtime services function pointers */
> - efi_native_runtime_setup();
> + /* Set up runtime services function pointers */
> + efi_native_runtime_setup();
> + } else {
> + /* Set up runtime services function pointers for Xen UEFI */
> + xen_efi_runtime_setup();
> + }
> set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>
> efi.runtime_version = efi.systab->hdr.revision;
> diff --git a/arch/arm64/xen/hypercall.S b/arch/arm64/xen/hypercall.S
> index 8bbe940..f6e15aa 100644
> --- a/arch/arm64/xen/hypercall.S
> +++ b/arch/arm64/xen/hypercall.S
> @@ -81,6 +81,7 @@ HYPERCALL2(physdev_op);
> HYPERCALL3(vcpu_op);
> HYPERCALL1(tmem_op);
> HYPERCALL2(multicall);
> +HYPERCALL1(dom0_op);
>
> ENTRY(privcmd_call)
> mov x16, x0
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 73708ac..59aec8b 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -268,7 +268,7 @@ config XEN_HAVE_PVMMU
>
> config XEN_EFI
> def_bool y
> - depends on X86_64 && EFI
> + depends on ARM64 || X86_64 && EFI
>
> config XEN_AUTO_XLATE
> def_bool y
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index f745db2..5246d89 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -33,6 +33,7 @@
>
> #define INIT_EFI_OP(name) \
> {.cmd = XENPF_efi_runtime_call, \
> +.interface_version = XENPF_INTERFACE_VERSION, \
> .u.efi_runtime_call.function = XEN_EFI_##name, \
> .u.efi_runtime_call.misc = 0}
>
> @@ -261,6 +262,7 @@ static efi_status_t
> xen_efi_query_capsule_ca
Re: [PATCH 08/13] Xen: EFI: Parse DT parameters for Xen specific UEFI
On 17 November 2015 at 10:57, wrote:
> From: Shannon Zhao
>
> Add a new function to parse DT parameters for Xen specific UEFI just
> like the way for normal UEFI. Then it could reuse the existing codes.
>
> Signed-off-by: Shannon Zhao
> ---
> drivers/firmware/efi/efi.c | 67
> ++
> 1 file changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index d6144e3..629bd06 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> struct efi __read_mostly efi = {
> .mps= EFI_INVALID_TABLE_ADDR,
> @@ -488,12 +489,60 @@ static __initdata struct {
> UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver",
> desc_ver)
> };
>
> +static __initdata struct {
> + const char name[32];
> + const char propname[32];
> + int offset;
> + int size;
> +} xen_dt_params[] = {
> + UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> + UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> + UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> + UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> + UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> +};
> +
We discussed (and agreed afair) that dropping the "linux," prefix from
the DT properties was an acceptable change. If we do that, and reuse
the same names in the xen version (i.e., drop the "xen," prefix), we
could make this change a *lot* simpler.
> struct param_info {
> int verbose;
> int found;
> void *params;
> };
>
> +static int __init fdt_find_xen_uefi_params(unsigned long node,
> + const char *uname, int depth,
> + void *data)
> +{
> + struct param_info *info = data;
> + const void *prop;
> + void *dest;
> + u64 val;
> + int i, len;
> +
> + if (xen_initial_domain() && (depth != 2 || strcmp(uname, "uefi") !=
> 0))
> + return 0;
> +
> + for (i = 0; i < ARRAY_SIZE(xen_dt_params); i++) {
> + prop = of_get_flat_dt_prop(node, xen_dt_params[i].propname,
> + &len);
> + if (!prop)
> + return 0;
> + dest = info->params + xen_dt_params[i].offset;
> + info->found++;
> +
> + val = of_read_number(prop, len / sizeof(u32));
> +
> + if (dt_params[i].size == sizeof(u32))
> + *(u32 *)dest = val;
> + else
> + *(u64 *)dest = val;
> +
> + if (info->verbose)
> + pr_info(" %s: 0x%0*llx\n", xen_dt_params[i].name,
> + xen_dt_params[i].size * 2, val);
> + }
> +
> + return 1;
> +}
> static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
>int depth, void *data)
> {
> @@ -538,12 +587,20 @@ int __init efi_get_fdt_params(struct efi_fdt_params
> *params, int verbose)
> info.found = 0;
> info.params = params;
>
> - ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> - if (!info.found)
> + if (xen_initial_domain())
> + ret = of_scan_flat_dt(fdt_find_xen_uefi_params, &info);
> + else
> + ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> + if (!info.found) {
> pr_info("UEFI not found.\n");
> - else if (!ret)
> - pr_err("Can't find '%s' in device tree!\n",
> - dt_params[info.found].name);
> + } else if (!ret) {
> + if (xen_initial_domain())
> + pr_err("Can't find '%s' in device tree!\n",
> + xen_dt_params[info.found].name);
> + else
> + pr_err("Can't find '%s' in device tree!\n",
> + xen_dt_params[info.found].name);
Wrong array here
> + }
>
> return ret;
> }
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/13] xen: memory : Add new XENMAPSPACE type XENMAPSPACE_dev_mmio
On 17 November 2015 at 10:57, wrote: > From: Shannon Zhao > No empty commit logs please. > Signed-off-by: Shannon Zhao > --- > include/xen/interface/memory.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/xen/interface/memory.h b/include/xen/interface/memory.h > index 2ecfe4f..9aa8988 100644 > --- a/include/xen/interface/memory.h > +++ b/include/xen/interface/memory.h > @@ -160,6 +160,7 @@ DEFINE_GUEST_HANDLE_STRUCT(xen_machphys_mapping_t); > #define XENMAPSPACE_gmfn_foreign 4 /* GMFN from another dom, > * XENMEM_add_to_physmap_range only. > */ > +#define XENMAPSPACE_dev_mmio 5 /* device mmio region */ > > /* > * Sets the GPFN at which a particular page appears in the specified guest's > -- > 2.1.0 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On 14 September 2015 at 10:42, Shannon Zhao wrote:
[..]
>
> It only needs to apply following patch to fix a bug in Linux kernel when
> mapping EFI_MEMORY_RUNTIME memory.
>
Could you explain why you think efi_virtmap_init() should fail if
there are no EFI_MEMORY_RUNTIME regions?
The absence of such regions is allowed by the spec, so
efi_virtmap_init() is correct imo to return success.
If you are trying to work around the issue where Xen does not expose
any Runtime Services regions, there is simply no way to do that and be
still UEFI compliant. I have suggested before that we should perhaps
tolerate this anyway, by considering the case where the EFI System
Table has a NULL runtime services pointer. But rigging
efi_virtmap_init() like this is really not the way to achieve that.
--
Ard.
> Author: Shannon Zhao
> Date: Thu Aug 20 14:54:58 2015 +0800
>
> arm64/efi: Fix a bug when no EFI_MEMORY_RUNTIME memory found
>
> Currently if the attribute type of all the EFI Memory Descriptors are
> not EFI_MEMORY_RUNTIME, efi_virtmap_init will return true. But at this
> case, it expect false as there are no EFI memory for RUNTIME. Fix it by
> introducing a status to show whether it finds EFI_MEMORY_RUNTIME.
>
> Signed-off-by: Shannon Zhao
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e8ca6ea..bad7f87 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -233,6 +233,7 @@ void __init efi_init(void)
> static bool __init efi_virtmap_init(void)
> {
> efi_memory_desc_t *md;
> + bool status = false;
>
> for_each_efi_memory_desc(&memmap, md) {
> u64 paddr, npages, size;
> @@ -264,8 +265,11 @@ static bool __init efi_virtmap_init(void)
> prot = PAGE_KERNEL;
>
> create_pgd_mapping(&efi_mm, paddr, md->virt_addr, size,
> prot);
> + status = true;
> }
> - return true;
> + if (status)
> + return true;
> + return false;
> }
>
> --
> Shannon
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On 11 September 2015 at 15:14, Stefano Stabellini wrote: > On Fri, 11 Sep 2015, Daniel Kiper wrote: >> On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: >> > > > C) When you could go: >> > > > >> > > >DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI >> > > > discovery >> > > >> > > I take you mean discovering Xen with the usual Xen hypervisor node on >> > > device tree. I think that C) is a good option actually. I like it. Not >> > > sure why we didn't think about this earlier. Is there anything EFI or >> > > ACPI which is needed before Xen support is discovered by >> > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()? >> > >> > Currently lots (including the memory map). With the stuff to support >> > SPCR, the ACPI discovery would be moved before xen_early_init(). >> > >> > > If not, we could just go for this. A lot of complexity would go away. >> > >> > I suspect this would still be fairly complex, but would at least prevent >> > the Xen-specific EFI handling from adversely affecting the native case. >> > >> > > > D) If you want to be generic: >> > > >EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific >> > > > stuff >> > > > \--/ >> > > >(virtualize these, provide shims to Dom0, but handle >> > > > everything in Xen itself) >> > > >> > > I think that this is good in theory but could turn out to be a lot of >> > > work in practice. We could probably virtualize the RuntimeServices but >> > > the BootServices are troublesome. >> > >> > What's troublesome with the boot services? >> > >> > What can't be simulated? >> >> How do you want to access bare metal EFI boot services from dom0 if they >> were shutdown long time ago before loading dom0 image? What do you need >> from EFI boot services in dom0? > > That's right. Trying to emulate BootServices after the real > ExitBootServices has already been called seems like a very bad plan. > > I think that whatever interface we come up with, would need to be past > ExitBootServices. It feels like this discussion is going in circles. When we discussed this six months ago, we already concluded that, since UEFI is the only specified way that the presence of ACPI is advertised on an ARM system, we need to emulate UEFI to some extent. So we need the EFI system table to expose the UEFI configuration table that carries the ACPI root pointer. Since ACPI support also relies on the UEFI memory map (I think?), we need that as well. These two items are exactly what we pass via the UEFI DT properties, so we should indeed promote the current de-facto binding to a proper binding, and renaming the properties makes sense in that context. I agree that this should also include a description of the expected state of the firmware, i.e., that ExitBootServices() has been called, and that the memory map has been populated with virtual address, which have been installed using SetVirtualAddressMap() if they differ from the physical addresses. (The current implementation on the kernel side is perfectly capable of dealing with a 1:1 mapping). Beyond that, there is no point in pretending to be a full UEFI implementation, imo. Boot services are not required, nor are runtime services (only the current EFI init code on arm needs to be modified to deal with a NULL runtime services pointer) -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
On 25 August 2015 at 17:37, Leif Lindholm wrote: > On Tue, Aug 25, 2015 at 04:51:22PM +0200, Ard Biesheuvel wrote: >> >>Arm kernel should either fetch memory information from >> >>efi or DT. >> > >> > Absolutely. >> > >> >>Currently arm kernel fetch both efi memory information and >> >>reserved buffer from DTB at the same time. >> > >> > No, it does not. >> >> It should not, but it does. Due to an oversight, the stub removes >> /memreserve/ entries but ignores the reserved-memory node completely. > > Urgh. > >> This was reported here in fact >> >> http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742 >> >> but there has not been a followup to this series. > > Are all of those patches still relevant, or did some of them go in > already? > The first two patches are in v4.2-rc1 and up, the others should still apply on top of that. -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
On 25 August 2015 at 16:24, Leif Lindholm wrote: > On Tue, Aug 25, 2015 at 09:43:14PM +0800, Haojian Zhuang wrote: >> Since we discussed a lot on this, let's make a conclusion on it. >> >> 1. UEFI could append the reserved buffer in it's memory mapping. > > Yes. It needs to. > > (I will let Mark comment on points 2-4.) > >> 5. A patch is necessary in kernel. If efi stub feature is enabled, >>arm kernel should not parse memory node or reserved memory buffer in >>DT any more. > > This is already the case. The stub deletes any present memory nodes and > reserved entries in drivers/firmware/efi/libstub/fdt.c:update_fdt(). > > Then, during setup_arch(), arch/arm64/kernel/efi.c:efi_init() calls > reserve_regions(), which adds only those memory regions available for > use by Linux as RAM to memblock. > >>Arm kernel should either fetch memory information from >>efi or DT. > > Absolutely. > >>Currently arm kernel fetch both efi memory information and >>reserved buffer from DTB at the same time. > > No, it does not. > It should not, but it does. Due to an oversight, the stub removes /memreserve/ entries but ignores the reserved-memory node completely. This was reported here in fact http://thread.gmane.org/gmane.linux.kernel.efi/5736/focus=5742 but there has not been a followup to this series. I think it is fine to keep those memory reservations in the DT, but you should simply understand that UEFI does not parse the DT, so you need to tell it which memory it cannot touch. Otherwise, the firmware itself or anything that executes under it (UEFI drivers, the UEFI Shell, GRUB, the UEFI stub in the kernel) will think it is available and may allocate it for its own use. This may include runtime services regions that will remain reserved even after exiting boot services. -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] of/fdt: split off FDT self reservation from memreserve processing
On 12 March 2015 at 22:54, Rob Herring wrote:
> On Wed, Mar 11, 2015 at 11:00 AM, Ard Biesheuvel
> wrote:
>> This splits off the reservation of the memory occupied by the FDT
>> binary itself from the processing of the memory reservations it
>> contains. This is necessary because the physical address of the FDT,
>> which is needed to perform the reservation, may not be known to the
>> FDT driver core, i.e., it may be mapped outside the linear direct
>> mapping, in which case __pa() returns a bogus value.
>>
>> Cc: Benjamin Herrenschmidt
>> Cc: Paul Mackerras
>> Signed-off-by: Ard Biesheuvel
>
> Not really the direction I like going by moving more things back to
> arch code, but I haven't come up with any better suggestion so:
>
> Acked-by: Rob Herring
>
Thanks.
@Russell, Benjamin, Paul: do any of you perhaps have any issues with this?
Regards,
Ard.
>> ---
>> arch/arm/mm/init.c | 1 +
>> arch/arm64/mm/init.c | 1 +
>> arch/powerpc/kernel/prom.c | 1 +
>> drivers/of/fdt.c | 19 ++-
>> include/linux/of_fdt.h | 2 ++
>> 5 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
>> index 1609b022a72f..0b8657c36fe4 100644
>> --- a/arch/arm/mm/init.c
>> +++ b/arch/arm/mm/init.c
>> @@ -317,6 +317,7 @@ void __init arm_memblock_init(const struct machine_desc
>> *mdesc)
>> if (mdesc->reserve)
>> mdesc->reserve();
>>
>> + early_init_fdt_reserve_self();
>> early_init_fdt_scan_reserved_mem();
>>
>> /* reserve memory for DMA contiguous allocations */
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index ae85da6307bb..fa2389b0f7f0 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -170,6 +170,7 @@ void __init arm64_memblock_init(void)
>> memblock_reserve(__virt_to_phys(initrd_start), initrd_end -
>> initrd_start);
>> #endif
>>
>> + early_init_fdt_reserve_self();
>> early_init_fdt_scan_reserved_mem();
>>
>> /* 4GB maximum for 32-bit only capable devices */
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index b8e15c678960..093ccfb384af 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -573,6 +573,7 @@ static void __init early_reserve_mem_dt(void)
>> int len;
>> const __be32 *prop;
>>
>> + early_init_fdt_reserve_self();
>> early_init_fdt_scan_reserved_mem();
>>
>> dt_root = of_get_flat_dt_root();
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index 3a896c9aeb74..bbb35cdb06e8 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -561,11 +561,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
>> if (!initial_boot_params)
>> return;
>>
>> - /* Reserve the dtb region */
>> - early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
>> - fdt_totalsize(initial_boot_params),
>> - 0);
>> -
>> /* Process header /memreserve/ fields */
>> for (n = 0; ; n++) {
>> fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
>> @@ -579,6 +574,20 @@ void __init early_init_fdt_scan_reserved_mem(void)
>> }
>>
>> /**
>> + * early_init_fdt_reserve_self() - reserve the memory used by the FDT blob
>> + */
>> +void __init early_init_fdt_reserve_self(void)
>> +{
>> + if (!initial_boot_params)
>> + return;
>> +
>> + /* Reserve the dtb region */
>> + early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
>> + fdt_totalsize(initial_boot_params),
>> + 0);
>> +}
>> +
>> +/**
>> * of_scan_flat_dt - scan flattened tree blob and call callback on each.
>> * @it: callback function
>> * @data: context data pointer
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 0ff360d5b3b3..6ef6b33238d3 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -62,6 +62,7 @@ extern int early_init_dt_scan_chosen(unsigned long node,
>> const char *uname,
>> extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
>>
[PATCH v2 5/5] arm64/efi: adapt to relaxed FDT placement requirements
With the relaxed FDT placement requirements in place, we can change
the allocation strategy used by the stub to put the FDT image higher
up in memory. At the same time, reduce the minimal alignment to a
power of 2 upper bound of the size: this way, we are still guaranteed
not to cross a 2 MB boundary, but will potentially waste less memory.
Signed-off-by: Ard Biesheuvel
---
arch/arm64/include/asm/efi.h| 8 +++-
drivers/firmware/efi/libstub/arm-stub.c | 5 ++---
drivers/firmware/efi/libstub/efistub.h | 1 -
drivers/firmware/efi/libstub/fdt.c | 10 +++---
4 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index ef572206f1c3..6f526fcc6b70 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -39,12 +39,10 @@ extern void efi_init(void);
/* arch specific definitions used by the stub code */
/*
- * AArch64 requires the DTB to be 8-byte aligned in the first 512MiB from
- * start of kernel and may not cross a 2MiB boundary. We set alignment to
- * 2MiB so we know it won't cross a 2MiB boundary.
+ * AArch64 requires the DTB to be 8-byte aligned and not cross a 2MiB boundary.
+ * So align to a power of 2 upper bound of the FDT size.
*/
-#define EFI_FDT_ALIGN SZ_2M /* used by allocate_new_fdt_and_exit_boot() */
-#define MAX_FDT_OFFSET SZ_512M
+#define EFI_FDT_ALIGN(x) roundup_pow_of_two(x)
#define efi_call_early(f, ...) sys_table_arg->boottime->f(__VA_ARGS__)
diff --git a/drivers/firmware/efi/libstub/arm-stub.c
b/drivers/firmware/efi/libstub/arm-stub.c
index dcae482a9a17..f54c76a4fd32 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -269,9 +269,8 @@ unsigned long efi_entry(void *handle, efi_system_table_t
*sys_table,
new_fdt_addr = fdt_addr;
status = allocate_new_fdt_and_exit_boot(sys_table, handle,
- &new_fdt_addr, dram_base + MAX_FDT_OFFSET,
- initrd_addr, initrd_size, cmdline_ptr,
- fdt_addr, fdt_size);
+ &new_fdt_addr, initrd_addr, initrd_size,
+ cmdline_ptr, fdt_addr, fdt_size);
/*
* If all went well, we need to return the FDT address to the
diff --git a/drivers/firmware/efi/libstub/efistub.h
b/drivers/firmware/efi/libstub/efistub.h
index 47437b16b186..c8e096094ea9 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -35,7 +35,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void
*orig_fdt,
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
void *handle,
unsigned long *new_fdt_addr,
- unsigned long max_addr,
u64 initrd_addr, u64 initrd_size,
char *cmdline_ptr,
unsigned long fdt_addr,
diff --git a/drivers/firmware/efi/libstub/fdt.c
b/drivers/firmware/efi/libstub/fdt.c
index 91da56c4fd54..48218e91cce1 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -165,10 +165,6 @@ fdt_set_fail:
return EFI_LOAD_ERROR;
}
-#ifndef EFI_FDT_ALIGN
-#define EFI_FDT_ALIGN EFI_PAGE_SIZE
-#endif
-
/*
* Allocate memory for a new FDT, then add EFI, commandline, and
* initrd related fields to the FDT. This routine increases the
@@ -186,7 +182,6 @@ fdt_set_fail:
efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
void *handle,
unsigned long *new_fdt_addr,
- unsigned long max_addr,
u64 initrd_addr, u64 initrd_size,
char *cmdline_ptr,
unsigned long fdt_addr,
@@ -223,8 +218,9 @@ efi_status_t
allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
*/
new_fdt_size = fdt_size + EFI_PAGE_SIZE;
while (1) {
- status = efi_high_alloc(sys_table, new_fdt_size, EFI_FDT_ALIGN,
- new_fdt_addr, max_addr);
+ status = efi_high_alloc(sys_table, new_fdt_size,
+ EFI_FDT_ALIGN(new_fdt_size),
+ new_fdt_addr, ULONG_MAX);
if (status != EFI_SUCCESS) {
pr_efi_err(sys_table, "Unable to allocate memory for
new device tree.\n");
goto fail;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree&q
[PATCH v2 4/5] arm64/efi: ensure that Image does not cross a 512 MB boundary
Update the Image placement logic used by the stub to make absolutely
sure that the Image is placed such that the early init code will
always be able to map it. This means the entire static memory footprint
of the Image should be inside the same naturally aligned 512 MB region.
First of all, the preferred offset of dram_base + TEXT_OFFSET is only
suitable if it doesn't result in the Image crossing a 512 MB
alignment boundary, which could be the case if dram_base itself is
close to the end of a naturally aligned 512 MB region.
Also, when moving the kernel Image, we need to verify that the new
destination region does not cross a 512 MB alignment boundary either.
If that is the case, we retry the allocation with the alignment
chosen such that the resulting region will always be suitable.
Signed-off-by: Ard Biesheuvel
---
arch/arm64/kernel/efi-stub.c | 41 +
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c
index f5374065ad53..3b67ca4e2f2e 100644
--- a/arch/arm64/kernel/efi-stub.c
+++ b/arch/arm64/kernel/efi-stub.c
@@ -21,15 +21,40 @@ efi_status_t __init handle_kernel_image(efi_system_table_t
*sys_table,
unsigned long dram_base,
efi_loaded_image_t *image)
{
+ const unsigned long kernel_size = _edata - _text;
+ const unsigned long kernel_memsize = _end - _text;
+ unsigned long preferred_offset;
efi_status_t status;
- unsigned long kernel_size, kernel_memsize = 0;
-
- /* Relocate the image, if required. */
- kernel_size = _edata - _text;
- if (*image_addr != (dram_base + TEXT_OFFSET)) {
- kernel_memsize = kernel_size + (_end - _edata);
- status = efi_low_alloc(sys_table, kernel_memsize + TEXT_OFFSET,
- SZ_2M, reserve_addr);
+
+ /*
+* The kernel Image should be located as close as possible to the
+* base of system RAM, but its static memory footprint must not
+* cross a 512 MB alignment boundary.
+*/
+ preferred_offset = dram_base + TEXT_OFFSET;
+ if ((preferred_offset & (SZ_512M - 1)) + kernel_memsize > SZ_512M)
+ preferred_offset = round_up(dram_base, SZ_512M) + TEXT_OFFSET;
+
+ if (*image_addr != preferred_offset) {
+ const unsigned long alloc_size = kernel_memsize + TEXT_OFFSET;
+
+ status = efi_low_alloc(sys_table, alloc_size, SZ_2M,
+ reserve_addr);
+
+ /*
+* Check whether the new allocation crosses a 512 MB alignment
+* boundary. If so, retry with the alignment set to a power of
+* two upper bound of the allocation size. That is guaranteed
+* to produce a suitable allocation, but may waste more memory.
+*/
+ if (status == EFI_SUCCESS &&
+ ((*reserve_addr & (SZ_512M - 1)) + alloc_size) > SZ_512M) {
+ efi_free(sys_table, alloc_size, *reserve_addr);
+
+ status = efi_low_alloc(sys_table, alloc_size,
+ roundup_pow_of_two(alloc_size),
+ reserve_addr);
+ }
if (status != EFI_SUCCESS) {
pr_efi_err(sys_table, "Failed to relocate kernel\n");
return status;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] of/fdt: split off FDT self reservation from memreserve processing
This splits off the reservation of the memory occupied by the FDT
binary itself from the processing of the memory reservations it
contains. This is necessary because the physical address of the FDT,
which is needed to perform the reservation, may not be known to the
FDT driver core, i.e., it may be mapped outside the linear direct
mapping, in which case __pa() returns a bogus value.
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Signed-off-by: Ard Biesheuvel
---
arch/arm/mm/init.c | 1 +
arch/arm64/mm/init.c | 1 +
arch/powerpc/kernel/prom.c | 1 +
drivers/of/fdt.c | 19 ++-
include/linux/of_fdt.h | 2 ++
5 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 1609b022a72f..0b8657c36fe4 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -317,6 +317,7 @@ void __init arm_memblock_init(const struct machine_desc
*mdesc)
if (mdesc->reserve)
mdesc->reserve();
+ early_init_fdt_reserve_self();
early_init_fdt_scan_reserved_mem();
/* reserve memory for DMA contiguous allocations */
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index ae85da6307bb..fa2389b0f7f0 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -170,6 +170,7 @@ void __init arm64_memblock_init(void)
memblock_reserve(__virt_to_phys(initrd_start), initrd_end -
initrd_start);
#endif
+ early_init_fdt_reserve_self();
early_init_fdt_scan_reserved_mem();
/* 4GB maximum for 32-bit only capable devices */
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index b8e15c678960..093ccfb384af 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -573,6 +573,7 @@ static void __init early_reserve_mem_dt(void)
int len;
const __be32 *prop;
+ early_init_fdt_reserve_self();
early_init_fdt_scan_reserved_mem();
dt_root = of_get_flat_dt_root();
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3a896c9aeb74..bbb35cdb06e8 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -561,11 +561,6 @@ void __init early_init_fdt_scan_reserved_mem(void)
if (!initial_boot_params)
return;
- /* Reserve the dtb region */
- early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
- fdt_totalsize(initial_boot_params),
- 0);
-
/* Process header /memreserve/ fields */
for (n = 0; ; n++) {
fdt_get_mem_rsv(initial_boot_params, n, &base, &size);
@@ -579,6 +574,20 @@ void __init early_init_fdt_scan_reserved_mem(void)
}
/**
+ * early_init_fdt_reserve_self() - reserve the memory used by the FDT blob
+ */
+void __init early_init_fdt_reserve_self(void)
+{
+ if (!initial_boot_params)
+ return;
+
+ /* Reserve the dtb region */
+ early_init_dt_reserve_memory_arch(__pa(initial_boot_params),
+ fdt_totalsize(initial_boot_params),
+ 0);
+}
+
+/**
* of_scan_flat_dt - scan flattened tree blob and call callback on each.
* @it: callback function
* @data: context data pointer
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 0ff360d5b3b3..6ef6b33238d3 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -62,6 +62,7 @@ extern int early_init_dt_scan_chosen(unsigned long node,
const char *uname,
extern int early_init_dt_scan_memory(unsigned long node, const char *uname,
int depth, void *data);
extern void early_init_fdt_scan_reserved_mem(void);
+extern void early_init_fdt_reserve_self(void);
extern void early_init_dt_add_memory_arch(u64 base, u64 size);
extern int early_init_dt_reserve_memory_arch(phys_addr_t base, phys_addr_t
size,
bool no_map);
@@ -89,6 +90,7 @@ extern u64 fdt_translate_address(const void *blob, int
node_offset);
extern void of_fdt_limit_memory(int limit);
#else /* CONFIG_OF_FLATTREE */
static inline void early_init_fdt_scan_reserved_mem(void) {}
+static inline void early_init_fdt_reserve_self(void) {}
static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
static inline void unflatten_device_tree(void) {}
static inline void unflatten_and_copy_device_tree(void) {}
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] arm64: Documentation: clarify Image placement in physical RAM
The early init code maps the kernel image using statically allocated page tables. This means that we can only allow Image to be placed such that we can map its entire static footprint using a single table entry at all but the lowest level. So update the documentation to reflect that the Image should not cross a 512 MB boundary, which ensures the above on both 4k and 64k pages kernels. Reviewed-by: Mark Rutland Signed-off-by: Ard Biesheuvel --- Documentation/arm64/booting.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt index ab5a90adece3..5949bdbe7aac 100644 --- a/Documentation/arm64/booting.txt +++ b/Documentation/arm64/booting.txt @@ -115,8 +115,9 @@ The Image must be placed text_offset bytes from a 2MB aligned base address near the start of usable system RAM and called there. Memory below that base address is currently unusable by Linux, and therefore it is strongly recommended that this location is the start of system RAM. -At least image_size bytes from the start of the image must be free for -use by the kernel. +The physical memory region consisting of image_size bytes counting from +the start of the image must be free for use by the kernel, and must not +cross a 512 MB physical alignment boundary. Any memory described to the kernel (even that below the 2MB aligned base address) which is not marked as reserved from the kernel e.g. with a -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/5] arm64: use fixmap region for permanent FDT mapping
Currently, the FDT blob needs to be in the same naturally aligned
512 MB region as the kernel, so that it can be mapped into the
kernel virtual memory space very early on using a minimal set of
statically allocated translation tables.
Now that we have early fixmap support, we can relax this restriction,
by moving the permanent FDT mapping to the fixmap region instead.
This way, the FDT blob may be anywhere in memory.
This also moves the vetting of the FDT to setup.c, since the early
init code in head.S does not handle mapping of the FDT anymore.
At the same time, fix up some comments in head.S that have gone stale.
Signed-off-by: Ard Biesheuvel
---
Documentation/arm64/booting.txt | 10 +++---
arch/arm64/include/asm/fixmap.h | 9 ++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/head.S| 38 +-
arch/arm64/kernel/setup.c | 72 +
arch/arm64/mm/init.c| 1 -
6 files changed, 69 insertions(+), 62 deletions(-)
diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
index f3c05b5f9f08..ab5a90adece3 100644
--- a/Documentation/arm64/booting.txt
+++ b/Documentation/arm64/booting.txt
@@ -45,11 +45,13 @@ sees fit.)
Requirement: MANDATORY
-The device tree blob (dtb) must be placed on an 8-byte boundary within
-the first 512 megabytes from the start of the kernel image and must not
-cross a 2-megabyte boundary. This is to allow the kernel to map the
-blob using a single section mapping in the initial page tables.
+The device tree blob (dtb) must be placed on an 8-byte boundary and must
+not cross a 2-megabyte boundary. This is to allow the kernel to map the
+blob using a single section mapping in the fixmap region.
+NOTE: versions prior to v4.1 require, in addition to the requirements
+listed above, that the dtb be placed above the kernel Image inside the
+same naturally aligned 512 MB region.
3. Decompress the kernel image
--
diff --git a/arch/arm64/include/asm/fixmap.h b/arch/arm64/include/asm/fixmap.h
index defa0ff98250..4ad240a60898 100644
--- a/arch/arm64/include/asm/fixmap.h
+++ b/arch/arm64/include/asm/fixmap.h
@@ -32,6 +32,15 @@
*/
enum fixed_addresses {
FIX_HOLE,
+
+ /*
+* Reserve 2 MB of virtual space for the FDT at the top of the fixmap
+* region. Keep this at the top so it remains 2 MB aligned.
+*/
+#define FIX_FDT_SIZE SZ_2M
+ FIX_FDT_END,
+ FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
+
FIX_EARLYCON_MEM_BASE,
__end_of_permanent_fixed_addresses,
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 5ee07eee80c2..e60885766936 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -6,6 +6,7 @@ CPPFLAGS_vmlinux.lds:= -DTEXT_OFFSET=$(TEXT_OFFSET)
AFLAGS_head.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
CFLAGS_efi-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
CFLAGS_armv8_deprecated.o := -I$(src)
+CFLAGS_setup.o := -I$(srctree)/scripts/dtc/libfdt/
CFLAGS_REMOVE_ftrace.o = -pg
CFLAGS_REMOVE_insn.o = -pg
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 9bf1ba1a8363..c87423a6d0b2 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -254,7 +254,6 @@ ENTRY(stext)
cbnzx23, 1f // invalid processor (x23=0)?
b __error_p
1:
- bl __vet_fdt
bl __create_page_tables// x25=TTBR0, x26=TTBR1
/*
* The following calls CPU specific code in a position independent
@@ -273,24 +272,6 @@ ENTRY(stext)
ENDPROC(stext)
/*
- * Determine validity of the x21 FDT pointer.
- * The dtb must be 8-byte aligned and live in the first 512M of memory.
- */
-__vet_fdt:
- tst x21, #0x7
- b.ne1f
- cmp x21, x24
- b.lt1f
- mov x0, #(1 << 29)
- add x0, x0, x24
- cmp x21, x0
- b.ge1f
- ret
-1:
- mov x21, #0
- ret
-ENDPROC(__vet_fdt)
-/*
* Macro to create a table entry to the next page.
*
* tbl:page table address
@@ -351,8 +332,7 @@ ENDPROC(__vet_fdt)
* required to get the kernel running. The following sections are required:
* - identity mapping to enable the MMU (low address, TTBR0)
* - first few MB of the kernel linear mapping to jump to once the MMU has
- * been enabled, including the FDT blob (TTBR1)
- * - pgd entry for fixed mappings (TTBR1)
+ * been enabled
*/
__create_page_tables:
pgtbl x25, x26, x28 // idmap_pg_dir and
swapper_pg_dir addresses
@@ -440,22 +420,6 @@ __create_page_tables:
create_block_map x0, x7, x3, x5, x6
/*
-* Map the FDT blob (maximum 2MB; must be within 512MB of
-* PHYS_OFFSET).
-*/
- mov x3, x21 // FDT phys address
- and
[PATCH v2 0/5] arm64: update/clarify/relax Image and FDT placement rules
This series came about after Mark Rutland brought up the fact that the current FDT placement logic used by the EFI stub is flawed. But actually, it turned out that the documentation for both the Image and FDT placement was incorrect as well, or confusing at the very least. So this series does two things: - It relaxes the FDT placement requirements, and updates the documentation and EFI stub FDT placement logic accordingly. - It clarifies the Image placement requirements in the documentation, and brings the EFI stub Image placement logic in line with it Changes since v1: - patch #1: split off reservation of the FDT binary itself from the memreserve processing, since the former assumes the FDT is accessed via the linear mapping, which we are about to change - patch #2: mention the older, stricter FDT placement rules in booting.txt, get rid of early_print, use correct format specifier for phys_addr_t, use R/O mapping for FDT, - patches #3 .. #5: add R-b, minor style and grammar tweaks Ard Biesheuvel (5): of/fdt: split off FDT self reservation from memreserve processing arm64: use fixmap region for permanent FDT mapping arm64: Documentation: clarify Image placement in physical RAM arm64/efi: ensure that Image does not cross a 512 MB boundary arm64/efi: adapt to relaxed FDT placement requirements Documentation/arm64/booting.txt | 15 --- arch/arm/mm/init.c | 1 + arch/arm64/include/asm/efi.h| 8 ++-- arch/arm64/include/asm/fixmap.h | 9 + arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/efi-stub.c| 41 +++ arch/arm64/kernel/head.S| 38 + arch/arm64/kernel/setup.c | 72 - arch/powerpc/kernel/prom.c | 1 + drivers/firmware/efi/libstub/arm-stub.c | 5 +-- drivers/firmware/efi/libstub/efistub.h | 1 - drivers/firmware/efi/libstub/fdt.c | 10 ++--- drivers/of/fdt.c| 19 ++--- include/linux/of_fdt.h | 2 + 14 files changed, 131 insertions(+), 92 deletions(-) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH v2 3/4] arm64:thunder: Add initial dts for Cavium's Thunder SoC in 2 Node topology.
On 24 November 2014 at 18:01, Arnd Bergmann wrote: > On Monday 24 November 2014 11:32:46 Roy Franz wrote: >> > >> > I don't know how much history is behind this binding. Have you looked >> > at the sPAPR way of doing this? I don't remember exactly how that is >> > done, but we'd need a good reason to discard that and implement >> > something else for arm64. >> > >> > If we create a new binding, I don't think the 'numa-map' node you >> > have here is the best solution. We already have device nodes for each >> > memory segment and each CPU in the system. Why not work with those >> > nodes directly? >> >> The DT memory nodes don't exist in an EFI system, as the EFI memory >> map is used instead. >> Using EFI as the boot firmware doesn't require the use of ACPI for >> hardware description, >> so the EFI/DT case is one that we should support. > > But they /could/ exist, right? Can we just require them to be > present in order to use NUMA features? > Actually, currently the memory nodes are stripped from the device tree by the EFI stub, so the kernel will never get to see them. This is done more or less as a fixup, under the assumption that EFI systems should not have DT memory nodes in the first place. We could revisit this, of course, but it needs to be taken into account in this discussion. -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5.5] of/fdt: export fdt blob as /sys/firmware/fdt
On 19 November 2014 00:11, Rob Herring wrote: > On Tue, Nov 18, 2014 at 4:11 PM, Grant Likely wrote: >> On Tue, 18 Nov 2014 17:25:45 + >> , Mark Rutland >> wrote: >>> On Tue, Nov 18, 2014 at 04:51:45PM +, Grant Likely wrote: >>> > On Fri, 14 Nov 2014 18:05:35 +0100 >>> > , Ard Biesheuvel >>> > wrote: >>> > > Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob >>> > > that was passed to the kernel by the bootloader. This allows userland >>> > > applications such as kexec to access the raw binary. > > [...] > >>> > * It also helps with exposing the reserved map to userspace, but kexec >>> > has done without that feature for years, and it is in the process of >>> > being deprecated in favour of /reserved-memory anyway. >>> >>> This is the first I'd heard of the reserve map being deprecated, and >>> we're going to have DTs with reserved map entries for a long time going >>> forwards. >> >> Deprecated, not removed or disabled. It will still work pretty much >> forever, but users should be encouraged to move to the reserve-memory >> tree. > > I thought you had said reserve map was still the right way for memory > the kernel should never touch. > >>> Can't we expose the header fields under something like >>> /sys/firmware/devicetree/dtb-header/, parallel to the usual >>> /sys/firmware/devicetree/base for nodes? >> >> We could do that too. >> >> Honestly though, I'm just unsure of what the best thing to do is. If you >> and a few others tell me that, "no, exporting the raw dtb is the right >> thing to do", then I'll be okay, merge the patch and sleep properly. > > I always sleep better when others can take the blame. > > What happens when we rev the dtb format? Is the ABI the blob or the > format of the blob? > > I lean towards we should add this. This is providing what is "in the > firmware" while /proc/devicetree provides the live tree state > including overlays. > Well, my pov is that FDT != devicetree ever since we started (ab)using the FDT container format to pass just the UEFI entry points to the kernel. The boot protocol describes what should be passed in x0, and /that/ is what we expose in /sys/firmware/fdt, regardless of how the kernel decided to configure itself. (perhaps we need to fix the wording in the document to refer to FDT not dtb) So that also means I don't care about memreserve vs reserved-memory or other DT specific details: as long as x0 points to something libfdt understands at boot, we expose it and not interpret it any further. -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5.5] of/fdt: export fdt blob as /sys/firmware/fdt
Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary.
The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.
A CRC32 checksum is calculated over the entire FDT blob, and verified
at late_initcall time. The sysfs entry is instantiated only if the
checksum is valid, i.e., if the FDT blob has not been modified in the
mean time. Otherwise, a warning is printed.
Signed-off-by: Ard Biesheuvel
---
v5.5: based on Grant's changes
"""
Actually, I made a couple of changes when I merged it. I removed the
older debugfs interface since it overlaps, and I added tests for
initial_boot_params to make sure it doesn't try to run on an invalid
FDT
"""
but I removed the second fdt_check_header() again in the late init code, as it
would result in a corrupted FDT to be silently ignored. Note that
initial_boot_params can only be set if fdt_check_header() succeeded the first
time, so a failure occurring the second time should produce a warning, and
the CRC check will catch it anyway. The CRC check itself is fixed to use the
API as it is supposed to. I also moved the CRC variable definition inside the
#ifdef OF_EARLY_FLATTREE region.
v4: use pr_warn() instead of WARN()
v3: keep checksum instead of copying the entire blob, and WARN on mismatch
drivers/of/Kconfig | 1 +
drivers/of/fdt.c | 43 +++
2 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 1a13f5b722c5..0348c208343c 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -23,6 +23,7 @@ config OF_FLATTREE
bool
select DTC
select LIBFDT
+ select CRC32
config OF_EARLY_FLATTREE
bool
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..2bbda0775f57 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -9,6 +9,7 @@
* version 2 as published by the Free Software Foundation.
*/
+#include
#include
#include
#include
@@ -22,6 +23,7 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
@@ -425,6 +427,8 @@ void *initial_boot_params;
#ifdef CONFIG_OF_EARLY_FLATTREE
+static u32 of_fdt_crc32;
+
/**
* res_mem_reserve_reg() - reserve all memory described in 'reg' property
*/
@@ -996,6 +1000,8 @@ bool __init early_init_dt_verify(void *params)
/* Setup flat device-tree pointer */
initial_boot_params = params;
+ of_fdt_crc32 = crc32_be(~0, initial_boot_params,
+ fdt_totalsize(initial_boot_params));
/* check device tree validity */
if (fdt_check_header(params)) {
@@ -1080,27 +1086,32 @@ void __init unflatten_and_copy_device_tree(void)
unflatten_device_tree();
}
-#if defined(CONFIG_DEBUG_FS) && defined(DEBUG)
-static struct debugfs_blob_wrapper flat_dt_blob;
-
-static int __init of_flat_dt_debugfs_export_fdt(void)
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
{
- struct dentry *d = debugfs_create_dir("device-tree", NULL);
-
- if (!d)
- return -ENOENT;
+ memcpy(buf, initial_boot_params + off, count);
+ return count;
+}
- flat_dt_blob.data = initial_boot_params;
- flat_dt_blob.size = fdt_totalsize(initial_boot_params);
+static int __init of_fdt_raw_init(void)
+{
+ static struct bin_attribute of_fdt_raw_attr =
+ __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
- d = debugfs_create_blob("flat-device-tree", S_IFREG | S_IRUSR,
- d, &flat_dt_blob);
- if (!d)
- return -ENOENT;
+ if (!initial_boot_params)
+ return 0;
- return 0;
+ if (of_fdt_crc32 != crc32_be(~0, initial_boot_params,
+fdt_totalsize(initial_boot_params))) {
+ pr_warn("fdt: not creating '/sys/firmware/fdt': CRC check
failed\n");
+ return 0;
+ }
+ of_fdt_raw_attr.size = fdt_totalsize(initial_boot_params);
+ return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
}
-module_init(of_flat_dt_debugfs_export_fdt);
+late_initcall(of_fdt_raw_init);
#endif
#endif /* CONFIG_OF_EARLY_FLATTREE */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] of/fdt: export fdt blob as /sys/firmware/fdt
On 12 November 2014 17:08, Rob Herring wrote:
> On Wed, Nov 12, 2014 at 6:28 AM, Ard Biesheuvel
> wrote:
>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>> that was passed to the kernel by the bootloader. This allows userland
>> applications such as kexec to access the raw binary.
>>
>> The fact that this node does not reside under /sys/firmware/device-tree
>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>> communicate just the UEFI and ACPI entry points, but the FDT is never
>> unflattened and used to configure the system.
>>
>> A CRC32 checksum is calculated over the entire FDT blob, and verified
>> at late_initcall time. The sysfs entry is instantiated only if the
>> checksum is valid, i.e., if the FDT blob has not been modified in the
>> mean time. Otherwise, a warning is printed.
>>
>> Signed-off-by: Ard Biesheuvel
>> ---
>> v4: use pr_warn() instead of WARN()
>> v3: keep checksum instead of copying the entire blob, and WARN on mismatch
>>
>> drivers/of/Kconfig | 1 +
>> drivers/of/fdt.c | 37 +
>> 2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
>> index 1a13f5b722c5..0348c208343c 100644
>> --- a/drivers/of/Kconfig
>> +++ b/drivers/of/Kconfig
>> @@ -23,6 +23,7 @@ config OF_FLATTREE
>> bool
>> select DTC
>> select LIBFDT
>> + select CRC32
>>
>> config OF_EARLY_FLATTREE
>> bool
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index d1ffca8b34ea..0004871ebccf 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -9,6 +9,7 @@
>> * version 2 as published by the Free Software Foundation.
>> */
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -22,10 +23,20 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include /* for COMMAND_LINE_SIZE */
>> #include
>>
>> +static u32 of_fdt_crc32;
>> +
>> +static u32 of_fdt_get_crc32(void *fdt)
>> +{
>> + static u32 const OF_FDT_CRC32_SEED = 0x04c11db7;
>
> Where did this value come from? A comment would be useful.
>
Oops, it appears I was a bit sloppy here. I misread 'seed' in the
crc32 header as poly, but actually the implementation refers to this
polynomial directly as CRCPOLY_BE and the seed should be 0 here (or in
the verify case, the original crc, in which case the return value
should be 0)
>> +
>> + return crc32_be(OF_FDT_CRC32_SEED, fdt, fdt_totalsize(fdt));
>> +}
>> +
>> /*
>> * of_fdt_limit_memory - limit the number of regions in the /memory node
>> * @limit: maximum entries
>> @@ -1003,6 +1014,8 @@ bool __init early_init_dt_verify(void *params)
>> return false;
>> }
>>
>> + of_fdt_crc32 = of_fdt_get_crc32(initial_boot_params);
>> +
>> return true;
>> }
>>
>> @@ -1103,4 +1116,28 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>> module_init(of_flat_dt_debugfs_export_fdt);
>> #endif
>>
>> +#ifdef CONFIG_SYSFS
>> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
>> + struct bin_attribute *bin_attr,
>> + char *buf, loff_t off, size_t count)
>> +{
>> + memcpy(buf, initial_boot_params + off, count);
>> + return count;
>> +}
>> +
>> +static int __init of_fdt_raw_init(void)
>> +{
>> + static struct bin_attribute of_fdt_raw_attr =
>> + __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
>> +
>> + if (of_fdt_crc32 != of_fdt_get_crc32(initial_boot_params)) {
>> + pr_warn("fdt: not creating '/sys/firmware/fdt': CRC check
>> failed");
>> + return 0;
>> + }
>> + of_fdt_raw_attr.size = fdt_totalsize(initial_boot_params);
>> + return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
>> +}
>> +late_initcall(of_fdt_raw_init);
>> +#endif
>> +
>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>
> You have a mixture of things inside and outside of
> CONFIG_OF_EARLY_FLATTREE which in theory could cause warnings. However
> looking more closely at this, it is fine for now. Instead I think we
> need to get rid of CONFIG_OF_EARLY_FLATTREE (or merge with
> OF_FLATTREE) . Every platform selects it except Sparc.
>
Well, the crc will only be computed if early_init_dt_verify() was
called, so it seems putting the initcall() here is not entirely
inappropriate.
@Grant; I will respin based on the version you posted in this thread,
and fix the crc bit.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] of/fdt: export fdt blob as /sys/firmware/fdt
Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary.
The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.
A CRC32 checksum is calculated over the entire FDT blob, and verified
at late_initcall time. The sysfs entry is instantiated only if the
checksum is valid, i.e., if the FDT blob has not been modified in the
mean time. Otherwise, a warning is printed.
Signed-off-by: Ard Biesheuvel
---
v4: use pr_warn() instead of WARN()
v3: keep checksum instead of copying the entire blob, and WARN on mismatch
drivers/of/Kconfig | 1 +
drivers/of/fdt.c | 37 +
2 files changed, 38 insertions(+)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 1a13f5b722c5..0348c208343c 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -23,6 +23,7 @@ config OF_FLATTREE
bool
select DTC
select LIBFDT
+ select CRC32
config OF_EARLY_FLATTREE
bool
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..0004871ebccf 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -9,6 +9,7 @@
* version 2 as published by the Free Software Foundation.
*/
+#include
#include
#include
#include
@@ -22,10 +23,20 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
+static u32 of_fdt_crc32;
+
+static u32 of_fdt_get_crc32(void *fdt)
+{
+ static u32 const OF_FDT_CRC32_SEED = 0x04c11db7;
+
+ return crc32_be(OF_FDT_CRC32_SEED, fdt, fdt_totalsize(fdt));
+}
+
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
@@ -1003,6 +1014,8 @@ bool __init early_init_dt_verify(void *params)
return false;
}
+ of_fdt_crc32 = of_fdt_get_crc32(initial_boot_params);
+
return true;
}
@@ -1103,4 +1116,28 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
module_init(of_flat_dt_debugfs_export_fdt);
#endif
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ memcpy(buf, initial_boot_params + off, count);
+ return count;
+}
+
+static int __init of_fdt_raw_init(void)
+{
+ static struct bin_attribute of_fdt_raw_attr =
+ __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
+
+ if (of_fdt_crc32 != of_fdt_get_crc32(initial_boot_params)) {
+ pr_warn("fdt: not creating '/sys/firmware/fdt': CRC check
failed");
+ return 0;
+ }
+ of_fdt_raw_attr.size = fdt_totalsize(initial_boot_params);
+ return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
+}
+late_initcall(of_fdt_raw_init);
+#endif
+
#endif /* CONFIG_OF_EARLY_FLATTREE */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt
On 12 November 2014 13:01, Grant Likely wrote: > On Wed, Nov 12, 2014 at 11:51 AM, Ard Biesheuvel > wrote: >> On 12 November 2014 12:45, Grant Likely wrote: >>> On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel >>> wrote: >>>> On 11 November 2014 21:08, Grant Likely wrote: >>>>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring >>>>> wrote: >>>>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely >>>>>> wrote: >>>>>>> On Mon, 10 Nov 2014 19:47:01 +0100 >>>>>>> , Ard Biesheuvel >>>>>>> wrote: >>>>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob >>>>>>>> that was passed to the kernel by the bootloader. This allows userland >>>>>>>> applications such as kexec to access the raw binary. The blob needs to >>>>>>>> be preserved as early as possible by calling preserve_fdt(). >>>>>>>> >>>>>>>> The fact that this node does not reside under /sys/firmware/device-tree >>>>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to >>>>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never >>>>>>>> unflattened and used to configure the system. >>>>>>>> >>>>>>>> Signed-off-by: Ard Biesheuvel >>>>>>> >>>>>>> On further thought, nak. I want tree modifications to be treated as >>>>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended >>>>>>> to us an in-blob CRC when the file format gets upreved to include one. >>>>>> >>>>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications >>>>>> or fixups have to be on the unflattened tree? >>>>> >>>>> That, or we require the fixups to explicitly clean up the CRC after >>>>> themselves. That will force them to be visible. Anything using the >>>>> fdt_*() write functions could potentially take care of it >>>>> automatically. That would make it immediately discoverable where >>>>> changes are being made in the tree. >>>>> >>>> >>>> Doesn't that defeat the purpose of capturing the FDT as it was passed >>>> by the bootloader? Those fixups will be reapplied by the kexec'ed >>>> kernel anyway. >>> >>> We're always going to be in a grey area here. There are some things >>> that should not be passed over. For example, a framebuffer that is >>> passed in from firmware will no longer be valid on kexec. Right now on >>> arch/arm, the exynos platform fixes up the DT because the memory node >>> is outright /corrupt/. In the MIPS case, they do a bunch of stuff in >>> the kernel to figure out the hardware as configured by firmware. I >>> don't agree with that approach, but I would be very surprised if the >>> full original DT has any validity after the early boot fixups. >>> >> >> Well, yes, but those fixups will presumably be done on every boot, >> including the kexec ones. >> >>> Early boot fixups to the .dtb are always going to be oddball cases. >>> >> >> OK, so generally speaking, whether the original DTB is more suitable >> to be presented through /sys/firmware/fdt than the 'current' FDT >> (i.e., whatever is in memory at the time the sysfs node is accessed) >> is not obvious at all. So why not just do the latter? > > We're now talking theoreticals. In the case you're working on > (ACPI+FDT), the FDT is generated by Grub or the EFI stub as an > intermediary format, and there isn't currently any code in your boot > path that modifies the FDT. You're arguing for a future situation > where the kernel makes a fixup that is not wanted in the second > kernel. I'm arguing that 1) I want to /know/ when that happens because > there are very few situations where it is a valid thing to do, and 2) > the likelyhood of that happening in the FDT+ACPI case is somewhere > between slim and none because you're working with an intermediary FDT, > not a real hardware description. > But do you still want the warning? Or only if you access the sysfs node? Or just a pr_warn() perhaps rather than a full blown WARN() ? MIPS will start seeing the warning as well even if they don't care about what is in /sys/firmware/fdt -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt
On 12 November 2014 12:45, Grant Likely wrote: > On Tue, Nov 11, 2014 at 8:34 PM, Ard Biesheuvel > wrote: >> On 11 November 2014 21:08, Grant Likely wrote: >>> On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring wrote: >>>> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely >>>> wrote: >>>>> On Mon, 10 Nov 2014 19:47:01 +0100 >>>>> , Ard Biesheuvel >>>>> wrote: >>>>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob >>>>>> that was passed to the kernel by the bootloader. This allows userland >>>>>> applications such as kexec to access the raw binary. The blob needs to >>>>>> be preserved as early as possible by calling preserve_fdt(). >>>>>> >>>>>> The fact that this node does not reside under /sys/firmware/device-tree >>>>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to >>>>>> communicate just the UEFI and ACPI entry points, but the FDT is never >>>>>> unflattened and used to configure the system. >>>>>> >>>>>> Signed-off-by: Ard Biesheuvel >>>>> >>>>> On further thought, nak. I want tree modifications to be treated as >>>>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended >>>>> to us an in-blob CRC when the file format gets upreved to include one. >>>> >>>> Why? MIPS Octeon is full of them BTW. So the rule is any modifications >>>> or fixups have to be on the unflattened tree? >>> >>> That, or we require the fixups to explicitly clean up the CRC after >>> themselves. That will force them to be visible. Anything using the >>> fdt_*() write functions could potentially take care of it >>> automatically. That would make it immediately discoverable where >>> changes are being made in the tree. >>> >> >> Doesn't that defeat the purpose of capturing the FDT as it was passed >> by the bootloader? Those fixups will be reapplied by the kexec'ed >> kernel anyway. > > We're always going to be in a grey area here. There are some things > that should not be passed over. For example, a framebuffer that is > passed in from firmware will no longer be valid on kexec. Right now on > arch/arm, the exynos platform fixes up the DT because the memory node > is outright /corrupt/. In the MIPS case, they do a bunch of stuff in > the kernel to figure out the hardware as configured by firmware. I > don't agree with that approach, but I would be very surprised if the > full original DT has any validity after the early boot fixups. > Well, yes, but those fixups will presumably be done on every boot, including the kexec ones. > Early boot fixups to the .dtb are always going to be oddball cases. > OK, so generally speaking, whether the original DTB is more suitable to be presented through /sys/firmware/fdt than the 'current' FDT (i.e., whatever is in memory at the time the sysfs node is accessed) is not obvious at all. So why not just do the latter? -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt
On 11 November 2014 21:08, Grant Likely wrote: > On Tue, Nov 11, 2014 at 4:25 PM, Rob Herring wrote: >> On Tue, Nov 11, 2014 at 8:42 AM, Grant Likely >> wrote: >>> On Mon, 10 Nov 2014 19:47:01 +0100 >>> , Ard Biesheuvel >>> wrote: >>>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob >>>> that was passed to the kernel by the bootloader. This allows userland >>>> applications such as kexec to access the raw binary. The blob needs to >>>> be preserved as early as possible by calling preserve_fdt(). >>>> >>>> The fact that this node does not reside under /sys/firmware/device-tree >>>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to >>>> communicate just the UEFI and ACPI entry points, but the FDT is never >>>> unflattened and used to configure the system. >>>> >>>> Signed-off-by: Ard Biesheuvel >>> >>> On further thought, nak. I want tree modifications to be treated as >>> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended >>> to us an in-blob CRC when the file format gets upreved to include one. >> >> Why? MIPS Octeon is full of them BTW. So the rule is any modifications >> or fixups have to be on the unflattened tree? > > That, or we require the fixups to explicitly clean up the CRC after > themselves. That will force them to be visible. Anything using the > fdt_*() write functions could potentially take care of it > automatically. That would make it immediately discoverable where > changes are being made in the tree. > Doesn't that defeat the purpose of capturing the FDT as it was passed by the bootloader? Those fixups will be reapplied by the kexec'ed kernel anyway. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] of/fdt: export fdt blob as /sys/firmware/fdt
Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary.
The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.
A CRC32 checksum is calculated over the entire FDT blob, and verified
at late_initcall time. The sysfs entry is instantiated only if the
checksum is valid, i.e., if the FDT blob has not been modified in the
mean time. Otherwise, a warning is printed.
Signed-off-by: Ard Biesheuvel
---
v3: keep checksum instead of copying the entire blob, and WARN on mismatch
drivers/of/Kconfig | 1 +
drivers/of/fdt.c | 36
2 files changed, 37 insertions(+)
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 1a13f5b722c5..0348c208343c 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -23,6 +23,7 @@ config OF_FLATTREE
bool
select DTC
select LIBFDT
+ select CRC32
config OF_EARLY_FLATTREE
bool
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..6742b6ae1b89 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -9,6 +9,7 @@
* version 2 as published by the Free Software Foundation.
*/
+#include
#include
#include
#include
@@ -22,10 +23,20 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
+static u32 of_fdt_crc32;
+
+static u32 of_fdt_get_crc32(void *fdt)
+{
+ static u32 const OF_FDT_CRC32_SEED = 0x04c11db7;
+
+ return crc32_be(OF_FDT_CRC32_SEED, fdt, fdt_totalsize(fdt));
+}
+
/*
* of_fdt_limit_memory - limit the number of regions in the /memory node
* @limit: maximum entries
@@ -1003,6 +1014,8 @@ bool __init early_init_dt_verify(void *params)
return false;
}
+ of_fdt_crc32 = of_fdt_get_crc32(initial_boot_params);
+
return true;
}
@@ -1103,4 +1116,27 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
module_init(of_flat_dt_debugfs_export_fdt);
#endif
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ memcpy(buf, initial_boot_params + off, count);
+ return count;
+}
+
+static int __init of_fdt_raw_init(void)
+{
+ static struct bin_attribute of_fdt_raw_attr =
+ __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
+
+ if (WARN(of_fdt_crc32 != of_fdt_get_crc32(initial_boot_params),
+"CRC check failed on binary FDT"))
+ return -EFAULT;
+ of_fdt_raw_attr.size = fdt_totalsize(initial_boot_params);
+ return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
+}
+late_initcall(of_fdt_raw_init);
+#endif
+
#endif /* CONFIG_OF_EARLY_FLATTREE */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt
On 11 November 2014 15:42, Grant Likely wrote:
> On Mon, 10 Nov 2014 19:47:01 +0100
> , Ard Biesheuvel
> wrote:
>> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
>> that was passed to the kernel by the bootloader. This allows userland
>> applications such as kexec to access the raw binary. The blob needs to
>> be preserved as early as possible by calling preserve_fdt().
>>
>> The fact that this node does not reside under /sys/firmware/device-tree
>> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
>> communicate just the UEFI and ACPI entry points, but the FDT is never
>> unflattened and used to configure the system.
>>
>> Signed-off-by: Ard Biesheuvel
>
> On further thought, nak. I want tree modifications to be treated as
> bugs. Do a checksum CRC32 or check. It's simpler and it can be extended
> to us an in-blob CRC when the file format gets upreved to include one.
>
Ah, right, I was under the impression that tree modifications were
expected and allowed.
In that case, yes, let me respin to calculate a CRC early on, and
WARN() if it gets corrupted.
--
Ard.
> g.
>
> ---
>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index be16ce2ffd69..adda0a16934f 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -23,6 +23,7 @@ config OF_FLATTREE
> bool
> select DTC
> select LIBFDT
> + select CRC
>
> config OF_EARLY_FLATTREE
> bool
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 83a8e1154602..80db46a2eab9 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -10,6 +10,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -420,6 +421,7 @@ int __initdata dt_root_addr_cells;
> int __initdata dt_root_size_cells;
>
> void *initial_boot_params;
> +u32 initial_boot_params_crc;
>
> #ifdef CONFIG_OF_EARLY_FLATTREE
>
> @@ -1003,6 +1005,9 @@ bool __init early_init_dt_verify(void *params)
>
> /* Setup flat device-tree pointer */
> initial_boot_params = params;
> + initial_boot_params_crc = crc32_be(0, params, fdt_totalsize(params));
> + pr_info("FDT CRC: %x\n", initial_boot_params_crc);
> +
> return true;
> }
>
>
>
>
>> ---
>> drivers/of/fdt.c | 34 ++
>> include/linux/of_fdt.h | 2 ++
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index d1ffca8b34ea..e9ee3d5f7ea4 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -22,6 +22,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include /* for COMMAND_LINE_SIZE */
>> #include
>> @@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
>> module_init(of_flat_dt_debugfs_export_fdt);
>> #endif
>>
>> +static u8 *raw_fdt_copy;
>> +
>> +void __init preserve_fdt(void)
>> +{
>> + u32 fdt_size;
>> +
>> + fdt_size = fdt_totalsize(initial_boot_params);
>> + raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
>> + initial_boot_params, fdt_size);
>> +}
>> +
>> +#ifdef CONFIG_SYSFS
>> +static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
>> +struct bin_attribute *bin_attr,
>> +char *buf, loff_t off, size_t count)
>> +{
>> + memcpy(buf, raw_fdt_copy + off, count);
>> + return count;
>> +}
>> +
>> +static int __init of_fdt_raw_init(void)
>> +{
>> + static struct bin_attribute of_fdt_raw_attr =
>> + __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
>> +
>> + if (!raw_fdt_copy)
>> + return 0;
>> + of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
>> + return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
>> +}
>> +module_init(of_fdt_raw_init);
>> +#endif
>> +
>> #endif /* CONFIG_OF_EARLY_FLATTREE */
>> diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
>> index 0ff360d5b3b3..7672a26305a5 100644
>> --- a/include/linux/of_fdt.h
>> +++ b/include/linux/of_fdt.h
>> @@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void
>> *default_match,
>> /* Other Prototypes */
>> extern void unflatten_device_tree(void);
>> extern void unflatten_and_copy_device_tree(void);
>> +extern void preserve_fdt(void);
>> e
[PATCH v2 2/2] arm64: fdt: call preserve_fdt() before unflattening it
To support the new /sys/firmware/fdt entry that gives access to the FDT blob as passed by the bootloader, call preserve_fdt() in the early arch code to make a copy of it before the unflattening code gets its hands on it. Also, make the early mapping of the FDT read only so we are certain it will not get clobbered by anything called by setup_machine_fdt(), which handles the FDT blob even earlier. Signed-off-by: Ard Biesheuvel --- arch/arm64/kernel/head.S | 3 +++ arch/arm64/kernel/setup.c | 1 + 2 files changed, 4 insertions(+) diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 0a6e4f924df8..df25dffcda9f 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -79,8 +79,10 @@ #ifdef CONFIG_ARM64_64K_PAGES #define MM_MMUFLAGSPTE_ATTRINDX(MT_NORMAL) | PTE_FLAGS +#define MM_MMUFLAGS_RO PTE_ATTRINDX(MT_NORMAL) | PTE_FLAGS | PTE_RDONLY #else #define MM_MMUFLAGSPMD_ATTRINDX(MT_NORMAL) | PMD_FLAGS +#define MM_MMUFLAGS_RO PMD_ATTRINDX(MT_NORMAL) | PMD_FLAGS | PMD_SECT_RDONLY #endif /* @@ -607,6 +609,7 @@ __create_page_tables: * Map the FDT blob (maximum 2MB; must be within 512MB of * PHYS_OFFSET). */ + ldr x7, =MM_MMUFLAGS_RO mov x3, x21 // FDT phys address and x3, x3, #~((1 << 21) - 1) // 2MB aligned mov x6, #PAGE_OFFSET diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 2437196cc5d4..1b535cc42981 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -394,6 +394,7 @@ void __init setup_arch(char **cmdline_p) efi_idmap_init(); + preserve_fdt(); unflatten_device_tree(); psci_init(); -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] of/fdt: export fdt blob as /sys/firmware/fdt
Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary. The blob needs to
be preserved as early as possible by calling preserve_fdt().
The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.
Signed-off-by: Ard Biesheuvel
---
drivers/of/fdt.c | 34 ++
include/linux/of_fdt.h | 2 ++
2 files changed, 36 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..e9ee3d5f7ea4 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
@@ -1103,4 +1104,37 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
module_init(of_flat_dt_debugfs_export_fdt);
#endif
+static u8 *raw_fdt_copy;
+
+void __init preserve_fdt(void)
+{
+ u32 fdt_size;
+
+ fdt_size = fdt_totalsize(initial_boot_params);
+ raw_fdt_copy = memcpy(__va(memblock_alloc(fdt_size, PAGE_SIZE)),
+ initial_boot_params, fdt_size);
+}
+
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ memcpy(buf, raw_fdt_copy + off, count);
+ return count;
+}
+
+static int __init of_fdt_raw_init(void)
+{
+ static struct bin_attribute of_fdt_raw_attr =
+ __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
+
+ if (!raw_fdt_copy)
+ return 0;
+ of_fdt_raw_attr.size = fdt_totalsize(raw_fdt_copy);
+ return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
+}
+module_init(of_fdt_raw_init);
+#endif
+
#endif /* CONFIG_OF_EARLY_FLATTREE */
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 0ff360d5b3b3..7672a26305a5 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -83,6 +83,7 @@ extern const void *of_flat_dt_match_machine(const void
*default_match,
/* Other Prototypes */
extern void unflatten_device_tree(void);
extern void unflatten_and_copy_device_tree(void);
+extern void preserve_fdt(void);
extern void early_init_devtree(void *);
extern void early_get_first_memblock_info(void *, phys_addr_t *);
extern u64 fdt_translate_address(const void *blob, int node_offset);
@@ -92,6 +93,7 @@ static inline void early_init_fdt_scan_reserved_mem(void) {}
static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
static inline void unflatten_device_tree(void) {}
static inline void unflatten_and_copy_device_tree(void) {}
+static inline void preserve_fdt(void) {}
#endif /* CONFIG_OF_FLATTREE */
#endif /* __ASSEMBLY__ */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] preserve FDT blob and present as /sys/firmware/fdt
Meh, I am not necessarily a lot happier with this version, now split into a generic and a arch specific part, but it addresses Grant's concerns with respect to modification, without resorting to checksums. Checksums only solve half of the problem, because /not/ presenting the blob at all if the checksum is incorrect is not really an option imo. Ard Biesheuvel (2): of/fdt: export fdt blob as /sys/firmware/fdt arm64: fdt: call preserve_fdt() before unflattening it arch/arm64/kernel/head.S | 3 +++ arch/arm64/kernel/setup.c | 1 + drivers/of/fdt.c | 34 ++ include/linux/of_fdt.h| 2 ++ 4 files changed, 40 insertions(+) -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] of/fdt: export fdt blob as /sys/firmware/fdt
On 10 November 2014 17:57, Arnd Bergmann wrote: > On Monday 10 November 2014 17:51:45 Ard Biesheuvel wrote: >> Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob >> that was passed to the kernel by the bootloader. This allows userland >> applications such as kexec to access the raw binary. >> >> The fact that this node does not reside under /sys/firmware/device-tree >> is deliberate: FDT is also used on arm64 UEFI/ACPI systems to >> communicate just the UEFI and ACPI entry points, but the FDT is never >> unflattened and used to configure the system. >> >> Signed-off-by: Ard Biesheuvel > > Can you elaborate on the motivation? Initially the fdt format was > introduced to make it easy to pass the information from /proc/device-tree > to the next kernel for kexec. Are you interested in cases where this does > not work? > /sys/firmware/device-tree only gets populated if you are really booting in DT mode, i.e., when the device tree is unflattened. However, in ACPI mode we still need to access the data that was communicated between UEFI and the kernel in the /chosen node if we want to support kexec under UEFI. -- Ard. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] of/fdt: export fdt blob as /sys/firmware/fdt
Create a new /sys entry '/sys/firmware/fdt' to export the FDT blob
that was passed to the kernel by the bootloader. This allows userland
applications such as kexec to access the raw binary.
The fact that this node does not reside under /sys/firmware/device-tree
is deliberate: FDT is also used on arm64 UEFI/ACPI systems to
communicate just the UEFI and ACPI entry points, but the FDT is never
unflattened and used to configure the system.
Signed-off-by: Ard Biesheuvel
---
Resending with CC LAKML and devicetree@vger.kernel.org added.
Apologies for the noise.
drivers/of/fdt.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..60f9c67dc52a 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include /* for COMMAND_LINE_SIZE */
#include
@@ -1103,4 +1104,23 @@ static int __init of_flat_dt_debugfs_export_fdt(void)
module_init(of_flat_dt_debugfs_export_fdt);
#endif
+#ifdef CONFIG_SYSFS
+static ssize_t of_fdt_raw_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ memcpy(buf, initial_boot_params + off, count);
+ return count;
+}
+
+static int __init of_fdt_raw_init(void)
+{
+ static struct bin_attribute of_fdt_raw_attr =
+ __BIN_ATTR(fdt, S_IRUSR, of_fdt_raw_read, NULL, 0);
+ of_fdt_raw_attr.size = fdt_totalsize(initial_boot_params);
+ return sysfs_create_bin_file(firmware_kobj, &of_fdt_raw_attr);
+}
+module_init(of_fdt_raw_init);
+#endif
+
#endif /* CONFIG_OF_EARLY_FLATTREE */
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] of: check for size < 0 after rounding in early_init_dt_add_memory_arch
Memory regions passed to early_init_dt_add_memory_arch() are rounded to
PAGE_SIZE by subtracting the size of the leading fractional page from
the 'size' argument. However, size being a u64 type, if its value is
sufficiently small, the subtraction wraps around and produces a bogus
value, potentially leading to crashes.
Fix this by ignoring the memory range in such cases.
Signed-off-by: Ard Biesheuvel
---
drivers/of/fdt.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..3b423f5b99c6 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -930,6 +930,11 @@ void __init __weak early_init_dt_add_memory_arch(u64 base,
u64 size)
const u64 phys_offset = __pa(PAGE_OFFSET);
if (!PAGE_ALIGNED(base)) {
+ if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
+ pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
+ base, base + size);
+ return;
+ }
size -= PAGE_SIZE - (base & ~PAGE_MASK);
base = PAGE_ALIGN(base);
}
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
