Re: [PATCH] efi/libstub/arm*: Set default address and size cells values for an empty dtb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/7/2017 12:01 PM, Mark Rutland wrote:
On Tue, Feb 07, 2017 at 11:54:55AM -0700, Jeffrey Hugo wrote:
On 2/7/2017 11:12 AM, Ard Biesheuvel wrote:
On 7 February 2017 at 17:59, Jeffrey Hugo <jhugo@xxxxxxxxxxxxxx> wrote:
From: Sameer Goel <sgoel@xxxxxxxxxxxxxx>

In cases where a device tree is not provided (ie ACPI based system), an
empty fdt is generated by efistub.  Sets the address and size cell values
in a generated fdt to support 64 bit addressing.

This enables kexec/kdump on Qualcomm Technologies QDF24XX platforms as those
utilities will read the address/size values from the fdt, and such values
may exceed the range provided by the 32 bit default.


As far as I know, those properties are explicitly associated with the
'reg' properties of subordinate nodes. So which nodes are we talking
about here? Are we producing an incorrect DT by not setting these? Or
is this simply a convenience to work around bugs in the tooling?

I think we are producing an incorrect DT, in some instances.

So we are starting from the same baseline, this is specific to ACPI
systems, as an ACPI system won't have a DT from the bootloader.  DT
based systems will already have a DT from the bootloader which is
assumed to be correct.  On ACPI systems without a DT, efistub
generates a default one.

That default is assumed to be for a 32-bit system.  The cell width
defaults to 1, which is 4 bytes.  You cannot represent a 64-bit
value in that instance.

What happens is that kexec inserts properties into the fdt which
contain the start address and size on the crash kernel.  On our
system, the start address is a 64-bit value, and while its not the
case today, I see no reason why size could not also be a 64-bit
value.  However the values that are inserted into the fdt are
governed by the address and size cell values already present in the
fdt.

Kexec attempts to insert these values in the fdt.  The fdt only
accepts 32-bit values, so it truncates what is put in.  Then later
kexec/kdump read the values from the fdt, and get garbage.

I take it this is specific to the kdump properties?

I can't immediately see what would matter for the !kdump case.
properties inserted under /chosen are not truncated?

The kexec/kdump properties are added under /chosen, therefore yes, properties added under /chosen are truncated, per our observations.


By changing the defaults to 2 (the proposed change), 64-bit values
can be inserted into the fdt, so the values we put in don't get
truncated, and thus kexec/kdump read the correct thing when they
need the values.

I don't see how the tools could be fixed - fdt is truncating the
values, and the generated fdt is already "static" at the point the
tools run. We haven't had luck changing the cell size at the point
the tools run. Additionally, this seems to be an issue for
everything using the fdt - pushing the problem to every tool instead
of fixing once at the top seems like playing a game of whack-a-mole.

Does that clarify that issue for you?  Obviously the commit text
needs some work, but I'd like to get on the same page first.

If you could update the commit message to explicitly mention the
properties being inserted for which this matters, this generally sounds
fine to me.

Please Cc me on v2.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



--
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux