> On 26 Nov 2024, at 13:29, Jan Beulich <[email protected]> wrote:
>
> On 26.11.2024 14:25, Luca Fancellu wrote:
>>> This reads better, thanks. Follow-on question: Is what is statically
>>> configured for the heap guaranteed to never overlap with anything passed
>>> to init_domheap_pages() in those places that you touch?
>>
>> I think so, the places of the check are mainly memory regions related to
>> boot modules,
>> when we add a boot module we also do a check in order to see if it clashes
>> with any
>> reserved regions already defined, which the static heap is part of.
>>
>> Could you explain me why the question?
>
> Well, if there was a chance of overlap, then parts of the free region would
> need to go one way, and the rest the other way.
oh ok, sure of course, thanks for answering.
>
>>>>>> --- a/xen/include/xen/bootfdt.h
>>>>>> +++ b/xen/include/xen/bootfdt.h
>>>>>> @@ -132,7 +132,6 @@ struct bootinfo {
>>>>>> #ifdef CONFIG_STATIC_SHM
>>>>>> struct shared_meminfo shmem;
>>>>>> #endif
>>>>>> - bool static_heap;
>>>>>> };
>>>>>>
>>>>>> #ifdef CONFIG_ACPI
>>>>>> @@ -157,6 +156,10 @@ struct bootinfo {
>>>>>>
>>>>>> extern struct bootinfo bootinfo;
>>>>>>
>>>>>> +#ifdef CONFIG_STATIC_MEMORY
>>>>>> +extern bool static_heap;
>>>>>> +#endif
>>>>>
>>>>> Just to double check Misra-wise: Is there a guarantee that this header
>>>>> will
>>>>> always be included by page-alloc.c, so that the definition of the symbol
>>>>> has an earlier declaration already visible? I ask because this header
>>>>> doesn't look like one where symbols defined in page-alloc.c would normally
>>>>> be declared. And I sincerely hope that this header isn't one of those that
>>>>> end up being included virtually everywhere.
>>>>
>>>> I’ve read again MISRA rule 8.4 and you are right, I should have included
>>>> bootfdt.h in
>>>> page-alloc.c in order to have the declaration visible before defining
>>>> static_heap.
>>>>
>>>> I will include the header in page-alloc.c
>>>
>>> Except that, as said, I don't think that header should be included in this
>>> file.
>>> Instead I think the declaration wants to move elsewhere.
>>
>> Ok sorry, I misunderstood your comment, I thought you were suggesting to
>> have the
>> declaration visible in that file since we are defining there the variable.
>>
>> So Julien suggested that file, it was hosted before in
>> common/device-tree/device-tree.c,
>> see the comment here:
>> https://patchwork.kernel.org/project/xen-devel/patch/[email protected]/#26125054
>>
>> Since it seems you disagree with Julien, could you suggest a more suitable
>> place?
>
> Anything defined in page-alloc.c should by default have its declaration in
> xen/mm.h, imo. Exceptions would need justification.
I would be fine to have the declaration in xen/mm.h, I just need to import
xen/mm.h in bootfdt.h so that it is visible to
“using_static_heap”, @Julien would you be ok with that?
>
> Obviously a possible alternative is to move the definition, not the
> declaration.
>
> Jan
Cheers,
Luca
