From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [PATCH for-4.6] libxl: move calling libxl__arch_domain_construct_memmap to right place Date: Thu, 06 Aug 2015 08:45:16 +0800 Message-ID: <55C2AE1C.2040607@intel.com> References: <1438770198-5581-1-git-send-email-wei.liu2@citrix.com> <1438771134.9747.51.camel@citrix.com> <20150805104342.GH26074@zion.uk.xensource.com> <1438771735.9747.54.camel@citrix.com> <20150805105807.GI26074@zion.uk.xensource.com> <1438772782.9747.59.camel@citrix.com> <20150805112517.GJ26074@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZN9JB-00014x-KP for xen-devel@lists.xenproject.org; Thu, 06 Aug 2015 00:45:21 +0000 In-Reply-To: <20150805112517.GJ26074@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu , Ian Campbell Cc: xen-devel@lists.xenproject.org, Ian Jackson List-Id: xen-devel@lists.xenproject.org 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 >> > > >> > >> > 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? Thanks Tiejun > > Sorry for the noise! > > Wei. > >