David Vrabel
2012-Feb-28 18:27 UTC
[PATCH 0/2] libxc: allow size of MMIO hole for HVM guests to be set
This series allows a user of libxc to set the size of the MMIO hole of HVM guests. The rationale for this is included in the patch 2. As part of this, the API calls were tidied up to allow new parameters to be added more easily (by using a transparent structure). Note that this breaks backwards compatibility and that adding more parameters will also break backwards compatibility. I took a stab at fixing the ia64 version of xc_hvm_domain_build() but I haven''t even tried a ia64 build... David
David Vrabel
2012-Feb-28 18:27 UTC
[PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
From: David Vrabel <david.vrabel@citrix.com> To allow new parameters to be added to the xc_hvm_build*() family of functions, pass them in a structure. Make the other variants fill in the structure and call xc_hvm_build() (except for xc_hvm_build_mem() which had no users and is removed). Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/libxc/ia64/xc_ia64_hvm_build.c | 21 ++++-- tools/libxc/xc_hvm_build.c | 115 +++++++++------------------------- tools/libxc/xenguest.h | 27 +++++--- tools/libxc/xg_private.c | 3 +- 4 files changed, 62 insertions(+), 104 deletions(-) diff --git a/tools/libxc/ia64/xc_ia64_hvm_build.c b/tools/libxc/ia64/xc_ia64_hvm_build.c index 18be616..16dace0 100644 --- a/tools/libxc/ia64/xc_ia64_hvm_build.c +++ b/tools/libxc/ia64/xc_ia64_hvm_build.c @@ -912,8 +912,8 @@ setup_guest(xc_interface *xch, uint32_t dom, unsigned long memsize, char *image, unsigned long image_size) { xen_pfn_t *pfn_list; - unsigned long dom_memsize = memsize << 20; - unsigned long nr_pages = memsize << (20 - PAGE_SHIFT); + unsigned long dom_memsize = memsize; + unsigned long nr_pages = memsize >> PAGE_SHIFT; unsigned long vcpus; unsigned long nr_special_pages; unsigned long memmap_info_pfn; @@ -1072,14 +1072,14 @@ error_out: } int -xc_hvm_build(xc_interface *xch, uint32_t domid, int memsize, const char *image_name) +xc_hvm_build(xc_interface *xch, uint32_t domid, const struct xc_hvm_params *params) { vcpu_guest_context_any_t st_ctxt_any; vcpu_guest_context_t *ctxt = &st_ctxt_any.c; char *image = NULL; unsigned long image_size; - image = xc_read_image(xch, image_name, &image_size); + image = xc_read_image(xch, params->image_file_name, &image_size); if (image == NULL) { PERROR("Could not read guest firmware image %s", image_name); goto error_out; @@ -1087,7 +1087,7 @@ xc_hvm_build(xc_interface *xch, uint32_t domid, int memsize, const char *image_n image_size = (image_size + PAGE_SIZE - 1) & PAGE_MASK; - if (setup_guest(xch, domid, (unsigned long)memsize, image, + if (setup_guest(xch, domid, (unsigned long)params->mem_size, image, image_size) < 0) { ERROR("Error constructing guest OS"); goto error_out; @@ -1114,6 +1114,8 @@ error_out: * files/filenames. If target < memsize, domain is created with * memsize pages marked populate-on-demand, and with a PoD cache size * of target. If target == memsize, pages are populated normally. + * + * XXX:PoD isn''t supported yet so setting target does nothing. */ int xc_hvm_build_target_mem(xc_interface *xch, uint32_t domid, @@ -1121,8 +1123,13 @@ int xc_hvm_build_target_mem(xc_interface *xch, int target, const char *image_name) { - /* XXX:PoD isn''t supported yet */ - return xc_hvm_build(xch, domid, target, image_name); + struct xc_hvm_params params; + + params.mem_size = (uint64_t)memsize << 20; + params.mem_target = (uint64_t)target << 20; + params.image_file_name = image_name; + + return xc_hvm_build(xch, domid, ¶ms); } /* diff --git a/tools/libxc/xc_hvm_build.c b/tools/libxc/xc_hvm_build.c index 1fa5658..31d4734 100644 --- a/tools/libxc/xc_hvm_build.c +++ b/tools/libxc/xc_hvm_build.c @@ -136,12 +136,12 @@ static int check_mmio_hole(uint64_t start, uint64_t memsize) } static int setup_guest(xc_interface *xch, - uint32_t dom, int memsize, int target, + uint32_t dom, const struct xc_hvm_params *params, char *image, unsigned long image_size) { xen_pfn_t *page_array = NULL; - unsigned long i, nr_pages = (unsigned long)memsize << (20 - PAGE_SHIFT); - unsigned long target_pages = (unsigned long)target << (20 - PAGE_SHIFT); + unsigned long i, nr_pages = params->mem_size >> PAGE_SHIFT; + unsigned long target_pages = params->mem_target >> PAGE_SHIFT; unsigned long entry_eip, cur_pages, cur_pfn; void *hvm_info_page; uint32_t *ident_pt; @@ -153,11 +153,7 @@ static int setup_guest(xc_interface *xch, stat_1gb_pages = 0; int pod_mode = 0; - /* An HVM guest must be initialised with at least 2MB memory. */ - if ( memsize < 2 || target < 2 ) - goto error_out; - - if ( memsize > target ) + if ( nr_pages > target_pages ) pod_mode = 1; memset(&elf, 0, sizeof(elf)); @@ -168,7 +164,7 @@ static int setup_guest(xc_interface *xch, elf_parse_binary(&elf); v_start = 0; - v_end = (unsigned long long)memsize << 20; + v_end = params->mem_size; if ( xc_version(xch, XENVER_capabilities, &caps) != 0 ) { @@ -393,39 +389,34 @@ static int setup_guest(xc_interface *xch, return -1; } -static int xc_hvm_build_internal(xc_interface *xch, - uint32_t domid, - int memsize, - int target, - char *image, - unsigned long image_size) -{ - if ( (image == NULL) || (image_size == 0) ) - { - ERROR("Image required"); - return -1; - } - - return setup_guest(xch, domid, memsize, target, image, image_size); -} - /* xc_hvm_build: * Create a domain for a virtualized Linux, using files/filenames. */ -int xc_hvm_build(xc_interface *xch, - uint32_t domid, - int memsize, - const char *image_name) +int xc_hvm_build(xc_interface *xch, uint32_t domid, + const struct xc_hvm_params *hvm_params) { - char *image; - int sts; + struct xc_hvm_params params = *hvm_params; + void *image; unsigned long image_size; + int sts; - if ( (image_name == NULL) || - ((image = xc_read_image(xch, image_name, &image_size)) == NULL) ) + if ( domid == 0 ) + return -1; + if ( params.image_file_name == NULL ) return -1; - sts = xc_hvm_build_internal(xch, domid, memsize, memsize, image, image_size); + if ( params.mem_target == 0 ) + params.mem_target = params.mem_size; + + /* An HVM guest must be initialised with at least 2MB memory. */ + if ( params.mem_size < (2ull << 20) || params.mem_target < (2ull << 20) ) + return -1; + + image = xc_read_image(xch, params.image_file_name, &image_size); + if ( image == NULL ) + return -1; + + sts = setup_guest(xch, domid, ¶ms, image, image_size); free(image); @@ -445,59 +436,13 @@ int xc_hvm_build_target_mem(xc_interface *xch, int target, const char *image_name) { - char *image; - int sts; - unsigned long image_size; + struct xc_hvm_params params = {}; - if ( (image_name == NULL) || - ((image = xc_read_image(xch, image_name, &image_size)) == NULL) ) - return -1; + params.mem_size = (uint64_t)memsize << 20; + params.mem_target = (uint64_t)target << 20; + params.image_file_name = image_name; - sts = xc_hvm_build_internal(xch, domid, memsize, target, image, image_size); - - free(image); - - return sts; -} - -/* xc_hvm_build_mem: - * Create a domain for a virtualized Linux, using memory buffers. - */ -int xc_hvm_build_mem(xc_interface *xch, - uint32_t domid, - int memsize, - const char *image_buffer, - unsigned long image_size) -{ - int sts; - unsigned long img_len; - char *img; - - /* Validate that there is a kernel buffer */ - - if ( (image_buffer == NULL) || (image_size == 0) ) - { - ERROR("kernel image buffer not present"); - return -1; - } - - img = xc_inflate_buffer(xch, image_buffer, image_size, &img_len); - if ( img == NULL ) - { - ERROR("unable to inflate ram disk buffer"); - return -1; - } - - sts = xc_hvm_build_internal(xch, domid, memsize, memsize, - img, img_len); - - /* xc_inflate_buffer may return the original buffer pointer (for - for already inflated buffers), so exercise some care in freeing */ - - if ( (img != NULL) && (img != image_buffer) ) - free(img); - - return sts; + return xc_hvm_build(xch, domid, ¶ms); } /* diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 533e702..19b0382 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -171,10 +171,23 @@ int xc_linux_build_mem(xc_interface *xch, unsigned int console_evtchn, unsigned long *console_mfn); -int xc_hvm_build(xc_interface *xch, - uint32_t domid, - int memsize, - const char *image_name); +struct xc_hvm_params { + uint64_t mem_size; /**< Memory size in bytes. */ + uint64_t mem_target; /**< Memory target in bytes. */ + const char *image_file_name; /**< File name of the image to load. */ +}; + +/** + * Build a HVM domain using a set of parameters. + * @parm xch libxc context handle. + * @parm domid domain ID for the new domain. + * @parm params parameters for the new domain. + * + * The memory size and image file parameters are required, the reset + * are optional. + */ +int xc_hvm_build(xc_interface *xch, uint32_t domid, + const struct xc_hvm_params *hvm_params); int xc_hvm_build_target_mem(xc_interface *xch, uint32_t domid, @@ -182,12 +195,6 @@ int xc_hvm_build_target_mem(xc_interface *xch, int target, const char *image_name); -int xc_hvm_build_mem(xc_interface *xch, - uint32_t domid, - int memsize, - const char *image_buffer, - unsigned long image_size); - int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn); int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port); diff --git a/tools/libxc/xg_private.c b/tools/libxc/xg_private.c index 4b624a5..f7e2139 100644 --- a/tools/libxc/xg_private.c +++ b/tools/libxc/xg_private.c @@ -192,8 +192,7 @@ unsigned long csum_page(void *page) __attribute__((weak)) int xc_hvm_build(xc_interface *xch, uint32_t domid, - int memsize, - const char *image_name) + const struct xc_hvm_params *hvm_params) { errno = ENOSYS; return -1; -- 1.7.2.5
David Vrabel
2012-Feb-28 18:27 UTC
[PATCH 2/2] libxc: add MMIO hole size parameter to xc_hvm_build()
From: David Vrabel <david.vrabel@citrix.com> Add a parameter for the MMIO hole size when building a HVM domain. This is useful for specifying a larger than normal MMIO hole to ensure that no PCI device''s MMIO region overlaps with the guest physical addresses of RAM. This is needed on certain systems with PCI bridges with ACS versions that are broken. On these systems, if a device behind the bridge attempts a DMA to a guest physical address that overlaps with the MMIO region of another device behind the bridge, then the bridge intercepts the access and forwards it directly to the device and the read or write never hits RAM. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- tools/libxc/xc_hvm_build.c | 29 ++++++++++++++++++----------- tools/libxc/xenguest.h | 1 + 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/tools/libxc/xc_hvm_build.c b/tools/libxc/xc_hvm_build.c index 31d4734..fbeddac 100644 --- a/tools/libxc/xc_hvm_build.c +++ b/tools/libxc/xc_hvm_build.c @@ -46,7 +46,8 @@ #define NR_SPECIAL_PAGES 5 #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x)) -static void build_hvm_info(void *hvm_info_page, uint64_t mem_size) +static void build_hvm_info(void *hvm_info_page, uint64_t mem_size, + uint64_t mmio_start, uint64_t mmio_size) { struct hvm_info_table *hvm_info = (struct hvm_info_table *) (((unsigned char *)hvm_info_page) + HVM_INFO_OFFSET); @@ -54,10 +55,10 @@ static void build_hvm_info(void *hvm_info_page, uint64_t mem_size) uint8_t sum; int i; - if ( lowmem_end > HVM_BELOW_4G_RAM_END ) + if ( lowmem_end > mmio_start ) { - highmem_end = lowmem_end + (1ull<<32) - HVM_BELOW_4G_RAM_END; - lowmem_end = HVM_BELOW_4G_RAM_END; + highmem_end = (1ull<<32) + (lowmem_end - mmio_start); + lowmem_end = mmio_start; } memset(hvm_info_page, 0, PAGE_SIZE); @@ -126,10 +127,10 @@ static int loadelfimage( * Check whether there exists mmio hole in the specified memory range. * Returns 1 if exists, else returns 0. */ -static int check_mmio_hole(uint64_t start, uint64_t memsize) +static int check_mmio_hole(uint64_t start, uint64_t memsize, + uint64_t mmio_start, uint64_t mmio_size) { - if ( start + memsize <= HVM_BELOW_4G_MMIO_START || - start >= HVM_BELOW_4G_MMIO_START + HVM_BELOW_4G_MMIO_LENGTH ) + if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size ) return 0; else return 1; @@ -142,6 +143,8 @@ static int setup_guest(xc_interface *xch, xen_pfn_t *page_array = NULL; unsigned long i, nr_pages = params->mem_size >> PAGE_SHIFT; unsigned long target_pages = params->mem_target >> PAGE_SHIFT; + uint64_t mmio_start = (1ull << 32) - params->mmio_size; + uint64_t mmio_size = params->mmio_size; unsigned long entry_eip, cur_pages, cur_pfn; void *hvm_info_page; uint32_t *ident_pt; @@ -188,8 +191,8 @@ static int setup_guest(xc_interface *xch, for ( i = 0; i < nr_pages; i++ ) page_array[i] = i; - for ( i = HVM_BELOW_4G_RAM_END >> PAGE_SHIFT; i < nr_pages; i++ ) - page_array[i] += HVM_BELOW_4G_MMIO_LENGTH >> PAGE_SHIFT; + for ( i = mmio_start >> PAGE_SHIFT; i < nr_pages; i++ ) + page_array[i] += mmio_size >> PAGE_SHIFT; /* * Allocate memory for HVM guest, skipping VGA hole 0xA0000-0xC0000. @@ -230,7 +233,8 @@ static int setup_guest(xc_interface *xch, if ( ((count | cur_pfn) & (SUPERPAGE_1GB_NR_PFNS - 1)) == 0 && /* Check if there exists MMIO hole in the 1GB memory range */ !check_mmio_hole(cur_pfn << PAGE_SHIFT, - SUPERPAGE_1GB_NR_PFNS << PAGE_SHIFT) ) + SUPERPAGE_1GB_NR_PFNS << PAGE_SHIFT, + mmio_start, mmio_size) ) { long done; unsigned long nr_extents = count >> SUPERPAGE_1GB_SHIFT; @@ -327,7 +331,7 @@ static int setup_guest(xc_interface *xch, xch, dom, PAGE_SIZE, PROT_READ | PROT_WRITE, HVM_INFO_PFN)) == NULL ) goto error_out; - build_hvm_info(hvm_info_page, v_end); + build_hvm_info(hvm_info_page, v_end, mmio_start, mmio_size); munmap(hvm_info_page, PAGE_SIZE); /* Allocate and clear special pages. */ @@ -408,6 +412,9 @@ int xc_hvm_build(xc_interface *xch, uint32_t domid, if ( params.mem_target == 0 ) params.mem_target = params.mem_size; + if ( params.mmio_size == 0 ) + params.mmio_size = HVM_BELOW_4G_MMIO_LENGTH; + /* An HVM guest must be initialised with at least 2MB memory. */ if ( params.mem_size < (2ull << 20) || params.mem_target < (2ull << 20) ) return -1; diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h index 19b0382..b06788c 100644 --- a/tools/libxc/xenguest.h +++ b/tools/libxc/xenguest.h @@ -174,6 +174,7 @@ int xc_linux_build_mem(xc_interface *xch, struct xc_hvm_params { uint64_t mem_size; /**< Memory size in bytes. */ uint64_t mem_target; /**< Memory target in bytes. */ + uint64_t mmio_size; /**< Size of the MMIO hole in bytes. */ const char *image_file_name; /**< File name of the image to load. */ }; -- 1.7.2.5
George Dunlap
2012-Feb-29 11:40 UTC
Re: [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
On Tue, Feb 28, 2012 at 6:27 PM, David Vrabel <david.vrabel@citrix.com> wrote:> From: David Vrabel <david.vrabel@citrix.com> > > To allow new parameters to be added to the xc_hvm_build*() family of > functions, pass them in a structure. Make the other variants fill in > the structure and call xc_hvm_build() (except for xc_hvm_build_mem() > which had no users and is removed). > > Signed-off-by: David Vrabel <david.vrabel@citrix.com>You should probably mention in the commit message that you''re changing from megabytes to bytes. Other than that, looks good to me: Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> --- > tools/libxc/ia64/xc_ia64_hvm_build.c | 21 ++++-- > tools/libxc/xc_hvm_build.c | 115 +++++++++------------------------- > tools/libxc/xenguest.h | 27 +++++--- > tools/libxc/xg_private.c | 3 +- > 4 files changed, 62 insertions(+), 104 deletions(-) > > diff --git a/tools/libxc/ia64/xc_ia64_hvm_build.c b/tools/libxc/ia64/xc_ia64_hvm_build.c > index 18be616..16dace0 100644 > --- a/tools/libxc/ia64/xc_ia64_hvm_build.c > +++ b/tools/libxc/ia64/xc_ia64_hvm_build.c > @@ -912,8 +912,8 @@ setup_guest(xc_interface *xch, uint32_t dom, unsigned long memsize, > char *image, unsigned long image_size) > { > xen_pfn_t *pfn_list; > - unsigned long dom_memsize = memsize << 20; > - unsigned long nr_pages = memsize << (20 - PAGE_SHIFT); > + unsigned long dom_memsize = memsize; > + unsigned long nr_pages = memsize >> PAGE_SHIFT; > unsigned long vcpus; > unsigned long nr_special_pages; > unsigned long memmap_info_pfn; > @@ -1072,14 +1072,14 @@ error_out: > } > > int > -xc_hvm_build(xc_interface *xch, uint32_t domid, int memsize, const char *image_name) > +xc_hvm_build(xc_interface *xch, uint32_t domid, const struct xc_hvm_params *params) > { > vcpu_guest_context_any_t st_ctxt_any; > vcpu_guest_context_t *ctxt = &st_ctxt_any.c; > char *image = NULL; > unsigned long image_size; > > - image = xc_read_image(xch, image_name, &image_size); > + image = xc_read_image(xch, params->image_file_name, &image_size); > if (image == NULL) { > PERROR("Could not read guest firmware image %s", image_name); > goto error_out; > @@ -1087,7 +1087,7 @@ xc_hvm_build(xc_interface *xch, uint32_t domid, int memsize, const char *image_n > > image_size = (image_size + PAGE_SIZE - 1) & PAGE_MASK; > > - if (setup_guest(xch, domid, (unsigned long)memsize, image, > + if (setup_guest(xch, domid, (unsigned long)params->mem_size, image, > image_size) < 0) { > ERROR("Error constructing guest OS"); > goto error_out; > @@ -1114,6 +1114,8 @@ error_out: > * files/filenames. If target < memsize, domain is created with > * memsize pages marked populate-on-demand, and with a PoD cache size > * of target. If target == memsize, pages are populated normally. > + * > + * XXX:PoD isn''t supported yet so setting target does nothing. > */ > int xc_hvm_build_target_mem(xc_interface *xch, > uint32_t domid, > @@ -1121,8 +1123,13 @@ int xc_hvm_build_target_mem(xc_interface *xch, > int target, > const char *image_name) > { > - /* XXX:PoD isn''t supported yet */ > - return xc_hvm_build(xch, domid, target, image_name); > + struct xc_hvm_params params; > + > + params.mem_size = (uint64_t)memsize << 20; > + params.mem_target = (uint64_t)target << 20; > + params.image_file_name = image_name; > + > + return xc_hvm_build(xch, domid, ¶ms); > } > > /* > diff --git a/tools/libxc/xc_hvm_build.c b/tools/libxc/xc_hvm_build.c > index 1fa5658..31d4734 100644 > --- a/tools/libxc/xc_hvm_build.c > +++ b/tools/libxc/xc_hvm_build.c > @@ -136,12 +136,12 @@ static int check_mmio_hole(uint64_t start, uint64_t memsize) > } > > static int setup_guest(xc_interface *xch, > - uint32_t dom, int memsize, int target, > + uint32_t dom, const struct xc_hvm_params *params, > char *image, unsigned long image_size) > { > xen_pfn_t *page_array = NULL; > - unsigned long i, nr_pages = (unsigned long)memsize << (20 - PAGE_SHIFT); > - unsigned long target_pages = (unsigned long)target << (20 - PAGE_SHIFT); > + unsigned long i, nr_pages = params->mem_size >> PAGE_SHIFT; > + unsigned long target_pages = params->mem_target >> PAGE_SHIFT; > unsigned long entry_eip, cur_pages, cur_pfn; > void *hvm_info_page; > uint32_t *ident_pt; > @@ -153,11 +153,7 @@ static int setup_guest(xc_interface *xch, > stat_1gb_pages = 0; > int pod_mode = 0; > > - /* An HVM guest must be initialised with at least 2MB memory. */ > - if ( memsize < 2 || target < 2 ) > - goto error_out; > - > - if ( memsize > target ) > + if ( nr_pages > target_pages ) > pod_mode = 1; > > memset(&elf, 0, sizeof(elf)); > @@ -168,7 +164,7 @@ static int setup_guest(xc_interface *xch, > > elf_parse_binary(&elf); > v_start = 0; > - v_end = (unsigned long long)memsize << 20; > + v_end = params->mem_size; > > if ( xc_version(xch, XENVER_capabilities, &caps) != 0 ) > { > @@ -393,39 +389,34 @@ static int setup_guest(xc_interface *xch, > return -1; > } > > -static int xc_hvm_build_internal(xc_interface *xch, > - uint32_t domid, > - int memsize, > - int target, > - char *image, > - unsigned long image_size) > -{ > - if ( (image == NULL) || (image_size == 0) ) > - { > - ERROR("Image required"); > - return -1; > - } > - > - return setup_guest(xch, domid, memsize, target, image, image_size); > -} > - > /* xc_hvm_build: > * Create a domain for a virtualized Linux, using files/filenames. > */ > -int xc_hvm_build(xc_interface *xch, > - uint32_t domid, > - int memsize, > - const char *image_name) > +int xc_hvm_build(xc_interface *xch, uint32_t domid, > + const struct xc_hvm_params *hvm_params) > { > - char *image; > - int sts; > + struct xc_hvm_params params = *hvm_params; > + void *image; > unsigned long image_size; > + int sts; > > - if ( (image_name == NULL) || > - ((image = xc_read_image(xch, image_name, &image_size)) == NULL) ) > + if ( domid == 0 ) > + return -1; > + if ( params.image_file_name == NULL ) > return -1; > > - sts = xc_hvm_build_internal(xch, domid, memsize, memsize, image, image_size); > + if ( params.mem_target == 0 ) > + params.mem_target = params.mem_size; > + > + /* An HVM guest must be initialised with at least 2MB memory. */ > + if ( params.mem_size < (2ull << 20) || params.mem_target < (2ull << 20) ) > + return -1; > + > + image = xc_read_image(xch, params.image_file_name, &image_size); > + if ( image == NULL ) > + return -1; > + > + sts = setup_guest(xch, domid, ¶ms, image, image_size); > > free(image); > > @@ -445,59 +436,13 @@ int xc_hvm_build_target_mem(xc_interface *xch, > int target, > const char *image_name) > { > - char *image; > - int sts; > - unsigned long image_size; > + struct xc_hvm_params params = {}; > > - if ( (image_name == NULL) || > - ((image = xc_read_image(xch, image_name, &image_size)) == NULL) ) > - return -1; > + params.mem_size = (uint64_t)memsize << 20; > + params.mem_target = (uint64_t)target << 20; > + params.image_file_name = image_name; > > - sts = xc_hvm_build_internal(xch, domid, memsize, target, image, image_size); > - > - free(image); > - > - return sts; > -} > - > -/* xc_hvm_build_mem: > - * Create a domain for a virtualized Linux, using memory buffers. > - */ > -int xc_hvm_build_mem(xc_interface *xch, > - uint32_t domid, > - int memsize, > - const char *image_buffer, > - unsigned long image_size) > -{ > - int sts; > - unsigned long img_len; > - char *img; > - > - /* Validate that there is a kernel buffer */ > - > - if ( (image_buffer == NULL) || (image_size == 0) ) > - { > - ERROR("kernel image buffer not present"); > - return -1; > - } > - > - img = xc_inflate_buffer(xch, image_buffer, image_size, &img_len); > - if ( img == NULL ) > - { > - ERROR("unable to inflate ram disk buffer"); > - return -1; > - } > - > - sts = xc_hvm_build_internal(xch, domid, memsize, memsize, > - img, img_len); > - > - /* xc_inflate_buffer may return the original buffer pointer (for > - for already inflated buffers), so exercise some care in freeing */ > - > - if ( (img != NULL) && (img != image_buffer) ) > - free(img); > - > - return sts; > + return xc_hvm_build(xch, domid, ¶ms); > } > > /* > diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h > index 533e702..19b0382 100644 > --- a/tools/libxc/xenguest.h > +++ b/tools/libxc/xenguest.h > @@ -171,10 +171,23 @@ int xc_linux_build_mem(xc_interface *xch, > unsigned int console_evtchn, > unsigned long *console_mfn); > > -int xc_hvm_build(xc_interface *xch, > - uint32_t domid, > - int memsize, > - const char *image_name); > +struct xc_hvm_params { > + uint64_t mem_size; /**< Memory size in bytes. */ > + uint64_t mem_target; /**< Memory target in bytes. */ > + const char *image_file_name; /**< File name of the image to load. */ > +}; > + > +/** > + * Build a HVM domain using a set of parameters. > + * @parm xch libxc context handle. > + * @parm domid domain ID for the new domain. > + * @parm params parameters for the new domain. > + * > + * The memory size and image file parameters are required, the reset > + * are optional. > + */ > +int xc_hvm_build(xc_interface *xch, uint32_t domid, > + const struct xc_hvm_params *hvm_params); > > int xc_hvm_build_target_mem(xc_interface *xch, > uint32_t domid, > @@ -182,12 +195,6 @@ int xc_hvm_build_target_mem(xc_interface *xch, > int target, > const char *image_name); > > -int xc_hvm_build_mem(xc_interface *xch, > - uint32_t domid, > - int memsize, > - const char *image_buffer, > - unsigned long image_size); > - > int xc_suspend_evtchn_release(xc_interface *xch, xc_evtchn *xce, int domid, int suspend_evtchn); > > int xc_suspend_evtchn_init(xc_interface *xch, xc_evtchn *xce, int domid, int port); > diff --git a/tools/libxc/xg_private.c b/tools/libxc/xg_private.c > index 4b624a5..f7e2139 100644 > --- a/tools/libxc/xg_private.c > +++ b/tools/libxc/xg_private.c > @@ -192,8 +192,7 @@ unsigned long csum_page(void *page) > __attribute__((weak)) > int xc_hvm_build(xc_interface *xch, > uint32_t domid, > - int memsize, > - const char *image_name) > + const struct xc_hvm_params *hvm_params) > { > errno = ENOSYS; > return -1; > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > lists.xen.org/xen-devel
George Dunlap
2012-Feb-29 11:41 UTC
Re: [PATCH 2/2] libxc: add MMIO hole size parameter to xc_hvm_build()
On Tue, Feb 28, 2012 at 6:27 PM, David Vrabel <david.vrabel@citrix.com> wrote:> From: David Vrabel <david.vrabel@citrix.com> > > Add a parameter for the MMIO hole size when building a HVM domain. > > This is useful for specifying a larger than normal MMIO hole to ensure > that no PCI device''s MMIO region overlaps with the guest physical > addresses of RAM. This is needed on certain systems with PCI bridges > with ACS versions that are broken. On these systems, if a device > behind the bridge attempts a DMA to a guest physical address that > overlaps with the MMIO region of another device behind the bridge, > then the bridge intercepts the access and forwards it directly to the > device and the read or write never hits RAM. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com>Acked-by: George Dunlap <george.dunlap@eu.citrix.com>> --- > tools/libxc/xc_hvm_build.c | 29 ++++++++++++++++++----------- > tools/libxc/xenguest.h | 1 + > 2 files changed, 19 insertions(+), 11 deletions(-) > > diff --git a/tools/libxc/xc_hvm_build.c b/tools/libxc/xc_hvm_build.c > index 31d4734..fbeddac 100644 > --- a/tools/libxc/xc_hvm_build.c > +++ b/tools/libxc/xc_hvm_build.c > @@ -46,7 +46,8 @@ > #define NR_SPECIAL_PAGES 5 > #define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x)) > > -static void build_hvm_info(void *hvm_info_page, uint64_t mem_size) > +static void build_hvm_info(void *hvm_info_page, uint64_t mem_size, > + uint64_t mmio_start, uint64_t mmio_size) > { > struct hvm_info_table *hvm_info = (struct hvm_info_table *) > (((unsigned char *)hvm_info_page) + HVM_INFO_OFFSET); > @@ -54,10 +55,10 @@ static void build_hvm_info(void *hvm_info_page, uint64_t mem_size) > uint8_t sum; > int i; > > - if ( lowmem_end > HVM_BELOW_4G_RAM_END ) > + if ( lowmem_end > mmio_start ) > { > - highmem_end = lowmem_end + (1ull<<32) - HVM_BELOW_4G_RAM_END; > - lowmem_end = HVM_BELOW_4G_RAM_END; > + highmem_end = (1ull<<32) + (lowmem_end - mmio_start); > + lowmem_end = mmio_start; > } > > memset(hvm_info_page, 0, PAGE_SIZE); > @@ -126,10 +127,10 @@ static int loadelfimage( > * Check whether there exists mmio hole in the specified memory range. > * Returns 1 if exists, else returns 0. > */ > -static int check_mmio_hole(uint64_t start, uint64_t memsize) > +static int check_mmio_hole(uint64_t start, uint64_t memsize, > + uint64_t mmio_start, uint64_t mmio_size) > { > - if ( start + memsize <= HVM_BELOW_4G_MMIO_START || > - start >= HVM_BELOW_4G_MMIO_START + HVM_BELOW_4G_MMIO_LENGTH ) > + if ( start + memsize <= mmio_start || start >= mmio_start + mmio_size ) > return 0; > else > return 1; > @@ -142,6 +143,8 @@ static int setup_guest(xc_interface *xch, > xen_pfn_t *page_array = NULL; > unsigned long i, nr_pages = params->mem_size >> PAGE_SHIFT; > unsigned long target_pages = params->mem_target >> PAGE_SHIFT; > + uint64_t mmio_start = (1ull << 32) - params->mmio_size; > + uint64_t mmio_size = params->mmio_size; > unsigned long entry_eip, cur_pages, cur_pfn; > void *hvm_info_page; > uint32_t *ident_pt; > @@ -188,8 +191,8 @@ static int setup_guest(xc_interface *xch, > > for ( i = 0; i < nr_pages; i++ ) > page_array[i] = i; > - for ( i = HVM_BELOW_4G_RAM_END >> PAGE_SHIFT; i < nr_pages; i++ ) > - page_array[i] += HVM_BELOW_4G_MMIO_LENGTH >> PAGE_SHIFT; > + for ( i = mmio_start >> PAGE_SHIFT; i < nr_pages; i++ ) > + page_array[i] += mmio_size >> PAGE_SHIFT; > > /* > * Allocate memory for HVM guest, skipping VGA hole 0xA0000-0xC0000. > @@ -230,7 +233,8 @@ static int setup_guest(xc_interface *xch, > if ( ((count | cur_pfn) & (SUPERPAGE_1GB_NR_PFNS - 1)) == 0 && > /* Check if there exists MMIO hole in the 1GB memory range */ > !check_mmio_hole(cur_pfn << PAGE_SHIFT, > - SUPERPAGE_1GB_NR_PFNS << PAGE_SHIFT) ) > + SUPERPAGE_1GB_NR_PFNS << PAGE_SHIFT, > + mmio_start, mmio_size) ) > { > long done; > unsigned long nr_extents = count >> SUPERPAGE_1GB_SHIFT; > @@ -327,7 +331,7 @@ static int setup_guest(xc_interface *xch, > xch, dom, PAGE_SIZE, PROT_READ | PROT_WRITE, > HVM_INFO_PFN)) == NULL ) > goto error_out; > - build_hvm_info(hvm_info_page, v_end); > + build_hvm_info(hvm_info_page, v_end, mmio_start, mmio_size); > munmap(hvm_info_page, PAGE_SIZE); > > /* Allocate and clear special pages. */ > @@ -408,6 +412,9 @@ int xc_hvm_build(xc_interface *xch, uint32_t domid, > if ( params.mem_target == 0 ) > params.mem_target = params.mem_size; > > + if ( params.mmio_size == 0 ) > + params.mmio_size = HVM_BELOW_4G_MMIO_LENGTH; > + > /* An HVM guest must be initialised with at least 2MB memory. */ > if ( params.mem_size < (2ull << 20) || params.mem_target < (2ull << 20) ) > return -1; > diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h > index 19b0382..b06788c 100644 > --- a/tools/libxc/xenguest.h > +++ b/tools/libxc/xenguest.h > @@ -174,6 +174,7 @@ int xc_linux_build_mem(xc_interface *xch, > struct xc_hvm_params { > uint64_t mem_size; /**< Memory size in bytes. */ > uint64_t mem_target; /**< Memory target in bytes. */ > + uint64_t mmio_size; /**< Size of the MMIO hole in bytes. */ > const char *image_file_name; /**< File name of the image to load. */ > }; > > -- > 1.7.2.5 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > lists.xen.org/xen-devel
Ian Campbell
2012-Feb-29 12:12 UTC
Re: [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
On Tue, 2012-02-28 at 18:27 +0000, David Vrabel wrote:> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h > index 533e702..19b0382 100644 > --- a/tools/libxc/xenguest.h > +++ b/tools/libxc/xenguest.h > @@ -171,10 +171,23 @@ int xc_linux_build_mem(xc_interface *xch, > unsigned int console_evtchn, > unsigned long *console_mfn); > > -int xc_hvm_build(xc_interface *xch, > - uint32_t domid, > - int memsize, > - const char *image_name); > +struct xc_hvm_params { > + uint64_t mem_size; /**< Memory size in bytes. */ > + uint64_t mem_target; /**< Memory target in bytes. */ > + const char *image_file_name; /**< File name of the image to load. > */ > +};Only a minor nit but Xen already has a concept of an "HVM param" which is something else. Might this be confusing? What parses that "/**<" syntax? Don''t see it anywhere else in the tree apart from tools/libxl/libxlu_disk_l.h etcv which are autogenerated. Ian.
David Vrabel
2012-Feb-29 12:16 UTC
Re: [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
On 29/02/12 12:12, Ian Campbell wrote:> On Tue, 2012-02-28 at 18:27 +0000, David Vrabel wrote: >> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h >> index 533e702..19b0382 100644 >> --- a/tools/libxc/xenguest.h >> +++ b/tools/libxc/xenguest.h >> @@ -171,10 +171,23 @@ int xc_linux_build_mem(xc_interface *xch, >> unsigned int console_evtchn, >> unsigned long *console_mfn); >> >> -int xc_hvm_build(xc_interface *xch, >> - uint32_t domid, >> - int memsize, >> - const char *image_name); >> +struct xc_hvm_params { >> + uint64_t mem_size; /**< Memory size in bytes. */ >> + uint64_t mem_target; /**< Memory target in bytes. */ >> + const char *image_file_name; /**< File name of the image to load. >> */ >> +}; > > Only a minor nit but Xen already has a concept of an "HVM param" which > is something else. Might this be confusing?Probably. Suggestions for a better name? struct xc_hvm_options?> What parses that "/**<" syntax? Don''t see it anywhere else in the tree > apart from tools/libxl/libxlu_disk_l.h etcv which are autogenerated.Doxygen. I added the markers out of habit. Shall I remove them? David
Ian Campbell
2012-Feb-29 12:18 UTC
Re: [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
On Wed, 2012-02-29 at 12:16 +0000, David Vrabel wrote:> On 29/02/12 12:12, Ian Campbell wrote: > > On Tue, 2012-02-28 at 18:27 +0000, David Vrabel wrote: > >> diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h > >> index 533e702..19b0382 100644 > >> --- a/tools/libxc/xenguest.h > >> +++ b/tools/libxc/xenguest.h > >> @@ -171,10 +171,23 @@ int xc_linux_build_mem(xc_interface *xch, > >> unsigned int console_evtchn, > >> unsigned long *console_mfn); > >> > >> -int xc_hvm_build(xc_interface *xch, > >> - uint32_t domid, > >> - int memsize, > >> - const char *image_name); > >> +struct xc_hvm_params { > >> + uint64_t mem_size; /**< Memory size in bytes. */ > >> + uint64_t mem_target; /**< Memory target in bytes. */ > >> + const char *image_file_name; /**< File name of the image to load. > >> */ > >> +}; > > > > Only a minor nit but Xen already has a concept of an "HVM param" which > > is something else. Might this be confusing? > > Probably. Suggestions for a better name? struct xc_hvm_options?strcut xc_hvm_build_params? Consistent with the name of the function?> > What parses that "/**<" syntax? Don''t see it anywhere else in the tree > > apart from tools/libxl/libxlu_disk_l.h etcv which are autogenerated. > > Doxygen. I added the markers out of habit. Shall I remove them?I doubt we''ll ever use doxygen so if you are resending you may as well. Ian.
Ian Jackson
2012-Feb-29 12:21 UTC
Re: [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure
David Vrabel writes ("Re: [Xen-devel] [PATCH 1/2] libxc: pass parameters to xc_hvm_build() in a structure"):> On 29/02/12 12:12, Ian Campbell wrote: > > Only a minor nit but Xen already has a concept of an "HVM param" which > > is something else. Might this be confusing? > > Probably. Suggestions for a better name? struct xc_hvm_options?Or args maybe.> > What parses that "/**<" syntax? Don''t see it anywhere else in the tree > > apart from tools/libxl/libxlu_disk_l.h etcv which are autogenerated. > > Doxygen. I added the markers out of habit. Shall I remove them?Please. Ian.