[Xen-devel,v3,18/39] ARM: new VGIC: Add CTLR, TYPER and IIDR handlers

Message ID 20180321163235.12529-19-andre.przywara@linaro.org
State New
Headers show
Series
  • New VGIC(-v2) implementation
Related show

Commit Message

Andre Przywara March 21, 2018, 4:32 p.m.
Those three registers are v2 emulation specific, so their implementation
lives entirely in vgic-mmio-v2.c. Also they are handled in one function,
as their implementation is pretty simple.
We choose to piggy-back on the existing KVM identification registers,
but use a different variant (major revision).
When the guest enables the distributor, we kick all VCPUs to get
potentially pending interrupts serviced.

This is based on Linux commit 2b0cda878965, written by Marc Zyngier.

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
---
Changelog v2 ... v3:
- fix misleading comment about PRODUCT_ID letter
- clarify on meaning of VARIANT_ID_XEN

Changelog v1 ... v2:
use new IIDR values (KVM product ID, Xen revision)
- add comment on handling GICD enablement
- use new vcpu_kick() function

 xen/arch/arm/vgic/vgic-mmio-v2.c | 63 +++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/vgic/vgic.c         | 15 ++++++++++
 xen/arch/arm/vgic/vgic.h         |  9 ++++++
 3 files changed, 86 insertions(+), 1 deletion(-)

Comments

Julien Grall March 22, 2018, 7:33 a.m. | #1
Hi Andre,

On 03/21/2018 04:32 PM, Andre Przywara wrote:
> Those three registers are v2 emulation specific, so their implementation
> lives entirely in vgic-mmio-v2.c. Also they are handled in one function,
> as their implementation is pretty simple.
> We choose to piggy-back on the existing KVM identification registers,
> but use a different variant (major revision).
> When the guest enables the distributor, we kick all VCPUs to get
> potentially pending interrupts serviced.
> 
> This is based on Linux commit 2b0cda878965, written by Marc Zyngier.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,
Stefano Stabellini March 27, 2018, 8:38 p.m. | #2
On Wed, 21 Mar 2018, Andre Przywara wrote:
> Those three registers are v2 emulation specific, so their implementation
> lives entirely in vgic-mmio-v2.c. Also they are handled in one function,
> as their implementation is pretty simple.
> We choose to piggy-back on the existing KVM identification registers,
> but use a different variant (major revision).

CC'ing Marc.

Reusing the KVM product ID could cause issues, for instance we could
clash with KVM if Linux changes the Major number. If we were to actually
reuse KVM's PRODUCT_ID we would need a better coordination in place.

I suggest to either introduce a Xen specific PRODUCT_ID, or simply reuse
the hardware value like the old vgic does for now. We can fix this
later.

In fact, I would be happy to swith to KVM's product id after appropriate
discussions with the KVM community. At the very least, it would need to
be written down somewhere under docs/ and/or Documentation.

Marc, what do you think?


> When the guest enables the distributor, we kick all VCPUs to get
> potentially pending interrupts serviced.
> 
> This is based on Linux commit 2b0cda878965, written by Marc Zyngier.
> 
> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
> ---
> Changelog v2 ... v3:
> - fix misleading comment about PRODUCT_ID letter
> - clarify on meaning of VARIANT_ID_XEN
> 
> Changelog v1 ... v2:
> use new IIDR values (KVM product ID, Xen revision)
> - add comment on handling GICD enablement
> - use new vcpu_kick() function
> 
>  xen/arch/arm/vgic/vgic-mmio-v2.c | 63 +++++++++++++++++++++++++++++++++++++++-
>  xen/arch/arm/vgic/vgic.c         | 15 ++++++++++
>  xen/arch/arm/vgic/vgic.h         |  9 ++++++
>  3 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c
> index 6f10cf16ca..43c1ab5906 100644
> --- a/xen/arch/arm/vgic/vgic-mmio-v2.c
> +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
> @@ -20,9 +20,70 @@
>  #include "vgic.h"
>  #include "vgic-mmio.h"
>  
> +static unsigned long vgic_mmio_read_v2_misc(struct vcpu *vcpu,
> +                                            paddr_t addr, unsigned int len)
> +{
> +    uint32_t value;
> +
> +    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
> +    {
> +    case GICD_CTLR:
> +        value = vcpu->domain->arch.vgic.enabled ? GICD_CTL_ENABLE : 0;
> +        break;
> +    case GICD_TYPER:
> +        value = vcpu->domain->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS;
> +        value = (value >> 5) - 1;
> +        value |= (vcpu->domain->max_vcpus - 1) << 5;

NIT: You can reuse GICD_TYPE_CPUS_SHIFT


> +        break;
> +    case GICD_IIDR:
> +        value = (PRODUCT_ID_KVM << 24) |
> +                (VARIANT_ID_XEN << 16) |
> +                (IMPLEMENTER_ARM << 0);
> +        break;
> +    default:
> +        return 0;
> +    }
> +
> +    return value;
> +}
> +
> +static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
> +                                    paddr_t addr, unsigned int len,
> +                                    unsigned long val)
> +{
> +    struct vgic_dist *dist = &vcpu->domain->arch.vgic;
> +    bool enabled;
> +
> +    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
> +    {
> +    case GICD_CTLR:
> +        domain_lock(vcpu->domain);
> +
> +        /*
> +         * Store the new enabled state in our distributor structure.
> +         * Work out whether it was disabled before and now got enabled,
> +         * so that we signal all VCPUs to check for interrupts to be injected.
> +         */
> +        enabled = dist->enabled;
> +        dist->enabled = val & GICD_CTL_ENABLE;
> +        enabled = !enabled && dist->enabled;
> +
> +        domain_unlock(vcpu->domain);
> +
> +        if ( enabled )
> +            vgic_kick_vcpus(vcpu->domain);
> +
> +        break;
> +    case GICD_TYPER:
> +    case GICD_IIDR:
> +        /* read-only, writes ignored */
> +        return;
> +    }
> +}
> +
>  static const struct vgic_register_region vgic_v2_dist_registers[] = {
>      REGISTER_DESC_WITH_LENGTH(GICD_CTLR,
> -        vgic_mmio_read_raz, vgic_mmio_write_wi, 12,
> +        vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
>          VGIC_ACCESS_32bit),
>      REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_IGROUPR,
>          vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
> index 925cda4580..37b425a16c 100644
> --- a/xen/arch/arm/vgic/vgic.c
> +++ b/xen/arch/arm/vgic/vgic.c
> @@ -684,6 +684,21 @@ int vgic_vcpu_pending_irq(struct vcpu *vcpu)
>      return ret;
>  }
>  
> +void vgic_kick_vcpus(struct domain *d)
> +{
> +    struct vcpu *vcpu;
> +
> +    /*
> +     * We've injected an interrupt, time to find out who deserves
> +     * a good kick...
> +     */
> +    for_each_vcpu( d, vcpu )
> +    {
> +        if ( vgic_vcpu_pending_irq(vcpu) )
> +            vcpu_kick(vcpu);
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
> index 7f221fd195..aed7e4179a 100644
> --- a/xen/arch/arm/vgic/vgic.h
> +++ b/xen/arch/arm/vgic/vgic.h
> @@ -17,6 +17,14 @@
>  #ifndef __XEN_ARM_VGIC_VGIC_H__
>  #define __XEN_ARM_VGIC_VGIC_H__
>  
> +/*
> + * We piggy-back on the already used KVM product ID,  but use a different
> + * variant (major revision) for Xen.
> + */
> +#define PRODUCT_ID_KVM          0x4b        /* ASCII code K */
> +#define VARIANT_ID_XEN          0x01
> +#define IMPLEMENTER_ARM         0x43b
> +
>  #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
>  
>  static inline bool irq_is_pending(struct vgic_irq *irq)
> @@ -37,6 +45,7 @@ struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
>  void vgic_put_irq(struct domain *d, struct vgic_irq *irq);
>  void vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq,
>                             unsigned long flags);
> +void vgic_kick_vcpus(struct domain *d);
>  
>  static inline void vgic_get_irq_kref(struct vgic_irq *irq)
>  {
Andre Przywara March 28, 2018, 10:36 a.m. | #3
Hi,

On 27/03/18 21:38, Stefano Stabellini wrote:
> On Wed, 21 Mar 2018, Andre Przywara wrote:
>> Those three registers are v2 emulation specific, so their implementation
>> lives entirely in vgic-mmio-v2.c. Also they are handled in one function,
>> as their implementation is pretty simple.
>> We choose to piggy-back on the existing KVM identification registers,
>> but use a different variant (major revision).
> 
> CC'ing Marc.
> 
> Reusing the KVM product ID could cause issues, for instance we could
> clash with KVM if Linux changes the Major number. If we were to actually
> reuse KVM's PRODUCT_ID we would need a better coordination in place.

Marc is aware of this, as this approach was decided after a brief
discussion with Julien and him last week.
If we need a quirk on the (guest) GIC driver side for the KVM VGIC, KVM
would bump the revision number, not the variant.
This is a good as coordination can realistically ever get.

> I suggest to either introduce a Xen specific PRODUCT_ID,

This is a bit of a stretch. Firstly, by the nature of this port the two
implementations are really close. Secondly: we can't really just occupy
any product ID. Technically the "K" isn't even reserved, but this ship
has sailed, so we could just piggy back on that for simplicity.

> or simply reuse
> the hardware value like the old vgic does for now. We can fix this
> later.

This implementation is totally different from anything existing. And we
don't want the guest to apply any errata fixes for the particular
hardware GIC. So using the hardware ID is not the best idea.

Cheers,
Andre

> In fact, I would be happy to swith to KVM's product id after appropriate
> discussions with the KVM community. At the very least, it would need to
> be written down somewhere under docs/ and/or Documentation.
> 
> Marc, what do you think?
> 
> 
>> When the guest enables the distributor, we kick all VCPUs to get
>> potentially pending interrupts serviced.
>>
>> This is based on Linux commit 2b0cda878965, written by Marc Zyngier.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>> ---
>> Changelog v2 ... v3:
>> - fix misleading comment about PRODUCT_ID letter
>> - clarify on meaning of VARIANT_ID_XEN
>>
>> Changelog v1 ... v2:
>> use new IIDR values (KVM product ID, Xen revision)
>> - add comment on handling GICD enablement
>> - use new vcpu_kick() function
>>
>>  xen/arch/arm/vgic/vgic-mmio-v2.c | 63 +++++++++++++++++++++++++++++++++++++++-
>>  xen/arch/arm/vgic/vgic.c         | 15 ++++++++++
>>  xen/arch/arm/vgic/vgic.h         |  9 ++++++
>>  3 files changed, 86 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c
>> index 6f10cf16ca..43c1ab5906 100644
>> --- a/xen/arch/arm/vgic/vgic-mmio-v2.c
>> +++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
>> @@ -20,9 +20,70 @@
>>  #include "vgic.h"
>>  #include "vgic-mmio.h"
>>  
>> +static unsigned long vgic_mmio_read_v2_misc(struct vcpu *vcpu,
>> +                                            paddr_t addr, unsigned int len)
>> +{
>> +    uint32_t value;
>> +
>> +    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
>> +    {
>> +    case GICD_CTLR:
>> +        value = vcpu->domain->arch.vgic.enabled ? GICD_CTL_ENABLE : 0;
>> +        break;
>> +    case GICD_TYPER:
>> +        value = vcpu->domain->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS;
>> +        value = (value >> 5) - 1;
>> +        value |= (vcpu->domain->max_vcpus - 1) << 5;
> 
> NIT: You can reuse GICD_TYPE_CPUS_SHIFT
> 
> 
>> +        break;
>> +    case GICD_IIDR:
>> +        value = (PRODUCT_ID_KVM << 24) |
>> +                (VARIANT_ID_XEN << 16) |
>> +                (IMPLEMENTER_ARM << 0);
>> +        break;
>> +    default:
>> +        return 0;
>> +    }
>> +
>> +    return value;
>> +}
>> +
>> +static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
>> +                                    paddr_t addr, unsigned int len,
>> +                                    unsigned long val)
>> +{
>> +    struct vgic_dist *dist = &vcpu->domain->arch.vgic;
>> +    bool enabled;
>> +
>> +    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
>> +    {
>> +    case GICD_CTLR:
>> +        domain_lock(vcpu->domain);
>> +
>> +        /*
>> +         * Store the new enabled state in our distributor structure.
>> +         * Work out whether it was disabled before and now got enabled,
>> +         * so that we signal all VCPUs to check for interrupts to be injected.
>> +         */
>> +        enabled = dist->enabled;
>> +        dist->enabled = val & GICD_CTL_ENABLE;
>> +        enabled = !enabled && dist->enabled;
>> +
>> +        domain_unlock(vcpu->domain);
>> +
>> +        if ( enabled )
>> +            vgic_kick_vcpus(vcpu->domain);
>> +
>> +        break;
>> +    case GICD_TYPER:
>> +    case GICD_IIDR:
>> +        /* read-only, writes ignored */
>> +        return;
>> +    }
>> +}
>> +
>>  static const struct vgic_register_region vgic_v2_dist_registers[] = {
>>      REGISTER_DESC_WITH_LENGTH(GICD_CTLR,
>> -        vgic_mmio_read_raz, vgic_mmio_write_wi, 12,
>> +        vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
>>          VGIC_ACCESS_32bit),
>>      REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_IGROUPR,
>>          vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
>> diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
>> index 925cda4580..37b425a16c 100644
>> --- a/xen/arch/arm/vgic/vgic.c
>> +++ b/xen/arch/arm/vgic/vgic.c
>> @@ -684,6 +684,21 @@ int vgic_vcpu_pending_irq(struct vcpu *vcpu)
>>      return ret;
>>  }
>>  
>> +void vgic_kick_vcpus(struct domain *d)
>> +{
>> +    struct vcpu *vcpu;
>> +
>> +    /*
>> +     * We've injected an interrupt, time to find out who deserves
>> +     * a good kick...
>> +     */
>> +    for_each_vcpu( d, vcpu )
>> +    {
>> +        if ( vgic_vcpu_pending_irq(vcpu) )
>> +            vcpu_kick(vcpu);
>> +    }
>> +}
>> +
>>  /*
>>   * Local variables:
>>   * mode: C
>> diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
>> index 7f221fd195..aed7e4179a 100644
>> --- a/xen/arch/arm/vgic/vgic.h
>> +++ b/xen/arch/arm/vgic/vgic.h
>> @@ -17,6 +17,14 @@
>>  #ifndef __XEN_ARM_VGIC_VGIC_H__
>>  #define __XEN_ARM_VGIC_VGIC_H__
>>  
>> +/*
>> + * We piggy-back on the already used KVM product ID,  but use a different
>> + * variant (major revision) for Xen.
>> + */
>> +#define PRODUCT_ID_KVM          0x4b        /* ASCII code K */
>> +#define VARIANT_ID_XEN          0x01
>> +#define IMPLEMENTER_ARM         0x43b
>> +
>>  #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
>>  
>>  static inline bool irq_is_pending(struct vgic_irq *irq)
>> @@ -37,6 +45,7 @@ struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
>>  void vgic_put_irq(struct domain *d, struct vgic_irq *irq);
>>  void vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq,
>>                             unsigned long flags);
>> +void vgic_kick_vcpus(struct domain *d);
>>  
>>  static inline void vgic_get_irq_kref(struct vgic_irq *irq)
>>  {
Stefano Stabellini March 28, 2018, 5:20 p.m. | #4
On Wed, 28 Mar 2018, Andre Przywara wrote:
> On 27/03/18 21:38, Stefano Stabellini wrote:
> > On Wed, 21 Mar 2018, Andre Przywara wrote:
> >> Those three registers are v2 emulation specific, so their implementation
> >> lives entirely in vgic-mmio-v2.c. Also they are handled in one function,
> >> as their implementation is pretty simple.
> >> We choose to piggy-back on the existing KVM identification registers,
> >> but use a different variant (major revision).
> > 
> > CC'ing Marc.
> > 
> > Reusing the KVM product ID could cause issues, for instance we could
> > clash with KVM if Linux changes the Major number. If we were to actually
> > reuse KVM's PRODUCT_ID we would need a better coordination in place.
> 
> Marc is aware of this, as this approach was decided after a brief
> discussion with Julien and him last week.
> If we need a quirk on the (guest) GIC driver side for the KVM VGIC, KVM
> would bump the revision number, not the variant.
> This is a good as coordination can realistically ever get.

Sure, as long as it gets documented. It needs to be written down
somewhere and committed.
Stefano Stabellini March 28, 2018, 5:22 p.m. | #5
On Wed, 28 Mar 2018, Andre Przywara wrote:
> >> +static void vgic_mmio_write_sgipends(struct vcpu *vcpu,
> >> +                                     paddr_t addr, unsigned int len,
> >> +                                     unsigned long val)
> >> +{
> >> +    uint32_t intid = VGIC_ADDR_TO_INTID(addr, 8);
> >> +    unsigned int i;
> >> +    unsigned long flags;
> >> +
> >> +    ASSERT(intid < VGIC_NR_SGIS);
> >> +
> >> +    for ( i = 0; i < len; i++ )
> >> +    {
> >> +        struct vgic_irq *irq = vgic_get_irq(vcpu->domain, vcpu, intid + i);
> >> +
> >> +        spin_lock_irqsave(&irq->irq_lock, flags);
> >> +
> >> +        irq->source |= (val >> (i * 8)) & 0xff;
> >> +
> >> +        if ( irq->source )
> >> +        {
> >> +            irq->pending_latch = true;
> >> +            vgic_queue_irq_unlock(vcpu->domain, irq, flags);
> >> +        }
> >> +        else
> >> +        {
> >> +            spin_unlock_irqrestore(&irq->irq_lock, flags);
> >> +        }
> > 
> > NIT: it should be safe to call vgic_queue_irq_unlock regardless, right?
> 
> I don't think vgic_queue_irq_unlock() and subsequent functions can deal
> with the IRQ being pending, but not having a source bit set:
> http://www.linux-arm.org/git?p=xen-ap.git;a=blob;f=xen/arch/arm/vgic/vgic-v2.c;h=6a84e741ee0a#l205
> 
> Besides, this scheme of:
> 	if ( needs to be injected )
> 	{
> 		make_pending();
> 		vgic_queue_irq_unlock();
> 	}
> 	else
> 	{
> 		spin_unlock_irqrestore();
> 	}
> 
> is all over the place, and I don't want to deviate from that and the KVM
> implementation needlessly.

OK

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Patch

diff --git a/xen/arch/arm/vgic/vgic-mmio-v2.c b/xen/arch/arm/vgic/vgic-mmio-v2.c
index 6f10cf16ca..43c1ab5906 100644
--- a/xen/arch/arm/vgic/vgic-mmio-v2.c
+++ b/xen/arch/arm/vgic/vgic-mmio-v2.c
@@ -20,9 +20,70 @@ 
 #include "vgic.h"
 #include "vgic-mmio.h"
 
+static unsigned long vgic_mmio_read_v2_misc(struct vcpu *vcpu,
+                                            paddr_t addr, unsigned int len)
+{
+    uint32_t value;
+
+    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
+    {
+    case GICD_CTLR:
+        value = vcpu->domain->arch.vgic.enabled ? GICD_CTL_ENABLE : 0;
+        break;
+    case GICD_TYPER:
+        value = vcpu->domain->arch.vgic.nr_spis + VGIC_NR_PRIVATE_IRQS;
+        value = (value >> 5) - 1;
+        value |= (vcpu->domain->max_vcpus - 1) << 5;
+        break;
+    case GICD_IIDR:
+        value = (PRODUCT_ID_KVM << 24) |
+                (VARIANT_ID_XEN << 16) |
+                (IMPLEMENTER_ARM << 0);
+        break;
+    default:
+        return 0;
+    }
+
+    return value;
+}
+
+static void vgic_mmio_write_v2_misc(struct vcpu *vcpu,
+                                    paddr_t addr, unsigned int len,
+                                    unsigned long val)
+{
+    struct vgic_dist *dist = &vcpu->domain->arch.vgic;
+    bool enabled;
+
+    switch ( addr & 0x0c )      /* filter for the 4 registers handled here */
+    {
+    case GICD_CTLR:
+        domain_lock(vcpu->domain);
+
+        /*
+         * Store the new enabled state in our distributor structure.
+         * Work out whether it was disabled before and now got enabled,
+         * so that we signal all VCPUs to check for interrupts to be injected.
+         */
+        enabled = dist->enabled;
+        dist->enabled = val & GICD_CTL_ENABLE;
+        enabled = !enabled && dist->enabled;
+
+        domain_unlock(vcpu->domain);
+
+        if ( enabled )
+            vgic_kick_vcpus(vcpu->domain);
+
+        break;
+    case GICD_TYPER:
+    case GICD_IIDR:
+        /* read-only, writes ignored */
+        return;
+    }
+}
+
 static const struct vgic_register_region vgic_v2_dist_registers[] = {
     REGISTER_DESC_WITH_LENGTH(GICD_CTLR,
-        vgic_mmio_read_raz, vgic_mmio_write_wi, 12,
+        vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
         VGIC_ACCESS_32bit),
     REGISTER_DESC_WITH_BITS_PER_IRQ(GICD_IGROUPR,
         vgic_mmio_read_rao, vgic_mmio_write_wi, 1,
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index 925cda4580..37b425a16c 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -684,6 +684,21 @@  int vgic_vcpu_pending_irq(struct vcpu *vcpu)
     return ret;
 }
 
+void vgic_kick_vcpus(struct domain *d)
+{
+    struct vcpu *vcpu;
+
+    /*
+     * We've injected an interrupt, time to find out who deserves
+     * a good kick...
+     */
+    for_each_vcpu( d, vcpu )
+    {
+        if ( vgic_vcpu_pending_irq(vcpu) )
+            vcpu_kick(vcpu);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/vgic/vgic.h b/xen/arch/arm/vgic/vgic.h
index 7f221fd195..aed7e4179a 100644
--- a/xen/arch/arm/vgic/vgic.h
+++ b/xen/arch/arm/vgic/vgic.h
@@ -17,6 +17,14 @@ 
 #ifndef __XEN_ARM_VGIC_VGIC_H__
 #define __XEN_ARM_VGIC_VGIC_H__
 
+/*
+ * We piggy-back on the already used KVM product ID,  but use a different
+ * variant (major revision) for Xen.
+ */
+#define PRODUCT_ID_KVM          0x4b        /* ASCII code K */
+#define VARIANT_ID_XEN          0x01
+#define IMPLEMENTER_ARM         0x43b
+
 #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS)
 
 static inline bool irq_is_pending(struct vgic_irq *irq)
@@ -37,6 +45,7 @@  struct vgic_irq *vgic_get_irq(struct domain *d, struct vcpu *vcpu,
 void vgic_put_irq(struct domain *d, struct vgic_irq *irq);
 void vgic_queue_irq_unlock(struct domain *d, struct vgic_irq *irq,
                            unsigned long flags);
+void vgic_kick_vcpus(struct domain *d);
 
 static inline void vgic_get_irq_kref(struct vgic_irq *irq)
 {