Re: [PATCH] xen/arm: Move static event channel feature to a separate module

[Michal Orzel <michal.orzel@xxxxxxx>]

On 29/11/2023 18:54, Julien Grall wrote:
> 
> 
> Hi Michal,
> 
> On 29/11/2023 18:41, Michal Orzel wrote:
>> On 29/11/2023 18:17, Julien Grall wrote:
>>> That said, I could settle on defining the two helpers in the *.c
>>> directly because they are not meant to be used outside of a single *.c.
>>>
>>> Simarly...
>>>
>>>> diff --git a/xen/arch/arm/include/asm/static-evtchn.h 
>>>> b/xen/arch/arm/include/asm/static-evtchn.h
>>>> new file mode 100644
>>>> index 000000000000..472673fae345
>>>> --- /dev/null
>>>> +++ b/xen/arch/arm/include/asm/static-evtchn.h
>>>> @@ -0,0 +1,41 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +
>>>> +#ifndef __ASM_STATIC_EVTCHN_H_
>>>> +#define __ASM_STATIC_EVTCHN_H_
>>>> +
>>>> +#ifdef CONFIG_STATIC_EVTCHN
>>>> +
>>>> +#include <xen/device_tree.h>
>>>> +
>>>> +#define STATIC_EVTCHN_NODE_SIZE_CELLS 2
>>>
>>> ... this used to be defined in domain_build.c. AFAICT the only use is
>>> now in static-evtchn.c. So why is it defined in the header?
>>>
>>> If this is moved in the *.c, then the header static-evtchn.h would only
>>> contain alloc_static_evtchn(). This could be moved in domain_build.h or
>>> setup.h (I don't have any preference).
>> Apart from a prototype, we still need a stub. Therefore I would prefer to 
>> still have a header (will
>> be needed for future upgrades e.g. port exposure in fdt) and move the 
>> prototype and a stub there (the macro
>> I can move to *.c). It just looks better for me + we reduce ifdefery in 
>> domain_build/setup.h.
>> Would you be ok with that?
> 
> I don't much like headers containing just one prototype. But I can live
> with them if you think more will be added.
I can at least think of adding support for discovering static ports.
Thank you for meeting me halfway.

~Michal