On Tue, Jun 23, 2015 at 05:57:25PM +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. >
RAM -> 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 <kevint.t...@intel.com> > --- > v4: > > * Consistent to use term "RDM". > * Unconditionally set *nr_entries to 0 > * Grab to all sutffs to provide a parameter to set our predefined boundary > dynamically to as a separated patch later > > tools/libxl/libxl_create.c | 2 +- > tools/libxl/libxl_dm.c | 259 > +++++++++++++++++++++++++++++++++++++++++++ > tools/libxl/libxl_dom.c | 17 ++- > tools/libxl/libxl_internal.h | 11 +- > tools/libxl/libxl_types.idl | 7 ++ > 5 files changed, 293 insertions(+), 3 deletions(-) > > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 6c8ec63..30e6593 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -460,7 +460,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 33f9ce6..5436bcf 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -90,6 +90,265 @@ const char *libxl__domain_device_model(libxl__gc *gc, > return dm; > } > > +static struct xen_reserved_device_memory > +*xc_device_get_rdm(libxl__gc *gc, > + uint32_t flag, > + uint16_t seg, > + uint8_t bus, > + uint8_t devfn, > + unsigned int *nr_entries) I just notice this function lives in libxl_dm.c. The function should be renamed to libxl__xc_device_get_rdm. This function should return proper libxl error code (ERROR_FAIL or something more appropriate). The allocated RDM entries should be returned with an out parameter. I had always thought this lived in libxc. Sorry for not having noticed this earlier. > +{ > + struct xen_reserved_device_memory *xrdm; > + int rc; > + > + /* > + * We really can't presume how many entries we can get in advance. > + */ > + *nr_entries = 0; > + rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn, > + NULL, nr_entries); > + assert(rc <= 0); > + /* "0" means we have no any rdm entry. */ > + if (!rc) > + goto out; > + > + if (errno == ENOBUFS) { > + xrdm = malloc(*nr_entries * sizeof(xen_reserved_device_memory_t)); libxl__malloc(gc, ...); > + if (!xrdm) { > + LOG(ERROR, "Could not allocate RDM buffer!\n"); > + goto out; > + } Get rid of this. > + rc = xc_reserved_device_memory_map(CTX->xch, flag, seg, bus, devfn, > + xrdm, nr_entries); > + if (rc) { > + LOG(ERROR, "Could not get reserved device memory maps.\n"); > + *nr_entries = 0; > + free(xrdm); > + xrdm = NULL; Get rid of free. > + } > + } else > + LOG(ERROR, "Could not get reserved device memory maps.\n"); > + > + out: > + return xrdm; > +} The reset of this patch looks good to me. It does what we've discussed. Wei. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel