Re: [Xen-devel] [PATCH 2/6] xen/pvh: place the trampoline at page 0x1

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 17, 2018 at 09:48:10AM +0000, Roger Pau Monne wrote:
> Since PVH guest jump straight into trampoline_setup trampoline_phys is
> not initialized, thus the trampoline is relocated to address 0.
> 
> This works, but has the undesirable effect of having VA 0 mapped to
> MFN 0, which means NULL pointed dereferences no longer trigger a page
> fault.
> 
> In order to solve this, place the trampoline at page 0x1 and reserve
> the memory used by it.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
> Should be backported to the 4.10.0-shim-comet branch.
> ---
>  xen/arch/x86/boot/head.S | 3 +++
>  xen/arch/x86/mm.c        | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 4fe5a776b1..7829e3f07c 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -411,6 +411,9 @@ __pvh_start:
>          /* Skip bootloader setup and bios setup, go straight to trampoline */
>          movb    $1, sym_esi(pvh_boot)
>          movb    $1, sym_esi(skip_realmode)
> +
> +        /* Set trampoline_phys to use page 1. */

Could you please add the rationale here -- to avoid having VA 0 mapped.

> +        movw    $0x1000, sym_esi(trampoline_phys)
>          jmp     trampoline_setup
>  
>  #endif /* CONFIG_PVH_GUEST */
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 1147a1afb1..586af2ba9a 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -292,9 +292,13 @@ void __init arch_init_memory(void)
>      /*
>       * First 1MB of RAM is historically marked as I/O.  If we booted PVH,
>       * reclaim the space.  Irrespective, leave MFN 0 as special for the sake
> -     * of 0 being a very common default value.
> +     * of 0 being a very common default value. Also reserve page 0x1 which is
> +     * used by the trampoline code.
>       */
> -    for ( i = 0; i < (pvh_boot ? 1 : 0x100); i++ )
> +    for ( i = 0;
> +          i < (pvh_boot ? (1 + PFN_UP(trampoline_end - trampoline_start))
> +                        : 0x100);
> +          i++ )

There is now a hardcoded assumption that the trampoline is mapped at
page 0x1. Maybe there should be a dedicated loop to share the trampoline
with dom_io?

Wei.

>          share_xen_page_with_guest(mfn_to_page(_mfn(i)),
>                                    dom_io, XENSHARE_writable);
>  
> -- 
> 2.15.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel