Re: [PATCH v2 02/14] x86/mm: introduce populate_perdomain_mapping()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.09.2026 11:43, George Dunlap wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -6334,6 +6334,130 @@ int create_perdomain_mapping(struct domain *d, 
> unsigned long va,
>      return rc;
>  }
>  
> +/*
> + * Map @nr pages, @mfn[0..nr-1], at consecutive pages from @va in v's view of
> + * the per-domain area, with page-table @flags.  The range must lie within a
> + * single per-domain slot, and must already have been plumbed down to the L1
> + * tables by create_perdomain_mapping(): missing structure is a bug.  A
> + * present entry not owned by the area (no _PAGE_AVAIL0) is silently
> + * replaced, as that is how callers update their mappings; a present
> + * area-owned entry is freed and replaced, which constrains the calling
> + * context (see the comment in the body).  No TLB flushing is done: the
> + * caller decides whether the old translations can still be cached
> + * anywhere.
> + *
> + * When v's page-tables are loaded on this pCPU the L1 entries are reached
> + * through the recursive linear mappings; otherwise the walk maps the
> + * per-domain page-table pages transiently with IRQs off, so it needs
> + * nothing from the current address space and is usable from any context --
> + * including the context switch, before the incoming vcpu's page-tables are
> + * loaded.
> + */
> +void populate_perdomain_mapping(const struct vcpu *v, unsigned long va,
> +                                const mfn_t *mfn, unsigned int nr,
> +                                unsigned int flags)
> +{
> +    l1_pgentry_t *l1tab = NULL, *pl1e;
> +    const l3_pgentry_t *l3tab;
> +    const l2_pgentry_t *l2tab;
> +    struct domain *d = v->domain;
> +    unsigned long irq_flags;
> +
> +    ASSERT(va >= PERDOMAIN_VIRT_START &&
> +           va < PERDOMAIN_VIRT_SLOT(PERDOMAIN_SLOTS));
> +    ASSERT(!nr || !l3_table_offset(va ^ (va + nr * PAGE_SIZE - 1)));
> +    /* Area-owned pages are installed by create_perdomain_mapping() only. */
> +    ASSERT(!(flags & _PAGE_AVAIL0));
> +
> +    if ( likely(this_cpu(pgtable_vcpu) == v) )
> +    {
> +        unsigned int i;
> +
> +        /*
> +         * Fast path: v's page-tables are loaded on this pCPU, so the L1
> +         * entries can be reached using the recursive linear mappings.
> +         */
> +        pl1e = &__linear_l1_table[l1_linear_offset(va)];
> +
> +        for ( i = 0; i < nr; i++, pl1e++ )
> +        {
> +            /*
> +             * An area-owned entry (installed by create_perdomain_mapping(),
> +             * marked _PAGE_AVAIL0) holds the only reference to its page, so
> +             * displacing it means freeing it.  Nothing in this series
> +             * replaces area-owned backing, hence the ASSERT_UNREACHABLE();
> +             * any future caller doing so must run where freeing is
> +             * permitted -- IRQs enabled, not in interrupt context (see
> +             * ASSERT_ALLOC_CONTEXT()) -- which the context switch path is
> +             * not.
> +             */
> +            if ( unlikely(perdomain_l1e_needs_freeing(*pl1e)) )
> +            {
> +                ASSERT_UNREACHABLE();
> +                free_domheap_page(l1e_get_page(*pl1e));
> +            }
> +            l1e_write(pl1e, l1e_from_mfn(mfn[i], flags));
> +        }
> +
> +        return;
> +    }
> +
> +    BUG_ON(!d->arch.perdomain_l3_pg);
> +
> +    /*
> +     * Slow path: walk v's per-domain page-table pages.  All mappings are
> +     * local to this function, so disabling interrupts for the duration of
> +     * the walk satisfies the map_domain_page_irqoff() contract.  This in
> +     * turn makes this function usable from the context switch path, where
> +     * a plain map_domain_page() could recurse into __context_switch() via
> +     * sync_local_execstate().
> +     */
> +    local_irq_save(irq_flags);
> +
> +    l3tab = __map_domain_page_irqoff(d->arch.perdomain_l3_pg);
> +
> +    /*
> +     * Missing page-table structure is a hypervisor bug: there is no safe
> +     * continuation, least of all from the context switch, where the next
> +     * descriptor fetch through an unmapped GDT slot would be fatal.
> +     */
> +    BUG_ON(!(l3e_get_flags(l3tab[l3_table_offset(va)]) & _PAGE_PRESENT));
> +
> +    l2tab = map_domain_page_irqoff(l3e_get_mfn(l3tab[l3_table_offset(va)]));
> +
> +    for ( ; nr--; va += PAGE_SIZE, mfn++ )
> +    {
> +        if ( !l1tab || !l1_table_offset(va) )
> +        {
> +            const l2_pgentry_t *pl2e = l2tab + l2_table_offset(va);
> +
> +            BUG_ON(!(l2e_get_flags(*pl2e) & _PAGE_PRESENT));
> +
> +            unmap_domain_page_irqoff(l1tab);
> +            l1tab = map_domain_page_irqoff(l2e_get_mfn(*pl2e));
> +        }
> +
> +        pl1e = &l1tab[l1_table_offset(va)];
> +
> +        /*
> +         * As the fast path -- and the slow path holds IRQs off throughout,
> +         * so replacing area-owned backing here is never permitted.
> +         */
> +        if ( unlikely(perdomain_l1e_needs_freeing(*pl1e)) )
> +        {
> +            ASSERT_UNREACHABLE();
> +            free_domheap_page(l1e_get_page(*pl1e));
> +        }

Just for the possible case of freeing here really becoming necessary: This
could be deferred until ...

> +        l1e_write(pl1e, l1e_from_mfn(*mfn, flags));
> +    }
> +
> +    unmap_domain_page_irqoff(l1tab);
> +    unmap_domain_page_irqoff(l2tab);
> +    unmap_domain_page_irqoff(l3tab);
> +
> +    local_irq_restore(irq_flags);

... here. Easily for the nr == 1 case (just requires a local variable to
hold MFN or struct page_info *), and with a slight change to the contract
with the caller (allowing mfn[] to be altered) also in the general case.

Jan