Re: [PATCH v3 1/2] EmbeddedPkg/AndroidBoot: boot android kernel from storage

[Leif Lindholm <leif.lindholm@linaro.org>]

On Sun, Jul 30, 2017 at 10:22:15PM +0800, Jun Nie wrote:
> >> >>>>+  Status = EfiGetSystemConfigurationTable (&gFdtTableGuid, (VOID **)&FdtBase);
> >> >>>>+  if (!EFI_ERROR (Status)) {
> >> >>>
> >> >>>Should this not be
> >> >>>   if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND)
> >> >>>?
> >> >>
> >> >>I mean, if fdt is found, we shall return to avoid installing another fdt.
> >> >
> >> >An FDT presented to you by firmware is just the hardware description.
> >> >Any command line or initrd updates that are required will still need
> >> >to happen in order to boot. So the same manipulations that happen to
> >> >the DT embedded in boot.img need to happen to one presented via a
> >> >configuration table.
> >> >
> >> >>But actually, I expect fdt is tied with kernel in Android boot image in
> >> >>standard Android boot image usage cases.
> >> >>Though it is agreed to decouple fdt and kernel in community in 2013,
> >> >>Android boot image format has been decided several years before that :) .
> >> >>
> >> >>We can make change in future if Android boot image usage case changes.
> >> >>Maybe I can add a warning message to highlight the new case.
> >> >
> >> >No.
> >> >
> >> >We can tolerate booting broken existing images, but we should not
> >> >design to intentionally break things ourselves.
> >> >
> >> >I guess as this is an application, you could even add a command-line
> >> >option to let you override an existing registered DT with one embedded
> >> >in boot.img.
> >> >
> >> >But ignoring an existing registered DT is not an option.
> >>
> >> So you suggest to override existing registered fdt data with the one in
> >> boot.img.
> >
> > No, I think that is a horrible idea, but you are saying that some
> > platforms are so fundamentally broken as to need a different DT for
> > every different kernel image.
> >
> > If this is the case, I can stretch to accepting a mechanism to
> > override the sane default behaviour of using the existing device tree
> > provided by the platform.
> >
> > But the platform registered device tree will still need the same
> > chosen node updates as the one extracted from boot.img.
> >
> >> How to add a command-line, in kernel argument that is embedded in
> >> boot.img? It is a bit strange if so.
> >
> > The command line argument would be to AndroidBootApp, not the kernel.
> >
> >> I prefer to override existing registered fdt data directly in current
> >> Android boot image usage.
> >
> > The alternative to using the registered device tree by default, with a
> > mechanism to override, is to always use the registered device tree and
> > always ignore anything provided in boot.img.
> 
> So the solution should be like this. the existing registered device tree will be
> used and chosen node will be updated. If no existing registered device tree
> is found, the one provided by boot.img will be used. As I do not expect
> existing registered device tree case for Android boot usage, fdt in boot.img
> will be used in most cases. If exception happens, we can check the detail
> situation.

Sure, that works for me.

/
    Leif