[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/arm: Move static event channel feature to a separate module
Hi Julien, On 29/11/2023 18:17, Julien Grall wrote: > > > Hi Michal > > On 29/11/2023 16:34, Michal Orzel wrote: >> Move static event channel feature related code to a separate module >> (static-evtchn.{c,h}) in the spirit of fine granular configuration, so >> that the feature can be disabled if not needed. >> >> Introduce Kconfig option CONFIG_STATIC_EVTCHN, enabled by default (to >> keep the current behavior) dependent on CONFIG_DOM0LESS. While it could >> be possible to create a loopback connection for dom0 only, this use case >> does not really need this feature and all the docs and commit messages >> refer explicitly to the use in dom0less system. >> >> The only function visible externally is alloc_static_evtchn(), so move >> the prototype to static-evtchn.h and provide a stub in case a feature >> is disabled. Guard static_evtchn_created in struct dt_device_node and >> move its helpers to static-evtchn.h. > > I guess this is a matter of taste, but I think > dt_device_set_static_evtchn_created() and > dt_device_static_evtchn_created() are better suited in device_tree.h > because they are touching a field in the device tree structure. > > This would stay consistent with the helper dt_device_set_protected() > which is only used by the IOMMU code yet is defined in device_tree.h. Good point about consistency. I will keep the helpers in device_tree.h + add guards. > > 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? ~Michal
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |