From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][PATCH 04/13] tools/libxl: detect and avoid conflicts with RDM Date: Thu, 23 Apr 2015 20:31:38 +0800 Message-ID: <5538E62A.9080502@intel.com> References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com> <1428657724-3498-5-git-send-email-tiejun.chen@intel.com> <21806.25404.276583.454770@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21806.25404.276583.454770@mariner.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: Ian Jackson Cc: kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, tim@xen.org, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, JBeulich@suse.com, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2015/4/15 21:10, Ian Jackson wrote: > Tiejun Chen writes ("[RFC][PATCH 04/13] tools/libxl: detect and avoid conflicts with RDM"): >> 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. > Thanks for your review. > I'm not really qualified to understand all of this, because I'm not an > x86 expert - I don't even know what RDM is. But this does all seem > very complicated. Can I have a second opinion from an x86 expert ? I hope Kevin's reply is fine to you. But if you still have further question let me know and I'd like to make this point clear to you :) > > I had a quick look at the libxl code and it at the very least needs > updating to conform to tools/libxl/CODING_STYLE. > I took a look at some libxl codes and found some obvious code style issues. diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 8a5f589..ff40c65 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -97,7 +97,7 @@ const char *libxl__domain_device_model(libxl__gc *gc, static int check_rdm_hole(uint64_t start, uint64_t memsize, uint64_t rdm_start, uint64_t rdm_size) { - if ( start + memsize <= rdm_start || start >= rdm_start + rdm_size ) + if (start + memsize <= rdm_start || start >= rdm_start + rdm_size) return 0; else return 1; @@ -163,7 +163,8 @@ int libxl__domain_device_check_rdm(libxl__gc *gc, if (!nr_entries) continue; - /* Need to check whether this entry is already saved in the array. + /* + * Need to check whether this entry is already saved in the array. * This could come from two cases: * * - user may configure to get all RMRRs in this platform, which @@ -207,7 +208,8 @@ int libxl__domain_device_check_rdm(libxl__gc *gc, /* Fix highmem. */ if (args->mem_size > args->lowmem_size) highmem_end += (args->mem_size - args->lowmem_size); - /* Next step is to check and avoid potential conflict between RDM entries + /* + * Next step is to check and avoid potential conflict between RDM entries * and guest RAM. To avoid intrusive impact to existing memory layout * {lowmem, mmio, highmem} which is passed around various function blocks, * below conflicts are not handled which are rare and handling them would Is this good? I know Jan had some comments on this patch as well so actually something needs to be changed but here just lets focus on code style firstly :) Thanks Tiejun