From: Hongyan Xia <hongyxia@amazon.com>

The root page table is allocated from the domheap and isn't
mapped by default. Map it on demand to build pv shim domain.

Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
Signed-off-by: Elias El Yandouzi <eliasely@amazon.com>

----

    Changes in v2:
        * New patch

diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index dc5e9fe117..fc51c7d362 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -991,8 +991,12 @@ do {                                                    \
      * !CONFIG_VIDEO case so the logic here can be simplified.
      */
     if ( pv_shim )
+    {
+        l4start = map_domain_page(l4start_mfn);
         pv_shim_setup_dom(d, l4start, v_start, vxenstore_start, vconsole_start,
                           vphysmap_start, si);
+        UNMAP_DOMAIN_PAGE(l4start);
+    }
 
 #ifdef CONFIG_COMPAT
     if ( compat )
-- 
2.40.1

On 16.01.2024 20:25, Elias El Yandouzi wrote:
> From: Hongyan Xia <hongyxia@amazon.com>
> 
> The root page table is allocated from the domheap and isn't
> mapped by default. Map it on demand to build pv shim domain.
> 
> Signed-off-by: Hongyan Xia <hongyxia@amazon.com>
> Signed-off-by: Elias El Yandouzi <eliasely@amazon.com>

The patch looks correct as is, so
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Still I would have wished that ...

> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -991,8 +991,12 @@ do {                                                    \
>       * !CONFIG_VIDEO case so the logic here can be simplified.
>       */
>      if ( pv_shim )
> +    {
> +        l4start = map_domain_page(l4start_mfn);
>          pv_shim_setup_dom(d, l4start, v_start, vxenstore_start, vconsole_start,
>                            vphysmap_start, si);
> +        UNMAP_DOMAIN_PAGE(l4start);
> +    }

... the function wide "l4start" wasn't clobbered like this.

In fact I think this patch needs either folding into the earlier one,
or moving ahead: The respective UNMAP_DOMAIN_PAGE() added there breaks
the use of l4start here. Yet then why not simply move that
UNMAP_DOMAIN_PAGE() below here, eliminating the need for this patch.

Jan