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

[Mark Rutland]

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 at codeaurora.org> wrote:
> >>From: Sameer Goel <sgoel at codeaurora.org>
> >>
> >>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?

> 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.