On 07/02/2019 13:56, Juergen Gross wrote:
> On 07/02/2019 14:45, Jan Beulich wrote:
>>
>>>>> @@ -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.
Oh - so there is. That is rather more helpful at explaining what is
going on, but there really needs to be a comment in the code.
>
>> 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".
Obviously this is a bug needing fixing.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
