Re: [PATCH v7 12/14] x86: make Xen early boot code relocatable

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, david.vrabel@citrix.com,
	xen-devel@lists.xenproject.org, qiaowei.ren@intel.com,
	gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v7 12/14] x86: make Xen early boot code relocatable
Date: Wed, 28 Sep 2016 04:19:49 -0600	[thread overview]
Message-ID: <57EBB56502000078001133BF@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20160928095633.GQ28753@olila.local.net-space.pl>

>>> On 28.09.16 at 11:56, <daniel.kiper@oracle.com> wrote:
> On Wed, Sep 28, 2016 at 03:06:31AM -0600, Jan Beulich wrote:
>> >>> On 27.09.16 at 21:55, <daniel.kiper@oracle.com> wrote:
>> > On Mon, Sep 26, 2016 at 09:03:30AM -0600, Jan Beulich wrote:
>> >> >>> On 23.09.16 at 23:47, <daniel.kiper@oracle.com> wrote:
>> >> > +1:      mov     %eax,sym_fs(l2_bootmap)-8(%ebx,%ecx,8)
>> >> > +        mov     %eax,sym_fs(l2_identmap)-8(%ebx,%ecx,8)
>> >> > +        sub     $(1<<L2_PAGETABLE_SHIFT),%eax
>> >> > +        loop    1b
>> >> > +
>> >> > +        /* Initialize L3 boot-map page directory entry. */
>> >> > +        lea     __PAGE_HYPERVISOR+(L2_PAGETABLE_ENTRIES*8)*3+sym_esi(l2_bootmap),%eax
>> >> > +        mov     $4,%ecx
>> >> > +1:      mov     %eax,sym_fs(l3_bootmap)-8(,%ecx,8)
>> >> > +        sub     $(L2_PAGETABLE_ENTRIES*8),%eax
>> >> > +        loop    1b
>> >> >
>> >> >          /*
>> >> >           * During boot, hook 4kB mappings of first 2MB of memory into L2.
>> >> > -         * This avoids mixing cachability for the legacy VGA region, and is
>> >> > -         * corrected when Xen relocates itself.
>> >> > +         * This avoids mixing cachability for the legacy VGA region.
>> >> >           */
>> >> > -        mov     $sym_phys(l1_identmap)+__PAGE_HYPERVISOR,%edi
>> >> > -        mov     %edi,sym_phys(l2_xenmap)
>> >> > +        lea     __PAGE_HYPERVISOR+sym_esi(l1_identmap),%edi
>> >> > +        mov     %edi,sym_fs(l2_bootmap)
>> >>
>> >> Switching from l2_xenmap to l2_bootmap here?
>> >
>> > Do we need first 2 MiB mapped in Xen image mapping? It looks that we do not.
>> > Am I missing something?
>>
>> My point here isn't that the change is wrong, but that it's not
>> immediately obvious and hence should be explained in the commit
>> message. After all the need for the mapping of these 2Mb does not
>> - aiui - go away with this patch, but (presumably) with the earlier one
>> moving the load address up to 2Mb. I.e. it can generally be viewed as
>> an independent adjustment.
> 
> Probably. Should I move this change to patch #10 (x86: change default load
> address from 1 MiB to 2 MiB) or leave it here with relevant comment?
> Or do you wish separate patch for it?

I think putting it in the other patch would be best, unless it would
end up as non-obvious as it did here, in which case a separate
patch would probably be better.

Jan

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