git: 0c839497c174 - stable/13 - loader.efi: There are systems without ConOut, also use ConOutDev

[Toomas Soome]

> On 5. Feb 2021, at 22:21, Warner Losh <imp at bsdimp.com> wrote:
> 
> 
> 
> On Fri, Feb 5, 2021 at 1:09 PM Toomas Soome <tsoome at me.com <mailto:tsoome at me.com>> wrote:
> 
> 
>> On 5. Feb 2021, at 21:43, Warner Losh <imp at bsdimp.com <mailto:imp at bsdimp.com>> wrote:
>> 
>> 
>> 
>> On Fri, Feb 5, 2021 at 10:24 AM Toomas Soome <tsoome at me.com <mailto:tsoome at me.com>> wrote:
>> 
>> 
>>> On 5. Feb 2021, at 18:44, Warner Losh <imp at bsdimp.com <mailto:imp at bsdimp.com>> wrote:
>>> 
>>> 
>>> 
>>> On Thu, Feb 4, 2021 at 11:38 PM Toomas Soome <tsoome at me.com <mailto:tsoome at me.com>> wrote:
>>> 
>>> 
>>>> On 5. Feb 2021, at 01:56, Warner Losh <imp at bsdimp.com <mailto:imp at bsdimp.com>> wrote:
>>>> 
>>>> 
>>>> And why the instaMFC? Changes are supposed to cook force days before merging... I have questions about the wisdom of this change...
>>>> 
>>>> Warner 
>>>> 
>>> 
>>> Reason is in PR. There is someone with the system without ConOut but ConOutDev is set. Instead of falling back to arbitrary device (which in this case was totally wrong choice), we can try the possible devices list. We do not change the ConOut parsing.
>>> 
>>> We could have the same effect defaulting to Video. This bug should have been discussed / reviewed before it was committed.
>> 
>> How is is different from defaulting to serial, it is just as bad? we can not guess there, thats why we do have ConOutDev list.
>> 
>> 
>>> 
>>> If it would appear, there are systems with unusable devices listed in ConOutDev, then we need to think how to handle such case.
>>> 
>>> Yes. We fall back to the arbitrary device... It's just a flag that can be overridden. We can easily fall back to video too.
>> 
>> We *should not* fall back on arbitrary devices when there is source for alternate options. And that option is from specification:
>> 
>> "The ConInDev, ConOutDev, and ErrOutDev variables each contain an EFI_DEVICE_PATH_PROTOCOL descriptor that defines all the possible default devices to use on boot. These variables are volatile, and are set dynamically on every boot. ConIn, ConOut, and ErrOut are always proper subsets of ConInDev, ConOutDev, and ErrOutDev.”
>> 
>> Right. Except they aren't a proper subset in this case. Since they aren't a proper subset, can you count on them having any meaningful meaning? In the cases you found they do, but it's just as arbitrary.
> 
> Well, we can argue if empty set is or is not subset of (any) other set. But, we do have specification. And we should not arbitrary pick what part we are going to follow and what not.
> 
> The empty set isn't a proper subset. It is a subset, but not a proper subset, by definition. Therefore, it's not standards complaint.
>>  
>> UEFI Spec 2.8A, Page 82.
>> 
>> There may or may not be Video (or Serial) device listed. If not, there are good chance we will be in trouble because the firmware internals may not be set up properly and we can just as well end up in hung system.
>> 
>> For x86, there's almost always Video. For !x86 it gets more troublesome.
> 
> There is almost always ConOut as well. Except when there is not. And in this case, there is not.
> 
> I still do not get what it is we are arguing about. Do we have case where ConOut is not set and ConOutDev does have garbage?
> 
> We have one sighting of 'ConOut' being missing. Any extrapolation from there is tricky. We know in this one case the info appears to be good. But this is not standards compliant, so how can we be sure that others will be similar?
> 
> Warner

It is  tricky, agree. But we also have other assumptions. Like we can get handles for all devices (vmware devs at one point in time did decide to only give handles for devices used at boot time - no handles fot non-boot disks, serial ports and such).

But we are not going to tell user to buy better computer, do we?:) Otherwise, we should tell the same for other non-standard cases we already do know about…

Thanks for all the feedback,
toomas