| Message ID | 20180315203050.19791-2-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 15/03/18 20:30, Andre Przywara wrote: > gic_event_needs_delivery() is not named very intuitively, especially > the gic_ prefix is somewhat misleading. > Rename it to vgic_vcpu_pending_irq(), which makes it clear that this > relates to the virtual GIC and is about interrupts. > Also add a VCPU parameter, which makes the code more flexible in the > future. The current VGIC expect this to be the current VCPU, so add > an assert to spot any regressions. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> Reviewed-by: Julien Grall <julien.grall@arm.com> Cheers, > --- > Changelog v1 .. v2: > - rename to vgic_vcpu_pending_irq() > - add VCPU parameter > > xen/arch/arm/gic-vgic.c | 16 ++++++++++++++-- > xen/include/asm-arm/event.h | 2 +- > xen/include/asm-arm/gic.h | 2 +- > 3 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index ecb07ceb40..61f093db50 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -339,9 +339,18 @@ void gic_clear_pending_irqs(struct vcpu *v) > gic_remove_from_lr_pending(v, p); > } > > -int gic_events_need_delivery(void) > +/** > + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected > + * @vcpu: The vCPU on which to check for interrupts. > + * > + * Checks whether there is an interrupt on the given VCPU which needs > + * handling in the guest. This requires at least one IRQ to be pending > + * and enabled. > + * > + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise. > + */ > +int vgic_vcpu_pending_irq(struct vcpu *v) > { > - struct vcpu *v = current; > struct pending_irq *p; > unsigned long flags; > const unsigned long apr = gic_hw_ops->read_apr(0); > @@ -349,6 +358,9 @@ int gic_events_need_delivery(void) > int active_priority; > int rc = 0; > > + /* We rely on reading the VMCR, which is only accessible locally. */ > + ASSERT(v == current); > + > mask_priority = gic_hw_ops->read_vmcr_priority(); > active_priority = find_next_bit(&apr, 32, 0); > > diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h > index e8c2a6cb44..c7a415ef57 100644 > --- a/xen/include/asm-arm/event.h > +++ b/xen/include/asm-arm/event.h > @@ -24,7 +24,7 @@ static inline int local_events_need_delivery_nomask(void) > * interrupts disabled so this shouldn't be a problem in the general > * case. > */ > - if ( gic_events_need_delivery() ) > + if ( vgic_vcpu_pending_irq(current) ) > return 1; > > if ( !vcpu_info(current, evtchn_upcall_pending) ) > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index d568957dd1..49cb94f792 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -238,7 +238,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, > > extern void vgic_sync_to_lrs(void); > extern void gic_clear_pending_irqs(struct vcpu *v); > -extern int gic_events_need_delivery(void); > +extern int vgic_vcpu_pending_irq(struct vcpu *v); > > extern void init_maintenance_interrupt(void); > extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq, >
On Thu, 15 Mar 2018, Andre Przywara wrote: > gic_event_needs_delivery() is not named very intuitively, especially > the gic_ prefix is somewhat misleading. > Rename it to vgic_vcpu_pending_irq(), which makes it clear that this > relates to the virtual GIC and is about interrupts. > Also add a VCPU parameter, which makes the code more flexible in the > future. The current VGIC expect this to be the current VCPU, so add > an assert to spot any regressions. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changelog v1 .. v2: > - rename to vgic_vcpu_pending_irq() > - add VCPU parameter > > xen/arch/arm/gic-vgic.c | 16 ++++++++++++++-- > xen/include/asm-arm/event.h | 2 +- > xen/include/asm-arm/gic.h | 2 +- > 3 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index ecb07ceb40..61f093db50 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -339,9 +339,18 @@ void gic_clear_pending_irqs(struct vcpu *v) > gic_remove_from_lr_pending(v, p); > } > > -int gic_events_need_delivery(void) > +/** > + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected > + * @vcpu: The vCPU on which to check for interrupts. > + * > + * Checks whether there is an interrupt on the given VCPU which needs > + * handling in the guest. This requires at least one IRQ to be pending > + * and enabled. > + * > + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise. > + */ > +int vgic_vcpu_pending_irq(struct vcpu *v) > { > - struct vcpu *v = current; > struct pending_irq *p; > unsigned long flags; > const unsigned long apr = gic_hw_ops->read_apr(0); > @@ -349,6 +358,9 @@ int gic_events_need_delivery(void) > int active_priority; > int rc = 0; > > + /* We rely on reading the VMCR, which is only accessible locally. */ > + ASSERT(v == current); > + > mask_priority = gic_hw_ops->read_vmcr_priority(); > active_priority = find_next_bit(&apr, 32, 0); > > diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h > index e8c2a6cb44..c7a415ef57 100644 > --- a/xen/include/asm-arm/event.h > +++ b/xen/include/asm-arm/event.h > @@ -24,7 +24,7 @@ static inline int local_events_need_delivery_nomask(void) > * interrupts disabled so this shouldn't be a problem in the general > * case. > */ > - if ( gic_events_need_delivery() ) > + if ( vgic_vcpu_pending_irq(current) ) > return 1; > > if ( !vcpu_info(current, evtchn_upcall_pending) ) > diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h > index d568957dd1..49cb94f792 100644 > --- a/xen/include/asm-arm/gic.h > +++ b/xen/include/asm-arm/gic.h > @@ -238,7 +238,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, > > extern void vgic_sync_to_lrs(void); > extern void gic_clear_pending_irqs(struct vcpu *v); > -extern int gic_events_need_delivery(void); > +extern int vgic_vcpu_pending_irq(struct vcpu *v); > > extern void init_maintenance_interrupt(void); > extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq, > -- > 2.14.1 >
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c index ecb07ceb40..61f093db50 100644 --- a/xen/arch/arm/gic-vgic.c +++ b/xen/arch/arm/gic-vgic.c @@ -339,9 +339,18 @@ void gic_clear_pending_irqs(struct vcpu *v) gic_remove_from_lr_pending(v, p); } -int gic_events_need_delivery(void) +/** + * vgic_vcpu_pending_irq() - determine if interrupts need to be injected + * @vcpu: The vCPU on which to check for interrupts. + * + * Checks whether there is an interrupt on the given VCPU which needs + * handling in the guest. This requires at least one IRQ to be pending + * and enabled. + * + * Returns: 1 if the guest should run to handle interrupts, 0 otherwise. + */ +int vgic_vcpu_pending_irq(struct vcpu *v) { - struct vcpu *v = current; struct pending_irq *p; unsigned long flags; const unsigned long apr = gic_hw_ops->read_apr(0); @@ -349,6 +358,9 @@ int gic_events_need_delivery(void) int active_priority; int rc = 0; + /* We rely on reading the VMCR, which is only accessible locally. */ + ASSERT(v == current); + mask_priority = gic_hw_ops->read_vmcr_priority(); active_priority = find_next_bit(&apr, 32, 0); diff --git a/xen/include/asm-arm/event.h b/xen/include/asm-arm/event.h index e8c2a6cb44..c7a415ef57 100644 --- a/xen/include/asm-arm/event.h +++ b/xen/include/asm-arm/event.h @@ -24,7 +24,7 @@ static inline int local_events_need_delivery_nomask(void) * interrupts disabled so this shouldn't be a problem in the general * case. */ - if ( gic_events_need_delivery() ) + if ( vgic_vcpu_pending_irq(current) ) return 1; if ( !vcpu_info(current, evtchn_upcall_pending) ) diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index d568957dd1..49cb94f792 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -238,7 +238,7 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned int virq, extern void vgic_sync_to_lrs(void); extern void gic_clear_pending_irqs(struct vcpu *v); -extern int gic_events_need_delivery(void); +extern int vgic_vcpu_pending_irq(struct vcpu *v); extern void init_maintenance_interrupt(void); extern void gic_raise_guest_irq(struct vcpu *v, unsigned int irq,
gic_event_needs_delivery() is not named very intuitively, especially the gic_ prefix is somewhat misleading. Rename it to vgic_vcpu_pending_irq(), which makes it clear that this relates to the virtual GIC and is about interrupts. Also add a VCPU parameter, which makes the code more flexible in the future. The current VGIC expect this to be the current VCPU, so add an assert to spot any regressions. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- Changelog v1 .. v2: - rename to vgic_vcpu_pending_irq() - add VCPU parameter xen/arch/arm/gic-vgic.c | 16 ++++++++++++++-- xen/include/asm-arm/event.h | 2 +- xen/include/asm-arm/gic.h | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-)