On 16.01.2024 20:25, Elias El Yandouzi wrote:
> From: Hongyan Xia <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: Elias El Yandouzi <[email protected]>

The patch looks correct as is, so
Reviewed-by: Jan Beulich <[email protected]>
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

Reply via email to