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

From: "Jan Beulich" <JBeulich@suse.com>
To: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Juergen Gross <JGross@suse.com>,
	sstabellini@kernel.org, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	julien.grall@arm.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v15 05/10] x86: add multiboot2 protocol support for EFI platforms
Date: Thu, 16 Feb 2017 02:29:45 -0700	[thread overview]
Message-ID: <58A57F19020000780013A903@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20170215215313.GI3484@olila.local.net-space.pl>

>>> On 15.02.17 at 22:53, <daniel.kiper@oracle.com> wrote:
> On Wed, Feb 15, 2017 at 03:22:02AM -0700, Jan Beulich wrote:
>> >>> On 14.02.17 at 19:38, <daniel.kiper@oracle.com> 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.

Jan

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