Re: [PATCH v3 4/7] xen/arm: dom0less seed xenstore grant table entry

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.04.2025 23:46, Jason Andryuk wrote:
> xenstored maps other domains' xenstore pages.  Currently this relies on
> init-dom0less or xl to seed the grants from Dom0.  With split
> hardware/control/xenstore domains, this is problematic since we don't
> want the hardware domain to be able to map other domains' resources
> without their permission.  Instead have the hypervisor seed the grant
> table entry for every dom0less domain.  The grant is then accessible as
> normal.
> 
> C xenstored uses grants, so it can map the xenstore pages from a
> non-dom0 xenstore domain.  OCaml xenstored uses foreign mappings, so it
> can only run from a privileged domain (dom0).
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> ---
> v3:
> Expand commit message about C vs. OCaml xenstored.
> Remove __init and flags from gnttab_seed_entry()
> Change frame to uint32_t
> ASSERT gfn fits in a uint32_t

Ehem. For this you need to use ...

> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -788,6 +788,12 @@ static void __init initialize_domU_xenstore(void)
>          rc = alloc_xenstore_evtchn(d);
>          if ( rc < 0 )
>              panic("%pd: Failed to allocate xenstore_evtchn\n", d);
> +
> +        if ( gfn != ~0ULL )
> +        {
> +            ASSERT(gfn <= UINT_MAX);

... UINT32_MAX here. Furthermore may I remind you that INVALID_GFN ==
UINT32_MAX in 32-bit environments.

The ~0ULL may also better be UINT64_MAX.

> @@ -85,6 +89,9 @@ static inline void grant_table_destroy(struct domain *d) {}
>  
>  static inline void grant_table_init_vcpu(struct vcpu *v) {}
>  
> +static inline void gnttab_seed_entry(struct domain *d, int idx,
> +                                     domid_t be_domid, uint32_t frame) {}

Hmm. While generally I prefer using such wrappers, I wonder if in this
case it wouldn't end up more clear if a conditional was added in
initialize_domU_xenstore(). Ideally using IS_ENABLED(), which - aiui -
would require moving the declaration of the function.

Jan