Re: [RFC][PATCH 04/13] tools/libxl: detect and avoid conflicts with RDM

From: "Tian, Kevin" <kevin.tian@intel.com>
To: Wei Liu <wei.liu2@citrix.com>, "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: "ian.campbell@citrix.com" <ian.campbell@citrix.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"tim@xen.org" <tim@xen.org>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	"stefano.stabellini@citrix.com" <stefano.stabellini@citrix.com>,
	"JBeulich@suse.com" <JBeulich@suse.com>,
	"Zhang, Yang Z" <yang.z.zhang@intel.com>,
	"Ian.Jackson@eu.citrix.com" <Ian.Jackson@eu.citrix.com>
Subject: Re: [RFC][PATCH 04/13] tools/libxl: detect and avoid conflicts with RDM
Date: Tue, 19 May 2015 01:32:27 +0000	[thread overview]
Message-ID: <AADFC41AFE54684AB9EE6CBC0274A5D126258415@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20150518200017.GA9893@zion.uk.xensource.com>

> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: Tuesday, May 19, 2015 4:00 AM
> 
> On Thu, May 14, 2015 at 04:27:45PM +0800, Chen, Tiejun wrote:
> > On 2015/5/11 19:32, Wei Liu wrote:
> > >On Mon, May 11, 2015 at 04:09:53PM +0800, Chen, Tiejun wrote:
> > >>On 2015/5/8 22:43, Wei Liu wrote:
> > >>>Sorry for the late review. This series fell through the crack.
> > >>>
> > >>
> > >>Thanks for your review.
> > >>
> > >>>On Fri, Apr 10, 2015 at 05:21:55PM +0800, Tiejun Chen wrote:
> > >>>>While building a VM, HVM domain builder provides struct
> hvm_info_table{}
> > >>>>to help hvmloader. Currently it includes two fields to construct guest
> > >>>>e820 table by hvmloader, low_mem_pgend and high_mem_pgend. So
> we should
> > >>>>check them to fix any conflict with RAM.
> > >>>>
> > >>>>RMRR can reside in address space beyond 4G theoretically, but we
> never
> > >>>>see this in real world. So in order to avoid breaking highmem layout
> > >>>
> > >>>How does this break highmem layout?
> > >>
> > >>In most cases highmen is always continuous like [4G, ...] but RMRR is
> > >>theoretically beyond 4G but very rarely. Especially we don't see this
> > >>happened in real world. So we don't want to such a case breaking the
> > >>highmem.
> > >>
> > >
> > >The problem is  we take this approach just because this rarely happens
> > >*now* is not future proof.  It needs to be clearly documented somewhere
> > >in the manual (or any other Intel docs) and be referenced in the code.
> > >Otherwise we might end up in a situation that no-one knows how it is
> > >supposed to work and no-one can fix it if it breaks in the future, that
> > >is, every single device on earth requires RMRR > 4G overnight (I'm
> > >exaggerating).
> > >
> > >Or you can just make it works with highmem. How much more work do you
> > >envisage?
> > >
> > >(If my above comment makes no sense do let me know. I only have very
> > >shallow understanding of RMRR)
> >
> > Maybe I'm misleading you :)
> >
> > I don't mean RMRR > 4G is not allowed to work in our implementation. What
> > I'm saying is that our *policy* is just simple for this kind of rare highmem
> > case...
> >
> 
> Yes, but this policy is hardcoded in code (as in, you bail when
> detecting conflict in highmem region). I don't think we have the
> expertise to un-hardcode it in the future (it might require x86 specific
> insider information and specific hardware to test). So I would like to
> make it as future proof as possible.
> 
> I know you're limited by hvm_info. I would accept this hardcoded policy
> as short term solution, but I would like commitment from Intel to
> maintain this piece of code and properly work out a flexible solution to
> deal with >4G case.
> 

We made this simplification by balancing implementation complexity and
real-world possibility. Yes Intel will commit to maintain it if future platform
does break this assumption.

Thanks
Kevin