| Message ID | 20180321163235.12529-16-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | New VGIC(-v2) implementation | expand |
Hi Andre, On 03/21/2018 04:32 PM, Andre Przywara wrote: > Tell Xen whether a particular VCPU has an IRQ that needs handling > in the guest. This is used to decide whether a VCPU is runnable or > if a hypercall should be preempted to let the guest handle the IRQ. > > This is based on Linux commit 90eee56c5f90, written by Eric Auger. > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > Changelog v2 ... v3: > - adjust vgic_vcpu_pending_irq() to return integers, not false/true I would have preferred to have the return switch to bool instead. I guess this can be done on a follow-up. With one comment below: Reviewed-by: Julien Grall <julien.grall@arm.com> > > Changelog v1 ... v2: > - adjust to new vgic_vcpu_pending_irq() prototype, drop wrapper > > xen/arch/arm/vgic/vgic.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c > index 2fa595f4f7..925cda4580 100644 > --- a/xen/arch/arm/vgic/vgic.c > +++ b/xen/arch/arm/vgic/vgic.c > @@ -647,6 +647,43 @@ void vgic_sync_to_lrs(void) > gic_hw_ops->update_hcr_status(GICH_HCR_EN, 1); > } > > +/** > + * 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. NIT: Because of "ret = irq_is_pending(irq) && irq->enabled", you will return a non-zero value if the guest should run to handle interrupts. > + */ > +int vgic_vcpu_pending_irq(struct vcpu *vcpu) > +{ > + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic; > + struct vgic_irq *irq; > + unsigned long flags; > + int ret = 0; > + > + if ( !vcpu->domain->arch.vgic.enabled ) > + return 0; > + > + spin_lock_irqsave(&vgic_cpu->ap_list_lock, flags); > + > + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) > + { > + spin_lock(&irq->irq_lock); > + ret = irq_is_pending(irq) && irq->enabled; > + spin_unlock(&irq->irq_lock); > + > + if ( ret ) > + break; > + } > + > + spin_unlock_irqrestore(&vgic_cpu->ap_list_lock, flags); > + > + return ret; > +} > + > /* > * Local variables: > * mode: C > Cheers,
Hi, On 22/03/18 03:52, Julien Grall wrote: > Hi Andre, > > On 03/21/2018 04:32 PM, Andre Przywara wrote: >> Tell Xen whether a particular VCPU has an IRQ that needs handling >> in the guest. This is used to decide whether a VCPU is runnable or >> if a hypercall should be preempted to let the guest handle the IRQ. >> >> This is based on Linux commit 90eee56c5f90, written by Eric Auger. >> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> >> --- >> Changelog v2 ... v3: >> - adjust vgic_vcpu_pending_irq() to return integers, not false/true > > I would have preferred to have the return switch to bool instead. I > guess this can be done on a follow-up. With one comment below: I did that originally, but then you meanwhile merged that first patch already. So I didn't want to add another patch to this series. I am fine with changing this afterwards, probably as part of a fixup series. > Reviewed-by: Julien Grall <julien.grall@arm.com> Thanks! Cheers, Andre. >> >> Changelog v1 ... v2: >> - adjust to new vgic_vcpu_pending_irq() prototype, drop wrapper >> >> xen/arch/arm/vgic/vgic.c | 37 +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 37 insertions(+) >> >> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c >> index 2fa595f4f7..925cda4580 100644 >> --- a/xen/arch/arm/vgic/vgic.c >> +++ b/xen/arch/arm/vgic/vgic.c >> @@ -647,6 +647,43 @@ void vgic_sync_to_lrs(void) >> gic_hw_ops->update_hcr_status(GICH_HCR_EN, 1); >> } >> +/** >> + * 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. > > NIT: Because of "ret = irq_is_pending(irq) && irq->enabled", you will > return a non-zero value if the guest should run to handle interrupts. > >> + */ >> +int vgic_vcpu_pending_irq(struct vcpu *vcpu) >> +{ >> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic; >> + struct vgic_irq *irq; >> + unsigned long flags; >> + int ret = 0; >> + >> + if ( !vcpu->domain->arch.vgic.enabled ) >> + return 0; >> + >> + spin_lock_irqsave(&vgic_cpu->ap_list_lock, flags); >> + >> + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) >> + { >> + spin_lock(&irq->irq_lock); >> + ret = irq_is_pending(irq) && irq->enabled; >> + spin_unlock(&irq->irq_lock); >> + >> + if ( ret ) >> + break; >> + } >> + >> + spin_unlock_irqrestore(&vgic_cpu->ap_list_lock, flags); >> + >> + return ret; >> +} >> + >> /* >> * Local variables: >> * mode: C >> > > Cheers, >
On Thu, 22 Mar 2018, Andre Przywara wrote: > Hi, > > On 22/03/18 03:52, Julien Grall wrote: > > Hi Andre, > > > > On 03/21/2018 04:32 PM, Andre Przywara wrote: > >> Tell Xen whether a particular VCPU has an IRQ that needs handling > >> in the guest. This is used to decide whether a VCPU is runnable or > >> if a hypercall should be preempted to let the guest handle the IRQ. > >> > >> This is based on Linux commit 90eee56c5f90, written by Eric Auger. > >> > >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > >> --- > >> Changelog v2 ... v3: > >> - adjust vgic_vcpu_pending_irq() to return integers, not false/true > > > > I would have preferred to have the return switch to bool instead. I > > guess this can be done on a follow-up. With one comment below: > > I did that originally, but then you meanwhile merged that first patch > already. So I didn't want to add another patch to this series. > I am fine with changing this afterwards, probably as part of a fixup series. > > > Reviewed-by: Julien Grall <julien.grall@arm.com> > > Thanks! Acked-by: Stefano Stabellini <sstabellini@kernel.org> > Cheers, > Andre. > > >> > >> Changelog v1 ... v2: > >> - adjust to new vgic_vcpu_pending_irq() prototype, drop wrapper > >> > >> xen/arch/arm/vgic/vgic.c | 37 +++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 37 insertions(+) > >> > >> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c > >> index 2fa595f4f7..925cda4580 100644 > >> --- a/xen/arch/arm/vgic/vgic.c > >> +++ b/xen/arch/arm/vgic/vgic.c > >> @@ -647,6 +647,43 @@ void vgic_sync_to_lrs(void) > >> gic_hw_ops->update_hcr_status(GICH_HCR_EN, 1); > >> } > >> +/** > >> + * 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. > > > > NIT: Because of "ret = irq_is_pending(irq) && irq->enabled", you will > > return a non-zero value if the guest should run to handle interrupts. > > > >> + */ > >> +int vgic_vcpu_pending_irq(struct vcpu *vcpu) > >> +{ > >> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic; > >> + struct vgic_irq *irq; > >> + unsigned long flags; > >> + int ret = 0; > >> + > >> + if ( !vcpu->domain->arch.vgic.enabled ) > >> + return 0; > >> + > >> + spin_lock_irqsave(&vgic_cpu->ap_list_lock, flags); > >> + > >> + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) > >> + { > >> + spin_lock(&irq->irq_lock); > >> + ret = irq_is_pending(irq) && irq->enabled; > >> + spin_unlock(&irq->irq_lock); > >> + > >> + if ( ret ) > >> + break; > >> + } > >> + > >> + spin_unlock_irqrestore(&vgic_cpu->ap_list_lock, flags); > >> + > >> + return ret; > >> +} > >> + > >> /* > >> * Local variables: > >> * mode: C > >> > > > > Cheers, > > >
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c index 2fa595f4f7..925cda4580 100644 --- a/xen/arch/arm/vgic/vgic.c +++ b/xen/arch/arm/vgic/vgic.c @@ -647,6 +647,43 @@ void vgic_sync_to_lrs(void) gic_hw_ops->update_hcr_status(GICH_HCR_EN, 1); } +/** + * 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 *vcpu) +{ + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic; + struct vgic_irq *irq; + unsigned long flags; + int ret = 0; + + if ( !vcpu->domain->arch.vgic.enabled ) + return 0; + + spin_lock_irqsave(&vgic_cpu->ap_list_lock, flags); + + list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) + { + spin_lock(&irq->irq_lock); + ret = irq_is_pending(irq) && irq->enabled; + spin_unlock(&irq->irq_lock); + + if ( ret ) + break; + } + + spin_unlock_irqrestore(&vgic_cpu->ap_list_lock, flags); + + return ret; +} + /* * Local variables: * mode: C
Tell Xen whether a particular VCPU has an IRQ that needs handling in the guest. This is used to decide whether a VCPU is runnable or if a hypercall should be preempted to let the guest handle the IRQ. This is based on Linux commit 90eee56c5f90, written by Eric Auger. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- Changelog v2 ... v3: - adjust vgic_vcpu_pending_irq() to return integers, not false/true Changelog v1 ... v2: - adjust to new vgic_vcpu_pending_irq() prototype, drop wrapper xen/arch/arm/vgic/vgic.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)