Re: [Xen-devel] [PATCH for-4.12 1/8] dom0/pvh: align allocation and mapping order to start address

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Jan 30, 2019 at 02:58:42PM +0100, Roger Pau Monné wrote:
> On Wed, Jan 30, 2019 at 12:37:28PM +0000, Wei Liu wrote:
> > On Wed, Jan 30, 2019 at 11:36:39AM +0100, Roger Pau Monne wrote:
> > > Due to the recent changes in the iommu mapping logic, the start
> > > addresses provided need to be aligned to the order intended to be
> > > mapped.
> > > 
> > 
> > Can you reference some commits here? What would happen if the address is
> > not aligned?
> 
> See:
> https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01030.html
> 
> and
> 
> https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01503.html
> 
> > > dom0 PVH domain builder didn't take this into account when populating
> > > the p2m, fix this by making sure the order is chosen so that the start
> > > address is aligned to 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>
> > > ---
> > > Without this patch trying to create a PVH dom0 will trigger an assert
> > > on certain hardware depending on the memory map.
> > > ---
> > >  xen/arch/x86/hvm/dom0_build.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> > > index 51cf490811..a571d15c13 100644
> > > --- a/xen/arch/x86/hvm/dom0_build.c
> > > +++ b/xen/arch/x86/hvm/dom0_build.c
> > > @@ -152,6 +152,8 @@ static int __init pvh_populate_memory_range(struct 
> > > domain *d,
> > >  
> > >          order = get_order_from_pages(end - start + 1);
> > >          order = min(order ? order - 1 : 0, max_order);
> > > +        /* The order allocated and populated must be aligned to the 
> > > address. */
> > > +        order = min(order, start ? find_first_set_bit(start) : 
> > > MAX_ORDER);
> > 
> > Isn't max_order better here?
> 
> It will yield the same result because order has already been limited
> by max_order. I've used MAX_ORDER directly because it's a constant and
> could be faster than loading the value in max_order. You could also
> use 'order' instead of MAX_ORDER and will also yield the same result.
> 

Reviewed-by: Wei Liu <wei.liu2@xxxxxxxxxx>

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