Re: [Xen-devel] [PATCH for-4.6] libxl: move calling libxl__arch_domain_construct_memmap to right place

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Aug 05, 2015 at 11:38:54AM +0100, Ian Campbell wrote:
> On Wed, 2015-08-05 at 11:23 +0100, Wei Liu wrote:
> > This function was called in the wrong place, because both
> > libxl__vnuma_build_vmemrange_hvm and xc_hvm_build rely on its output.
> 
> What is the effect of this call being in the wrong place? Presumably one or
> the other of those functions reaches the wrong conclusion?

Originally, by the time that function got called, all guest pages were
already populated.  The end result is E820 map disagrees with what vNUMA
says and what address ranges memory actually resides, i.e. risk of guest
accessing region that doesn't have backing pages.

> 
> > 
> > Move the call of said function to the right place -- before the other
> > two functions which reply on its output.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> > Cc: "Chen, Tiejun" <tiejun.chen@xxxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > 
> > Discovered this issue by code inspection. This issue is not discovered
> > by osstest because we don't have hardware or test case to test that
> > code path.
> > 
> > Tiejun, can you confirm this is the right fix? Can you test this
> > change?
> > ---
> >  tools/libxl/libxl_dom.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index 5555fea..811f7da 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -960,6 +960,11 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
> >          goto out;
> >      }
> >  
> > +    if (libxl__arch_domain_construct_memmap(gc, d_config, domid, &args)) 
> > {
> > +        LOG(ERROR, "setting domain memory map failed");
> > +        goto out;
> > +    }
> > +
> >      if (info->num_vnuma_nodes != 0) {
> >          int i;
> >  
> > @@ -997,11 +1002,6 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
> >          goto out;
> >      }
> >  
> > -    if (libxl__arch_domain_construct_memmap(gc, d_config, domid, &args)) 
> > {
> > -        LOG(ERROR, "setting domain memory map failed");
> > -        goto out;
> > -    }
> > -
> >      ret = hvm_build_set_params(ctx->xch, domid, info, state->store_port,
> >                                 &state->store_mfn, state->console_port,
> >                                 &state->console_mfn, state->store_domid,

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel