On 07/02/2019 14:45, Jan Beulich wrote:
>>>> On 07.02.19 at 14:29, <andrew.coop...@citrix.com> wrote:
>> On 07/02/2019 12:58, Jan Beulich wrote:
>>>>>> On 06.02.19 at 21:41, <andrew.coop...@citrix.com> wrote:
>>>> 2) The reported
>>>>
>>>> Dom0 alloc.: 000000003e800000->000000003ec00000 (240470 pages to be
>>>> allocated)
>>>>
>>>> line changes by 1 page because of the alloc_domheap_page() moving ahead
>>>> of
>>>> the printk(), but I'm fairly sure this is benign. There is a matching
>>>> reduction in the length of the constructed m2p which is perhaps less
>>>> benign.
>>> Well, the M2P of course has to be correctly sized. An off-by-one would
>>> likely result in hard to repro bug reports.
>>
>> The delta in output (with some of my own debugging) is:
>>
>> @@ -22,13 +22,13 @@
>> (XEN) p2m_base = 0xffffffffffffffff
>> (XEN) Xen kernel: 64-bit, lsb, compat32
>> (XEN) Dom0 kernel: 32-bit, PAE, lsb, paddr 0x100000 -> 0x112000
>> -(XEN) ** nr_pages 241494
>> +(XEN) ** nr_pages 241493
>> (XEN) PHYSICAL MEMORY ARRANGEMENT:
>> -(XEN) Dom0 alloc.: 000000003e800000->000000003ec00000 (240470 pages to
>> be allocated) (tot 1024, nr 241494)
>> +(XEN) Dom0 alloc.: 000000003e800000->000000003ec00000 (240469 pages to
>> be allocated) (tot 1024, nr 241493)
>> (XEN) VIRTUAL MEMORY ARRANGEMENT:
>> (XEN) Loaded kernel: 0000000000100000->0000000000112000
>> (XEN) Init. ramdisk: 0000000000112000->0000000000112000
>> -(XEN) Phys-Mach map: 0000000000112000->00000000001fdd58
>> +(XEN) Phys-Mach map: 0000000000112000->00000000001fdd54
>> (XEN) Start info: 00000000001fe000->00000000001fe4b4
>> (XEN) Xenstore ring: 0000000000000000->0000000000000000
>> (XEN) Console ring: 0000000000000000->0000000000000000
>>
>> I meant the P2M rather than M2P, and it is different by 1 entry which is
>> expected, given the change by 1 page. I've positively identified the
>> 1-page change to be the alloc_domheap_page() for the monitor table moving.
>
> But the P2M size isn't supposed to change overall - the same number
> of pages get added to the domain. IOW I can see why the "Dom0
> alloc.:" changes (and without bad side effects), but I'm having trouble
> seeing how a P2M size change can be correct (and I suspect there
> would be a problem if previously it went just one slot past a page
> boundary).
>
>>>> @@ -606,23 +598,14 @@ int __init dom0_construct_pv(struct domain *d,
>>>> {
>>>> maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l4_page_table;
>>>> l4start = l4tab = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
>>>> + clear_page(l4tab);
>>>> + init_xen_l4_slots(l4tab, _mfn(virt_to_mfn(l4start)),
>>>> + d, INVALID_MFN, true);
>>>> + v->arch.guest_table = pagetable_from_paddr(__pa(l4start));
>>>> }
>>>> else
>>>> - {
>>>> - page = alloc_domheap_page(d, MEMF_no_owner | MEMF_no_scrub);
>>>> - if ( !page )
>>>> - panic("Not enough RAM for domain 0 PML4\n");
>>>> - page->u.inuse.type_info = PGT_l4_page_table|PGT_validated|1;
>>>> - l4start = l4tab = page_to_virt(page);
>>>> - maddr_to_page(mpt_alloc)->u.inuse.type_info = PGT_l3_page_table;
>>>> - l3start = __va(mpt_alloc); mpt_alloc += PAGE_SIZE;
>>> This one is lost without replacement, but is needed. Commit
>>> 7a9d764630 ("x86/32-on-64: adjust Dom0 initial page table layout")
>>> specifically introduced it to make sure the guest-perceived top level
>>> page table is allocated first (and hence marks the beginning of the
>>> boot page tables, so Dom0 can later put all of them into general use).
>>
>> I did call this out specifically in the commit message. I had no idea
>> about that commit when editing the code, but I still don't understand
>> why it is important that the guests top level needs to be first.
>
> The start info field "pt_base" is specified to point at the root table.
> If the root table isn't first, it's harder for the kernel to know where
> the counting of "nr_pt_frames" actually starts (see Linux'es
> xen_find_pt_base(), which tells me that nowadays they do that
> extra scanning, but iirc this hadn't been there from the beginning).
Before I introduced xen_find_pt_base() 32-bit pv domains just assumed
there could be 2 page tables located before PGD.
There is an exhaustive comment in Xen's include/public/xen.h in this
regard.
> Furthermore your change even violates the specification, as
> "pt_base" no longer points at the root table; you'd have to undo
This is of course a major problem.
pt_base is similar to "where cr3 is supposed to point at".
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
