diff mbox series

xen: add kconfig for event_fifo

Message ID alpine.DEB.2.22.394.2503181637100.2000798@ubuntu-linux-20-04-desktop (mailing list archive)
State New
Headers show
Series xen: add kconfig for event_fifo | expand

Commit Message

Stefano Stabellini March 18, 2025, 11:40 p.m. UTC
Evtchn fifos are not needed on smaller systems; the older interface is
lightweight and sufficient. Make it possible to disable evtchn fifo.

Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Comments

Jan Beulich March 20, 2025, 4:54 p.m. UTC | #1
On 19.03.2025 00:40, Stefano Stabellini wrote:
> Evtchn fifos are not needed on smaller systems; the older interface is
> lightweight and sufficient. Make it possible to disable evtchn fifo.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>

Let me ask a more general question, before considering whether to ack:
When kconfig was introduced into Xen, the expectation was (iirc) that
we wouldn't grow its use to the fine grained granularity that it has
been having in Linux. The more build-time controls we have, the harder
it is to know whether a certain piece of code was actually included in
a build someone, say, reports a problem with. Are we knowingly moving
away from that earlier position? evtchn_fifo.c isn't actually that
much code to exclude, after all ...

Jan
Stefano Stabellini March 21, 2025, 9:30 p.m. UTC | #2
On Thu, 20 Mar 2025, Jan Beulich wrote:
> On 19.03.2025 00:40, Stefano Stabellini wrote:
> > Evtchn fifos are not needed on smaller systems; the older interface is
> > lightweight and sufficient. Make it possible to disable evtchn fifo.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
> 
> Let me ask a more general question, before considering whether to ack:
> When kconfig was introduced into Xen, the expectation was (iirc) that
> we wouldn't grow its use to the fine grained granularity that it has
> been having in Linux. The more build-time controls we have, the harder
> it is to know whether a certain piece of code was actually included in
> a build someone, say, reports a problem with. Are we knowingly moving
> away from that earlier position? evtchn_fifo.c isn't actually that
> much code to exclude, after all ...


I think we need to be more flexible with build-time controls. The extent
to which we should be flexible is an interesting discussion to have.

However, this patch is not for code size. This code causes unnecessary
runtime anonymous memory allocations, which are highly undesirable. In
fact, it is the primary source of such allocations. Additionally, it
exposes an extra interface to the guest, which is also undesirable
unless necessary.

In other words, the issue is not the size of the code, but its impact.
Jan Beulich March 24, 2025, 9:26 a.m. UTC | #3
On 21.03.2025 22:30, Stefano Stabellini wrote:
> On Thu, 20 Mar 2025, Jan Beulich wrote:
>> On 19.03.2025 00:40, Stefano Stabellini wrote:
>>> Evtchn fifos are not needed on smaller systems; the older interface is
>>> lightweight and sufficient. Make it possible to disable evtchn fifo.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
>>
>> Let me ask a more general question, before considering whether to ack:
>> When kconfig was introduced into Xen, the expectation was (iirc) that
>> we wouldn't grow its use to the fine grained granularity that it has
>> been having in Linux. The more build-time controls we have, the harder
>> it is to know whether a certain piece of code was actually included in
>> a build someone, say, reports a problem with. Are we knowingly moving
>> away from that earlier position? evtchn_fifo.c isn't actually that
>> much code to exclude, after all ...
> 
> 
> I think we need to be more flexible with build-time controls. The extent
> to which we should be flexible is an interesting discussion to have.
> 
> However, this patch is not for code size. This code causes unnecessary
> runtime anonymous memory allocations, which are highly undesirable. In
> fact, it is the primary source of such allocations. Additionally, it
> exposes an extra interface to the guest, which is also undesirable
> unless necessary.
> 
> In other words, the issue is not the size of the code, but its impact.

This may help if it was said in the description.

Jan
diff mbox series

Patch

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index a6aa2c5c14..14d82923c5 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -23,6 +23,16 @@  config GRANT_TABLE
 
 	  If unsure, say Y.
 
+config EVTCHN_FIFO
+	bool "Event Channel Fifo support" if EXPERT
+	default y
+	help
+	  The Event channel Fifo extends support for event channels
+	  beyond 1024 event channels for 32-bit guests and 4096 for
+	  64-bit guests.
+
+	  If unsure, say Y.
+
 config PDX_COMPRESSION
 	bool "PDX (Page inDeX) compression" if EXPERT && !X86 && !RISCV
 	default ARM || PPC
diff --git a/xen/common/Makefile b/xen/common/Makefile
index ac23120d7d..9da8a7244d 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -13,7 +13,7 @@  obj-$(CONFIG_IOREQ_SERVER) += dm.o
 obj-y += domain.o
 obj-y += event_2l.o
 obj-y += event_channel.o
-obj-y += event_fifo.o
+obj-$(CONFIG_EVTCHN_FIFO) += event_fifo.o
 obj-$(CONFIG_GRANT_TABLE) += grant_table.o
 obj-y += guestcopy.o
 obj-y += gzip/
diff --git a/xen/common/event_channel.h b/xen/common/event_channel.h
index 45219ca67c..a778ae775b 100644
--- a/xen/common/event_channel.h
+++ b/xen/common/event_channel.h
@@ -45,12 +45,27 @@  void evtchn_2l_init(struct domain *d);
 
 /* FIFO */
 
+#ifdef CONFIG_EVTCHN_FIFO
 struct evtchn_init_control;
 struct evtchn_expand_array;
 
 int evtchn_fifo_init_control(struct evtchn_init_control *init_control);
 int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array);
 void evtchn_fifo_destroy(struct domain *d);
+#else
+static inline int evtchn_fifo_init_control(struct evtchn_init_control *init_control)
+{
+    return -EOPNOTSUPP;
+}
+static inline int evtchn_fifo_expand_array(const struct evtchn_expand_array *expand_array)
+{
+    return -EOPNOTSUPP;
+}
+static inline void evtchn_fifo_destroy(struct domain *d)
+{
+    return;
+}
+#endif /* CONFIG_EVTCHN_FIFO */
 
 /*
  * Local variables: