Mailing List Archive

[PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one
Having a FIFO specific header is not (or at least no longer) warranted
with just three function declarations left there. Introduce a private
header instead, moving there some further items from xen/event.h.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: New.
---
TBD: If - considering the layering violation that's there anyway - we
allowed PV shim code to make use of this header, a few more items
could be moved out of "public sight".

--- a/xen/common/event_2l.c
+++ b/xen/common/event_2l.c
@@ -7,11 +7,12 @@
* Version 2 or later. See the file COPYING for more details.
*/

+#include "event_channel.h"
+
#include <xen/init.h>
#include <xen/lib.h>
#include <xen/errno.h>
#include <xen/sched.h>
-#include <xen/event.h>

#include <asm/guest_atomics.h>

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -14,17 +14,17 @@
* along with this program; If not, see <http://www.gnu.org/licenses/>.
*/

+#include "event_channel.h"
+
#include <xen/init.h>
#include <xen/lib.h>
#include <xen/errno.h>
#include <xen/sched.h>
-#include <xen/event.h>
#include <xen/irq.h>
#include <xen/iocap.h>
#include <xen/compat.h>
#include <xen/guest_access.h>
#include <xen/keyhandler.h>
-#include <xen/event_fifo.h>
#include <asm/current.h>

#include <public/xen.h>
--- /dev/null
+++ b/xen/common/event_channel.h
@@ -0,0 +1,63 @@
+/* Event channel handling private header. */
+
+#include <xen/event.h>
+
+static inline unsigned int max_evtchns(const struct domain *d)
+{
+ return d->evtchn_fifo ? EVTCHN_FIFO_NR_CHANNELS
+ : BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d);
+}
+
+static inline bool evtchn_is_busy(const struct domain *d,
+ const struct evtchn *evtchn)
+{
+ return d->evtchn_port_ops->is_busy &&
+ d->evtchn_port_ops->is_busy(d, evtchn);
+}
+
+static inline void evtchn_port_unmask(struct domain *d,
+ struct evtchn *evtchn)
+{
+ if ( evtchn_usable(evtchn) )
+ d->evtchn_port_ops->unmask(d, evtchn);
+}
+
+static inline int evtchn_port_set_priority(struct domain *d,
+ struct evtchn *evtchn,
+ unsigned int priority)
+{
+ if ( !d->evtchn_port_ops->set_priority )
+ return -ENOSYS;
+ if ( !evtchn_usable(evtchn) )
+ return -EACCES;
+ return d->evtchn_port_ops->set_priority(d, evtchn, priority);
+}
+
+static inline void evtchn_port_print_state(struct domain *d,
+ const struct evtchn *evtchn)
+{
+ d->evtchn_port_ops->print_state(d, evtchn);
+}
+
+/* 2-level */
+
+void evtchn_2l_init(struct domain *d);
+
+/* 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);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -7,12 +7,12 @@
* Version 2 or later. See the file COPYING for more details.
*/

+#include "event_channel.h"
+
#include <xen/init.h>
#include <xen/lib.h>
#include <xen/errno.h>
#include <xen/sched.h>
-#include <xen/event.h>
-#include <xen/event_fifo.h>
#include <xen/paging.h>
#include <xen/mm.h>
#include <xen/domain_page.h>
--- a/xen/include/xen/event.h
+++ b/xen/include/xen/event.h
@@ -105,12 +105,6 @@ void notify_via_xen_event_channel(struct
#define bucket_from_port(d, p) \
((group_from_port(d, p))[((p) % EVTCHNS_PER_GROUP) / EVTCHNS_PER_BUCKET])

-static inline unsigned int max_evtchns(const struct domain *d)
-{
- return d->evtchn_fifo ? EVTCHN_FIFO_NR_CHANNELS
- : BITS_PER_EVTCHN_WORD(d) * BITS_PER_EVTCHN_WORD(d);
-}
-
static inline bool_t port_is_valid(struct domain *d, unsigned int p)
{
if ( p >= read_atomic(&d->valid_evtchns) )
@@ -176,8 +170,6 @@ static bool evtchn_usable(const struct e

void evtchn_check_pollers(struct domain *d, unsigned int port);

-void evtchn_2l_init(struct domain *d);
-
/* Close all event channels and reset to 2-level ABI. */
int evtchn_reset(struct domain *d, bool resuming);

@@ -227,13 +219,6 @@ static inline void evtchn_port_clear_pen
d->evtchn_port_ops->clear_pending(d, evtchn);
}

-static inline void evtchn_port_unmask(struct domain *d,
- struct evtchn *evtchn)
-{
- if ( evtchn_usable(evtchn) )
- d->evtchn_port_ops->unmask(d, evtchn);
-}
-
static inline bool evtchn_is_pending(const struct domain *d,
const struct evtchn *evtchn)
{
@@ -259,13 +244,6 @@ static inline bool evtchn_port_is_masked
return rc;
}

-static inline bool evtchn_is_busy(const struct domain *d,
- const struct evtchn *evtchn)
-{
- return d->evtchn_port_ops->is_busy &&
- d->evtchn_port_ops->is_busy(d, evtchn);
-}
-
/* Returns negative errno, zero for not pending, or positive for pending. */
static inline int evtchn_port_poll(struct domain *d, evtchn_port_t port)
{
@@ -285,21 +263,4 @@ static inline int evtchn_port_poll(struc
return rc;
}

-static inline int evtchn_port_set_priority(struct domain *d,
- struct evtchn *evtchn,
- unsigned int priority)
-{
- if ( !d->evtchn_port_ops->set_priority )
- return -ENOSYS;
- if ( !evtchn_usable(evtchn) )
- return -EACCES;
- return d->evtchn_port_ops->set_priority(d, evtchn, priority);
-}
-
-static inline void evtchn_port_print_state(struct domain *d,
- const struct evtchn *evtchn)
-{
- d->evtchn_port_ops->print_state(d, evtchn);
-}
-
#endif /* __XEN_EVENT_H__ */
--- a/xen/include/xen/event_fifo.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * FIFO-based event channel ABI.
- *
- * Copyright (C) 2013 Citrix Systems R&D Ltd.
- *
- * This source code is licensed under the GNU General Public License,
- * Version 2 or later. See the file COPYING for more details.
- */
-#ifndef __XEN_EVENT_FIFO_H__
-#define __XEN_EVENT_FIFO_H__
-
-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 *domain);
-
-#endif /* __XEN_EVENT_FIFO_H__ */
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
Re: [PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one [ In reply to ]
On Tue, Oct 20, 2020 at 04:08:33PM +0200, Jan Beulich wrote:
> Having a FIFO specific header is not (or at least no longer) warranted
> with just three function declarations left there. Introduce a private
> header instead, moving there some further items from xen/event.h.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> v2: New.
> ---
> TBD: If - considering the layering violation that's there anyway - we
> allowed PV shim code to make use of this header, a few more items
> could be moved out of "public sight".
>
> --- a/xen/common/event_2l.c
> +++ b/xen/common/event_2l.c
> @@ -7,11 +7,12 @@
> * Version 2 or later. See the file COPYING for more details.
> */
>
> +#include "event_channel.h"
> +
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/errno.h>
> #include <xen/sched.h>
> -#include <xen/event.h>
>
> #include <asm/guest_atomics.h>
>
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -14,17 +14,17 @@
> * along with this program; If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include "event_channel.h"
> +
> #include <xen/init.h>
> #include <xen/lib.h>
> #include <xen/errno.h>
> #include <xen/sched.h>
> -#include <xen/event.h>
> #include <xen/irq.h>
> #include <xen/iocap.h>
> #include <xen/compat.h>
> #include <xen/guest_access.h>
> #include <xen/keyhandler.h>
> -#include <xen/event_fifo.h>
> #include <asm/current.h>
>
> #include <public/xen.h>
> --- /dev/null
> +++ b/xen/common/event_channel.h
> @@ -0,0 +1,63 @@
> +/* Event channel handling private header. */

I've got no idea whether it matters or not, but the code moved here
from xen/events.h had an explicit copyright banner with Keirs name,
should this be kept?

Thanks, Roger.
Re: [PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one [ In reply to ]
Hi Jan,

On 20/10/2020 15:08, Jan Beulich wrote:
> Having a FIFO specific header is not (or at least no longer) warranted
> with just three function declarations left there. Introduce a private
> header instead, moving there some further items from xen/event.h.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Julien Grall <jgrall@amazon.com>

> ---
> v2: New.
> ---
> TBD: If - considering the layering violation that's there anyway - we
> allowed PV shim code to make use of this header, a few more items
> could be moved out of "public sight".

Are you referring to PV shim calling function such as evtchn_bind_vcpu()?

Cheers,

--
Julien Grall
Re: [PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one [ In reply to ]
On 30.10.2020 11:21, Julien Grall wrote:
> On 20/10/2020 15:08, Jan Beulich wrote:
>> Having a FIFO specific header is not (or at least no longer) warranted
>> with just three function declarations left there. Introduce a private
>> header instead, moving there some further items from xen/event.h.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks; perhaps you didn't notice this went in already?

>> ---
>> v2: New.
>> ---
>> TBD: If - considering the layering violation that's there anyway - we
>> allowed PV shim code to make use of this header, a few more items
>> could be moved out of "public sight".
>
> Are you referring to PV shim calling function such as evtchn_bind_vcpu()?

Yes.

Jan
Re: [PATCH v2 2/8] evtchn: replace FIFO-specific header by generic private one [ In reply to ]
On 30/10/2020 10:42, Jan Beulich wrote:
> On 30.10.2020 11:21, Julien Grall wrote:
>> On 20/10/2020 15:08, Jan Beulich wrote:
>>> Having a FIFO specific header is not (or at least no longer) warranted
>>> with just three function declarations left there. Introduce a private
>>> header instead, moving there some further items from xen/event.h.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
>
> Thanks; perhaps you didn't notice this went in already?

I only noticed it afterwards. I find useful when committers send a quick
message mentioning that part of the series has been committed.

Cheers,

--
Julien Grall