[PATCH 0/3] cmd: pxe: support INITRD and FDT selection with FIT

Tue Jan 3 17:32:05 CET 2023

Hi Patrick,

On 1/3/23 17:10, Patrick DELAUNAY wrote:
> 
> On 12/13/22 15:48, Quentin Schulz wrote:
>> Hi Patrick,
>>
>> On 12/13/22 15:34, neil.armstrong at linaro.org wrote:
>>> On 13/12/2022 15:31, Patrick DELAUNAY wrote:
>>>> Hi,
>>>>
>>>> On 11/22/22 20:43, Neil Armstrong wrote:
>>>>> On 22/11/2022 20:11, Neil Armstrong wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 21/11/2022 13:23, Quentin Schulz wrote:
>>>>>>> Hi Patrick,
>>>>>>>
>>>>>>> Thanks for looking at it.
>>>>>>>
>>>>>>> On 10/28/22 11:01, Patrick Delaunay wrote:
>>>>>>>>
>>>>>>>> Since the commit d5ba6188dfbf ("cmd: pxe_utils: Check 
>>>>>>>> fdtcontroladdr
>>>>>>>> in label_boot") the FDT or the FDTDIR label is required in 
>>>>>>>> extlinux.conf
>>>>>>>> and the fallback done by bootm command when only the device tree 
>>>>>>>> is present
>>>>>>>> in this command parameters is no more performed when FIT is used 
>>>>>>>> for
>>>>>>>> kernel.
>>>>>>>>
>>>>>>>> The next file "extlinux.conf" no more selects the device tree in 
>>>>>>>> FIT
>>>>>>>> but use the pxe fallback with the device tree of U-Boot.
>>>>>>>>
>>>>>>>> menu title Select the boot mode
>>>>>>>> DEFAULT test
>>>>>>>> LABEL test
>>>>>>>>          KERNEL /fitImage
>>>>>>>>
>>>>>>>> This serie restores the possibility to use a FIT in extlinux.conf
>>>>>>>> by using FDT label with the same string.
>>>>>>>>
>>>>>>>> menu title Select the boot mode
>>>>>>>> DEFAULT test
>>>>>>>> LABEL test
>>>>>>>>          KERNEL /fitImage
>>>>>>>>          FDT /fitImage
>>>>>>>>
>>>>>>>> even when a specific FIT config is used:
>>>>>>>>
>>>>>>>> menu title Select the boot mode
>>>>>>>> DEFAULT test
>>>>>>>> LABEL test
>>>>>>>>          KERNEL /fitImage#config
>>>>>>>>          FDT /fitImage#config
>>>>>>>>
>>>>>>>> The last commit of the series is only a minor improvement.
>>>>>>>>
>>>>>>>
>>>>>>> I tested this on my Puma RK3399 and it does work again, thanks.
>>>>>>>
>>>>>>> However, I'm not sure this is what we should go for?
>>>>>>>
>>>>>>> My worry is the following:
>>>>>>> What happens for old releases (pre-v2022.04) where KERNEL worked 
>>>>>>> just fine without the FDT to load the fdt from the fitimage conf 
>>>>>>> specified in KERNEL field? Would we now need to keep an 
>>>>>>> extlinux.conf for pre-2022.04 releases where FDT wouldn't be set 
>>>>>>> and one for 2023.01 and later where FDT would be mentioned? That 
>>>>>>> does not seem like a good thing for distros.
>>>>>>>
>>>>>>> I unfortunately cannot answer the question myself without 
>>>>>>> spending significant effort patching v2022.01 to get it to work 
>>>>>>> on our Puma module. Does anyone have access to a board to quickly 
>>>>>>> check an extlinux.conf with KERNEL and FDT set to /fitImage on a 
>>>>>>> v2022.01 release?
>>>>>>
>>>>>> I'm building kirkstone images with meta-meson having v2022.01, 
>>>>>> I'll test with FDT set to /fitImage to see if it breaks.
>>>>>
>>>>> It breaks:
>>>>> Found /extlinux/extlinux.conf
>>>>> Retrieving file: /extlinux/extlinux.conf
>>>>> 1:    Yocto
>>>>> Retrieving file: /fitImage
>>>>> append: root=PARTUUID=3ebc0005-02 rootwait console=ttyAML0,115200
>>>>> Retrieving file: /fitImage
>>>>> Bad Linux ARM64 Image magic!
>>>>> SCRIPT FAILED: continuing...
>>>>>
>>>>
>>>> Can you share the extlinux file used for your test ?do you have my 
>>>> patch ?
>>>
>>> It was explicitly without your patch as Quentin asked, he hoped addingh
>>> "FDT /fitImage" would not break u-boot pre d5ba6188dfbf, but no.
>>>
>>
>> Your implementation requires changes in extlinux.conf (which could be 
>> fine, I'm not arguing about this specifically). I however think we 
>> need those required changes to be backward compatible with older U-Boot.
>>
>> This means, a new extlinux.conf that works on newer U-Boot should also 
>> work on older U-Boot without your patch.
>>
>> Otherwise, we would have the following:
>>
>> U-Boot \ extlinux.conf || old | new
>> ====================================
>> old                    || OK  | NOK
>> new                    || NOK | OK
>>
>> What I am hoping for is:
>> U-Boot \ extlinux.conf || old | new
>> ====================================
>> old                    || OK  | OK
>> new                    || NOK | OK
>>
>> or even fix the support for new U-Boot with old extlinux.conf (but 
>> that seems not possible because how d5ba6188dfbf ("cmd: pxe_utils: 
>> Check fdtcontroladdr in label_boot") works?).
>>
> 
> Yes but I don't see any possible solution to solve all the combination 
> with d5ba6188dfbf and without or without FIT:
> 
> the old extlinux.conf with FIT are already no more working as expected 
> with current U-Boot (1)
> 
> when FDT and FDTDIR are absent
> 
> 
>             extlinux.conf || kernel = uImage |    kernel= FIT    | 
> kernel = FIT
> 
>                           ||  FDT absent     | FDT absent        | FDT = 
> FIT (my proposal)
> 
> ============================================================================================
> 
> U-Boot <= v2022.01-rc2   || KO              | OK using DT in FIT   | not 
> supported
> 
> U-Boot >= 2022.01-rc3   || OK (1)         | OK(2)using U-Boot DT | not 
> supported
> 
> next with my proposal    || OK (1)          | OK(2)using U-Boot DT | OK 
> (using DT in FIT)
> 
> 
> (1) with d5ba6188dfbf
> 
> (2) Risk to have unaligned DT between U-Boot (old) and kernel (new)
> 
>      => U-Boot behavior change with 2022.01-rc3(1)
> 
> 
>> This would give an easy migration path to any creator of this 
>> extlinux.conf file by just migrating to the new format while not 
>> requiring it to care about the version of U-Boot being used.
> 
> 
> I agree, it is better to be backward compatible.
> 
> So I search a solution to keep the new feature introduced by 
> d5ba6188dfbf ("cmd: pxe_utils: Check fdtcontroladdr in label_boot") but 
> only when FIT is not used and fallback to previous behavior for FIT....
> 
> 
> but it is too complicated: pxe utils need to load the "kernel_addr" and 
> verify if it is a FIT before to check if "fdtcontroladdr" is provided, 
> but I can test an solution if you have a proposal.
> 
> 
> Moreover I assumed the FIT feature is not largely used in generated 
> extlinux.conf file as the regression introduced by 2022.01-rc2wasn't 
> detected until now.
> 

It's the default in Yocto when you're building fit images :) U-Boot 
2022.01 is only in Yocto since Kirkstone and it's only the default 
version, which is rarely used. Vendors also have a tendency to stay way 
behind and not upgrade U-Boot (we've done this, trying to fix it right 
now, you probably saw the number of patches I sent to fix our poor Puma 
module in the last year :) ).

Anyways, I think we've something we can work with now: 
https://lore.kernel.org/u-boot/20221214064518.753432-1-marex@denx.de/

We need to take this one and then apply 
https://lore.kernel.org/u-boot/20221217174113.7459-1-marex@denx.de/ again.

I'll ask Marek to make this a v3 all together and mark it urgent for 
next release so Tom knows he should merge it ASAP. Don't hesitate to 
provide feedback on those patches because I expect those to be merged 
quite fast.

Cheers,
Quentin