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@suse.com>
---
v2: Refine an expression. Add comment.
---
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) and an apparently wrong comment stating
that not only v_end but also v_start would be superpage aligned (in fact
v_end is 4MiB aligned, which is the superpage size only on long
abandoned [by us] non-PAE x86-32).
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -59,6 +59,16 @@ static __init void mark_pv_pt_pages_rdon
l1e_remove_flags(*pl1e, _PAGE_RW);
page = mfn_to_page(l1e_get_mfn(*pl1e));
+ /*
+ * Verify that
+ * - all pages have a valid L1...Ln page table type and
+ * - no other bits are set, in particular the type refcount is still
+ * zero.
+ */
+ ASSERT((page->u.inuse.type_info & PGT_type_mask) >= PGT_l1_page_table);
+ ASSERT((page->u.inuse.type_info & PGT_type_mask) <= PGT_root_page_table);
+ ASSERT(!(page->u.inuse.type_info & ~PGT_type_mask));
+
/* Read-only mapping + PGC_allocated + page-table page. */
page->count_info = PGC_allocated | 3;
page->u.inuse.type_info |= PGT_validated | 1;
On 17/08/2021 15: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@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com>, thanks. Are you planning further cleanups here imminently? If not, I can probably shuffle some of the easy ROUNDUP() refactoring in the direction of an intern or grad. ~Andrew
On 17.08.2021 19:25, Andrew Cooper wrote: > On 17/08/2021 15: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@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citirx.com>, thanks. > > Are you planning further cleanups here imminently? If not, I can > probably shuffle some of the easy ROUNDUP() refactoring in the direction > of an intern or grad. Not any cleanup, I don't think, but quite a few further changes to make Dom0 use large IOMMU page mappings where possible (without introducing logic yet to un-shatter split pages, not the least because relying on just that would then undermine the secondary effect of improving boot time). The two changes posted so far were really just fallout from that work, with it seeming reasonable to post up front to reduce the size of the actual series, which has grown quite a bit longer than I though it would. Jan
© 2016 - 2024 Red Hat, Inc.