[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/PV: assert page state in mark_pv_pt_pages_rdonly()

  • To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Mon, 16 Aug 2021 20:25:40 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=B3Y0Cyf/fLxod5qFC7RFXVKHndxprCsGWTyFr1qqzoo=; b=DBJnZHUR6ZRLb7eP90+kBqphvmnemAx20PQfknoBabCEWc/X8RRj9JtdnesP2ZCxUphIa5j4E9Wo0GCmYW9BWw3K5+GqLVHj86TIktNTxYtwD+Suw9zFG0bdeTv/R+IZB905W+2FVX8DahGwCPFGp6D1RWkzAJNcxGhIsqA5dpa0/bcWBSb4DrA7a0bplzRAnfKLRE13LPQALkORXQ+7Ivo9PIYyxCwxU1UqRihmQIwP8dES0apgFCw5PdzRyGTa74WhqmtKytaxRJ4Sc4RJCMrGaU4BByJStDySdpWlLZ1ba27WVwc6c1Md7SvR+1TmhJSn9BOdUZf1pmA3kKRbfw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oPyFQ6aPCt/gSTw77Q/VbDkVxHi58M3ASkfnMGZCFsAXQbpm7qDZmqAeCJsTvLGdKWusrirTRilUvugwI+D1oporWRPFI660v++rPe1zuvDjy+/DWJatJsuPpLIwBjyTbIQx4CxYK178sjpaXlkokt9UhyICxN+hpvF6FJZyIScAIbWtMsslTLbDBrUKKsNkz4t+3sunag5cHj9TbbfwgfJWhmRl0tUuliVTxKgRKWr/P1JE45GQpVNaBu0alUzAcSccMtxN2KHHiXOkQhNTPyW/nqdrc2mK29IrQpwEFddfXotfl9enPMGy0hUPL1cLp6ZYsyGXUP1xk/7r5uxYkQ==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Mon, 16 Aug 2021 19:26:16 +0000
  • Ironport-hdrordr: A9a23:pSVE0q/Nj342pcdlCWNuk+AiI+orL9Y04lQ7vn2ZKSY5TiVXra CTdZUgpHvJYVMqMk3I9uruBEDtex3hHP1OkOws1NWZLWrbUQKTRekP0WKL+Vbd8kbFh4xgPM lbEpSXCLfLfCVHZcSR2njFLz73quP3j5xBho3lvglQpRkBUdAG0+/gYDzraXGfQmN9dPwEPa vZ3OVrjRy6d08aa8yqb0N1JdQq97Xw5evbiQdtPW9e1DWz
  • Ironport-sdr: mRVdNEgzFTb0UO/nl2MLCBvvC7cuSfcdTU0SS7da11bua5VJfu0Jb7PEdHsuq+Q0iHPd2zdXVp 6tRRtMUZCmqTcZo9lAB8aRTEHfqmIpp1tBw+I2Y9HDBtI49ewS7QbhVuhZXYHjHY8bIxZ00ul7 v6zBGyE3MPpeaMI73AWaLkoFF8jaSaKJLGJIyl+1dv5k9RRRCqrOatYCgKrEG5mZ6g3x6ICd+9 i1vOzdaMMih7VKJQzXk7Sj0lrGQwAkZ5x27sqpL1aNg6abqnogMoyrsj6PvcZ5u9pGYJhXwcck E8ae0CG1Zs6tU7vysQq8j1zV
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 16/08/2021 16:29, Jan Beulich wrote:
> About every time I look at dom0_construct_pv()'s "calculation" of
> nr_pt_pages I question (myself) whether the result is precise or merely
> an upper bound. I think it is meant to be precise, but I think we would
> be better off having some checking in place. Hence add ASSERT()s to
> verify that
> - all pages have a valid L1...Ln (currently L4) page table type and
> - no other bits are set, in particular the type refcount is still zero.
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> There are (at least) two factors supporting my uncertainty about the
> "calculation" being precise: The loop starting from 2 (which clearly is
> too small for a possible result)

2 was the correct absolute minimum for 2-level guests.

XTF kernels don't exceed the 2M boundary (at least, not currently), so
they can be mapped with only 3 or 4 pagetables, except:

* 3-level guests are created with 4 L2's for no obvious reason.  This is
nothing to do with legacy PAE paging, nor with how a typical Linux/BSD
kernel works.  The requirement to make 3-level guests work (and even
then, only under 32bit Xen) is to create a PGT_pae_xen_l2 if not already
covered by the other mappings.  Any non-toy kernel discards these
pagetables in favour of its own idea of pagetables.

* v_end is rounded up to 4MB.

Most XTF guests will operate entirely happily in a few hundred kb of
space, and the same will be true of other microservices.  The rounding
up of memory might be helpful for the traditional big VMs case, but it
isn't correct or useful for other usecases.

> and an apparently wrong comment stating
> that not only v_end but also v_start would be superpage aligned

Which comment?  The only one I see about 4M has nothing to do with

>  (in fact
> v_end is 4MiB aligned, which is the superpage size only on long
> abandoned [by us] non-PAE x86-32).

Tangentially, that code needs some serious work to use ROUNDUP/DOWN
macros for clarity.

> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -59,6 +59,10 @@ static __init void mark_pv_pt_pages_rdon
>          l1e_remove_flags(*pl1e, _PAGE_RW);
>          page = mfn_to_page(l1e_get_mfn(*pl1e));
> +        ASSERT(page->u.inuse.type_info & PGT_type_mask);
> +        ASSERT((page->u.inuse.type_info & PGT_type_mask) <= 
> PGT_root_page_table);

This is an obfuscated

ASSERT((page->u.inuse.type_info & PGT_type_mask) >= PGT_l1_page_table &&
       (page->u.inuse.type_info & PGT_type_mask) <= PGT_root_page_table);


> +        ASSERT(!(page->u.inuse.type_info & ~PGT_type_mask));

this has no context.

At a bare minimum, you need a comment stating what properties we're
looking for, so anyone suffering an assertion failure has some clue as
to what may have gone wrong.




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.