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

AKASHI, Takahiro
2017-02-08T07:43:04Z

Hi Mark,

Thank you for this heads-up.

On Tue, Feb 07, 2017 at 07:55:59PM +0000, Mark Rutland wrote:
> On Tue, Feb 07, 2017 at 12:29:38PM -0700, Jeffrey Hugo wrote:
> > On 2/7/2017 12:13 PM, Mark Rutland wrote:
> > >On Tue, Feb 07, 2017 at 12:07:56PM -0700, Jeffrey Hugo wrote:
> > >>On 2/7/2017 12:01 PM, Mark Rutland wrote:
> > >>>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.
> > >
> > >Sorry for the dodgy (and confusing) reply above.
> > >
> > >What I was trying to ask was does this *only* affect kdump properties?
> > >I note that kdump is not yet upstream for arm64.
> > >
> > >Or are there regular kexec properties that this affects?
> > >
> > >Or !kdump && !kexec properties?
> > >
> > >Can you please enumerate the set of properties for which this matters?
> >
> > "linux,elfcorehdr" and "linux,usable-memory-range"
> >
> > We are not aware of !kdump && !kexec properties where this is an issue.
>
> Ok, I understand the problem now. Thanks for clarifying the !kdump
> situation.
>
> As per my reply to Timur, given this only affects kdump, this is all
> happening in code that is not upstream, and therefore this is not
> *currently* an issue.
>
> In future, please report this kind of issue in reply to relevant
> postings (e.g. [1]). At the very least, refer to these, with relevant
> people Cc'd (e.g. Takahiro-san in this case).
>
> I think there are three things which should happen:
>
> (a) The userspace kexec-tools kdump code should take /#address-cells and
> /#size-cells into account when inserting the linux,elfcorehdr and
> linux,usable-memory-range properties. There can be DTs for 64 bit
> platforms where these are not 2.
>
> Takahiro-san, from looking at your kexec-tools repo, this is not
> currently the case. Could you address that?

Yup, I will, but if this is the case,
we might need to think about another case where /#address-cells and /#size-cells
are <1> but the range of crash dump kernel can be still 64-bit wide since
the system memory information comes from ACPI table (not DT).

Therefore, the properties in this case should look like:
/ {
#address-cells = <1>;
#size-cells = <1>;
chosen {
...
linux,usable-memory-range {
#address-cells = <2>;
#size-cells = <2>; may be omitted if possible
reg = < ... >;
}
linux,elfcorehdr {
#address-cells = <2>;
#size-cells = <2>; may be omitted if possible
reg = < ... >;
}
...
}
}

Is this what you meant?
(Obviously, I will have to modify the kernel patches as well.)

Thanks,
-Takahiro AKASHI

> (b) The kdump documentation should updated to explicitly state that
> these properties are #address-cells + #size-cells tuples, since this
> is clearly going to be confusing either way. I'll try to come up
> with wording for that, and I'll reply on the current [1] kdump
> thread.
>
> (c) Regardless, when creating the empty DT the EFI stub should insert
> /#address-cells = <2> and /#size-cells = <2>. That better aligns
> with the usual requirements for an otherwise empty DTB, and avoids a
> tonne of fragility when the DTB is passed on or altered by other
> agents.
>
> So, please respin this patch, stating explicitly what this matters for.
>
> Thanks,
> Mark.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-February/486234.html