Re: [Minios-devel] [UNIKRAFT PATCH v2 2/3] plat/xen/common: Move event channel functions in events.h

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Hi Radu,

thank you for cleaning this up. I have a little note inline

- Yuri.

radunicolau102@xxxxxxxxx writes:

> From: Radu Nicolau <radunicolau102@xxxxxxxxx>
>
> Move mask_evtchn, unmask_evtchn and clear_evtchn function
> prototypes from hypervisor.h to events.h. Change parameter
> type to evtchn_port_t. Remove hypervisor.h inclusion from
> arm/arch_events.c and arm/arch_time.c
>
> Move active_evtchns() macro to events.h
>
> Signed-off-by: Radu Nicolau <radunicolau102@xxxxxxxxx>
> ---
>  plat/xen/arm/arch_events.c           |  1 -
>  plat/xen/arm/arch_time.c             |  1 -
>  plat/xen/events.c                    |  8 +++-----
>  plat/xen/hypervisor.c                |  6 +-----
>  plat/xen/include/common/events.h     | 18 ++++++++++++++++++
>  plat/xen/include/common/hypervisor.h |  3 ---
>  6 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/plat/xen/arm/arch_events.c b/plat/xen/arm/arch_events.c
> index 91f0ecb..2037aca 100644
> --- a/plat/xen/arm/arch_events.c
> +++ b/plat/xen/arm/arch_events.c
> @@ -26,7 +26,6 @@
>  
>  #include <xen-arm/os.h>
>  #include <common/events.h>
> -#include <common/hypervisor.h>
>  #include <uk/print.h>
>  #include <uk/assert.h>
>  #include <uk/essentials.h>
> diff --git a/plat/xen/arm/arch_time.c b/plat/xen/arm/arch_time.c
> index c5d4551..fec89da 100644
> --- a/plat/xen/arm/arch_time.c
> +++ b/plat/xen/arm/arch_time.c
> @@ -25,7 +25,6 @@
>   */
>  
>  #include <xen-arm/os.h>
> -#include <common/hypervisor.h>
>  #include <common/events.h>
>  #include <xen-arm/traps.h>
>  #include <uk/print.h>
> diff --git a/plat/xen/events.c b/plat/xen/events.c
> index 6df3e4b..3a1d155 100644
> --- a/plat/xen/events.c
> +++ b/plat/xen/events.c
> @@ -291,16 +291,14 @@ int evtchn_get_peercontext(evtchn_port_t local_port, 
> char *ctx, int size)
>       return rc;
>  }
>  
> -/* TODO - these were moved from hypervisor.c; integrate into evtchn */
> -
> -inline void mask_evtchn(uint32_t port)
> +inline void mask_evtchn(evtchn_port_t port)
>  {
>       shared_info_t *s = HYPERVISOR_shared_info;
>  
>       uk_set_bit(port, &s->evtchn_mask[0]);
>  }
>  
> -inline void unmask_evtchn(uint32_t port)
> +inline void unmask_evtchn(evtchn_port_t port)
>  {
>       shared_info_t *s = HYPERVISOR_shared_info;
>       vcpu_info_t *vcpu_info = &s->vcpu_info[smp_processor_id()];
> @@ -323,7 +321,7 @@ inline void unmask_evtchn(uint32_t port)
>       }
>  }
>  
> -inline void clear_evtchn(uint32_t port)
> +inline void clear_evtchn(evtchn_port_t port)
>  {
>       shared_info_t *s = HYPERVISOR_shared_info;
>  
> diff --git a/plat/xen/hypervisor.c b/plat/xen/hypervisor.c
> index ca16ed6..09d5fa6 100644
> --- a/plat/xen/hypervisor.c
> +++ b/plat/xen/hypervisor.c
> @@ -42,10 +42,6 @@
>  #include <uk/arch/lcpu.h>
>  #include <uk/arch/atomic.h>
>  
> -// TODO revisit: move to evtchn; why cpu param?
> -#define active_evtchns(cpu, sh, idx)                         \
> -     ((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
First things first, never combine the moving piece of code with
modifying it. That must be done in separate patches.

And I would not move this macro anyways, because there it is used only
in the hypervisor.c. It is still worthy to kill the "cpu" parameter, but
in a separate patch.

> -
>  int in_callback;
>  
>  #ifndef CONFIG_PARAVIRT
> @@ -73,7 +69,7 @@ void do_hypervisor_callback(struct __regs *regs)
>               l1i = ukarch_ffsl(l1);
>               l1 &= ~(1UL << l1i);
>  
> -             while ((l2 = active_evtchns(cpu, s, l1i)) != 0) {
> +             while ((l2 = active_evtchns(s, l1i)) != 0) {
>                       l2i = ukarch_ffsl(l2);
>                       l2 &= ~(1UL << l2i);
>  
> diff --git a/plat/xen/include/common/events.h 
> b/plat/xen/include/common/events.h
> index 971dff9..b70992d 100644
> --- a/plat/xen/include/common/events.h
> +++ b/plat/xen/include/common/events.h
> @@ -47,6 +47,8 @@
>  #include <xen/event_channel.h>
>  #include <uk/arch/lcpu.h>
>  
> +#define active_evtchns(sh, idx)                              \
> +     ((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
>  
>  typedef void (*evtchn_handler_t)(evtchn_port_t, struct __regs *, void *);
>  
> @@ -65,6 +67,22 @@ evtchn_port_t bind_pirq(uint32_t pirq, int will_share,
>  evtchn_port_t bind_evtchn(evtchn_port_t port, evtchn_handler_t handler,
>                         void *data);
>  void unbind_evtchn(evtchn_port_t port);
> +
> +/* Disable events for <port> by setting the masking bit */
> +void mask_evtchn(evtchn_port_t port);
> +
> +/*
> + * Enable events for <port> by unsetting the masking bit.
> + * If pending events are present, call ukplat_lcpu_irqs_handle_pending
> + */
> +void unmask_evtchn(evtchn_port_t port);
> +
> +/*
> + * Clear pending events from <port> by unsetting the pending
> + * events bit
> + */
> +void clear_evtchn(evtchn_port_t port);
> +
>  void init_events(void);
>  int evtchn_alloc_unbound(domid_t pal, evtchn_handler_t handler,
>                        void *data, evtchn_port_t *port);
> diff --git a/plat/xen/include/common/hypervisor.h 
> b/plat/xen/include/common/hypervisor.h
> index 4587b86..417498e 100644
> --- a/plat/xen/include/common/hypervisor.h
> +++ b/plat/xen/include/common/hypervisor.h
> @@ -71,9 +71,6 @@ shared_info_t *map_shared_info(void *p);
>  //TODO START from here on we have to cleanup/refactor/move stuff
>  /* hypervisor.c */
>  void do_hypervisor_callback(struct __regs *regs);
> -void mask_evtchn(uint32_t port);
> -void unmask_evtchn(uint32_t port);
> -void clear_evtchn(uint32_t port);
>  //TODO END
>  
>  extern int in_callback;
> -- 
> 2.7.4
>
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel