Re: [PATCH 1/3] xen/arm/dom0less-build: Alloc magic pages for Dom0less DomUs from hypervisor

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Henry,

On 26/04/2024 05:14, Henry Wang wrote:
> There are use cases (for example using the PV driver) in Dom0less
> setup that require Dom0less DomUs start immediately with Dom0, but
> initialize XenStore later after Dom0's successful boot and call to
> the init-dom0less application.
> 
> An error message can seen from the init-dom0less application on
> 1:1 direct-mapped domains:
> ```
> Allocating magic pages
> memory.c:238:d0v0 mfn 0x39000 doesn't belong to d1
> Error on alloc magic pages
> ```
> This is because currently the magic pages for Dom0less DomUs are
> populated by the init-dom0less app through populate_physmap(), and
> populate_physmap() automatically assumes gfn == mfn for 1:1 direct
> mapped domains. This cannot be true for the magic pages that are
> allocated later from the init-dom0less application executed in Dom0.
> For domain using statically allocated memory but not 1:1 direct-mapped,
> similar error "failed to retrieve a reserved page" can be seen as the
> reserved memory list is empty at that time.
> 
> To solve above issue, this commit allocates the magic pages for
> Dom0less DomUs at the domain construction time. The base address/PFN
> of the magic page region will be noted and communicated to the
> init-dom0less application in Dom0.
> 
> Reported-by: Alec Kwapis <alec.kwapis@xxxxxxxxxxxxx>
> Suggested-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> ---
>  tools/libs/guest/xg_dom_arm.c |  1 -
>  xen/arch/arm/dom0less-build.c | 22 ++++++++++++++++++++++
>  xen/include/public/arch-arm.h |  1 +
>  3 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
> index 2fd8ee7ad4..8cc7f27dbb 100644
> --- a/tools/libs/guest/xg_dom_arm.c
> +++ b/tools/libs/guest/xg_dom_arm.c
> @@ -25,7 +25,6 @@
>  
>  #include "xg_private.h"
>  
> -#define NR_MAGIC_PAGES 4
Moving only this macro to arch-arm.h while leaving the offsets does not make 
much sense to me.
I think they all should be moved. This would also allow init-dom0less.h not to 
re-define XENSTORE_PFN_OFFSET.

>  #define CONSOLE_PFN_OFFSET 0
>  #define XENSTORE_PFN_OFFSET 1
>  #define MEMACCESS_PFN_OFFSET 2
> diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> index fb63ec6fd1..40dc85c759 100644
> --- a/xen/arch/arm/dom0less-build.c
> +++ b/xen/arch/arm/dom0less-build.c
> @@ -834,11 +834,33 @@ static int __init construct_domU(struct domain *d,
>  
>      if ( kinfo.dom0less_feature & DOM0LESS_XENSTORE )
>      {
> +        struct page_info *magic_pg;
> +        mfn_t mfn;
> +        gfn_t gfn;
> +
>          ASSERT(hardware_domain);
>          rc = alloc_xenstore_evtchn(d);
>          if ( rc < 0 )
>              return rc;
>          d->arch.hvm.params[HVM_PARAM_STORE_PFN] = ~0ULL;
> +
> +        d->max_pages += NR_MAGIC_PAGES;
> +        magic_pg = alloc_domheap_pages(d, 
> get_order_from_pages(NR_MAGIC_PAGES), 0);
80 char exceeded

> +        if ( magic_pg == NULL )
> +            return -ENOMEM;
> +
> +        mfn = page_to_mfn(magic_pg);
> +        if ( !is_domain_direct_mapped(d) )
> +            gfn = gaddr_to_gfn(GUEST_MAGIC_BASE);
> +        else
> +            gfn = gaddr_to_gfn(mfn_to_maddr(mfn));
> +
> +        rc = guest_physmap_add_pages(d, gfn, mfn, NR_MAGIC_PAGES);
> +        if ( rc )
> +        {
> +            free_domheap_pages(magic_pg, 
> get_order_from_pages(NR_MAGIC_PAGES));
> +            return rc;
> +        }
Please create a function alloc_magic_pages to encapsulate the above block.

>      }
>  
>      return rc;
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index e167e14f8d..f24e7bbe37 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -475,6 +475,7 @@ typedef uint64_t xen_callback_t;
>  
>  #define GUEST_MAGIC_BASE  xen_mk_ullong(0x39000000)
>  #define GUEST_MAGIC_SIZE  xen_mk_ullong(0x01000000)
> +#define NR_MAGIC_PAGES 4
>  
>  #define GUEST_RAM_BANKS   2
>  

~Michal