Re: [PATCH v4 4/6] xen/p2m: move xenmem_access_to_p2m_access() to common p2m.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.01.2026 10:28, Penny Zheng wrote:
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2203,6 +2203,46 @@ void p2m_log_dirty_range(struct domain *d, unsigned 
> long begin_pfn,
>      guest_flush_tlb_mask(d, d->dirty_cpumask);
>  }
>  
> +#if defined(CONFIG_VM_EVENT) || defined(CONFIG_ALTP2M)
> +bool xenmem_access_to_p2m_access(const struct p2m_domain *p2m,
> +                                 xenmem_access_t xaccess,
> +                                 p2m_access_t *paccess)
> +{
> +    static const p2m_access_t memaccess[] = {
> +#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
> +        ACCESS(n),
> +        ACCESS(r),
> +        ACCESS(w),
> +        ACCESS(rw),
> +        ACCESS(x),
> +        ACCESS(rx),
> +        ACCESS(wx),
> +        ACCESS(rwx),
> +        ACCESS(rx2rw),
> +        ACCESS(n2rwx),
> +        ACCESS(r_pw),
> +#undef ACCESS
> +    };
> +
> +    switch ( xaccess )
> +    {
> +    case 0 ... ARRAY_SIZE(memaccess) - 1:
> +        xaccess = array_index_nospec(xaccess, ARRAY_SIZE(memaccess));
> +        *paccess = memaccess[xaccess];
> +        break;
> +
> +    case XENMEM_access_default:
> +        *paccess = p2m->default_access;
> +        break;
> +
> +    default:
> +        return false;
> +    }
> +
> +    return true;
> +}
> +#endif /* VM_EVENT || ALTP2M */
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
> index 4de651038d..8e7d9ea2e3 100644
> --- a/xen/include/xen/mem_access.h
> +++ b/xen/include/xen/mem_access.h
> @@ -73,11 +73,6 @@ typedef enum {
>      /* NOTE: Assumed to be only 4 bits right now on x86. */
>  } p2m_access_t;
>  
> -struct p2m_domain;
> -bool xenmem_access_to_p2m_access(const struct p2m_domain *p2m,
> -                                 xenmem_access_t xaccess,
> -                                 p2m_access_t *paccess);
> -
>  /*
>   * Set access type for a region of gfns.
>   * If gfn == INVALID_GFN, sets the default access type.
> diff --git a/xen/include/xen/p2m-common.h b/xen/include/xen/p2m-common.h
> index f0bd9a6b98..bd4169caee 100644
> --- a/xen/include/xen/p2m-common.h
> +++ b/xen/include/xen/p2m-common.h
> @@ -43,5 +43,8 @@ int __must_check check_get_page_from_gfn(struct domain *d, 
> gfn_t gfn,
>                                           bool readonly, p2m_type_t *p2mt_p,
>                                           struct page_info **page_p);
>  
> +bool xenmem_access_to_p2m_access(const struct p2m_domain *p2m,
> +                                 xenmem_access_t xaccess,
> +                                 p2m_access_t *paccess);

CI says "no" on both PPC and RISC-V. I wouldn't be surprised of build issues
on Arm or x86 either, seeing that p2m-common.h doesn't (and shouldn't) include
xen/mem_access.h. It's arch/<arch>/include/asm/p2m.h which is responsible for
the inclusion ahead of including p2m-common.h. Question though is: If this is
an x86-only function, why was its decl put in xen/mem_access.h rather than
x86'es asm/mem_access.h. I'll try that before giving up and handing this back
to you, but may I stress again that you please properly test your changes? It
is not the responsibility of the committer to deal with such fallout.

Jan