Re: [PATCH v2] xen/arm: vcpu: Correctly release resource when the VCPU failed to initialized

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 02/05/14 15:09, Julien Grall wrote:
> On 05/02/2014 01:25 PM, Ian Campbell wrote:
>> On Wed, 2014-04-30 at 20:15 +0100, Julien Grall wrote:
>>> While I was adding new failing code at the end of the function, I've noticed
>>> that the vtimers are not freed which mess all the timers and will crash Xen
>>> quickly when the page will be reused.
>>>
>>> Currently neither vcpu_vgic_init nor vcpu_vtimer_init fail, so we
>>> are safe for now. With the new GICv3 code, the former function will be able
>>> to fail. This will result to a memory leak.
>>>
>>> Call vcpu_destroy if the initialization has failed. We also need to add a
>>> boolean to know if the vtimers are correctly setup as the timer common code
>>> doesn't have safe guard against removing non-initialized timer.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> I was about to acked + apply but it failed to build on arm64 with:
>>
>>         domain.c: In function 'alloc_vcpu_struct':
>>         /local/scratch/ianc/devel/committer.git/xen/include/xen/lib.h:19:31: error: static assertion failed: "!(sizeof(*v) > PAGE_SIZE)"
>>          #define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
>>                                        ^
>>         domain.c:415:5: note: in expansion of macro 'BUILD_BUG_ON'
>>              BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
>>              ^
>> struct arch_vcpu is apparently now too large.
> Hmmm... I'm not sure what is the best solution. Can:
> 	1) Allocate 2 pages for the VCPU structure
> 	2) Allocate vgic structure outside.
>
> Any opinions?
>
> Regards,
>

2)

The reason structs vcpu/domain were reduced to this size to was avoid
needing multi-page allocations, which risk allocation failures on
systems with sufficiently fragmented memory.

~Andrew