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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 14/05/2025 08:59, Orzel, Michal wrote:
> 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?
I was providing an example of why we enforce to enforce __init for 
dom0-build.
Cheers,

--
Julien Grall