Re: [PATCH] ArmPkg/PlatformBootManagerLib: regenerate boot options on boot failure

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/17/20 4:35 PM, Leif Lindholm wrote:
> On Wed, Jun 17, 2020 at 14:59:45 +0200, Ard Biesheuvel wrote:
>>>>> Something like:
>>>>> "On entry, the UiApp instantiates the autogenerated boot options that
>>>>> we used to rely on - but it does not consume them. This breaks the
>>>>> unattended..."
>>>>
>>>> OK
>>>>
>>>>> I assume the UiApp only ever *adds* entries, which is why checking
>>>>> number of entries is sufficient?
>>>>>
>>>>
>>>> It only manages entries that it instantiated itself, but it may also remove
>>>> entries if the underlying hardware has disappeared.
>>>
>>> Hmm, that's a bit trickier then. I mean, it's unlikely, but I'm sure
>>> there's situations that could happen.
>>> Would we run the risk of getting bug reports like "system fails to
>>> boot from Ethernet when inifiniband switch powered off"? Or if some
>>> virtual devices presented by a BMC appear/disappear?
>>>
>>
>> If the boot entries are not refreshed, you will retain the old ones. So the
>> only way this could lead to a boot failure is when you rely on automatically
>> generated boot entries to device that disappear and reappear in a different
>> place, e.g., move a Ethernet PCIe card to a different slot. Note that USB
>> devices plugged into a different port will still work fine, though, as they
>> rely on the removable boot path in this case, which will be attempted anyway
>> before doing the UnableToBoot().
>>
>> Note that the failure mode here is being dropped into the menu, where before
>> you were always dropped into the Shell. The case we are trying to address
>> here is zero intervention network boot after putting the device into
>> circulation, and that should work correctly with this change: if the network
>> boot path did not exist before, it will be added, in which case the number
>> of boot options will increase.
> 
> OK. I'm not convinced we're not going to see a report of this
> somewhere down the line, but I think you've managed to convince me
> it's an unlikely enough situation, and a fallback, that we can bump it
> to then (and it *is* a behavioural improvement in all other cases).
> 
> Reviewed-by: Leif Lindholm <leif@nuviainc.com>
> (with the commit message update)
> 

Merged as 2d233af64b8f73d1b1e138b302e6344f7c2e0f4e

Thanks all,