Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-23 Thread Wei Liu 
On Thu, Jul 23, 2015 at 08:52:24AM +0800, Chen, Tiejun wrote:
 Ian,
 
 Thanks for your effort.
 
 A tiny change may be needed but I don't block this.
 
 +libxl__xc_device_get_rdm(libxl__gc *gc,
 + uint32_t flag,
 
 Since now we are sitting on xc_reserved_device_memory_map(, flags, xxx),
 s/flag/flags may be better.
 
 + uint16_t seg,
 + uint8_t bus,
 + uint8_t devfn,
 + unsigned int *nr_entries,
 + struct xen_reserved_device_memory **xrdm)
 +{
 
 [snip]
 
 +r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,
 
 Ditto.
 
 +  NULL, nr_entries);
 +assert(r = 0);
 +/* 0 means we have no any rdm entry. */
 +if (!r) goto out;
 +
 +if (errno != ENOBUFS) {
 +rc = ERROR_FAIL;
 +goto out;
 +}
 +
 +GCNEW_ARRAY(*xrdm, *nr_entries);
 +r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,
 
 Ditto.
 

These cosmetic changes can be fixed by a follow-up patch.

Wei.

 Thanks
 Tiejun

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

Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-23 Thread Chen, Tiejun 

These cosmetic changes can be fixed by a follow-up patch.



If Jackson would like not to fix this directly in his tree, I can post 
this a small patch but we'd better squash this into the predecessor just 
as one commit.


Thanks
Tiejun

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

Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-23 Thread Ian Jackson 
Wei Liu writes (Re: [PATCH 11/16] tools/libxl: detect and avoid conflicts with 
RDM):
 Ian, you forgot your S-o-B.

So I did, thanks.

 Acked-by: Wei Liu wei.l...@citrix.com

Thanks.

Also, I should mention that have retained the ack from Kevin Tian
(CC'd), since my changes were almost entirely mechanical.

Ian.

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

Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-23 Thread Ian Jackson 
Chen, Tiejun writes (Re: [PATCH 11/16] tools/libxl: detect and avoid conflicts 
with RDM):
 Thanks for your effort.
 
 A tiny change may be needed but I don't block this.

Thanks.  I have applied this change to my tree.  There is no
functional change and I don't feel the need to collect a new ack.

Ian.

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

[Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-22 Thread Ian Jackson 
From: Tiejun Chen tiejun.c...@intel.com

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 RDM.

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
we don't solve highmem conflict. Note this means highmem rmrr could still
be supported if no conflict.

But in the case of lowmem, RMRR probably scatter the whole RAM space.
Especially multiple RMRR entries would worsen this to lead a complicated
memory layout. And then its hard to extend hvm_info_table{} to work
hvmloader out. So here we're trying to figure out a simple solution to
avoid breaking existing layout. So when a conflict occurs,

#1. Above a predefined boundary (2G)
- move lowmem_end below reserved region to solve conflict;

#2. Below a predefined boundary (2G)
- Check strict/relaxed policy.
strict policy leads to fail libxl. Note when both policies
are specified on a given region, 'strict' is always preferred.
relaxed policy issue a warning message and also mask this entry 
INVALID
to indicate we shouldn't expose this entry to hvmloader.

Note later we need to provide a parameter to set that predefined boundary
dynamically.

CC: Ian Jackson ian.jack...@eu.citrix.com
CC: Stefano Stabellini stefano.stabell...@eu.citrix.com
CC: Ian Campbell ian.campb...@citrix.com
CC: Wei Liu wei.l...@citrix.com
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Reviewed-by: Kevin Tian kevin.t...@intel.com
---
v13: Mechanical changes to deal with changes to patch 01/
 XENMEM_reserved_device_memory_map.
---
 tools/libxl/libxl_create.c   |2 +-
 tools/libxl/libxl_dm.c   |  274 ++
 tools/libxl/libxl_dom.c  |   17 ++-
 tools/libxl/libxl_internal.h |   14 ++-
 tools/libxl/libxl_types.idl  |7 ++
 5 files changed, 311 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 7c884c4..5b57062 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -407,7 +407,7 @@ int libxl__domain_build(libxl__gc *gc,
 
 switch (info-type) {
 case LIBXL_DOMAIN_TYPE_HVM:
-ret = libxl__build_hvm(gc, domid, info, state);
+ret = libxl__build_hvm(gc, domid, d_config, state);
 if (ret)
 goto out;
 
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 634b8d2..40b2bba 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -92,6 +92,280 @@ const char *libxl__domain_device_model(libxl__gc *gc,
 return dm;
 }
 
+static int
+libxl__xc_device_get_rdm(libxl__gc *gc,
+ uint32_t flag,
+ uint16_t seg,
+ uint8_t bus,
+ uint8_t devfn,
+ unsigned int *nr_entries,
+ struct xen_reserved_device_memory **xrdm)
+{
+int rc = 0, r;
+
+/*
+ * We really can't presume how many entries we can get in advance.
+ */
+*nr_entries = 0;
+r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,
+  NULL, nr_entries);
+assert(r = 0);
+/* 0 means we have no any rdm entry. */
+if (!r) goto out;
+
+if (errno != ENOBUFS) {
+rc = ERROR_FAIL;
+goto out;
+}
+
+GCNEW_ARRAY(*xrdm, *nr_entries);
+r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,
+  *xrdm, nr_entries);
+if (r)
+rc = ERROR_FAIL;
+
+ out:
+if (rc) {
+*nr_entries = 0;
+*xrdm = NULL;
+LOG(ERROR, Could not get reserved device memory maps.\n);
+}
+return rc;
+}
+
+/*
+ * Check whether there exists rdm hole in the specified memory range.
+ * Returns true if exists, else returns false.
+ */
+static bool overlaps_rdm(uint64_t start, uint64_t memsize,
+ uint64_t rdm_start, uint64_t rdm_size)
+{
+return (start + memsize  rdm_start)  (start  rdm_start + rdm_size);
+}
+
+static void
+add_rdm_entry(libxl__gc *gc, libxl_domain_config *d_config,
+  uint64_t rdm_start, uint64_t rdm_size, int rdm_policy)
+{
+d_config-rdms = libxl__realloc(NOGC, d_config-rdms,
+(d_config-num_rdms+1) * sizeof(libxl_device_rdm));
+
+d_config-rdms[d_config-num_rdms].start = rdm_start;
+d_config-rdms[d_config-num_rdms].size = rdm_size;
+d_config-rdms[d_config-num_rdms].policy = rdm_policy;
+d_config-num_rdms++;
+}
+
+/*
+ * Check reported RDM regions and handle potential gfn conflicts according
+ * to user preferred policy.
+ *
+ * RDM can reside in address space beyond 4G theoretically, but we never
+ * see this in real world. So in order to

Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-22 Thread Wei Liu 
On Wed, Jul 22, 2015 at 04:44:14PM +0100, Ian Jackson wrote:
 From: Tiejun Chen tiejun.c...@intel.com
 
 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 RDM.
 
 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
 we don't solve highmem conflict. Note this means highmem rmrr could still
 be supported if no conflict.
 
 But in the case of lowmem, RMRR probably scatter the whole RAM space.
 Especially multiple RMRR entries would worsen this to lead a complicated
 memory layout. And then its hard to extend hvm_info_table{} to work
 hvmloader out. So here we're trying to figure out a simple solution to
 avoid breaking existing layout. So when a conflict occurs,
 
 #1. Above a predefined boundary (2G)
 - move lowmem_end below reserved region to solve conflict;
 
 #2. Below a predefined boundary (2G)
 - Check strict/relaxed policy.
 strict policy leads to fail libxl. Note when both policies
 are specified on a given region, 'strict' is always preferred.
 relaxed policy issue a warning message and also mask this entry 
 INVALID
 to indicate we shouldn't expose this entry to hvmloader.
 
 Note later we need to provide a parameter to set that predefined boundary
 dynamically.
 
 CC: Ian Jackson ian.jack...@eu.citrix.com
 CC: Stefano Stabellini stefano.stabell...@eu.citrix.com
 CC: Ian Campbell ian.campb...@citrix.com
 CC: Wei Liu wei.l...@citrix.com
 Signed-off-by: Tiejun Chen tiejun.c...@intel.com
 Reviewed-by: Kevin Tian kevin.t...@intel.com

Ian, you forgot your S-o-B.

Acked-by: Wei Liu wei.l...@citrix.com

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

Re: [Xen-devel] [PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM

2015-07-22 Thread Chen, Tiejun 

Ian,

Thanks for your effort.

A tiny change may be needed but I don't block this.


+libxl__xc_device_get_rdm(libxl__gc *gc,
+ uint32_t flag,


Since now we are sitting on xc_reserved_device_memory_map(, flags, xxx), 
s/flag/flags may be better.



+ uint16_t seg,
+ uint8_t bus,
+ uint8_t devfn,
+ unsigned int *nr_entries,
+ struct xen_reserved_device_memory **xrdm)
+{


[snip]


+r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,


Ditto.


+  NULL, nr_entries);
+assert(r = 0);
+/* 0 means we have no any rdm entry. */
+if (!r) goto out;
+
+if (errno != ENOBUFS) {
+rc = ERROR_FAIL;
+goto out;
+}
+
+GCNEW_ARRAY(*xrdm, *nr_entries);
+r = xc_reserved_device_memory_map(CTX-xch, flag, seg, bus, devfn,


Ditto.

Thanks
Tiejun

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