Re: [PATCH 2/2] xen/arm: Handle reserved heap pages in boot and heap allocator

[Michal Orzel <michal.orzel@xxxxxxx>]

On 30/08/2022 11:17, Henry Wang wrote:
> 
> Hi Michal,
> 
>> -----Original Message-----
>> From: Michal Orzel <michal.orzel@xxxxxxx>
>>>> This is totally fine. What I mean is that the check you introduced does not
>>>> correspond
>>>> to the panic message below. In case of reserved heap, its size is selected
>> by
>>>> the user.
>>>> "Not enough space for xenheap" means that there is not enough space to
>> be
>>>> reserved for heap,
>>>> meaning its size is too large. But your check is about size being too 
>>>> small.
>>>
>>> Actually my understanding of "Not enough space for xenheap" is xenheap
>>> is too large so we need to reserve more space, which is slightly different
>> than
>>> your opinion. But I am not the native speaker so it is highly likely that I 
>>> am
>>> making mistakes...
>> My understanding is exactly the same as yours :),
>> meaning heap is too large.
> 
> Oh I think get your point. Let me try to explain myself and thanks for your
> patience :))
> 
> The reserved heap region defined in the device tree should be used for both
> Xenheap and domain heap, so if we reserved a too small region (<32M),
> an error should pop because the reserved region is not enough for xenheap,
> and user should reserve more.
> [...]
> 
>> But your check is against heap being to small (less than 32M).
>> So basically if the following check fails:
>> "( reserved_heap && reserved_heap_pages < 32<<(20-PAGE_SHIFT) ) )"
>> it means that the heap region defined by a user is too small (not too large),
>> because according to requirements it should be at least 32M.
> 
> [...]
> So in that case, printing "Not enough space for xenheap" means the reserved
> region cannot satisfy the minimal requirement of the space of xenheap (at 
> least
> 32M), and this is in consistent with the check.

Ok, it clearly depends on the way someone understands this sentence.
Currently this panic can be triggered if the heap size is too large and
should be read as "heap is too large to fit in because there is not enough space
within RAM considering modules (e - s < size)". Usually (and also in this case)
space refers to a region to contain another one.

You are reusing the same message for different meaning, that is "user defined 
too
small heap and this space (read as size) is not enough".

Let's leave it to someone else to decide.

~Michal