Re: [Xen-devel] [PATCH] libxl: fix "xl mem-set" regression from 0c029c4da2

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.04.15 at 16:01, <ian.campbell@xxxxxxxxxx> wrote:
> On Wed, 2015-04-22 at 13:02 +0100, Jan Beulich wrote:
>> Said commit ("libxl_set_memory_target: retain the same maxmem offset on
>> top of the current target") caused a regression for "xl mem-set"
>> against Dom0: While prior to creation of the first domain this works,
>> the first domain creation involving ballooning breaks. Due to "enforce"
>> not being set in the domain creation case, and due to Dom0's initial
>> ->max_pages (in the hypervisor) being UINT_MAX, the calculation of
>> "memorykb" in the first "xl mem-set" adusting the target upwards
>> subsequent to domain creation and termination may cause an overflow,
>> resulting in Dom0's maximum getting to a very small value. This small
>> maximum will the make the subsequent setting of the PoD target fail.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> Note that this only fixes the immediate problem - there appear to be
>> further issues lurking here:
>> - libxl_set_memory_target()'s *_memkb variables all being 32-bit,
>> - libxl_domain_setmaxmem()'s max_memkb parameter being 32-bit,
> 
> I think that increasing the width of these variables wouldn't break the
> API guarantee which we make, at least not in a practical way, since any
> existing 32-bit arguments passed will just get promoted.

No, not even on 64-bit. On 32-bit, two arguments slots are needed
for what so far requires only one. On 64-bit (at least x86-64), the
calling code isn't required to zero-extend a value calculated in a
register (e.g. a result of earlier calculations which had more than
32 significant bits could be passed unchanged to the called function);
it just so happens that 32-bit arithmetic on registers would always
implicitly zero the upper halves (and iirc that's the same on ARM64).

>> - other similar code living elsewhere?
>> Note also that this requires
>> http://lists.xenproject.org/archives/html/xen-devel/2015-04/msg02485.html 
>> (or some other change avoiding truncation) to also be in place in order
>> to address the observed problem.
>> Note further that xc_domain_setmaxmem() is being used by upstream qemu
>> and hence the libxc interface change here may represent a compatibility
>> issue.
> 
> This might have been a problem wrt getting through the respective push
> gates in the right order, but actually doesn't automatic type promotion
> from unsigned int to uint64_t save us here too?

Same as above - sadly, no.

Jan


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