[PATCH v2 0/4] xen/arm: dom1 PV console up and running

[PATCH v2 0/4] xen/arm: dom1 PV console up and running

[Stefano Stabellini]

Hi all,
this patch series fixes few bugs and implements few missing pieces
needed to get the first PV console up and running. With this patch
series applied I can fully boot a Dom1 guest out of an initramfs and
connect to its pv console in dom0, using xenconsole.

Regarding the console and xenstore pfns, I have used the HVM way of
doing things (speaking in x86 gergo) because they fit naturally in our
scenario.



Changes in v2:

- move xenstore and console page allocation to alloc_magic_pages;

- do now add the magic pages to p2m_host;

- remove xcbuild patch;

- rebase on "xen: event channel remapping for emulated MSIs", plus Ian's
PV domain build patches and my patch series "xen/arm: multiple guests
support in the GIC and VGIC".


Stefano Stabellini (4):
      xen/arm: implement do_hvm_op for ARM
      xen/arm: gic and vgic fixes
      xen/arm: disable the event optimization in the gic
      libxc/arm: allocate xenstore and console pages

 tools/libxc/xc_dom_arm.c     |   36 +++++++++++++++++++++----
 xen/arch/arm/Makefile        |    1 +
 xen/arch/arm/gic.c           |   48 ++-------------------------------
 xen/arch/arm/hvm.c           |   60 ++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c         |    1 +
 xen/arch/arm/vgic.c          |    3 +-
 xen/include/asm-arm/domain.h |    7 +++++
 7 files changed, 103 insertions(+), 53 deletions(-)


Cheers,

Stefano

[PATCH v2 1/4] xen/arm: implement do_hvm_op for ARM

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/Makefile        |    1 +
 xen/arch/arm/hvm.c           |   60 ++++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c         |    1 +
 xen/include/asm-arm/domain.h |    7 +++++
 4 files changed, 69 insertions(+), 0 deletions(-)
 create mode 100644 xen/arch/arm/hvm.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 5a87ba6..634b620 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -26,6 +26,7 @@ obj-y += traps.o
 obj-y += vgic.o
 obj-y += vtimer.o
 obj-y += vpl011.o
+obj-y += hvm.o
 
 #obj-bin-y += ....o
 
diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
new file mode 100644
index 0000000..c11378d
--- /dev/null
+++ b/xen/arch/arm/hvm.c
@@ -0,0 +1,60 @@
+#include <xen/config.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/sched.h>
+
+#include <public/xen.h>
+#include <public/hvm/params.h>
+#include <public/hvm/hvm_op.h>
+
+#include <asm/hypercall.h>
+
+long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
+
+{
+    long rc = 0;
+
+    switch ( op )
+    {
+    case HVMOP_set_param:
+    case HVMOP_get_param:
+    {
+        struct xen_hvm_param a;
+        struct domain *d;
+
+        if ( copy_from_guest(&a, arg, 1) )
+            return -EFAULT;
+
+        if ( a.index >= HVM_NR_PARAMS )
+            return -EINVAL;
+
+        rc = rcu_lock_target_domain_by_id(a.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        if ( op == HVMOP_set_param )
+        {
+            d->arch.hvm_domain.params[a.index] = a.value;
+        }
+        else
+        {
+            a.value = d->arch.hvm_domain.params[a.index];
+            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+        }
+
+        rcu_unlock_domain(d);
+        break;
+    }
+
+    default:
+    {
+        printk("%s: Bad HVM op %ld.\n", __func__, op);
+        rc = -ENOSYS;
+        break;
+    }
+    }
+
+    return rc;
+}
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f5f43da..f6e6807 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -430,6 +430,7 @@ static arm_hypercall_t *arm_hypercall_table[] = {
     HYPERCALL(memory_op),
     HYPERCALL(physdev_op),
     HYPERCALL(sysctl),
+    HYPERCALL(hvm_op),
 };
 
 static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 2b14545..114a8f6 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -5,6 +5,7 @@
 #include <xen/cache.h>
 #include <asm/page.h>
 #include <asm/p2m.h>
+#include <public/hvm/params.h>
 
 /* Represents state corresponding to a block of 32 interrupts */
 struct vgic_irq_rank {
@@ -28,9 +29,15 @@ struct pending_irq
     struct list_head lr_queue;
 };
 
+struct hvm_domain
+{
+    uint64_t              params[HVM_NR_PARAMS];
+}  __cacheline_aligned;
+
 struct arch_domain
 {
     struct p2m_domain p2m;
+    struct hvm_domain hvm_domain;
 
     struct {
         /*
-- 
1.7.2.5

[PATCH v2 2/4] xen/arm: gic and vgic fixes

[Stefano Stabellini]

- the last argument of find_next_bit is the maximum number of bits, not
the maximum number of bytes;

- use list_for_each_entry_safe instead of list_for_each_entry in
gic_restore_pending_irqs because we are actually deleting entries in the
loop.

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

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index c8d7caa..3a995d4 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -477,9 +477,9 @@ out:
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
     int i;
-    struct pending_irq *p;
+    struct pending_irq *p, *t;
 
-    list_for_each_entry ( p, &v->arch.vgic.lr_pending, lr_queue )
+    list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
     {
         i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
         if ( i >= nr_lrs ) return;
@@ -583,7 +583,7 @@ static void events_maintenance(struct vcpu *v)
     if (!already_pending && gic.event_mask != 0) {
         spin_lock_irq(&gic.lock);
         while ((i = find_next_bit((const long unsigned int *) &gic.event_mask,
-                        sizeof(uint64_t), i)) < sizeof(uint64_t)) {
+                        64, i)) < 64) {
 
             GICH[GICH_LR + i] = 0;
             clear_bit(i, &gic.lr_mask);
@@ -605,7 +605,7 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
     events_maintenance(v);
 
     while ((i = find_next_bit((const long unsigned int *) &eisr,
-                              sizeof(eisr), i)) < sizeof(eisr)) {
+                              64, i)) < 64) {
         struct pending_irq *p;
 
         spin_lock_irq(&gic.lock);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 005f478..5b5fca7 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -355,8 +355,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
     unsigned int irq;
     int i = 0;
 
-    while ( (i = find_next_bit((const long unsigned int *) &r,
-                        sizeof(uint32_t), i)) < sizeof(uint32_t) ) {
+    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) {
         irq = i + (32 * n);
         p = irq_to_pending(v, irq);
         if ( !list_empty(&p->inflight) )
-- 
1.7.2.5

[PATCH v2 3/4] xen/arm: disable the event optimization in the gic

[Stefano Stabellini]

Currently we have an optimization in the GIC driver that allows us not
to request maintenance interrupts for Xen events. The basic idea is that
every time we are about to inject a new interrupt or event into a guest,
we check whether we can remove some old event irqs from one or more LRs.
We can do this because we know when a guest finishes processing event
notifications: it sets the evtchn_upcall_pending bit to 0.

However it is unsafe: the guest resets evtchn_upcall_pending before
EOI'ing the event irq, therefore a small window of time exists when Xen
could jump in and remove the event irq from the LR register before the
guest has EOI'ed it.

This is not a good practice according to the GIC manual.
Remove the optimization for now.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/gic.c |   42 ------------------------------------------
 1 files changed, 0 insertions(+), 42 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 3a995d4..378158b 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -37,7 +37,6 @@
                                      + (GIC_CR_OFFSET & 0xfff)))
 #define GICH ((volatile uint32_t *) (FIXMAP_ADDR(FIXMAP_GICH)  \
                                      + (GIC_HR_OFFSET & 0xfff)))
-static void events_maintenance(struct vcpu *v);
 static void gic_restore_pending_irqs(struct vcpu *v);
 
 /* Global state */
@@ -48,7 +47,6 @@ static struct {
     unsigned int lines;
     unsigned int cpus;
     spinlock_t lock;
-    uint64_t event_mask;
     uint64_t lr_mask;
 } gic;
 
@@ -62,7 +60,6 @@ void gic_save_state(struct vcpu *v)
     for ( i=0; i<nr_lrs; i++)
         v->arch.gic_lr[i] = GICH[GICH_LR + i];
     v->arch.lr_mask = gic.lr_mask;
-    v->arch.event_mask = gic.event_mask;
     /* Disable until next VCPU scheduled */
     GICH[GICH_HCR] = 0;
     isb();
@@ -76,7 +73,6 @@ void gic_restore_state(struct vcpu *v)
         return;
 
     gic.lr_mask = v->arch.lr_mask;
-    gic.event_mask = v->arch.event_mask;
     for ( i=0; i<nr_lrs; i++)
         GICH[GICH_LR + i] = v->arch.gic_lr[i];
     GICH[GICH_HCR] = GICH_HCR_EN;
@@ -318,7 +314,6 @@ int __init gic_init(void)
     gic_hyp_init();
 
     gic.lr_mask = 0ULL;
-    gic.event_mask = 0ULL;
 
     spin_unlock(&gic.lock);
 
@@ -421,9 +416,6 @@ static inline void gic_set_lr(int lr, unsigned int virtual_irq,
 
     BUG_ON(lr > nr_lrs);
 
-    if (virtual_irq == VGIC_IRQ_EVTCHN_CALLBACK && nr_lrs > 1)
-        maintenance_int = 0;
-
     GICH[GICH_LR + lr] = state |
         maintenance_int |
         ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) |
@@ -436,11 +428,6 @@ void gic_set_guest_irq(struct vcpu *v, unsigned int virtual_irq,
     int i;
     struct pending_irq *iter, *n;
 
-    if ( v->is_running )
-    {
-        events_maintenance(v);
-    }
-
     spin_lock_irq(&gic.lock);
 
     if ( v->is_running && list_empty(&v->arch.vgic.lr_pending) )
@@ -448,8 +435,6 @@ void gic_set_guest_irq(struct vcpu *v, 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;
         }
@@ -487,8 +472,6 @@ static void gic_restore_pending_irqs(struct vcpu *v)
         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);
     }
 
 }
@@ -574,27 +557,6 @@ int 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_irq(&gic.lock);
-        while ((i = find_next_bit((const long unsigned int *) &gic.event_mask,
-                        64, i)) < 64) {
-
-            GICH[GICH_LR + i] = 0;
-            clear_bit(i, &gic.lr_mask);
-            clear_bit(i, &gic.event_mask);
-
-            i++;
-        }
-        spin_unlock_irq(&gic.lock);
-    }
-}
-
 static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
 {
     int i = 0, virq;
@@ -602,8 +564,6 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
     struct vcpu *v = current;
     uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32);
 
-    events_maintenance(v);
-
     while ((i = find_next_bit((const long unsigned int *) &eisr,
                               64, i)) < 64) {
         struct pending_irq *p;
@@ -619,8 +579,6 @@ 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();
         }
-- 
1.7.2.5

[PATCH v2 4/4] libxc/arm: allocate xenstore and console pages

[Stefano Stabellini]

Allocate two additional pages at the end of the guest physical memory
for xenstore and console.
Set HVM_PARAM_STORE_PFN and HVM_PARAM_CONSOLE_PFN to the corresponding
values.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/libxc/xc_dom_arm.c |   36 ++++++++++++++++++++++++++++++------
 1 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index bb86139..724e7ad 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -25,6 +25,10 @@
 #include "xg_private.h"
 #include "xc_dom.h"
 
+#define NR_MAGIC_PAGES 2
+#define CONSOLE_PFN_OFFSET 0
+#define XENSTORE_PFN_OFFSET 1
+
 /* ------------------------------------------------------------------------ */
 /*
  * arm guests are hybrid and start off with paging disabled, therefore no
@@ -46,13 +50,33 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
 
 static int alloc_magic_pages(struct xc_dom_image *dom)
 {
+    int rc, i, allocsz;
+    xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES];
+
     DOMPRINTF_CALLED(dom->xch);
-    /* XXX
-     *   dom->p2m_guest
-     *   dom->start_info_pfn
-     *   dom->xenstore_pfn
-     *   dom->console_pfn
-     */
+
+    for (i = 0; i < NR_MAGIC_PAGES; i++)
+        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
+
+    for ( i = rc = allocsz = 0;
+          (i < NR_MAGIC_PAGES) && !rc;
+          i += allocsz) {
+        allocsz = NR_MAGIC_PAGES - i;
+        rc = xc_domain_populate_physmap_exact(
+                dom->xch, dom->guest_domid, allocsz,
+                0, 0, &p2m[i]);
+    }
+
+    console_pfn = dom->rambase_pfn + dom->total_pages + CONSOLE_PFN_OFFSET;
+    store_pfn = dom->rambase_pfn + dom->total_pages + XENSTORE_PFN_OFFSET;
+
+    xc_clear_domain_page(dom->xch, dom->guest_domid, console_pfn);
+    xc_clear_domain_page(dom->xch, dom->guest_domid, store_pfn);
+    xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_CONSOLE_PFN,
+            console_pfn);
+    xc_set_hvm_param(dom->xch, dom->guest_domid, HVM_PARAM_STORE_PFN,
+            store_pfn);
+
     return 0;
 }
 
-- 
1.7.2.5

Re: [PATCH v2 1/4] xen/arm: implement do_hvm_op for ARM

[Tim Deegan]

At 11:55 +0100 on 04 Jul (1341402946), Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Tim Deegan <tim@xen.org>

I think we should look at moving this kind of thing into common code,
but that can wait until we have a better idea of how many hvm_ops we
want to move (and until after 4.2 branches!)


> ---
>  xen/arch/arm/Makefile        |    1 +
>  xen/arch/arm/hvm.c           |   60 ++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c         |    1 +
>  xen/include/asm-arm/domain.h |    7 +++++
>  4 files changed, 69 insertions(+), 0 deletions(-)
>  create mode 100644 xen/arch/arm/hvm.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 5a87ba6..634b620 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -26,6 +26,7 @@ obj-y += traps.o
>  obj-y += vgic.o
>  obj-y += vtimer.o
>  obj-y += vpl011.o
> +obj-y += hvm.o
>  
>  #obj-bin-y += ....o
>  
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> new file mode 100644
> index 0000000..c11378d
> --- /dev/null
> +++ b/xen/arch/arm/hvm.c
> @@ -0,0 +1,60 @@
> +#include <xen/config.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/errno.h>
> +#include <xen/guest_access.h>
> +#include <xen/sched.h>
> +
> +#include <public/xen.h>
> +#include <public/hvm/params.h>
> +#include <public/hvm/hvm_op.h>
> +
> +#include <asm/hypercall.h>
> +
> +long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
> +
> +{
> +    long rc = 0;
> +
> +    switch ( op )
> +    {
> +    case HVMOP_set_param:
> +    case HVMOP_get_param:
> +    {
> +        struct xen_hvm_param a;
> +        struct domain *d;
> +
> +        if ( copy_from_guest(&a, arg, 1) )
> +            return -EFAULT;
> +
> +        if ( a.index >= HVM_NR_PARAMS )
> +            return -EINVAL;
> +
> +        rc = rcu_lock_target_domain_by_id(a.domid, &d);
> +        if ( rc != 0 )
> +            return rc;
> +
> +        if ( op == HVMOP_set_param )
> +        {
> +            d->arch.hvm_domain.params[a.index] = a.value;
> +        }
> +        else
> +        {
> +            a.value = d->arch.hvm_domain.params[a.index];
> +            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> +        }
> +
> +        rcu_unlock_domain(d);
> +        break;
> +    }
> +
> +    default:
> +    {
> +        printk("%s: Bad HVM op %ld.\n", __func__, op);
> +        rc = -ENOSYS;
> +        break;
> +    }
> +    }
> +
> +    return rc;
> +}
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index f5f43da..f6e6807 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -430,6 +430,7 @@ static arm_hypercall_t *arm_hypercall_table[] = {
>      HYPERCALL(memory_op),
>      HYPERCALL(physdev_op),
>      HYPERCALL(sysctl),
> +    HYPERCALL(hvm_op),
>  };
>  
>  static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 2b14545..114a8f6 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -5,6 +5,7 @@
>  #include <xen/cache.h>
>  #include <asm/page.h>
>  #include <asm/p2m.h>
> +#include <public/hvm/params.h>
>  
>  /* Represents state corresponding to a block of 32 interrupts */
>  struct vgic_irq_rank {
> @@ -28,9 +29,15 @@ struct pending_irq
>      struct list_head lr_queue;
>  };
>  
> +struct hvm_domain
> +{
> +    uint64_t              params[HVM_NR_PARAMS];
> +}  __cacheline_aligned;
> +
>  struct arch_domain
>  {
>      struct p2m_domain p2m;
> +    struct hvm_domain hvm_domain;
>  
>      struct {
>          /*
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2 2/4] xen/arm: gic and vgic fixes

[Tim Deegan]

At 11:55 +0100 on 04 Jul (1341402947), Stefano Stabellini wrote:
> - the last argument of find_next_bit is the maximum number of bits, not
> the maximum number of bytes;
> 
> - use list_for_each_entry_safe instead of list_for_each_entry in
> gic_restore_pending_irqs because we are actually deleting entries in the
> loop.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Tim Deegan <tim@xen.org>

> ---
>  xen/arch/arm/gic.c  |    8 ++++----
>  xen/arch/arm/vgic.c |    3 +--
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index c8d7caa..3a995d4 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -477,9 +477,9 @@ out:
>  static void gic_restore_pending_irqs(struct vcpu *v)
>  {
>      int i;
> -    struct pending_irq *p;
> +    struct pending_irq *p, *t;
>  
> -    list_for_each_entry ( p, &v->arch.vgic.lr_pending, lr_queue )
> +    list_for_each_entry_safe ( p, t, &v->arch.vgic.lr_pending, lr_queue )
>      {
>          i = find_first_zero_bit(&gic.lr_mask, nr_lrs);
>          if ( i >= nr_lrs ) return;
> @@ -583,7 +583,7 @@ static void events_maintenance(struct vcpu *v)
>      if (!already_pending && gic.event_mask != 0) {
>          spin_lock_irq(&gic.lock);
>          while ((i = find_next_bit((const long unsigned int *) &gic.event_mask,
> -                        sizeof(uint64_t), i)) < sizeof(uint64_t)) {
> +                        64, i)) < 64) {
>  
>              GICH[GICH_LR + i] = 0;
>              clear_bit(i, &gic.lr_mask);
> @@ -605,7 +605,7 @@ static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *r
>      events_maintenance(v);
>  
>      while ((i = find_next_bit((const long unsigned int *) &eisr,
> -                              sizeof(eisr), i)) < sizeof(eisr)) {
> +                              64, i)) < 64) {
>          struct pending_irq *p;
>  
>          spin_lock_irq(&gic.lock);
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 005f478..5b5fca7 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -355,8 +355,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>      unsigned int irq;
>      int i = 0;
>  
> -    while ( (i = find_next_bit((const long unsigned int *) &r,
> -                        sizeof(uint32_t), i)) < sizeof(uint32_t) ) {
> +    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 ) {
>          irq = i + (32 * n);
>          p = irq_to_pending(v, irq);
>          if ( !list_empty(&p->inflight) )
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2 4/4] libxc/arm: allocate xenstore and console pages

[Tim Deegan]

At 11:55 +0100 on 04 Jul (1341402949), Stefano Stabellini wrote:
>  static int alloc_magic_pages(struct xc_dom_image *dom)
>  {
> +    int rc, i, allocsz;
> +    xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES];
> +
>      DOMPRINTF_CALLED(dom->xch);
> -    /* XXX
> -     *   dom->p2m_guest
> -     *   dom->start_info_pfn
> -     *   dom->xenstore_pfn
> -     *   dom->console_pfn
> -     */
> +
> +    for (i = 0; i < NR_MAGIC_PAGES; i++)
> +        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
> +
> +    for ( i = rc = allocsz = 0;
> +          (i < NR_MAGIC_PAGES) && !rc;
> +          i += allocsz) {
> +        allocsz = NR_MAGIC_PAGES - i;
> +        rc = xc_domain_populate_physmap_exact(
> +                dom->xch, dom->guest_domid, allocsz,
> +                0, 0, &p2m[i]);
> +    }

What does this loop do?  It seems like it can only ever execute once.

Cheers,

Tim.

Re: [PATCH v2 4/4] libxc/arm: allocate xenstore and console pages

[Stefano Stabellini]

On Thu, 12 Jul 2012, Tim Deegan wrote:
> At 11:55 +0100 on 04 Jul (1341402949), Stefano Stabellini wrote:
> >  static int alloc_magic_pages(struct xc_dom_image *dom)
> >  {
> > +    int rc, i, allocsz;
> > +    xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES];
> > +
> >      DOMPRINTF_CALLED(dom->xch);
> > -    /* XXX
> > -     *   dom->p2m_guest
> > -     *   dom->start_info_pfn
> > -     *   dom->xenstore_pfn
> > -     *   dom->console_pfn
> > -     */
> > +
> > +    for (i = 0; i < NR_MAGIC_PAGES; i++)
> > +        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
> > +
> > +    for ( i = rc = allocsz = 0;
> > +          (i < NR_MAGIC_PAGES) && !rc;
> > +          i += allocsz) {
> > +        allocsz = NR_MAGIC_PAGES - i;
> > +        rc = xc_domain_populate_physmap_exact(
> > +                dom->xch, dom->guest_domid, allocsz,
> > +                0, 0, &p2m[i]);
> > +    }
> 
> What does this loop do?  It seems like it can only ever execute once.

I think that you are right.
In that case also the same loop in arch_setup_meminit in Ian's patch
should probably be removed:

http://marc.info/?l=xen-devel&m=134089793916569

Re: [PATCH v2 4/4] libxc/arm: allocate xenstore and console pages

[Ian Campbell]

On Fri, 2012-07-13 at 11:32 -0400, Stefano Stabellini wrote:
> On Thu, 12 Jul 2012, Tim Deegan wrote:
> > At 11:55 +0100 on 04 Jul (1341402949), Stefano Stabellini wrote:
> > >  static int alloc_magic_pages(struct xc_dom_image *dom)
> > >  {
> > > +    int rc, i, allocsz;
> > > +    xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES];
> > > +
> > >      DOMPRINTF_CALLED(dom->xch);
> > > -    /* XXX
> > > -     *   dom->p2m_guest
> > > -     *   dom->start_info_pfn
> > > -     *   dom->xenstore_pfn
> > > -     *   dom->console_pfn
> > > -     */
> > > +
> > > +    for (i = 0; i < NR_MAGIC_PAGES; i++)
> > > +        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
> > > +
> > > +    for ( i = rc = allocsz = 0;
> > > +          (i < NR_MAGIC_PAGES) && !rc;
> > > +          i += allocsz) {
> > > +        allocsz = NR_MAGIC_PAGES - i;
> > > +        rc = xc_domain_populate_physmap_exact(
> > > +                dom->xch, dom->guest_domid, allocsz,
> > > +                0, 0, &p2m[i]);
> > > +    }
> > 
> > What does this loop do?  It seems like it can only ever execute once.
> 
> I think that you are right.
> In that case also the same loop in arch_setup_meminit in Ian's patch
> should probably be removed:
> 
> http://marc.info/?l=xen-devel&m=134089793916569

In that loop it can make only partial progress in an iteration (due to
the "if (alloc_size > 1024*1024)" clamping the allocation size. In that
case we will need to go round the loop multiple times.

In your case you always allocate exactly enough (NR_MAGIC_PAGES - i) the
first time round the loop. Since NR_MAGIC_PAGES is small you don't need
to do the allocations in chunks.

Ian

Re: [PATCH v2 4/4] libxc/arm: allocate xenstore and console pages

[Stefano Stabellini]

On Fri, 13 Jul 2012, Ian Campbell wrote:
> On Fri, 2012-07-13 at 11:32 -0400, Stefano Stabellini wrote:
> > On Thu, 12 Jul 2012, Tim Deegan wrote:
> > > At 11:55 +0100 on 04 Jul (1341402949), Stefano Stabellini wrote:
> > > >  static int alloc_magic_pages(struct xc_dom_image *dom)
> > > >  {
> > > > +    int rc, i, allocsz;
> > > > +    xen_pfn_t store_pfn, console_pfn, p2m[NR_MAGIC_PAGES];
> > > > +
> > > >      DOMPRINTF_CALLED(dom->xch);
> > > > -    /* XXX
> > > > -     *   dom->p2m_guest
> > > > -     *   dom->start_info_pfn
> > > > -     *   dom->xenstore_pfn
> > > > -     *   dom->console_pfn
> > > > -     */
> > > > +
> > > > +    for (i = 0; i < NR_MAGIC_PAGES; i++)
> > > > +        p2m[i] = dom->rambase_pfn + dom->total_pages + i;
> > > > +
> > > > +    for ( i = rc = allocsz = 0;
> > > > +          (i < NR_MAGIC_PAGES) && !rc;
> > > > +          i += allocsz) {
> > > > +        allocsz = NR_MAGIC_PAGES - i;
> > > > +        rc = xc_domain_populate_physmap_exact(
> > > > +                dom->xch, dom->guest_domid, allocsz,
> > > > +                0, 0, &p2m[i]);
> > > > +    }
> > > 
> > > What does this loop do?  It seems like it can only ever execute once.
> > 
> > I think that you are right.
> > In that case also the same loop in arch_setup_meminit in Ian's patch
> > should probably be removed:
> > 
> > http://marc.info/?l=xen-devel&m=134089793916569
> 
> In that loop it can make only partial progress in an iteration (due to
> the "if (alloc_size > 1024*1024)" clamping the allocation size. In that
> case we will need to go round the loop multiple times.
> 
> In your case you always allocate exactly enough (NR_MAGIC_PAGES - i) the
> first time round the loop. Since NR_MAGIC_PAGES is small you don't need
> to do the allocations in chunks.

I see, OK then.

Re: [PATCH v2 0/4] xen/arm: dom1 PV console up and running

[Ian Campbell]

On Wed, 2012-07-04 at 11:55 +0100, Stefano Stabellini wrote:

> Stefano Stabellini (4):
>       xen/arm: implement do_hvm_op for ARM
>       xen/arm: gic and vgic fixes
>       xen/arm: disable the event optimization in the gic

applied these three.

>       libxc/arm: allocate xenstore and console pages

this depends on tools patches which didn't get applied yet.

Ian.

> 
>  tools/libxc/xc_dom_arm.c     |   36 +++++++++++++++++++++----
>  xen/arch/arm/Makefile        |    1 +
>  xen/arch/arm/gic.c           |   48 ++-------------------------------
>  xen/arch/arm/hvm.c           |   60 ++++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c         |    1 +
>  xen/arch/arm/vgic.c          |    3 +-
>  xen/include/asm-arm/domain.h |    7 +++++
>  7 files changed, 103 insertions(+), 53 deletions(-)
> 
> 
> Cheers,
>