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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 7 Apr 2025, 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).
> 
> Add a define to indicate the late alloc xsentore PFN, to better indicate
> what is being checked.  Use UINT64_MAX instead of ~0ULL as the HVM_PARAM
> field is a uint64_t.  UINT64_MAX is not defined, so add it.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> v4:
> ASSERT gfn against UINT32_MAX and use < to avoid INVALID_GFN on 32bit
> builds
> Stefano gave R-b, but holding off addition because of changes
> Use IS_ENABLED(CONFIG_GRANT_TABLE) instead of wrapper
> Add XENSTORE_PFN_LATE_ALLOC
> Add UINT64_MAX
> 
> 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
> Rebase on mem paging changes
> 
> v2:
> Tweak commit message
> Mark gnttab_seed_entry() __init and put inside CONFIG_DOM0LESS_BOOT
> Add ASSERT(!d->creation_finished) and ASSERT(gt->gt_version == 1);
> const struct domain & struct grant_table
> ---
>  xen/arch/arm/dom0less-build.c | 10 +++++++++-
>  xen/common/grant_table.c      | 14 ++++++++++++++
>  xen/include/xen/grant_table.h |  4 ++++
>  xen/include/xen/types.h       |  1 +
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index bb8cc3be43..188ef40b52 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -20,6 +20,8 @@
>  #include <asm/static-memory.h>
>  #include <asm/static-shmem.h>
>  
> +#define XENSTORE_PFN_LATE_ALLOC UINT64_MAX
> +
>  static domid_t __initdata xs_domid = DOMID_INVALID;
>  static bool __initdata need_xenstore;
>  
> @@ -756,7 +758,7 @@ static int __init alloc_xenstore_params(struct 
> kernel_info *kinfo)
>  
>      if ( (kinfo->dom0less_feature & (DOM0LESS_XENSTORE | DOM0LESS_XS_LEGACY))
>                                   == (DOM0LESS_XENSTORE | DOM0LESS_XS_LEGACY) 
> )
> -        d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
> +        d->arch.hvm.params[HVM_PARAM_STORE_PFN] = XENSTORE_PFN_LATE_ALLOC;
>      else if ( kinfo->dom0less_feature & DOM0LESS_XENSTORE )
>      {
>          rc = alloc_xenstore_page(d);
> @@ -788,6 +790,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 != XENSTORE_PFN_LATE_ALLOC && 
> IS_ENABLED(CONFIG_GRANT_TABLE) )
> +        {
> +            ASSERT(gfn < UINT32_MAX);
> +            gnttab_seed_entry(d, GNTTAB_RESERVED_XENSTORE, xs_domid, gfn);
> +        }
>      }
>  }
>  
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 6c77867f8c..e75ff98aff 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -4346,6 +4346,20 @@ static void gnttab_usage_print(struct domain *rd)
>          printk("no active grant table entries\n");
>  }
>  
> +#ifdef CONFIG_DOM0LESS_BOOT
> +void __init gnttab_seed_entry(const struct domain *d, unsigned int idx,
> +                              domid_t be_domid, uint32_t frame)
> +{
> +    const struct grant_table *gt = d->grant_table;
> +
> +    ASSERT(!d->creation_finished);
> +    ASSERT(gt->gt_version == 1);
> +    shared_entry_v1(gt, idx).flags = GTF_permit_access;
> +    shared_entry_v1(gt, idx).domid = be_domid;
> +    shared_entry_v1(gt, idx).frame = frame;
> +}
> +#endif
> +
>  static void cf_check gnttab_usage_print_all(unsigned char key)
>  {
>      struct domain *d;
> diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
> index 50edfecfb6..297d7669e9 100644
> --- a/xen/include/xen/grant_table.h
> +++ b/xen/include/xen/grant_table.h
> @@ -33,6 +33,10 @@
>  
>  struct grant_table;
>  
> +/* Seed a gnttab entry for Hyperlaunch/dom0less. */
> +void gnttab_seed_entry(const struct domain *d, unsigned int idx,
> +                       domid_t be_domid, uint32_t frame);
> +
>  #ifdef CONFIG_GRANT_TABLE
>  
>  extern unsigned int opt_gnttab_max_version;
> diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
> index e8d419b954..73ddccbbd5 100644
> --- a/xen/include/xen/types.h
> +++ b/xen/include/xen/types.h
> @@ -44,6 +44,7 @@ typedef __UINTPTR_TYPE__ uintptr_t;
>  #define UINT8_MAX       (255)
>  #define UINT16_MAX      (65535)
>  #define UINT32_MAX      (4294967295U)
> +#define UINT64_MAX      (18446744073709551615ULL)
>  
>  #define INT_MAX         ((int)(~0U>>1))
>  #define INT_MIN         (-INT_MAX - 1)
> -- 
> 2.49.0
>