[PATCH 7/7] x86/mm: simplify create_perdomain_mapping() interface
George Dunlap <[email protected]>
| Newsgroups | org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <[email protected]> |
From: Roger Pau Monné <[email protected]> create_perdomain_mapping()'s interface is richer than any caller needs. The paging structure for the requested range is built to a depth selected by the pl1tab and ppg arguments, each of which distinguishes NULL from NIL() from a real pointer: - nr == 0: only ensure the per-domain L3 exists; nothing else is allocated, and the other arguments are ignored. - pl1tab == a pointer: allocate the L1 tables covering the range, and return their addresses in the array -- the mode that existed to build the GDT/LDT stash. - pl1tab == NIL(): allocate the L1 tables, and return nothing. - pl1tab == NULL: do not plumb L1 tables for their own sake (they are still allocated on demand if data-page population requires them). - ppg == a pointer: allocate and install zeroed data pages across the range, and return their struct page_info pointers in the array. - ppg == NIL(): allocate and install the zeroed data pages, but hand nothing back; the pages are reachable only through the mapping. - ppg == NULL: do not allocate data pages. - both NULL, nr > 0: stop after the slot's L2; do not plumb L1 tables at all. Very few of these modes have users now. The last user of the pl1tab capture mode was removed when we removed the GDT/LDT stash. The ppg capture mode never had any users. Nothing uses the both-NULL L2-only mode with nr != 0. What remains is exactly one bit of information: whether the caller wants the range populated with zeroed, area-owned data pages, or merely plumbed down to the L1 tables, ready for populate_perdomain_mapping() to install caller-owned pages. Replace the two arguments with a boolean expressing that bit. With the stashing mode gone the NIL()/IS_NIL() macros lose their last user, so drop them as well; and document the resulting interface. No caller changes behaviour: every existing call maps onto the boolean exactly. Signed-off-by: Roger Pau Monné <[email protected]> Assisted-by: Claude Code:claude-fable-5 Signed-off-by: George Dunlap <[email protected]> --- Changes since the previously posted version: - Drop the now-unused NIL()/IS_NIL() macros as requested during review - Describe the prior interface in the commit message and add a doc comment for the simplified one. --- xen/arch/x86/domain_page.c | 10 ++++----- xen/arch/x86/hvm/hvm.c | 2 +- xen/arch/x86/include/asm/mm.h | 6 +----- xen/arch/x86/mm.c | 40 +++++++++++++++++++++++------------ xen/arch/x86/pv/domain.c | 4 +--- xen/arch/x86/x86_64/mm.c | 3 +-- 6 files changed, 35 insertions(+), 30 deletions(-) diff --git a/xen/arch/x86/domain_page.c b/xen/arch/x86/domain_page.c index 72c00194f3..1c1deeeebc 100644 --- a/xen/arch/x86/domain_page.c +++ b/xen/arch/x86/domain_page.c @@ -246,8 +246,7 @@ int mapcache_domain_init(struct domain *d) spin_lock_init(&dcache->lock); return create_perdomain_mapping(d, (unsigned long)dcache->inuse, - 2 * bitmap_pages + 1, - NIL(l1_pgentry_t *), NULL); + 2 * bitmap_pages + 1, false); } int mapcache_vcpu_init(struct vcpu *v) @@ -264,16 +263,15 @@ int mapcache_vcpu_init(struct vcpu *v) if ( ents > dcache->entries ) { /* Populate page tables. */ - int rc = create_perdomain_mapping(d, MAPCACHE_VIRT_START, ents, - NIL(l1_pgentry_t *), NULL); + int rc = create_perdomain_mapping(d, MAPCACHE_VIRT_START, ents, false); /* Populate bit maps. */ if ( !rc ) rc = create_perdomain_mapping(d, (unsigned long)dcache->inuse, - nr, NULL, NIL(struct page_info *)); + nr, true); if ( !rc ) rc = create_perdomain_mapping(d, (unsigned long)dcache->garbage, - nr, NULL, NIL(struct page_info *)); + nr, true); if ( rc ) return rc; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 955fc062a5..e8fbe7ba27 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -620,7 +620,7 @@ int hvm_domain_initialise(struct domain *d, INIT_LIST_HEAD(&d->arch.hvm.mmcfg_regions); INIT_LIST_HEAD(&d->arch.hvm.msix_tables); - rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, NULL, NULL); + rc = create_perdomain_mapping(d, PERDOMAIN_VIRT_START, 0, false); if ( rc ) goto fail; diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h index 1888807394..30eaec9179 100644 --- a/xen/arch/x86/include/asm/mm.h +++ b/xen/arch/x86/include/asm/mm.h @@ -600,12 +600,8 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg); long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg); int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg); -#define NIL(type) ((type *)-sizeof(type)) -#define IS_NIL(ptr) (!((uintptr_t)(ptr) + sizeof(*(ptr)))) - int create_perdomain_mapping(struct domain *d, unsigned long va, - unsigned int nr, l1_pgentry_t **pl1tab, - struct page_info **ppg); + unsigned int nr, bool populate); void populate_perdomain_mapping(const struct vcpu *v, unsigned long va, const mfn_t *mfn, unsigned int nr, unsigned int flags); diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 1810971677..aa26d12ac2 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -6211,9 +6211,33 @@ static bool perdomain_l1e_needs_freeing(l1_pgentry_t l1e) (_PAGE_PRESENT | _PAGE_AVAIL0); } +/* + * Ensure the paging structure for [va, va + nr * PAGE_SIZE) of d's + * per-domain area is in place, allocating whichever levels are missing: + * the (domain-wide) L3 root, the slot's L2, and all L1 tables covering + * the range. The page-tables come from the xenheap, so that they stay + * reachable through their always-mapped alias; populated pages come from + * the domain heap. The range must lie within a single per-domain slot + * (one L3 entry), and already-present levels and entries are left + * untouched, so calls are idempotent over existing ranges. + * + * nr == 0: only ensure the per-domain L3 itself exists; populate is + * ignored. Used to set the area up before any sub-range is known. + * + * populate == false: stop once the L1 tables are in place. The range is + * then ready for caller-owned pages to be mapped and unmapped via + * populate_perdomain_mapping() / destroy_perdomain_mapping(), which only + * fill (or clear) existing tables; populate treats missing structure as a + * bug, destroy skips it. + * + * populate == true: additionally install a freshly allocated, zeroed page + * at every not-yet-present entry in the range. Such pages are marked + * _PAGE_AVAIL0, "owned by the per-domain area": teardown frees them (see + * perdomain_l1e_needs_freeing()), whereas caller-owned mappings are only + * ever unmapped. + */ int create_perdomain_mapping(struct domain *d, unsigned long va, - unsigned int nr, l1_pgentry_t **pl1tab, - struct page_info **ppg) + unsigned int nr, bool populate) { struct page_info *pg; l3_pgentry_t *l3tab; @@ -6258,9 +6282,6 @@ int create_perdomain_mapping(struct domain *d, unsigned long va, else l2tab = maddr_to_virt(l3e_get_paddr(l3tab[l3_table_offset(va)])); - if ( !pl1tab && !ppg ) - return 0; - for ( l1tab = NULL; !rc && nr--; ) { l2_pgentry_t *pl2e = l2tab + l2_table_offset(va); @@ -6273,26 +6294,19 @@ int create_perdomain_mapping(struct domain *d, unsigned long va, rc = -ENOMEM; break; } - if ( pl1tab && !IS_NIL(pl1tab) ) - { - ASSERT(!pl1tab[l2_table_offset(va)]); - pl1tab[l2_table_offset(va)] = l1tab; - } clear_page(l1tab); *pl2e = l2e_from_mfn(virt_to_mfn(l1tab), __PAGE_HYPERVISOR_RW); } else if ( !l1tab ) l1tab = maddr_to_virt(l2e_get_paddr(*pl2e)); - if ( ppg && + if ( populate && !(l1e_get_flags(l1tab[l1_table_offset(va)]) & _PAGE_PRESENT) ) { pg = alloc_domheap_page(d, MEMF_no_owner); if ( pg ) { clear_domain_page(page_to_mfn(pg)); - if ( !IS_NIL(ppg) ) - *ppg++ = pg; l1tab[l1_table_offset(va)] = l1e_from_page(pg, __PAGE_HYPERVISOR_RW | _PAGE_AVAIL0); l2e_add_flags(*pl2e, _PAGE_AVAIL0); diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c index 35d1761c9c..15a8238aff 100644 --- a/xen/arch/x86/pv/domain.c +++ b/xen/arch/x86/pv/domain.c @@ -314,9 +314,7 @@ int switch_compat(struct domain *d) static int pv_create_gdt_ldt_l1tab(struct vcpu *v) { return create_perdomain_mapping(v->domain, GDT_VIRT_START(v), - 1U << GDT_LDT_VCPU_SHIFT, - NIL(l1_pgentry_t *), - NULL); + 1U << GDT_LDT_VCPU_SHIFT, false); } static void pv_destroy_gdt_ldt_l1tab(struct vcpu *v) diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c index 8eadab7933..ffeda06e08 100644 --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -733,8 +733,7 @@ void __init zap_low_mappings(void) int setup_compat_arg_xlat(struct vcpu *v) { return create_perdomain_mapping(v->domain, ARG_XLAT_START(v), - PFN_UP(COMPAT_ARG_XLAT_SIZE), - NULL, NIL(struct page_info *)); + PFN_UP(COMPAT_ARG_XLAT_SIZE), true); } void free_compat_arg_xlat(struct vcpu *v) -- 2.55.0