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 11:35:15AM +0000, Roger Pau Monné wrote:
> 
> > > +        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?
> 
> I could add a loop like:
> 
> for ( i = PFN_DOWN(trampoline_phys);
>       i < (pvh_boot ? (PFN_UP(trampoline_phys + trampoline_end - 
> trampoline_start))
>                     : 0);
>       i++ )
> 
> But it seems quite redundant.
> 
> The comment above the loop already explains the position of the
> trampoline for PVH, do you think it would be fine to add something

What I'm afraid is that once we move the trampoline somewhere else the
comment and code will be stale and we can't easily know about that.

> like:
> 
> BUG_ON(pvh_boot && trampoline_phys != 0x1000);
> 
> Just before the loop?

This will do, too.

Wei.

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