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

[Mark Rutland <mark.rutland@arm.com>]

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?

(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