[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v6 16/16] libxl/arm: Add the size of ACPI tables to maxmem
On 2016/9/28 3:00, Wei Liu wrote:
On Tue, Sep 27, 2016 at 11:43:38AM -0700, Shannon Zhao wrote:
On 2016/9/27 9:35, Wei Liu wrote:
On Tue, Sep 27, 2016 at 09:01:00AM -0700, Shannon Zhao wrote:
On 2016/9/27 2:41, Wei Liu wrote:
On Mon, Sep 26, 2016 at 02:54:55PM -0700, Shannon Zhao wrote:
On 2016/9/22 7:10, Wei Liu wrote:
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 2924629..118beab 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -408,8 +408,15 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
}
}
+
+ rc = libxl__arch_memory_constant(gc, info, state);
+ if (rc < 0) {
+ LOGE(ERROR, "Couldn't get arch constant memory size");
+ return ERROR_FAIL;
+ }
+
if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
- LIBXL_MAXMEM_CONSTANT) < 0) {
+ LIBXL_MAXMEM_CONSTANT + rc) < 0) {
I think this LIBXL_MAXMEM_CONSTANT should be pushed to your helper
function, too.
So that, we can have all LIBXL_MAXMEM_CONSTANT removed in libxl
functions (see libxl.c and libxl_dom.c)
If we push LIBXL_MAXMEM_CONSTANT to the libxl_arch_memory_constant and
remove it from libxl.c, do we need to call libxl_arch_memory_constant there
in libxl_set_memory_target()?
Yes, we need to call that function everywhere to get consistent results.
That's the reason I asked you to consolidate it to a function.
Well it's a little awkward I think, since in libxl_domain_setmaxmem() and
libxl_set_memory_target() it seems it can't get the parameters info and
state for libxl__arch_memory_constant().
I'm not sure how to solve it. Wei, any suggestion?
Hmm...
The first question is can state be derived from build_info ? From my
quick skim of the code the answer is likely yes.
I'm not familiar with the relationship between these structures and not sure
how to do this. Please give me some suggestion.
Oh, I was just reading the code in your patch series and existing code
in libxl_arm.c. Here is my analysis of the code, please point out any
inaccuracy.
In your patch that estimates the size of ACPI table(s), xc_config is
needed. In particular, you need to know the gic version -- in fact
that's the only thing you need to know as far as I can tell.
In libxl_arm.c, the gic version is finally saved to d_config, which
means you should be able to later extract that from d_config.
But, as I understand it, you can't use d_config only while *building*
the domain, because the gic version might be determined only after the
domain is constructed (_NATIVE case). If you want to do so, you need to
move some code around, which might or might not be feasible -- I haven't
checked.
So based on my analysis, it would make sense to have such function:
libxl__arch_extra_memory(gc, d_config)
This is the function that is used in libxl_set_memory_target and
friends.
Obviously x86 would only need to return a constant in that function.
Then, in arm implementation:
libxl__get_acpi_size(gc, info, gic_version /* not build_state anymore */)
/* also fix up libxl__estimate_madt_size */
/* this is the function called when constructing the domain etc, only
* in libxl_arm.c */
static acpi_extra_memory(gc, build_info, gic_version)
{
libxl__get_acpi_size...
}
libxl__arch_extra_memory(gc, d_config)
{
gic_version = d_config->..gic_version;
If user doesn't specify gic_version in xl config, the
d_config->b_info.arch_arm.gic_version will be LIBXL_GIC_VERSION_DEFAULT,
so we can't know the exact gic_version which will be constructed later.
Since the gic_version is now only used to determine if it should include
acpi_madt_generic_redistributor size, can we add a function
libxl__get_acpi_max_size which doesn't care about the gic_version and
just returns the max acpi size. And this max size is just for setting
the target maxmem and not for allocating the acpi tables.
What do you think about this?
Thanks,
--
Shannon
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
