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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 06, 2015 at 08:45:16AM +0800, Chen, Tiejun wrote:
> On 8/5/2015 7:25 PM, Wei Liu wrote:
> >On Wed, Aug 05, 2015 at 12:06:22PM +0100, Ian Campbell wrote:
> >>On Wed, 2015-08-05 at 11:58 +0100, Wei Liu wrote:
> >>> On Wed, Aug 05, 2015 at 11:48:55AM +0100, Ian Campbell wrote:
> >>> > On Wed, 2015-08-05 at 11:43 +0100, Wei Liu wrote:
> >>> > > 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.
> >>> >
> >>> > Ouch. This should certainly be explained in the commit message.
> >>> >
> >>> > With that: Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >>> >
> >>>
> >>> I will post v2 shortly with that information in commit message.
> >>>
> >>> > Although perhaps we should wait for confirmation this fix doesn't
> >>> > regress
> >>> > RMRR somehow?
> >>> >
> >>>
> >>> I doubt it will. The code as-is is already broken for RMRR. But let's
> >>> wait till tomorrow for Tiejun to reply.
> >>>
> >>> If he doesn't reply by tomorrow, I suggest we apply v2 first and
> >>> fix up any subsequent issues later.
> >>
> >>Agreed.
> >
> >Actually I want to retract this patch. I confused hvm path with pv path
> >and drew my conclusion when looking at both code paths.
> >
> >In hvm path, neither libxl__vnuma_build_vmemragen_hvm nor xc_hvm_build
> >depends on output of libxl__arch_domain_construct_memmap (in fact it
> >doesn't change anything). So the code is OK.
> >
> >In pv path, there is a path which relies on having a valid E820 map
> >first, but that path 1) relies on host E820 map; 2) doesn't involve RMRR
> >support.
> >
> >In the end, moving that function call has no effect whatsoever.
> 
> Sorry I don't go into this details but seems I have nothing to do about
> this, ultimately. Right?
> 

Yes. Nothing to worry about at the moment.

Wei.

> Thanks
> Tiejun
> 
> >
> >Sorry for the noise!
> >
> >Wei.
> >
> >

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