| Message ID | 1472784939-14404-17-git-send-email-zhaoshenglong@huawei.com |
|---|---|
| State | New |
| Headers | show |
Hi Shannon, On 02/09/16 03:55, Shannon Zhao wrote: > From: Shannon Zhao <shannon.zhao@linaro.org> > > Here it adds the ACPI tables size to set the target maxmem to avoid > providing less available memory for guest. > > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> > --- > tools/libxl/libxl_arch.h | 2 +- > tools/libxl/libxl_arm.c | 18 +++++++++++++++++- > tools/libxl/libxl_arm.h | 4 ++++ > tools/libxl/libxl_arm_acpi.c | 20 ++++++++++++++++++++ > tools/libxl/libxl_arm_no_acpi.c | 6 ++++++ > tools/libxl/libxl_dom.c | 2 +- > tools/libxl/libxl_x86.c | 2 +- > 7 files changed, 50 insertions(+), 4 deletions(-) > > diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h > index 337061f..d62fa4c 100644 > --- a/tools/libxl/libxl_arch.h > +++ b/tools/libxl/libxl_arch.h > @@ -30,7 +30,7 @@ int libxl__arch_domain_save_config(libxl__gc *gc, > /* arch specific internal domain creation function */ > _hidden > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > - uint32_t domid); > + libxl__domain_build_state *state, uint32_t domid); > > /* setup arch specific hardware description, i.e. DTB on ARM */ > _hidden > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index e73d65e..c7d4f65 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -101,8 +101,24 @@ int libxl__arch_domain_save_config(libxl__gc *gc, > } > > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > - uint32_t domid) > + libxl__domain_build_state *state, uint32_t domid) > { > + libxl_domain_build_info *const info = &d_config->b_info; > + libxl_ctx *ctx = libxl__gc_owner(gc); > + int size; > + > + /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */ > + if (libxl_defbool_val(info->acpi)) { > + size = libxl__get_acpi_size(gc, info, state); > + if (size < 0) > + return ERROR_FAIL; > + if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + > + LIBXL_MAXMEM_CONSTANT + (size + 1023) / 1024)) { I still have some concern about use info->target_memkb + LIBXL_MAXMEM_CONSTANT here. What if the generic code decide to change the computation? We may forgot to replicate here. My suggestion on the previous version was to have in the common code xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + LIBXL_MAXMEM_CONSTANT + libxl__arch_memory_constant()); Or a similar name. > + LOGE(ERROR, "Couldn't set max memory"); > + return ERROR_FAIL; > + } > + } > + > return 0; > } Regards,
Hi Shannon, On 13/09/16 08:03, Shannon Zhao wrote: > > > On 2016/9/12 23:18, Julien Grall wrote: >> Hi Shannon, >> >> On 02/09/16 03:55, Shannon Zhao wrote: >>> From: Shannon Zhao <shannon.zhao@linaro.org> >>> >>> Here it adds the ACPI tables size to set the target maxmem to avoid >>> providing less available memory for guest. >>> >>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org> >>> --- >>> tools/libxl/libxl_arch.h | 2 +- >>> tools/libxl/libxl_arm.c | 18 +++++++++++++++++- >>> tools/libxl/libxl_arm.h | 4 ++++ >>> tools/libxl/libxl_arm_acpi.c | 20 ++++++++++++++++++++ >>> tools/libxl/libxl_arm_no_acpi.c | 6 ++++++ >>> tools/libxl/libxl_dom.c | 2 +- >>> tools/libxl/libxl_x86.c | 2 +- >>> 7 files changed, 50 insertions(+), 4 deletions(-) >>> >>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h >>> index 337061f..d62fa4c 100644 >>> --- a/tools/libxl/libxl_arch.h >>> +++ b/tools/libxl/libxl_arch.h >>> @@ -30,7 +30,7 @@ int libxl__arch_domain_save_config(libxl__gc *gc, >>> /* arch specific internal domain creation function */ >>> _hidden >>> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >>> *d_config, >>> - uint32_t domid); >>> + libxl__domain_build_state *state, >>> uint32_t domid); >>> >>> /* setup arch specific hardware description, i.e. DTB on ARM */ >>> _hidden >>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c >>> index e73d65e..c7d4f65 100644 >>> --- a/tools/libxl/libxl_arm.c >>> +++ b/tools/libxl/libxl_arm.c >>> @@ -101,8 +101,24 @@ int libxl__arch_domain_save_config(libxl__gc *gc, >>> } >>> >>> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >>> *d_config, >>> - uint32_t domid) >>> + libxl__domain_build_state *state, >>> uint32_t domid) >>> { >>> + libxl_domain_build_info *const info = &d_config->b_info; >>> + libxl_ctx *ctx = libxl__gc_owner(gc); >>> + int size; >>> + >>> + /* Add the size of ACPI tables to maxmem if ACPI is enabled for >>> guest. */ >>> + if (libxl_defbool_val(info->acpi)) { >>> + size = libxl__get_acpi_size(gc, info, state); >>> + if (size < 0) >>> + return ERROR_FAIL; >>> + if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + >>> + LIBXL_MAXMEM_CONSTANT + (size + 1023) >>> / 1024)) { >> >> I still have some concern about use info->target_memkb + >> LIBXL_MAXMEM_CONSTANT here. What if the generic code decide to change >> the computation? >> >> We may forgot to replicate here. My suggestion on the previous version >> was to have in the common code >> >> xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + >> LIBXL_MAXMEM_CONSTANT + libxl__arch_memory_constant()); >> >> Or a similar name. >> > Just to confirm, do you mean that having a arch function to get the size > each arch needs and adding the size in libxl__build_pre()? That's correct. Wei, Ian, do you have any opinions on this? Regards,
diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h index 337061f..d62fa4c 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -30,7 +30,7 @@ int libxl__arch_domain_save_config(libxl__gc *gc, /* arch specific internal domain creation function */ _hidden int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, - uint32_t domid); + libxl__domain_build_state *state, uint32_t domid); /* setup arch specific hardware description, i.e. DTB on ARM */ _hidden diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index e73d65e..c7d4f65 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -101,8 +101,24 @@ int libxl__arch_domain_save_config(libxl__gc *gc, } int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, - uint32_t domid) + libxl__domain_build_state *state, uint32_t domid) { + libxl_domain_build_info *const info = &d_config->b_info; + libxl_ctx *ctx = libxl__gc_owner(gc); + int size; + + /* Add the size of ACPI tables to maxmem if ACPI is enabled for guest. */ + if (libxl_defbool_val(info->acpi)) { + size = libxl__get_acpi_size(gc, info, state); + if (size < 0) + return ERROR_FAIL; + if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb + + LIBXL_MAXMEM_CONSTANT + (size + 1023) / 1024)) { + LOGE(ERROR, "Couldn't set max memory"); + return ERROR_FAIL; + } + } + return 0; } diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h index a91ff93..37b1f15 100644 --- a/tools/libxl/libxl_arm.h +++ b/tools/libxl/libxl_arm.h @@ -24,6 +24,10 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, libxl__domain_build_state *state, struct xc_dom_image *dom); +_hidden +int libxl__get_acpi_size(libxl__gc *gc, libxl_domain_build_info *info, + libxl__domain_build_state *state); + static inline uint64_t libxl__compute_mpdir(unsigned int cpuid) { /* diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c index 30e4d66..9854c7a 100644 --- a/tools/libxl/libxl_arm_acpi.c +++ b/tools/libxl/libxl_arm_acpi.c @@ -94,6 +94,26 @@ static int libxl__estimate_madt_size(libxl__gc *gc, return rc; } +int libxl__get_acpi_size(libxl__gc *gc, libxl_domain_build_info *info, + libxl__domain_build_state *state) +{ + int size; + + size = libxl__estimate_madt_size(gc, info, &state->config); + if (size < 0) + goto out; + + size = ROUNDUP(size, 3) + + ROUNDUP(sizeof(struct acpi_table_rsdp), 3) + + ROUNDUP(sizeof(struct acpi_table_xsdt), 3) + + ROUNDUP(sizeof(struct acpi_table_gtdt), 3) + + ROUNDUP(sizeof(struct acpi_table_fadt), 3) + + ROUNDUP(sizeof(dsdt_anycpu_arm_len), 3); + +out: + return size; +} + static int libxl__estimate_acpi_size(libxl__gc *gc, libxl_domain_build_info *info, struct xc_dom_image *dom, diff --git a/tools/libxl/libxl_arm_no_acpi.c b/tools/libxl/libxl_arm_no_acpi.c index e7f7411..5eeb825 100644 --- a/tools/libxl/libxl_arm_no_acpi.c +++ b/tools/libxl/libxl_arm_no_acpi.c @@ -25,6 +25,12 @@ int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info, return ERROR_FAIL; } +int libxl__get_acpi_size(libxl__gc *gc, libxl_domain_build_info *info, + libxl__domain_build_state *state) +{ + return ERROR_FAIL; +} + /* * Local variables: * mode: C diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 7dbf614..a2cd350 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -437,7 +437,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid, #endif } - rc = libxl__arch_domain_create(gc, d_config, domid); + rc = libxl__arch_domain_create(gc, d_config, state, domid); return rc; } diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index e9127bb..c872089 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -285,7 +285,7 @@ static int libxl__e820_alloc(libxl__gc *gc, uint32_t domid, } int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, - uint32_t domid) + libxl__domain_build_state *state, uint32_t domid) { int ret = 0; int tsc_mode;