Re: [Xen-devel] [PATCH v15 05/10] x86: add multiboot2 protocol support for EFI platforms

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 2/16/17 3:49 PM, Daniel Kiper wrote:
> On Thu, Feb 16, 2017 at 02:29:45AM -0700, Jan Beulich wrote:
>>>>> On 15.02.17 at 22:53, <daniel.kiper@xxxxxxxxxx> wrote:
>>> On Wed, Feb 15, 2017 at 03:22:02AM -0700, Jan Beulich wrote:
>>>>>>> On 14.02.17 at 19:38, <daniel.kiper@xxxxxxxxxx> wrote:
>>>>> --- a/xen/arch/x86/boot/head.S
>>>>> +++ b/xen/arch/x86/boot/head.S
>>>>> @@ -394,10 +394,18 @@ __start:
>>>>>
>>>>>          /* EFI IA-32 platforms are not supported. */
>>>>>          cmpl    $MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx)
>>>>> +        /*
>>>>> +         * Here we should zap vga_text_buffer. However, we can disable
>>>>> +         * VGA updates in simpler and more reliable way later.
>>>>> +         */
>>>>>          je      .Lmb2_efi_ia_32
>>>>>
>>>>>          /* Bootloader shutdown EFI x64 boot services. */
>>>>>          cmpl    $MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx)
>>>>> +        /*
>>>>> +         * Here we should zap vga_text_buffer. However, we can disable
>>>>> +         * VGA updates in simpler and more reliable way later.
>>>>> +         */
>>>>>          je      .Lmb2_no_bs
>>>>
>>>> I'm afraid I don't view these comments as helpful in understanding
>>>> the whole situation. That's partly because I don't follow both the
>>>> "simpler" and "more reliable" parts (using just the information here,
>>>
>>> OK, I will clarify it.
>>>
>>>> i.e. leaving aside what you've given as explanation earlier, albeit I
>>>> don't think that was fully clarifying things either), and partly
>>>> because I continue to think that the explanation should go where
>>>> the labels are (which is what I had meant to suggest with my
>>>> comment placement in reply to v14). Nor does the adjustment
>>>
>>> OK.
>>>
>>>> above help (me) understand the correctness of the dual use of
>>>> .Lmb2_no_bs.
>>>
>>> What do you mean by "dual use of .Lmb2_no_bs."? I would like to be sure.
>>
>> As said in v14 review, it's being jumped to from two rather different
>> places, and hence the VGA aspect isn't obviously the same for both.
> 
> OK, I will try to clarify. If a bootloader called us using __efi64_mb2_start
> we are sure that we are running on EFI platform and there is no VGA there.
> It means that we can safely zap vga_text_buffer unconditionally in first steps
> (we do that in second instruction). Then we do not need to take care about
> that in case of error. And one of these errors is lack of 
> MULTIBOOT2_TAG_TYPE_EFI_BS
> tag. It means that EFI boot services are shutdown. So, we are in black hole.
> We have to inform user about that and halt the system. And that is why we

Not looking at the code but the words here. If ExitBootServices() has
been called we should be able to still boot if the memory map was passed
along. Are we deferring that use case to a follow on?

-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel