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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.09.16 at 11:56, <daniel.kiper@xxxxxxxxxx> wrote:
> On Wed, Sep 28, 2016 at 03:06:31AM -0600, Jan Beulich wrote:
>> >>> On 27.09.16 at 21:55, <daniel.kiper@xxxxxxxxxx> wrote:
>> > On Mon, Sep 26, 2016 at 09:03:30AM -0600, Jan Beulich wrote:
>> >> >>> On 23.09.16 at 23:47, <daniel.kiper@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel