Re: [PATCH v2 10/17] xen/riscv: implement guest_physmap_add_entry() for mapping GFNs to MFNs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.06.2025 15:05, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/p2m.c
> +++ b/xen/arch/riscv/p2m.c
> @@ -324,3 +324,44 @@ int p2m_set_allocation(struct domain *d, unsigned long 
> pages, bool *preempted)
>  
>      return 0;
>  }
> +
> +static int p2m_set_entry(struct p2m_domain *p2m,
> +                         gfn_t sgfn,
> +                         unsigned long nr,
> +                         mfn_t smfn,
> +                         p2m_type_t t,
> +                         p2m_access_t a)
> +{
> +    return -EOPNOTSUPP;
> +}
> +
> +static int p2m_insert_mapping(struct domain *d, gfn_t start_gfn,

This likely again wants to be struct p2m_domain *.

> +                              unsigned long nr, mfn_t mfn, p2m_type_t t)
> +{
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +    int rc;
> +
> +    p2m_write_lock(p2m);
> +    rc = p2m_set_entry(p2m, start_gfn, nr, mfn, t, p2m->default_access);
> +    p2m_write_unlock(p2m);
> +
> +    return rc;
> +}
> +
> +int map_regions_p2mt(struct domain *d,
> +                     gfn_t gfn,
> +                     unsigned long nr,
> +                     mfn_t mfn,
> +                     p2m_type_t p2mt)
> +{
> +    return p2m_insert_mapping(d, gfn, nr, mfn, p2mt);
> +}

What is this function doing here? The description says "for generic mapping
purposes", which really may mean anything. Plus, if and when you need it, it
wants to come with a name that fits with e.g. ...

> +int guest_physmap_add_entry(struct domain *d,
> +                            gfn_t gfn,
> +                            mfn_t mfn,
> +                            unsigned long page_order,
> +                            p2m_type_t t)

... this one, to understand their relationship / difference.

> +{
> +    return p2m_insert_mapping(d, gfn, (1 << page_order), mfn, t);

1UL please, while at the same time the parentheses could be omitted.

Jan