| Message ID | 20171019124847.5978-11-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 19 Oct 2017, Andre Przywara wrote: > At the moment we happily access VGIC internal data structures like > the rank and struct pending_irq in gic.c, which should be VGIC agnostic. > > Factor out a new function vgic_connect_hw_irq(), which allows a virtual > IRQ to be connected to a hardware IRQ (using the hw bit in the LR). > > This removes said accesses to VGIC data structures and improves abstraction. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/gic-vgic.c | 31 +++++++++++++++++++++++++++++++ > xen/arch/arm/gic.c | 42 ++++++------------------------------------ > xen/include/asm-arm/vgic.h | 2 ++ > 3 files changed, 39 insertions(+), 36 deletions(-) > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index 66cae21e82..bf9455a34e 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -385,6 +385,37 @@ void gic_inject(struct vcpu *v) > gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1); > } > > +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq, > + struct irq_desc *desc) > +{ > + unsigned long flags; > + /* Use vcpu0 to retrieve the pending_irq struct. Given that we only > + * route SPIs to guests, it doesn't make any difference. */ > + struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); > + struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); > + struct pending_irq *p = irq_to_pending(v_target, virq); > + int ret = 0; > + > + /* We are taking to rank lock to prevent parallel connections. */ > + vgic_lock_rank(v_target, rank, flags); > + > + if ( desc ) > + { > + /* The VIRQ should not be already enabled by the guest */ > + if ( !p->desc && > + !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) ) > + p->desc = desc; > + else > + ret = -EBUSY; > + } > + else > + p->desc = NULL; > + > + vgic_unlock_rank(v_target, rank, flags); > + > + return ret; > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 4cb74d449e..d46a6d54b3 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -128,27 +128,12 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) > int gic_route_irq_to_guest(struct domain *d, unsigned int virq, > struct irq_desc *desc, unsigned int priority) > { > - unsigned long flags; > - /* Use vcpu0 to retrieve the pending_irq struct. Given that we only > - * route SPIs to guests, it doesn't make any difference. */ > - struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); > - struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); > - struct pending_irq *p = irq_to_pending(v_target, virq); > - int res = -EBUSY; > - > ASSERT(spin_is_locked(&desc->lock)); > /* Caller has already checked that the IRQ is an SPI */ > ASSERT(virq >= 32); > ASSERT(virq < vgic_num_irqs(d)); > ASSERT(!is_lpi(virq)); > > - vgic_lock_rank(v_target, rank, flags); > - > - if ( p->desc || > - /* The VIRQ should not be already enabled by the guest */ > - test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) ) > - goto out; > - > desc->handler = gic_hw_ops->gic_guest_irq_type; > set_bit(_IRQ_GUEST, &desc->status); > > @@ -156,31 +141,19 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq, > gic_set_irq_type(desc, desc->arch.type); > gic_set_irq_priority(desc, priority); > > - p->desc = desc; > - res = 0; > - > -out: > - vgic_unlock_rank(v_target, rank, flags); > - > - return res; > + return vgic_connect_hw_irq(d, NULL, virq, desc); > } > > /* This function only works with SPIs for now */ > int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, > struct irq_desc *desc) > { > - struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); > - struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); > - struct pending_irq *p = irq_to_pending(v_target, virq); > - unsigned long flags; > + int ret; > > ASSERT(spin_is_locked(&desc->lock)); > ASSERT(test_bit(_IRQ_GUEST, &desc->status)); > - ASSERT(p->desc == desc); > ASSERT(!is_lpi(virq)); > > - vgic_lock_rank(v_target, rank, flags); > - > if ( d->is_dying ) > { > desc->handler->shutdown(desc); > @@ -198,19 +171,16 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, > */ > if ( test_bit(_IRQ_INPROGRESS, &desc->status) || > !test_bit(_IRQ_DISABLED, &desc->status) ) > - { > - vgic_unlock_rank(v_target, rank, flags); > return -EBUSY; > - } > } > > + ret = vgic_connect_hw_irq(d, NULL, virq, NULL); > + if ( ret ) > + return ret; > + > clear_bit(_IRQ_GUEST, &desc->status); > desc->handler = &no_irq_type; > > - p->desc = NULL; > - > - vgic_unlock_rank(v_target, rank, flags); > - > return 0; > } > > diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h > index dcdb1acaf3..cf02dc6394 100644 > --- a/xen/include/asm-arm/vgic.h > +++ b/xen/include/asm-arm/vgic.h > @@ -220,6 +220,8 @@ int vgic_v2_init(struct domain *d, int *mmio_count); > int vgic_v3_init(struct domain *d, int *mmio_count); > > bool vgic_evtchn_irq_pending(struct vcpu *v); > +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq, > + struct irq_desc *desc); > > extern int domain_vgic_register(struct domain *d, int *mmio_count); > extern int vcpu_vgic_free(struct vcpu *v); > -- > 2.14.1 >
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c index 66cae21e82..bf9455a34e 100644 --- a/xen/arch/arm/gic-vgic.c +++ b/xen/arch/arm/gic-vgic.c @@ -385,6 +385,37 @@ void gic_inject(struct vcpu *v) gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1); } +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq, + struct irq_desc *desc) +{ + unsigned long flags; + /* Use vcpu0 to retrieve the pending_irq struct. Given that we only + * route SPIs to guests, it doesn't make any difference. */ + struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); + struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); + struct pending_irq *p = irq_to_pending(v_target, virq); + int ret = 0; + + /* We are taking to rank lock to prevent parallel connections. */ + vgic_lock_rank(v_target, rank, flags); + + if ( desc ) + { + /* The VIRQ should not be already enabled by the guest */ + if ( !p->desc && + !test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) ) + p->desc = desc; + else + ret = -EBUSY; + } + else + p->desc = NULL; + + vgic_unlock_rank(v_target, rank, flags); + + return ret; +} + /* * Local variables: * mode: C diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 4cb74d449e..d46a6d54b3 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -128,27 +128,12 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority) int gic_route_irq_to_guest(struct domain *d, unsigned int virq, struct irq_desc *desc, unsigned int priority) { - unsigned long flags; - /* Use vcpu0 to retrieve the pending_irq struct. Given that we only - * route SPIs to guests, it doesn't make any difference. */ - struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); - struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); - struct pending_irq *p = irq_to_pending(v_target, virq); - int res = -EBUSY; - ASSERT(spin_is_locked(&desc->lock)); /* Caller has already checked that the IRQ is an SPI */ ASSERT(virq >= 32); ASSERT(virq < vgic_num_irqs(d)); ASSERT(!is_lpi(virq)); - vgic_lock_rank(v_target, rank, flags); - - if ( p->desc || - /* The VIRQ should not be already enabled by the guest */ - test_bit(GIC_IRQ_GUEST_ENABLED, &p->status) ) - goto out; - desc->handler = gic_hw_ops->gic_guest_irq_type; set_bit(_IRQ_GUEST, &desc->status); @@ -156,31 +141,19 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int virq, gic_set_irq_type(desc, desc->arch.type); gic_set_irq_priority(desc, priority); - p->desc = desc; - res = 0; - -out: - vgic_unlock_rank(v_target, rank, flags); - - return res; + return vgic_connect_hw_irq(d, NULL, virq, desc); } /* This function only works with SPIs for now */ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, struct irq_desc *desc) { - struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq); - struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq); - struct pending_irq *p = irq_to_pending(v_target, virq); - unsigned long flags; + int ret; ASSERT(spin_is_locked(&desc->lock)); ASSERT(test_bit(_IRQ_GUEST, &desc->status)); - ASSERT(p->desc == desc); ASSERT(!is_lpi(virq)); - vgic_lock_rank(v_target, rank, flags); - if ( d->is_dying ) { desc->handler->shutdown(desc); @@ -198,19 +171,16 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, */ if ( test_bit(_IRQ_INPROGRESS, &desc->status) || !test_bit(_IRQ_DISABLED, &desc->status) ) - { - vgic_unlock_rank(v_target, rank, flags); return -EBUSY; - } } + ret = vgic_connect_hw_irq(d, NULL, virq, NULL); + if ( ret ) + return ret; + clear_bit(_IRQ_GUEST, &desc->status); desc->handler = &no_irq_type; - p->desc = NULL; - - vgic_unlock_rank(v_target, rank, flags); - return 0; } diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index dcdb1acaf3..cf02dc6394 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -220,6 +220,8 @@ int vgic_v2_init(struct domain *d, int *mmio_count); int vgic_v3_init(struct domain *d, int *mmio_count); bool vgic_evtchn_irq_pending(struct vcpu *v); +int vgic_connect_hw_irq(struct domain *d, struct vcpu *v, unsigned int virq, + struct irq_desc *desc); extern int domain_vgic_register(struct domain *d, int *mmio_count); extern int vcpu_vgic_free(struct vcpu *v);
At the moment we happily access VGIC internal data structures like the rank and struct pending_irq in gic.c, which should be VGIC agnostic. Factor out a new function vgic_connect_hw_irq(), which allows a virtual IRQ to be connected to a hardware IRQ (using the hw bit in the LR). This removes said accesses to VGIC data structures and improves abstraction. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic-vgic.c | 31 +++++++++++++++++++++++++++++++ xen/arch/arm/gic.c | 42 ++++++------------------------------------ xen/include/asm-arm/vgic.h | 2 ++ 3 files changed, 39 insertions(+), 36 deletions(-)