| Message ID | 0315252d-0378-4699-bff7-ce745d753438@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] xen/events: fix global virq handling | expand |
On 07.03.2025 11:11, Jan Beulich wrote: Sorry, missed From: Juergen Gross <jgross@suse.com> Jan > VIRQs are split into "global" and "per vcpu" ones. Unfortunately in > reality there are "per domain" ones, too. > > send_global_virq() and set_global_virq_handler() make only sense for > the real "global" ones, so replace virq_is_global() with a new > function get_virq_type() returning one of the 3 possible types (global, > domain, vcpu VIRQ). > > To make its intended purpose more clear, also rename > send_guest_global_virq() to send_guest_domain_virq(). > > Fixes: 980822c5edd1 ("xen/events: allow setting of global virq handler only for unbound virqs") > Signed-off-by: Juergen Gross <jgross@suse.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > As I don't have much time today (I'm having a day off), only compile > tested. Please try a CI run with this patch before applying! > > Whether the "domain" infix in send_guest_domain_virq() is really useful > I don't know. Maybe we should go even father with renaming, to have > {vcpu,domain}_send_virq()? > --- > v2: Also rename send_guest_global_virq(). > > --- a/xen/arch/arm/include/asm/event.h > +++ b/xen/arch/arm/include/asm/event.h > @@ -47,9 +47,9 @@ static inline void local_event_delivery_ > } > > /* No arch specific virq definition now. Default to global. */ > -static inline bool arch_virq_is_global(unsigned int virq) > +static inline enum virq_type arch_get_virq_type(unsigned int virq) > { > - return true; > + return VIRQ_GLOBAL; > } > > #endif > --- a/xen/arch/ppc/include/asm/event.h > +++ b/xen/arch/ppc/include/asm/event.h > @@ -17,9 +17,9 @@ static inline int vcpu_event_delivery_is > } > > /* No arch specific virq definition now. Default to global. */ > -static inline bool arch_virq_is_global(unsigned int virq) > +static inline enum virq_type arch_get_virq_type(unsigned int virq) > { > - return true; > + return VIRQ_GLOBAL; > } > > static inline int local_events_need_delivery(void) > --- a/xen/arch/riscv/include/asm/event.h > +++ b/xen/arch/riscv/include/asm/event.h > @@ -24,9 +24,9 @@ static inline void local_event_delivery_ > } > > /* No arch specific virq definition now. Default to global. */ > -static inline bool arch_virq_is_global(unsigned int virq) > +static inline enum virq_type arch_get_virq_type(unsigned int virq) > { > - return true; > + return VIRQ_GLOBAL; > } > > #endif /* ASM__RISCV__EVENT_H */ > --- a/xen/arch/x86/include/asm/event.h > +++ b/xen/arch/x86/include/asm/event.h > @@ -41,10 +41,10 @@ static inline void local_event_delivery_ > vcpu_info(current, evtchn_upcall_mask) = 0; > } > > -/* No arch specific virq definition now. Default to global. */ > -static inline bool arch_virq_is_global(unsigned int virq) > +/* Only global arch specific virq definitions. */ > +static inline enum virq_type arch_get_virq_type(unsigned int virq) > { > - return true; > + return VIRQ_GLOBAL; > } > > #ifdef CONFIG_PV_SHIM > --- a/xen/common/argo.c > +++ b/xen/common/argo.c > @@ -440,7 +440,7 @@ signal_domain(struct domain *d) > { > argo_dprintk("signalling domid:%u\n", d->domain_id); > > - send_guest_global_virq(d, VIRQ_ARGO); > + send_guest_domain_virq(d, VIRQ_ARGO); > } > > static void > --- a/xen/common/event_channel.c > +++ b/xen/common/event_channel.c > @@ -127,7 +127,7 @@ static struct domain *get_global_virq_ha > return global_virq_handlers[virq] ?: hardware_domain; > } > > -static bool virq_is_global(unsigned int virq) > +static enum virq_type get_virq_type(unsigned int virq) > { > switch ( virq ) > { > @@ -135,14 +135,17 @@ static bool virq_is_global(unsigned int > case VIRQ_DEBUG: > case VIRQ_XENOPROF: > case VIRQ_XENPMU: > - return false; > + return VIRQ_VCPU; > + > + case VIRQ_ARGO: > + return VIRQ_DOMAIN; > > case VIRQ_ARCH_0 ... VIRQ_ARCH_7: > - return arch_virq_is_global(virq); > + return arch_get_virq_type(virq); > } > > ASSERT(virq < NR_VIRQS); > - return true; > + return VIRQ_GLOBAL; > } > > static struct evtchn *_evtchn_from_port(const struct domain *d, > @@ -476,7 +479,7 @@ int evtchn_bind_virq(evtchn_bind_virq_t > struct domain *d = current->domain; > int virq = bind->virq, vcpu = bind->vcpu; > int rc = 0; > - bool is_global; > + enum virq_type type; > > if ( (virq < 0) || (virq >= ARRAY_SIZE(v->virq_to_evtchn)) ) > return -EINVAL; > @@ -486,9 +489,9 @@ int evtchn_bind_virq(evtchn_bind_virq_t > * speculative execution. > */ > virq = array_index_nospec(virq, ARRAY_SIZE(v->virq_to_evtchn)); > - is_global = virq_is_global(virq); > + type = get_virq_type(virq); > > - if ( is_global && vcpu != 0 ) > + if ( type != VIRQ_VCPU && vcpu != 0 ) > return -EINVAL; > > if ( (v = domain_vcpu(d, vcpu)) == NULL ) > @@ -496,7 +499,7 @@ int evtchn_bind_virq(evtchn_bind_virq_t > > write_lock(&d->event_lock); > > - if ( is_global && get_global_virq_handler(virq) != d ) > + if ( type == VIRQ_GLOBAL && get_global_virq_handler(virq) != d ) > { > rc = -EBUSY; > goto out; > @@ -756,7 +759,8 @@ int evtchn_close(struct domain *d1, int > if ( chn1->u.virq == VIRQ_DOM_EXC ) > domain_deinit_states(d1); > > - v = d1->vcpu[virq_is_global(chn1->u.virq) ? 0 : chn1->notify_vcpu_id]; > + v = d1->vcpu[get_virq_type(chn1->u.virq) != VIRQ_VCPU > + ? 0 : chn1->notify_vcpu_id]; > > write_lock_irqsave(&v->virq_lock, flags); > ASSERT(read_atomic(&v->virq_to_evtchn[chn1->u.virq]) == port1); > @@ -900,7 +904,7 @@ bool evtchn_virq_enabled(const struct vc > if ( !v ) > return false; > > - if ( virq_is_global(virq) && v->vcpu_id ) > + if ( get_virq_type(virq) != VIRQ_VCPU && v->vcpu_id ) > v = domain_vcpu(v->domain, 0); > > return read_atomic(&v->virq_to_evtchn[virq]); > @@ -913,7 +917,7 @@ void send_guest_vcpu_virq(struct vcpu *v > struct domain *d; > struct evtchn *chn; > > - ASSERT(!virq_is_global(virq)); > + ASSERT(get_virq_type(virq) == VIRQ_VCPU); > > read_lock_irqsave(&v->virq_lock, flags); > > @@ -933,14 +937,14 @@ void send_guest_vcpu_virq(struct vcpu *v > read_unlock_irqrestore(&v->virq_lock, flags); > } > > -void send_guest_global_virq(struct domain *d, uint32_t virq) > +void send_guest_domain_virq(struct domain *d, uint32_t virq) > { > unsigned long flags; > int port; > struct vcpu *v; > struct evtchn *chn; > > - ASSERT(virq_is_global(virq)); > + ASSERT(get_virq_type(virq) != VIRQ_VCPU); > > if ( unlikely(d == NULL) || unlikely(d->vcpu == NULL) ) > return; > @@ -995,9 +999,9 @@ static DEFINE_SPINLOCK(global_virq_handl > > void send_global_virq(uint32_t virq) > { > - ASSERT(virq_is_global(virq)); > + ASSERT(get_virq_type(virq) == VIRQ_GLOBAL); > > - send_guest_global_virq(get_global_virq_handler(virq), virq); > + send_guest_domain_virq(get_global_virq_handler(virq), virq); > } > > int set_global_virq_handler(struct domain *d, uint32_t virq) > @@ -1008,7 +1012,7 @@ int set_global_virq_handler(struct domai > > if (virq >= NR_VIRQS) > return -EINVAL; > - if (!virq_is_global(virq)) > + if (get_virq_type(virq) != VIRQ_GLOBAL) > return -EINVAL; > > if (global_virq_handlers[virq] == d) > @@ -1204,7 +1208,7 @@ int evtchn_bind_vcpu(evtchn_port_t port, > switch ( chn->state ) > { > case ECS_VIRQ: > - if ( virq_is_global(chn->u.virq) ) > + if ( get_virq_type(chn->u.virq) != VIRQ_VCPU ) > chn->notify_vcpu_id = v->vcpu_id; > else > rc = -EINVAL; > --- a/xen/include/xen/event.h > +++ b/xen/include/xen/event.h > @@ -24,17 +24,17 @@ > void send_guest_vcpu_virq(struct vcpu *v, uint32_t virq); > > /* > - * send_global_virq: Notify the domain handling a global VIRQ. > - * @virq: Virtual IRQ number (VIRQ_*) > + * send_guest_domain_virq: > + * @d: Domain to which VIRQ should be sent > + * @virq: Virtual IRQ number (VIRQ_*), may not be per-vCPU > */ > -void send_global_virq(uint32_t virq); > +void send_guest_domain_virq(struct domain *d, uint32_t virq); > > /* > - * send_guest_global_virq: > - * @d: Domain to which VIRQ should be sent > - * @virq: Virtual IRQ number (VIRQ_*), must be global > + * send_global_virq: Notify the domain handling a global VIRQ. > + * @virq: Virtual IRQ number (VIRQ_*) > */ > -void send_guest_global_virq(struct domain *d, uint32_t virq); > +void send_global_virq(uint32_t virq); > > /* > * sent_global_virq_handler: Set a global VIRQ handler. > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -84,6 +84,12 @@ extern domid_t hardware_domid; > #define XEN_CONSUMER_BITS 3 > #define NR_XEN_CONSUMERS ((1 << XEN_CONSUMER_BITS) - 1) > > +enum virq_type { > + VIRQ_GLOBAL, > + VIRQ_DOMAIN, > + VIRQ_VCPU > +}; > + > struct evtchn > { > rwlock_t lock;
--- a/xen/arch/arm/include/asm/event.h +++ b/xen/arch/arm/include/asm/event.h @@ -47,9 +47,9 @@ static inline void local_event_delivery_ } /* No arch specific virq definition now. Default to global. */ -static inline bool arch_virq_is_global(unsigned int virq) +static inline enum virq_type arch_get_virq_type(unsigned int virq) { - return true; + return VIRQ_GLOBAL; } #endif --- a/xen/arch/ppc/include/asm/event.h +++ b/xen/arch/ppc/include/asm/event.h @@ -17,9 +17,9 @@ static inline int vcpu_event_delivery_is } /* No arch specific virq definition now. Default to global. */ -static inline bool arch_virq_is_global(unsigned int virq) +static inline enum virq_type arch_get_virq_type(unsigned int virq) { - return true; + return VIRQ_GLOBAL; } static inline int local_events_need_delivery(void) --- a/xen/arch/riscv/include/asm/event.h +++ b/xen/arch/riscv/include/asm/event.h @@ -24,9 +24,9 @@ static inline void local_event_delivery_ } /* No arch specific virq definition now. Default to global. */ -static inline bool arch_virq_is_global(unsigned int virq) +static inline enum virq_type arch_get_virq_type(unsigned int virq) { - return true; + return VIRQ_GLOBAL; } #endif /* ASM__RISCV__EVENT_H */ --- a/xen/arch/x86/include/asm/event.h +++ b/xen/arch/x86/include/asm/event.h @@ -41,10 +41,10 @@ static inline void local_event_delivery_ vcpu_info(current, evtchn_upcall_mask) = 0; } -/* No arch specific virq definition now. Default to global. */ -static inline bool arch_virq_is_global(unsigned int virq) +/* Only global arch specific virq definitions. */ +static inline enum virq_type arch_get_virq_type(unsigned int virq) { - return true; + return VIRQ_GLOBAL; } #ifdef CONFIG_PV_SHIM --- a/xen/common/argo.c +++ b/xen/common/argo.c @@ -440,7 +440,7 @@ signal_domain(struct domain *d) { argo_dprintk("signalling domid:%u\n", d->domain_id); - send_guest_global_virq(d, VIRQ_ARGO); + send_guest_domain_virq(d, VIRQ_ARGO); } static void --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -127,7 +127,7 @@ static struct domain *get_global_virq_ha return global_virq_handlers[virq] ?: hardware_domain; } -static bool virq_is_global(unsigned int virq) +static enum virq_type get_virq_type(unsigned int virq) { switch ( virq ) { @@ -135,14 +135,17 @@ static bool virq_is_global(unsigned int case VIRQ_DEBUG: case VIRQ_XENOPROF: case VIRQ_XENPMU: - return false; + return VIRQ_VCPU; + + case VIRQ_ARGO: + return VIRQ_DOMAIN; case VIRQ_ARCH_0 ... VIRQ_ARCH_7: - return arch_virq_is_global(virq); + return arch_get_virq_type(virq); } ASSERT(virq < NR_VIRQS); - return true; + return VIRQ_GLOBAL; } static struct evtchn *_evtchn_from_port(const struct domain *d, @@ -476,7 +479,7 @@ int evtchn_bind_virq(evtchn_bind_virq_t struct domain *d = current->domain; int virq = bind->virq, vcpu = bind->vcpu; int rc = 0; - bool is_global; + enum virq_type type; if ( (virq < 0) || (virq >= ARRAY_SIZE(v->virq_to_evtchn)) ) return -EINVAL; @@ -486,9 +489,9 @@ int evtchn_bind_virq(evtchn_bind_virq_t * speculative execution. */ virq = array_index_nospec(virq, ARRAY_SIZE(v->virq_to_evtchn)); - is_global = virq_is_global(virq); + type = get_virq_type(virq); - if ( is_global && vcpu != 0 ) + if ( type != VIRQ_VCPU && vcpu != 0 ) return -EINVAL; if ( (v = domain_vcpu(d, vcpu)) == NULL ) @@ -496,7 +499,7 @@ int evtchn_bind_virq(evtchn_bind_virq_t write_lock(&d->event_lock); - if ( is_global && get_global_virq_handler(virq) != d ) + if ( type == VIRQ_GLOBAL && get_global_virq_handler(virq) != d ) { rc = -EBUSY; goto out; @@ -756,7 +759,8 @@ int evtchn_close(struct domain *d1, int if ( chn1->u.virq == VIRQ_DOM_EXC ) domain_deinit_states(d1); - v = d1->vcpu[virq_is_global(chn1->u.virq) ? 0 : chn1->notify_vcpu_id]; + v = d1->vcpu[get_virq_type(chn1->u.virq) != VIRQ_VCPU + ? 0 : chn1->notify_vcpu_id]; write_lock_irqsave(&v->virq_lock, flags); ASSERT(read_atomic(&v->virq_to_evtchn[chn1->u.virq]) == port1); @@ -900,7 +904,7 @@ bool evtchn_virq_enabled(const struct vc if ( !v ) return false; - if ( virq_is_global(virq) && v->vcpu_id ) + if ( get_virq_type(virq) != VIRQ_VCPU && v->vcpu_id ) v = domain_vcpu(v->domain, 0); return read_atomic(&v->virq_to_evtchn[virq]); @@ -913,7 +917,7 @@ void send_guest_vcpu_virq(struct vcpu *v struct domain *d; struct evtchn *chn; - ASSERT(!virq_is_global(virq)); + ASSERT(get_virq_type(virq) == VIRQ_VCPU); read_lock_irqsave(&v->virq_lock, flags); @@ -933,14 +937,14 @@ void send_guest_vcpu_virq(struct vcpu *v read_unlock_irqrestore(&v->virq_lock, flags); } -void send_guest_global_virq(struct domain *d, uint32_t virq) +void send_guest_domain_virq(struct domain *d, uint32_t virq) { unsigned long flags; int port; struct vcpu *v; struct evtchn *chn; - ASSERT(virq_is_global(virq)); + ASSERT(get_virq_type(virq) != VIRQ_VCPU); if ( unlikely(d == NULL) || unlikely(d->vcpu == NULL) ) return; @@ -995,9 +999,9 @@ static DEFINE_SPINLOCK(global_virq_handl void send_global_virq(uint32_t virq) { - ASSERT(virq_is_global(virq)); + ASSERT(get_virq_type(virq) == VIRQ_GLOBAL); - send_guest_global_virq(get_global_virq_handler(virq), virq); + send_guest_domain_virq(get_global_virq_handler(virq), virq); } int set_global_virq_handler(struct domain *d, uint32_t virq) @@ -1008,7 +1012,7 @@ int set_global_virq_handler(struct domai if (virq >= NR_VIRQS) return -EINVAL; - if (!virq_is_global(virq)) + if (get_virq_type(virq) != VIRQ_GLOBAL) return -EINVAL; if (global_virq_handlers[virq] == d) @@ -1204,7 +1208,7 @@ int evtchn_bind_vcpu(evtchn_port_t port, switch ( chn->state ) { case ECS_VIRQ: - if ( virq_is_global(chn->u.virq) ) + if ( get_virq_type(chn->u.virq) != VIRQ_VCPU ) chn->notify_vcpu_id = v->vcpu_id; else rc = -EINVAL; --- a/xen/include/xen/event.h +++ b/xen/include/xen/event.h @@ -24,17 +24,17 @@ void send_guest_vcpu_virq(struct vcpu *v, uint32_t virq); /* - * send_global_virq: Notify the domain handling a global VIRQ. - * @virq: Virtual IRQ number (VIRQ_*) + * send_guest_domain_virq: + * @d: Domain to which VIRQ should be sent + * @virq: Virtual IRQ number (VIRQ_*), may not be per-vCPU */ -void send_global_virq(uint32_t virq); +void send_guest_domain_virq(struct domain *d, uint32_t virq); /* - * send_guest_global_virq: - * @d: Domain to which VIRQ should be sent - * @virq: Virtual IRQ number (VIRQ_*), must be global + * send_global_virq: Notify the domain handling a global VIRQ. + * @virq: Virtual IRQ number (VIRQ_*) */ -void send_guest_global_virq(struct domain *d, uint32_t virq); +void send_global_virq(uint32_t virq); /* * sent_global_virq_handler: Set a global VIRQ handler. --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -84,6 +84,12 @@ extern domid_t hardware_domid; #define XEN_CONSUMER_BITS 3 #define NR_XEN_CONSUMERS ((1 << XEN_CONSUMER_BITS) - 1) +enum virq_type { + VIRQ_GLOBAL, + VIRQ_DOMAIN, + VIRQ_VCPU +}; + struct evtchn { rwlock_t lock;
VIRQs are split into "global" and "per vcpu" ones. Unfortunately in reality there are "per domain" ones, too. send_global_virq() and set_global_virq_handler() make only sense for the real "global" ones, so replace virq_is_global() with a new function get_virq_type() returning one of the 3 possible types (global, domain, vcpu VIRQ). To make its intended purpose more clear, also rename send_guest_global_virq() to send_guest_domain_virq(). Fixes: 980822c5edd1 ("xen/events: allow setting of global virq handler only for unbound virqs") Signed-off-by: Juergen Gross <jgross@suse.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- As I don't have much time today (I'm having a day off), only compile tested. Please try a CI run with this patch before applying! Whether the "domain" infix in send_guest_domain_virq() is really useful I don't know. Maybe we should go even father with renaming, to have {vcpu,domain}_send_virq()? --- v2: Also rename send_guest_global_virq().