Re: [PATCH 1/2] xen/dom0less: mark domain_p2m_set_allocation __init

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 14/05/2025 09:55, Julien Grall wrote:
> 
> 
> On 14/05/2025 08:52, Orzel, Michal wrote:
>>
>>
>> On 14/05/2025 09:37, Julien Grall wrote:
>>> Hi Michal,
>>>
>>> On 14/05/2025 08:04, Orzel, Michal wrote:
>>>>
>>>>
>>>> On 14/05/2025 08:56, Jan Beulich wrote:
>>>>> On 14.05.2025 08:31, Orzel, Michal wrote:
>>>>>> On 14/05/2025 02:07, Stefano Stabellini wrote:
>>>>>>> On Tue, 13 May 2025, Stewart Hildebrand wrote:
>>>>>>>> All functions in dom0less-build.c should be __init.
>>>>>> Why? This patch is first in your series and by that time there is no 
>>>>>> build time
>>>>>> enforcement. Together with the Fixes tag it implies that this is somehow 
>>>>>> an
>>>>>> issue (i.e. build/runtime issue) other than inconsistency for which we 
>>>>>> surely
>>>>>> don't need Fixes tag.
>>>>>
>>>>> I disagree: Code not called post-init should be in .init.*. While not 
>>>>> formally
>>>>> a Misra violation (and wrongly so, I think), it imo effectively is: Such 
>>>>> code
>>>>> is otherwise unreachable post-init.
>>>> You have a point here, I agree. Although I don't think MISRA differentiates
>>>> between unreachable in general vs pre or post init. It defines it as code 
>>>> that
>>>> cannot be executed. It does not go into stages of runtime execution.
>>>>
>>>> I'm thinking how this is different from a function that is called e.g. 
>>>> only once
>>>> at specific point at runtime execution for which we did not come up with a
>>>> separate section?
>>>
>>> Along with what Jan said, in general there is some relaxation for the
>>> boot code. For instance, we could accept if it panic.
>>>
>>> There is at least one of the place in domain_build.c which panic() and
>>> the parsing is not meant to be fully robust. So this code either need to
>>> be __init (as this was the intention from when the feature was created)
>>> or you need to fully harden the code.
>> What is this place?
> 
> static void __init initialize_domU_xenstore(void)
> {
> [...]
>          rc = alloc_xenstore_evtchn(d);
>          if ( rc < 0 )
>              panic("%pd: Failed to allocate xenstore_evtchn\n", d);
> }
Sorry, I am a bit lost, maybe I don't understand your reply. Do you mean we need
to do sth about it (I can see it's __init and we have panic) or this is just an
example?

~Michal