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

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

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

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