thr3ads.net - Xen devel - [PATCH v4 0/6] xen/arm: event channels and shared

If this information is useful, please help other people find it:
Share via:

Stefano Stabellini

2012-May-25 16:21 UTC

[PATCH v4 0/6] xen/arm: event channels and shared_info page

Hi all,
this patch series implements support for injecting event channels into
the guest and enables a wider range of hypercalls for ARM guests.

In order to allow more flexibility I modified the hypercall protocol, in
particular the hypercall number is not passed as imm to hvc anymore,
because we might not always know it at compile time.
The hypercall number is now passed on the r12 register.

With this patch series and using the following Linux tree:

git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git vexpress-dt-privcmd-2

I am able to boot dom0, start xenstored and run basic xl commands, like
"xl list" and "xl uptime".
I have added at the beginning of this series few patches that were
previously sent separately (in particular "arm: support fewer LR
registers than virtual irqs").


Changes in v4:

- drop all the patches that have already been committed;

- rebase on "amd iommu: improve parse_event_log_entry()";

- move the initialization of gic.lr_pending and gic.lr_mask to gic_init;

- pass 0 as memflags to alloc_xenheap_pages in arch_domain_create.


Changes in v3:

- several fixes to "support fewer LR registers than virtual irqs";

- many more comments to the gic and vgic IRQ queues;

- merge Ian''s patch into "shared_info page allocation and
mapping";

- do not alloc the shared_info page for the idle domain;

- added "handle dom0_max_vcpus=0 case properly" by Ian, removed the
corresponding patch in my series;

- move XEN_HYPERCALL_TAG to a public header;

- clobber register in the debug build;

- document calling convention;

- check if arm_hypercall_table[regs->r12] != NULL;

- use a PPI for events injection (IRQ 31) and do not request maintenance
interrupts for it whenever possible.


Changes in v2:

- fixed tabs/spaces problem.



Stefano Stabellini (6):
      arm: support fewer LR registers than virtual irqs
      arm: replace list_del and INIT_LIST_HEAD with list_del_init
      arm: shared_info page allocation and mapping
      arm: implement flush_tlb_all_local and flush_tlb_local
      arm: remove VGIC_SOFTIRQ
      arm: implement event injection

 xen/arch/arm/domain.c          |   22 ++++++
 xen/arch/arm/dummy.S           |    1 -
 xen/arch/arm/gic.c             |  152 +++++++++++++++++++++++++++++++++-------
 xen/arch/arm/gic.h             |    7 ++-
 xen/arch/arm/mm.c              |   98 +++++++++++++++++++++++++-
 xen/arch/arm/p2m.c             |   26 +++++++-
 xen/arch/arm/traps.c           |    4 +-
 xen/arch/arm/vgic.c            |   15 ----
 xen/include/asm-arm/config.h   |    2 +
 xen/include/asm-arm/domain.h   |   10 +++
 xen/include/asm-arm/flushtlb.h |   23 ++++++-
 xen/include/asm-arm/mm.h       |    4 +
 xen/include/asm-arm/p2m.h      |    9 +++
 xen/include/asm-arm/paging.h   |    3 +
 xen/include/asm-arm/softirq.h  |    3 +-
 15 files changed, 326 insertions(+), 53 deletions(-)



A git branch is available here:

git://xenbits.xen.org/people/sstabellini/xen-unstable.git events-4

Cheers,

Stefano

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 1/6] arm: support fewer LR registers than virtual irqs

If the vgic needs to inject a virtual irq into the guest, but no free
LR registers are available, add the irq to a list and return.
Whenever an LR register becomes available we add the queued irq to it
and remove it from the list.
We use the gic lock to protect the list and the bitmask.


Changes in v4:

- move the initialization of gic.lr_pending and gic.lr_mask to gic_init.


Changes in v3:

- added some comments;

- rename lr_link to lr_queue;

- fix list handling in gic_set_guest_irq;

- use nr_lrs instead of sizeof(uint64_t) as argument to
find_first_zero_bit.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c           |  102 +++++++++++++++++++++++++++++++++---------
 xen/include/asm-arm/domain.h |   10 ++++
 2 files changed, 90 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 34a2c3f..ea5ce06 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -25,6 +25,7 @@
 #include <xen/sched.h>
 #include <xen/errno.h>
 #include <xen/softirq.h>
+#include <xen/list.h>
 #include <asm/p2m.h>
 #include <asm/domain.h>
 
@@ -45,6 +46,14 @@ static struct {
     unsigned int lines;
     unsigned int cpus;
     spinlock_t lock;
+    uint64_t lr_mask;
+    /* lr_pending is used to queue IRQs (struct pending_irq) that the
+     * vgic tried to inject in the guest (calling gic_set_guest_irq) but
+     * no LRs were available at the time.
+     * As soon as an LR is freed we remove the first IRQ from this
+     * list and write it to the LR register.
+     * lr_pending is a subset of vgic.inflight_irqs. */
+    struct list_head lr_pending;
 } gic;
 
 irq_desc_t irq_desc[NR_IRQS];
@@ -283,6 +292,9 @@ int __init gic_init(void)
     gic_cpu_init();
     gic_hyp_init();
 
+    gic.lr_mask = 0ULL;
+    INIT_LIST_HEAD(&gic.lr_pending);
+
     spin_unlock(&gic.lock);
 
     return gic.cpus;
@@ -377,16 +389,50 @@ int __init setup_irq(unsigned int irq, struct irqaction
*new)
     return rc;
 }
 
-void gic_set_guest_irq(unsigned int virtual_irq,
+static inline void gic_set_lr(int lr, unsigned int virtual_irq,
         unsigned int state, unsigned int priority)
 {
-    BUG_ON(virtual_irq > nr_lrs);
-    GICH[GICH_LR + virtual_irq] = state |
+    BUG_ON(lr > nr_lrs);
+    GICH[GICH_LR + lr] = state |
         GICH_LR_MAINTENANCE_IRQ |
         ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
         ((virtual_irq & GICH_LR_VIRTUAL_MASK) <<
GICH_LR_VIRTUAL_SHIFT);
 }
 
+void gic_set_guest_irq(unsigned int virtual_irq,
+        unsigned int state, unsigned int priority)
+{
+    int i;
+    struct pending_irq *iter, *n;
+
+    spin_lock(&gic.lock);
+
+    if ( list_empty(&gic.lr_pending) )
+    {
+        i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
+        if (i < nr_lrs) {
+            set_bit(i, &gic.lr_mask);
+            gic_set_lr(i, virtual_irq, state, priority);
+            goto out;
+        }
+    }
+
+    n = irq_to_pending(current, virtual_irq);
+    list_for_each_entry ( iter, &gic.lr_pending, lr_queue )
+    {
+        if ( iter->priority > priority )
+        {
+            list_add_tail(&n->lr_queue, &iter->lr_queue);
+            goto out;
+        }
+    }
+    list_add_tail(&n->lr_queue, &gic.lr_pending);
+
+out:
+    spin_unlock(&gic.lock);
+    return;
+}
+
 void gic_inject_irq_start(void)
 {
     uint32_t hcr;
@@ -463,30 +509,42 @@ void gicv_setup(struct domain *d)
 
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs
*regs)
 {
-    int i, virq;
+    int i = 0, virq;
     uint32_t lr;
     uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) <<
32);
 
-    for ( i = 0; i < 64; i++ ) {
-        if ( eisr & ((uint64_t)1 << i) ) {
-            struct pending_irq *p;
-
-            lr = GICH[GICH_LR + i];
-            virq = lr & GICH_LR_VIRTUAL_MASK;
-            GICH[GICH_LR + i] = 0;
-
-            spin_lock(&current->arch.vgic.lock);
-            p = irq_to_pending(current, virq);
-            if ( p->desc != NULL ) {
-                p->desc->status &= ~IRQ_INPROGRESS;
-                GICC[GICC_DIR] = virq;
-            }
+    while ((i = find_next_bit((const long unsigned int *) &eisr,
+                              sizeof(eisr), i)) < sizeof(eisr)) {
+        struct pending_irq *p;
+
+        spin_lock(&gic.lock);
+        lr = GICH[GICH_LR + i];
+        virq = lr & GICH_LR_VIRTUAL_MASK;
+        GICH[GICH_LR + i] = 0;
+        clear_bit(i, &gic.lr_mask);
+
+        if ( !list_empty(&gic.lr_pending) ) {
+            p = list_entry(gic.lr_pending.next, typeof(*p), lr_queue);
+            gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
+            list_del_init(&p->lr_queue);
+            set_bit(i, &gic.lr_mask);
+        } else {
             gic_inject_irq_stop();
-            list_del(&p->inflight);
-            INIT_LIST_HEAD(&p->inflight);
-            cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
-            spin_unlock(&current->arch.vgic.lock);
         }
+        spin_unlock(&gic.lock);
+
+        spin_lock(&current->arch.vgic.lock);
+        p = irq_to_pending(current, virq);
+        if ( p->desc != NULL ) {
+            p->desc->status &= ~IRQ_INPROGRESS;
+            GICC[GICC_DIR] = virq;
+        }
+        list_del(&p->inflight);
+        INIT_LIST_HEAD(&p->inflight);
+        cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
+        spin_unlock(&current->arch.vgic.lock);
+
+        i++;
     }
 }
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index d01534b..10ed540 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -23,6 +23,9 @@ struct pending_irq
     /* inflight is used to append instances of pending_irq to
      * vgic.inflight_irqs */
     struct list_head inflight;
+    /* lr_queue is used to append instances of pending_irq to
+     * gic.lr_pending */
+    struct list_head lr_queue;
 };
 
 struct arch_domain
@@ -75,6 +78,13 @@ struct arch_vcpu
 
     struct {
         struct vgic_irq_rank private_irqs;
+        /* This list is ordered by IRQ priority and it is used to keep
+         * track of the IRQs that the VGIC injected into the guest.
+         * Depending on the availability of LR registers, the IRQs might
+         * actually be in an LR, and therefore injected into the guest,
+         * or queued in gic.lr_pending.
+         * As soon as an IRQ is EOI''d by the guest and removed from
the
+         * corresponding LR it is also removed from this list. */
         struct list_head inflight_irqs;
         spinlock_t lock;
     } vgic;
-- 
1.7.2.5

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 2/6] arm: replace list_del and INIT_LIST_HEAD with list_del_init

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index ea5ce06..c05e598 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -539,8 +539,7 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
             p->desc->status &= ~IRQ_INPROGRESS;
             GICC[GICC_DIR] = virq;
         }
-        list_del(&p->inflight);
-        INIT_LIST_HEAD(&p->inflight);
+        list_del_init(&p->inflight);
         cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
         spin_unlock(&current->arch.vgic.lock);
 
-- 
1.7.2.5

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 3/6] arm: shared_info page allocation and mapping

Allocate the shared_info page at domain creation.

Implement arch_memory_op, only for XENMEM_add_to_physmap with space
=XENMAPSPACE_shared_info, so that the guest can map the shared_info page.


Changes in v4:

- pass 0 as memflags to alloc_xenheap_pages in arch_domain_create.


Changes in v3:

- /MEMF_bits(32)/MEMF_bits(64);

- do not alloc the shared_info page for the idle domain;

- define CONFIG_PAGING_ASSISTANCE;

- adjust the API to match what the common code expects;

- implement a dummy guest_physmap_remove_page.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain.c        |   11 +++++
 xen/arch/arm/mm.c            |   98 ++++++++++++++++++++++++++++++++++++++++--
 xen/arch/arm/p2m.c           |   24 ++++++++++-
 xen/include/asm-arm/config.h |    2 +
 xen/include/asm-arm/mm.h     |    4 ++
 xen/include/asm-arm/p2m.h    |    9 ++++
 xen/include/asm-arm/paging.h |    3 +
 7 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index edaff22..3a726c8 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -192,6 +192,17 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
     if ( (rc = p2m_init(d)) != 0 )
         goto fail;
 
+    if ( !is_idle_domain(d) )
+    {
+        rc = -ENOMEM;
+        if ( (d->shared_info = alloc_xenheap_pages(0, 0)) == NULL )
+            goto fail;
+
+        clear_page(d->shared_info);
+        share_xen_page_with_guest(
+                virt_to_page(d->shared_info), d, XENSHARE_writable);
+    }
+
     d->max_vcpus = 8;
 
     if ( (rc = domain_vgic_init(d)) != 0 )
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index fa2f5ec..10ff883 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -25,8 +25,11 @@
 #include <xen/mm.h>
 #include <xen/preempt.h>
 #include <xen/errno.h>
+#include <xen/guest_access.h>
 #include <asm/page.h>
 #include <asm/current.h>
+#include <public/memory.h>
+#include <xen/sched.h>
 
 struct domain *dom_xen, *dom_io;
 
@@ -376,17 +379,104 @@ void arch_dump_shared_mem_info(void)
 {
 }
 
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+int donate_page(struct domain *d, struct page_info *page, unsigned int
memflags)
 {
+    ASSERT(0);
     return -ENOSYS;
 }
 
-int donate_page(struct domain *d, struct page_info *page, unsigned int
memflags)
+void share_xen_page_with_guest(struct page_info *page,
+                          struct domain *d, int readonly)
 {
-    ASSERT(0);
-    return -ENOSYS;
+    if ( page_get_owner(page) == d )
+        return;
+
+    spin_lock(&d->page_alloc_lock);
+
+    /* The incremented type count pins as writable or read-only. */
+    page->u.inuse.type_info  = (readonly ? PGT_none : PGT_writable_page);
+    page->u.inuse.type_info |= PGT_validated | 1;
+
+    page_set_owner(page, d);
+    wmb(); /* install valid domain ptr before updating refcnt. */
+    ASSERT((page->count_info & ~PGC_xen_heap) == 0);
+
+    /* Only add to the allocation list if the domain isn''t dying. */
+    if ( !d->is_dying )
+    {
+        page->count_info |= PGC_allocated | 1;
+        if ( unlikely(d->xenheap_pages++ == 0) )
+            get_knownalive_domain(d);
+        page_list_add_tail(page, &d->xenpage_list);
+    }
+
+    spin_unlock(&d->page_alloc_lock);
+}
+
+static int xenmem_add_to_physmap_once(
+    struct domain *d,
+    const struct xen_add_to_physmap *xatp)
+{
+    unsigned long mfn = 0;
+    int rc;
+
+    switch ( xatp->space )
+    {
+        case XENMAPSPACE_shared_info:
+            if ( xatp->idx == 0 )
+                mfn = virt_to_mfn(d->shared_info);
+            break;
+        default:
+            return -ENOSYS;
+    }
+
+    domain_lock(d);
+
+    /* Map at new location. */
+    rc = guest_physmap_add_page(d, xatp->gpfn, mfn, 0);
+
+    domain_unlock(d);
+
+    return rc;
+}
+
+static int xenmem_add_to_physmap(struct domain *d,
+                                 struct xen_add_to_physmap *xatp)
+{
+    return xenmem_add_to_physmap_once(d, xatp);
 }
 
+long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+{
+    int rc;
+
+    switch ( op )
+    {
+    case XENMEM_add_to_physmap:
+    {
+        struct xen_add_to_physmap xatp;
+        struct domain *d;
+
+        if ( copy_from_guest(&xatp, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        rc = xenmem_add_to_physmap(d, &xatp);
+
+        rcu_unlock_domain(d);
+
+        return rc;
+    }
+
+    default:
+        return -ENOSYS;
+    }
+
+    return 0;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 14614fd..4c94ef0 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -118,7 +118,12 @@ static int create_p2m_entries(struct domain *d,
         }
         /* else: third already valid */
 
-        BUG_ON(third[third_table_offset(addr)].p2m.valid);
+        if ( third[third_table_offset(addr)].p2m.valid )
+        {
+            /* p2m entry already present */
+            free_domheap_page(
+                    mfn_to_page(third[third_table_offset(addr)].p2m.base));
+        }
 
         /* Allocate a new RAM page and attach */
         if (alloc)
@@ -172,6 +177,23 @@ int map_mmio_regions(struct domain *d,
     return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
 }
 
+int guest_physmap_add_page(struct domain *d,
+                           unsigned long gpfn,
+                           unsigned long mfn,
+                           unsigned int page_order)
+{
+    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
+                              (gpfn + (1<<page_order)) <<
PAGE_SHIFT,
+                              mfn << PAGE_SHIFT);
+}
+
+void guest_physmap_remove_page(struct domain *d,
+                               unsigned long gpfn,
+                               unsigned long mfn, unsigned int page_order)
+{
+    ASSERT(0);
+}
+
 int p2m_alloc_table(struct domain *d)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1e4f108..91e87e1 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -7,6 +7,8 @@
 #ifndef __ARM_CONFIG_H__
 #define __ARM_CONFIG_H__
 
+#define CONFIG_PAGING_ASSISTANCE 1
+
 #define CONFIG_PAGING_LEVELS 3
 
 #define CONFIG_ARM 1
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index ea27e4f..53801b0 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -78,6 +78,10 @@ struct page_info
 #define _PGT_pinned       PG_shift(5)
 #define PGT_pinned        PG_mask(1, 5)
 
+ /* Has this page been validated for use as its current type? */
+#define _PGT_validated    PG_shift(6)
+#define PGT_validated     PG_mask(1, 6)
+
  /* Count of uses of this frame as its current type. */
 #define PGT_count_width   PG_shift(9)
 #define PGT_count_mask    ((1UL<<PGT_count_width)-1)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index d88e7a9..349923a 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -40,6 +40,15 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t
end);
 int map_mmio_regions(struct domain *d, paddr_t start_gaddr,
                      paddr_t end_gaddr, paddr_t maddr);
 
+/* Untyped version for RAM only, for compatibility */
+int guest_physmap_add_page(struct domain *d,
+                           unsigned long gfn,
+                           unsigned long mfn,
+                           unsigned int page_order);
+void guest_physmap_remove_page(struct domain *d,
+                               unsigned long gpfn,
+                               unsigned long mfn, unsigned int page_order);
+
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
 /*
diff --git a/xen/include/asm-arm/paging.h b/xen/include/asm-arm/paging.h
index 4dc340f..3d7dd95 100644
--- a/xen/include/asm-arm/paging.h
+++ b/xen/include/asm-arm/paging.h
@@ -1,6 +1,9 @@
 #ifndef _XEN_PAGING_H
 #define _XEN_PAGING_H
 
+#define paging_mode_translate(d)              (0)
+#define paging_mode_external(d)               (0)
+
 #endif /* XEN_PAGING_H */
 
 /*
-- 
1.7.2.5

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 4/6] arm: implement flush_tlb_all_local and flush_tlb_local

Call flush_tlb_all_local from create_p2m_entries after removing a page
from the p2m.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/p2m.c             |    2 ++
 xen/include/asm-arm/flushtlb.h |   23 +++++++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 4c94ef0..051a0e8 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -3,6 +3,7 @@
 #include <xen/lib.h>
 #include <xen/errno.h>
 #include <xen/domain_page.h>
+#include <asm/flushtlb.h>
 
 void p2m_load_VTTBR(struct domain *d)
 {
@@ -123,6 +124,7 @@ static int create_p2m_entries(struct domain *d,
             /* p2m entry already present */
             free_domheap_page(
                     mfn_to_page(third[third_table_offset(addr)].p2m.base));
+            flush_tlb_all_local();
         }
 
         /* Allocate a new RAM page and attach */
diff --git a/xen/include/asm-arm/flushtlb.h b/xen/include/asm-arm/flushtlb.h
index c8486fc..210abfa 100644
--- a/xen/include/asm-arm/flushtlb.h
+++ b/xen/include/asm-arm/flushtlb.h
@@ -14,8 +14,27 @@ do {                                                         
\
 
 #define tlbflush_current_time()                 (0)
 
-/* Flush local TLBs */
-void flush_tlb_local(void);
+/* Flush local TLBs, current VMID only */
+static inline void flush_tlb_local(void)
+{
+    dsb();
+
+    WRITE_CP32((uint32_t) 0, TLBIALLIS);
+
+    dsb();
+    isb();
+}
+
+/* Flush local TLBs, all VMIDs, non-hypervisor mode */
+static inline void flush_tlb_all_local(void)
+{
+    dsb();
+
+    WRITE_CP32((uint32_t) 0, TLBIALLNSNHIS);
+
+    dsb();
+    isb();
+}
 
 /* Flush specified CPUs'' TLBs */
 void flush_tlb_mask(const cpumask_t *mask);
-- 
1.7.2.5

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 5/6] arm: remove VGIC_SOFTIRQ

Instead of using a softirq to check whether we need to set the VI bit in
the HCR (IRQ injection in the guest), always check the lr_mask on
leave_hypervisor_tail.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/gic.c            |   13 ++++++++++---
 xen/arch/arm/gic.h            |    4 ++--
 xen/arch/arm/traps.c          |    4 +++-
 xen/arch/arm/vgic.c           |   15 ---------------
 xen/include/asm-arm/softirq.h |    3 +--
 5 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c05e598..cdb4e4a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -433,7 +433,7 @@ out:
     return;
 }
 
-void gic_inject_irq_start(void)
+static void gic_inject_irq_start(void)
 {
     uint32_t hcr;
     hcr = READ_CP32(HCR);
@@ -441,7 +441,7 @@ void gic_inject_irq_start(void)
     isb();
 }
 
-void gic_inject_irq_stop(void)
+static void gic_inject_irq_stop(void)
 {
     uint32_t hcr;
     hcr = READ_CP32(HCR);
@@ -451,6 +451,14 @@ void gic_inject_irq_stop(void)
     }
 }
 
+void gic_inject(void)
+{
+    if (!gic.lr_mask)
+        gic_inject_irq_stop();
+    else
+        gic_inject_irq_start();
+}
+
 int gic_route_irq_to_guest(struct domain *d, unsigned int irq,
                            const char * devname)
 {
@@ -540,7 +548,6 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
             GICC[GICC_DIR] = virq;
         }
         list_del_init(&p->inflight);
-        cpu_raise_softirq(current->processor, VGIC_SOFTIRQ);
         spin_unlock(&current->arch.vgic.lock);
 
         i++;
diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
index 6b2be4f..2c5922e 100644
--- a/xen/arch/arm/gic.h
+++ b/xen/arch/arm/gic.h
@@ -128,11 +128,11 @@ extern struct pending_irq *irq_to_pending(struct vcpu *v,
unsigned int irq);
 
 extern void gic_route_irqs(void);
 
+extern void gic_inject(void);
+
 extern void __cpuinit init_maintenance_interrupt(void);
 extern void gic_set_guest_irq(unsigned int irq,
         unsigned int state, unsigned int priority);
-extern void gic_inject_irq_start(void);
-extern void gic_inject_irq_stop(void);
 extern int gic_route_irq_to_guest(struct domain *d, unsigned int irq,
                                   const char * devname);
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6304bbd..abc26a3 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -608,8 +608,10 @@ asmlinkage void leave_hypervisor_tail(void)
     while (1)
     {
         local_irq_disable();
-        if (!softirq_pending(smp_processor_id()))
+        if (!softirq_pending(smp_processor_id())) {
+            gic_inject();
             return;
+        }
         local_irq_enable();
         do_softirq();
     }
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 4d2a0e0..629a0da 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -577,23 +577,8 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,
int virtual)
     list_add_tail(&n->inflight, &v->arch.vgic.inflight_irqs);
     spin_unlock(&v->arch.vgic.lock);
     /* we have a new higher priority irq, inject it into the guest */
-    cpu_raise_softirq(v->processor, VGIC_SOFTIRQ);
 }
 
-static void vgic_softirq(void)
-{
-    if (list_empty(&current->arch.vgic.inflight_irqs))
-        return;
-
-    gic_inject_irq_start();
-}
-
-static int __init init_vgic_softirq(void)
-{
-    open_softirq(VGIC_SOFTIRQ, vgic_softirq);
-    return 0;
-}
-__initcall(init_vgic_softirq);
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/asm-arm/softirq.h b/xen/include/asm-arm/softirq.h
index 536af38..27818ae 100644
--- a/xen/include/asm-arm/softirq.h
+++ b/xen/include/asm-arm/softirq.h
@@ -1,8 +1,7 @@
 #ifndef __ASM_SOFTIRQ_H__
 #define __ASM_SOFTIRQ_H__
 
-#define VGIC_SOFTIRQ        (NR_COMMON_SOFTIRQS + 0)
-#define NR_ARCH_SOFTIRQS       1
+#define NR_ARCH_SOFTIRQS       0
 
 #endif /* __ASM_SOFTIRQ_H__ */
 /*
-- 
1.7.2.5

Stefano Stabellini

2012-May-25 16:23 UTC

head link

[PATCH v4 6/6] arm: implement event injection

Implement vcpu_mark_events_pending using the vgic to inject PPI 31, that
we reserve for Xen usage.
In the future the interrupt used for event injection might be dynamic
and could be written into the device tree.
Otherwise it could be an SGI choosen by the guest and passed to Xen
through an hypercall.


Considering that:

- it is easy to determine if an event notification
interrupt has already been EOI''d by the guest just looking at the
evtchn_upcall_pending bit in the shared_info page;

- we can safely assume that there is at most one event notification
interrupt pending at any time in any set of LR registers because we
never inject more than a single event notification interrupt in one vcpu
(see vcpu_mark_events_pending);

we can avoid requesting maintenance interrupts for
VGIC_IRQ_EVTCHN_CALLBACK, provided that we check for event notification
interrupts that need to be cleared in the following places:

- maintenance interrupt entry;

- gic_set_guest_irq;

that is every time we are about to write to an LR.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain.c |   11 +++++++++++
 xen/arch/arm/dummy.S  |    1 -
 xen/arch/arm/gic.c    |   40 +++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/gic.h    |    3 +++
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 3a726c8..5702399 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -232,6 +232,17 @@ void arch_dump_vcpu_info(struct vcpu *v)
 {
 }
 
+void vcpu_mark_events_pending(struct vcpu *v)
+{
+    int already_pending = test_and_set_bit(
+        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
+
+    if ( already_pending )
+        return;
+
+    vgic_vcpu_inject_irq(v, VGIC_IRQ_EVTCHN_CALLBACK, 1);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
index 8c6151c..016340c 100644
--- a/xen/arch/arm/dummy.S
+++ b/xen/arch/arm/dummy.S
@@ -27,7 +27,6 @@ DUMMY(arch_vcpu_reset);
 DUMMY(free_vcpu_guest_context);
 DUMMY(sync_vcpu_execstate);
 NOP(update_vcpu_system_time);
-DUMMY(vcpu_mark_events_pending);
 DUMMY(vcpu_show_execution_state);
 
 /* Page Reference & Type Maintenance */
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index cdb4e4a..cc9d37b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -37,6 +37,7 @@
                                      + (GIC_CR_OFFSET & 0xfff)))
 #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH)  \
                                      + (GIC_HR_OFFSET & 0xfff)))
+static void events_maintenance(struct vcpu *v);
 
 /* Global state */
 static struct {
@@ -46,6 +47,7 @@ static struct {
     unsigned int lines;
     unsigned int cpus;
     spinlock_t lock;
+    uint64_t event_mask;
     uint64_t lr_mask;
     /* lr_pending is used to queue IRQs (struct pending_irq) that the
      * vgic tried to inject in the guest (calling gic_set_guest_irq) but
@@ -293,6 +295,7 @@ int __init gic_init(void)
     gic_hyp_init();
 
     gic.lr_mask = 0ULL;
+    gic.event_mask = 0ULL;
     INIT_LIST_HEAD(&gic.lr_pending);
 
     spin_unlock(&gic.lock);
@@ -392,9 +395,15 @@ int __init setup_irq(unsigned int irq, struct irqaction
*new)
 static inline void gic_set_lr(int lr, unsigned int virtual_irq,
         unsigned int state, unsigned int priority)
 {
+    int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
+
     BUG_ON(lr > nr_lrs);
+
+    if (virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK && nr_lrs > 1)
+        maintenance_int = 0;
+
     GICH[GICH_LR + lr] = state |
-        GICH_LR_MAINTENANCE_IRQ |
+        maintenance_int |
         ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
         ((virtual_irq & GICH_LR_VIRTUAL_MASK) <<
GICH_LR_VIRTUAL_SHIFT);
 }
@@ -405,6 +414,8 @@ void gic_set_guest_irq(unsigned int virtual_irq,
     int i;
     struct pending_irq *iter, *n;
 
+    events_maintenance(current);
+
     spin_lock(&gic.lock);
 
     if ( list_empty(&gic.lr_pending) )
@@ -412,6 +423,8 @@ void gic_set_guest_irq(unsigned int virtual_irq,
         i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
         if (i < nr_lrs) {
             set_bit(i, &gic.lr_mask);
+            if ( virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK )
+                set_bit(i, &gic.event_mask);
             gic_set_lr(i, virtual_irq, state, priority);
             goto out;
         }
@@ -515,12 +528,35 @@ void gicv_setup(struct domain *d)
                         GIC_BASE_ADDRESS + GIC_VR_OFFSET);
 }
 
+static void events_maintenance(struct vcpu *v)
+{
+    int i = 0;
+    int already_pending = test_bit(0,
+            (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
+
+    if (!already_pending && gic.event_mask != 0) {
+        spin_lock(&gic.lock);
+        while ((i = find_next_bit((const long unsigned int *)
&gic.event_mask,
+                        sizeof(uint64_t), i)) < sizeof(uint64_t)) {
+
+            GICH[GICH_LR + i] = 0;
+            clear_bit(i, &gic.lr_mask);
+            clear_bit(i, &gic.event_mask);
+
+            i++;
+        }
+        spin_unlock(&gic.lock);
+    }
+}
+
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs
*regs)
 {
     int i = 0, virq;
     uint32_t lr;
     uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) <<
32);
 
+    events_maintenance(current);
+
     while ((i = find_next_bit((const long unsigned int *) &eisr,
                               sizeof(eisr), i)) < sizeof(eisr)) {
         struct pending_irq *p;
@@ -536,6 +572,8 @@ static void maintenance_interrupt(int irq, void *dev_id,
struct cpu_user_regs *r
             gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
             list_del_init(&p->lr_queue);
             set_bit(i, &gic.lr_mask);
+            if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK )
+                set_bit(i, &gic.event_mask);
         } else {
             gic_inject_irq_stop();
         }
diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
index 2c5922e..ff8d0a2 100644
--- a/xen/arch/arm/gic.h
+++ b/xen/arch/arm/gic.h
@@ -121,6 +121,9 @@
 #define GICH_LR_CPUID_SHIFT     9
 #define GICH_VTR_NRLRGS         0x3f
 
+/* XXX: write this into the DT */
+#define VGIC_IRQ_EVTCHN_CALLBACK 31
+
 extern int domain_vgic_init(struct domain *d);
 extern int vcpu_vgic_init(struct vcpu *v);
 extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int virtual);
-- 
1.7.2.5

Ian Campbell

2012-Jun-01 08:45 UTC

head link

Re: [PATCH v4 6/6] arm: implement event injection

On Fri, 2012-05-25 at 17:23 +0100, Stefano Stabellini
wrote:> Implement vcpu_mark_events_pending using the vgic to inject PPI 31, that
> we reserve for Xen usage.
> In the future the interrupt used for event injection might be dynamic
> and could be written into the device tree.
> Otherwise it could be an SGI choosen by the guest and passed to Xen
> through an hypercall.
We need to resolve this one way or another before we commit to any ABI
(i.e. before we upstream any Linux patches depending on it), but lets
have this for now.
> 
> 
> Considering that:
> 
> - it is easy to determine if an event notification
> interrupt has already been EOI''d by the guest just looking at the
> evtchn_upcall_pending bit in the shared_info page;
> 
> - we can safely assume that there is at most one event notification
> interrupt pending at any time in any set of LR registers because we
> never inject more than a single event notification interrupt in one vcpu
> (see vcpu_mark_events_pending);
> 
> we can avoid requesting maintenance interrupts for
> VGIC_IRQ_EVTCHN_CALLBACK, provided that we check for event notification
> interrupts that need to be cleared in the following places:
> 
> - maintenance interrupt entry;
> 
> - gic_set_guest_irq;
> 
> that is every time we are about to write to an LR.
> 
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/domain.c |   11 +++++++++++
>  xen/arch/arm/dummy.S  |    1 -
>  xen/arch/arm/gic.c    |   40 +++++++++++++++++++++++++++++++++++++++-
>  xen/arch/arm/gic.h    |    3 +++
>  4 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 3a726c8..5702399 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -232,6 +232,17 @@ void arch_dump_vcpu_info(struct vcpu *v)
>  {
>  }
>  
> +void vcpu_mark_events_pending(struct vcpu *v)
> +{
> +    int already_pending = test_and_set_bit(
> +        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
> +
> +    if ( already_pending )
> +        return;
> +
> +    vgic_vcpu_inject_irq(v, VGIC_IRQ_EVTCHN_CALLBACK, 1);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
> index 8c6151c..016340c 100644
> --- a/xen/arch/arm/dummy.S
> +++ b/xen/arch/arm/dummy.S
> @@ -27,7 +27,6 @@ DUMMY(arch_vcpu_reset);
>  DUMMY(free_vcpu_guest_context);
>  DUMMY(sync_vcpu_execstate);
>  NOP(update_vcpu_system_time);
> -DUMMY(vcpu_mark_events_pending);
>  DUMMY(vcpu_show_execution_state);
>  
>  /* Page Reference & Type Maintenance */
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index cdb4e4a..cc9d37b 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -37,6 +37,7 @@
>                                       + (GIC_CR_OFFSET & 0xfff)))
>  #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH)  \
>                                       + (GIC_HR_OFFSET & 0xfff)))
> +static void events_maintenance(struct vcpu *v);
>  
>  /* Global state */
>  static struct {
> @@ -46,6 +47,7 @@ static struct {
>      unsigned int lines;
>      unsigned int cpus;
>      spinlock_t lock;
> +    uint64_t event_mask;
>      uint64_t lr_mask;
>      /* lr_pending is used to queue IRQs (struct pending_irq) that the
>       * vgic tried to inject in the guest (calling gic_set_guest_irq) but
> @@ -293,6 +295,7 @@ int __init gic_init(void)
>      gic_hyp_init();
>  
>      gic.lr_mask = 0ULL;
> +    gic.event_mask = 0ULL;
>      INIT_LIST_HEAD(&gic.lr_pending);
>  
>      spin_unlock(&gic.lock);
> @@ -392,9 +395,15 @@ int __init setup_irq(unsigned int irq, struct
irqaction *new)
>  static inline void gic_set_lr(int lr, unsigned int virtual_irq,
>          unsigned int state, unsigned int priority)
>  {
> +    int maintenance_int = GICH_LR_MAINTENANCE_IRQ;
> +
>      BUG_ON(lr > nr_lrs);
> +
> +    if (virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK && nr_lrs > 1)
> +        maintenance_int = 0;
> +
>      GICH[GICH_LR + lr] = state |
> -        GICH_LR_MAINTENANCE_IRQ |
> +        maintenance_int |
>          ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
>          ((virtual_irq & GICH_LR_VIRTUAL_MASK) <<
GICH_LR_VIRTUAL_SHIFT);
>  }
> @@ -405,6 +414,8 @@ void gic_set_guest_irq(unsigned int virtual_irq,
>      int i;
>      struct pending_irq *iter, *n;
>  
> +    events_maintenance(current);
> +
>      spin_lock(&gic.lock);
>  
>      if ( list_empty(&gic.lr_pending) )
> @@ -412,6 +423,8 @@ void gic_set_guest_irq(unsigned int virtual_irq,
>          i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
>          if (i < nr_lrs) {
>              set_bit(i, &gic.lr_mask);
> +            if ( virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK )
> +                set_bit(i, &gic.event_mask);
>              gic_set_lr(i, virtual_irq, state, priority);
>              goto out;
>          }
> @@ -515,12 +528,35 @@ void gicv_setup(struct domain *d)
>                          GIC_BASE_ADDRESS + GIC_VR_OFFSET);
>  }
>  
> +static void events_maintenance(struct vcpu *v)
> +{
> +    int i = 0;
> +    int already_pending = test_bit(0,
> +            (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
> +
> +    if (!already_pending && gic.event_mask != 0) {
> +        spin_lock(&gic.lock);
> +        while ((i = find_next_bit((const long unsigned int *)
&gic.event_mask,
> +                        sizeof(uint64_t), i)) < sizeof(uint64_t)) {
> +
> +            GICH[GICH_LR + i] = 0;
> +            clear_bit(i, &gic.lr_mask);
> +            clear_bit(i, &gic.event_mask);
> +
> +            i++;
> +        }
> +        spin_unlock(&gic.lock);
> +    }
> +}
> +
>  static void maintenance_interrupt(int irq, void *dev_id, struct
cpu_user_regs *regs)
>  {
>      int i = 0, virq;
>      uint32_t lr;
>      uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1])
<< 32);
>  
> +    events_maintenance(current);
> +
>      while ((i = find_next_bit((const long unsigned int *) &eisr,
>                                sizeof(eisr), i)) < sizeof(eisr)) {
>          struct pending_irq *p;
> @@ -536,6 +572,8 @@ static void maintenance_interrupt(int irq, void
*dev_id, struct cpu_user_regs *r
>              gic_set_lr(i, p->irq, GICH_LR_PENDING, p->priority);
>              list_del_init(&p->lr_queue);
>              set_bit(i, &gic.lr_mask);
> +            if ( p->irq == VGIC_IRQ_EVTCHN_CALLBACK )
> +                set_bit(i, &gic.event_mask);
>          } else {
>              gic_inject_irq_stop();
>          }
> diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
> index 2c5922e..ff8d0a2 100644
> --- a/xen/arch/arm/gic.h
> +++ b/xen/arch/arm/gic.h
> @@ -121,6 +121,9 @@
>  #define GICH_LR_CPUID_SHIFT     9
>  #define GICH_VTR_NRLRGS         0x3f
>  
> +/* XXX: write this into the DT */
> +#define VGIC_IRQ_EVTCHN_CALLBACK 31
> +
>  extern int domain_vgic_init(struct domain *d);
>  extern int vcpu_vgic_init(struct vcpu *v);
>  extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int
virtual);

Ian Campbell

2012-Jun-01 09:31 UTC

head link

Re: [PATCH v4 0/6] xen/arm: event channels and shared_info page

On Fri, 2012-05-25 at 17:21 +0100, Stefano Stabellini
wrote:> this patch series implements support for injecting event channels into
> the guest and enables a wider range of hypercalls for ARM guests.
I''ve actually been running with the previous iteration of these patches
in my tree for ages now, so they are actually pretty well tested by me.

So I''ve acked+applied them...

Ian.

Xen devel - May 2012 - [PATCH v4 0/6] xen/arm: event channels and shared_info page

[PATCH v4 0/6] xen/arm: event channels and shared_info page

[PATCH v4 1/6] arm: support fewer LR registers than virtual irqs

[PATCH v4 2/6] arm: replace list_del and INIT_LIST_HEAD with list_del_init

[PATCH v4 3/6] arm: shared_info page allocation and mapping

[PATCH v4 4/6] arm: implement flush_tlb_all_local and flush_tlb_local

[PATCH v4 5/6] arm: remove VGIC_SOFTIRQ

[PATCH v4 6/6] arm: implement event injection

Re: [PATCH v4 6/6] arm: implement event injection

Re: [PATCH v4 0/6] xen/arm: event channels and shared_info page