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

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

[Stefano Stabellini]

Hi all,
this patch series is based upon Ian's "arm: boot a dom1 to "Calibrating
delay loop" then hang" (http://marc.info/?l=xen-devel&m=133856539418794)
and my "xen/arm: event channels and shared_info page"
(http://marc.info/?l=xen-devel&m=133796319005683) patches series.

It 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.

The corresponding Linux side patch series is going to be posted soon.


Stefano Stabellini (5):
      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
      xcbuild: add console and xenstore support

 tools/libxc/xc_dom_arm.c     |   32 +++++++++++++++-------
 tools/xcutils/Makefile       |    4 +-
 tools/xcutils/xcbuild.c      |   58 +++++++++++++++++++++++++++++++++++++++-
 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 +++++
 9 files changed, 154 insertions(+), 60 deletions(-)

Cheers,

Stefano

[PATCH 1/5] 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 ec74298..3900545 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 2/5] 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 87713c1..8190f84 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -480,13 +480,13 @@ out:
 static void gic_restore_pending_irqs(struct vcpu *v)
 {
     int i;
-    struct pending_irq *p;
+    struct pending_irq *p, *t;
 
     /* check for new pending irqs */
     if ( list_empty(&v->arch.vgic.lr_pending) )
         return;
 
-    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;
@@ -593,7 +593,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);
@@ -615,7 +615,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 653e8e5..b383e01 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 3/5] 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>
---
 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 8190f84..c6cee4b 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;
         }
@@ -494,8 +479,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);
     }
 
 }
@@ -584,27 +567,6 @@ 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_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;
@@ -612,8 +574,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;
@@ -629,8 +589,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 4/5] 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 |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index bb86139..df2eefe 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
@@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
 static int alloc_magic_pages(struct xc_dom_image *dom)
 {
     DOMPRINTF_CALLED(dom->xch);
-    /* XXX
-     *   dom->p2m_guest
-     *   dom->start_info_pfn
-     *   dom->xenstore_pfn
-     *   dom->console_pfn
-     */
     return 0;
 }
 
@@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 {
     int rc;
     xen_pfn_t pfn, allocsz, i;
+    xen_pfn_t store_pfn, console_pfn;
 
     fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__,
             dom->total_pages, dom->rambase_pfn);
 
     dom->shadow_enabled = 1;
 
-    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
+    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES));
 
     fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__,
             dom->rambase_pfn, dom->total_pages );
     /* setup initial p2m */
-    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
+    for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
         dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
 
     fprintf(stderr, "%s: init'd p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
@@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 
     /* allocate guest memory */
     for ( i = rc = allocsz = 0;
-          (i < dom->total_pages) && !rc;
+          (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
           i += allocsz )
     {
-        allocsz = dom->total_pages - i;
+        allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;
         if ( allocsz > 1024*1024 )
             allocsz = 1024*1024;
         fprintf(stderr, "alloc %"PRI_xen_pfn" at offset %"PRI_xen_pfn" which is %"PRI_xen_pfn"\n",
@@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     fprintf(stderr, "%s: popl'd p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n",
             __func__, dom->total_pages-1, dom->p2m_host[dom->total_pages-1]);
 
+    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);
+
+    fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn" xenstore_pfn=%"PRI_xen_pfn"\n",
+            __func__, console_pfn, store_pfn);
+
     return 0;
 }
 
-- 
1.7.2.5

[PATCH 5/5] xcbuild: add console and xenstore support

[Stefano Stabellini]

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 tools/xcutils/Makefile  |    4 +-
 tools/xcutils/xcbuild.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile
index 92c5a68..c88f286 100644
--- a/tools/xcutils/Makefile
+++ b/tools/xcutils/Makefile
@@ -20,7 +20,7 @@ CFLAGS_xc_save.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe
 CFLAGS_readnotes.o  := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
 CFLAGS_lsevtchn.o   := $(CFLAGS_libxenctrl)
 CFLAGS_xcversion.o  := $(CFLAGS_libxenctrl)
-CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
+CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore)
 
 .PHONY: all
 all: build
@@ -35,7 +35,7 @@ xc_save: xc_save.o
 	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
 xcbuild: xcbuild.o
-	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
+	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
 
 readnotes: readnotes.o
 	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c
index a70c3ca..7b5c0f8 100644
--- a/tools/xcutils/xcbuild.c
+++ b/tools/xcutils/xcbuild.c
@@ -1,6 +1,7 @@
 #include <unistd.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 
 #include <errno.h>
 
@@ -8,6 +9,7 @@
 //#include <xenguest.h>
 #include <xentoollog.h>
 #include <xc_dom.h>
+#include <xenstore.h>
 
 int main(int argc, char **argv)
 {
@@ -15,11 +17,19 @@ int main(int argc, char **argv)
 	xc_interface *xch;
 	int rv;
 	const char *image;
-	uint32_t domid;
+	uint32_t domid = 1;
 	xen_domain_handle_t handle;
 	int maxmem = 128; /* MB */ //atoi(argv[2]);
 	int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */
 	struct xc_dom_image *dom;
+	unsigned long console_mfn;
+	unsigned long console_port;
+	unsigned long store_mfn;
+	unsigned long store_port;
+	struct xs_handle *xs;
+	char *dom_path;
+	char path[256];
+	char val[256];
 
 	image = (argc < 2) ? "guest.img" : argv[1];
 	printf("Image: %s\n", image);
@@ -103,6 +113,52 @@ int main(int argc, char **argv)
 	if (rv) return rv;
 	rv = xc_dom_build_image(dom);
 	if (rv) return rv;
+
+	xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn);
+	console_port = xc_evtchn_alloc_unbound(xch, domid, 0);
+	xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port);
+
+	xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn);
+	store_port = xc_evtchn_alloc_unbound(xch, domid, 0);
+	xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port);
+	xs = xs_daemon_open();
+	if (xs == NULL) {
+		printf("Could not contact XenStore");
+		return errno;
+	}
+	dom_path = xs_get_domain_path(xs, domid);
+
+	snprintf(path, sizeof(path), "%s/console/port", dom_path);
+	snprintf(val, sizeof(val), "%lu", console_port);
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path);
+	snprintf(val, sizeof(val), "%lu", console_mfn);
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/console/type", dom_path);
+	snprintf(val, sizeof(val), "xenconsoled");
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/console/output", dom_path);
+	snprintf(val, sizeof(val), "pty");
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/console/limit", dom_path);
+	snprintf(val, sizeof(val), "%d", 1048576);
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/store/port", dom_path);
+	snprintf(val, sizeof(val), "%lu", store_port);
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+
+	snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path);
+	snprintf(val, sizeof(val), "%lu", store_mfn);
+	xs_write(xs, XBT_NULL, path, val, strlen(val));
+	xs_introduce_domain(xs, domid, store_mfn, store_port);
+
+	xs_daemon_close(xs);
+
 	rv = xc_dom_boot_image(dom);
 	if (rv) return rv;
 
-- 
1.7.2.5

Re: [PATCH 5/5] xcbuild: add console and xenstore support

[Ian Campbell]

Do you not have libxl and friends up and running sufficiently to use?
This xcbuild was really just intended to tide us over until that stuff
worked, not to be an actual thing...

On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  tools/xcutils/Makefile  |    4 +-
>  tools/xcutils/xcbuild.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile
> index 92c5a68..c88f286 100644
> --- a/tools/xcutils/Makefile
> +++ b/tools/xcutils/Makefile
> @@ -20,7 +20,7 @@ CFLAGS_xc_save.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe
>  CFLAGS_readnotes.o  := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
>  CFLAGS_lsevtchn.o   := $(CFLAGS_libxenctrl)
>  CFLAGS_xcversion.o  := $(CFLAGS_libxenctrl)
> -CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
> +CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore)
>  
>  .PHONY: all
>  all: build
> @@ -35,7 +35,7 @@ xc_save: xc_save.o
>  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
>  
>  xcbuild: xcbuild.o
> -	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> +	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
>  
>  readnotes: readnotes.o
>  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c
> index a70c3ca..7b5c0f8 100644
> --- a/tools/xcutils/xcbuild.c
> +++ b/tools/xcutils/xcbuild.c
> @@ -1,6 +1,7 @@
>  #include <unistd.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  
>  #include <errno.h>
>  
> @@ -8,6 +9,7 @@
>  //#include <xenguest.h>
>  #include <xentoollog.h>
>  #include <xc_dom.h>
> +#include <xenstore.h>
>  
>  int main(int argc, char **argv)
>  {
> @@ -15,11 +17,19 @@ int main(int argc, char **argv)
>  	xc_interface *xch;
>  	int rv;
>  	const char *image;
> -	uint32_t domid;
> +	uint32_t domid = 1;
>  	xen_domain_handle_t handle;
>  	int maxmem = 128; /* MB */ //atoi(argv[2]);
>  	int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */
>  	struct xc_dom_image *dom;
> +	unsigned long console_mfn;
> +	unsigned long console_port;
> +	unsigned long store_mfn;
> +	unsigned long store_port;
> +	struct xs_handle *xs;
> +	char *dom_path;
> +	char path[256];
> +	char val[256];
>  
>  	image = (argc < 2) ? "guest.img" : argv[1];
>  	printf("Image: %s\n", image);
> @@ -103,6 +113,52 @@ int main(int argc, char **argv)
>  	if (rv) return rv;
>  	rv = xc_dom_build_image(dom);
>  	if (rv) return rv;
> +
> +	xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn);
> +	console_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> +	xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port);
> +
> +	xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn);
> +	store_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> +	xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port);
> +	xs = xs_daemon_open();
> +	if (xs == NULL) {
> +		printf("Could not contact XenStore");
> +		return errno;
> +	}
> +	dom_path = xs_get_domain_path(xs, domid);
> +
> +	snprintf(path, sizeof(path), "%s/console/port", dom_path);
> +	snprintf(val, sizeof(val), "%lu", console_port);
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path);
> +	snprintf(val, sizeof(val), "%lu", console_mfn);
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/console/type", dom_path);
> +	snprintf(val, sizeof(val), "xenconsoled");
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/console/output", dom_path);
> +	snprintf(val, sizeof(val), "pty");
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/console/limit", dom_path);
> +	snprintf(val, sizeof(val), "%d", 1048576);
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/store/port", dom_path);
> +	snprintf(val, sizeof(val), "%lu", store_port);
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +
> +	snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path);
> +	snprintf(val, sizeof(val), "%lu", store_mfn);
> +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> +	xs_introduce_domain(xs, domid, store_mfn, store_port);
> +
> +	xs_daemon_close(xs);
> +
>  	rv = xc_dom_boot_image(dom);
>  	if (rv) return rv;
>  

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

[Ian Campbell]

We need to device how hybrid our hybrid arm guests are going to be.

The particular hvm params you are using here (evtchn port etc) typically
live in start_info for a PV guest. In principal we could define a start
info for ARM too but that leaves the question of how the guest can find
it (which loops up back to hvm_params...).

Looking at the other stuff in start_info, it has stuff like modules (aka
ramdisks) and command lines which ARM guest get via the normal ARM boot
protocol stuff (i.e. the domain builder does it) and a bunch of stuff
which seems to only make sense for proper-PV guests.

So maybe HVM PARAM is the right answer?

sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN.
Might we want that?

On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> 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 ec74298..3900545 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 {
>          /*

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

[Ian Campbell]

On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> - the last argument of find_next_bit is the maximum number of bits, not
> the maximum number of bytes;

I guess 8*sizeof(...) is pretty ugly too so just using the hardcoded
numbers is acceptable in this case.

> 
> - 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 87713c1..8190f84 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -480,13 +480,13 @@ out:
>  static void gic_restore_pending_irqs(struct vcpu *v)
>  {
>      int i;
> -    struct pending_irq *p;
> +    struct pending_irq *p, *t;
>  
>      /* check for new pending irqs */
>      if ( list_empty(&v->arch.vgic.lr_pending) )
>          return;
>  
> -    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;
> @@ -593,7 +593,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);
> @@ -615,7 +615,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 653e8e5..b383e01 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) )

Re: [PATCH 3/5] xen/arm: disable the event optimization in the gic

[Ian Campbell]

On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> 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 8190f84..c6cee4b 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;
>          }
> @@ -494,8 +479,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);
>      }
>  
>  }
> @@ -584,27 +567,6 @@ 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_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;
> @@ -612,8 +574,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;
> @@ -629,8 +589,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();
>          }

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

[Ian Campbell]

On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> 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 |   32 ++++++++++++++++++++++----------
>  1 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index bb86139..df2eefe 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
> @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
>  static int alloc_magic_pages(struct xc_dom_image *dom)
>  {
>      DOMPRINTF_CALLED(dom->xch);
> -    /* XXX
> -     *   dom->p2m_guest
> -     *   dom->start_info_pfn
> -     *   dom->xenstore_pfn
> -     *   dom->console_pfn
> -     */
>      return 0;
>  }
>  
> @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>  {
>      int rc;
>      xen_pfn_t pfn, allocsz, i;
> +    xen_pfn_t store_pfn, console_pfn;
>  
>      fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__,
>              dom->total_pages, dom->rambase_pfn);
>  
>      dom->shadow_enabled = 1;
>  
> -    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
> +    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES));
>  
>      fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__,
>              dom->rambase_pfn, dom->total_pages );
>      /* setup initial p2m */
> -    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
> +    for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
>          dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
>  
>      fprintf(stderr, "%s: init'd p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
> @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>  
>      /* allocate guest memory */
>      for ( i = rc = allocsz = 0;
> -          (i < dom->total_pages) && !rc;
> +          (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
>            i += allocsz )
>      {
> -        allocsz = dom->total_pages - i;
> +        allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;

All these "+ NR_MAGIC_PAGES" are a bit troublesome looking.

Do these pages need to be in p2m_host or would it be fine to just insert
them into the guest p2m individually outside the main allocation logic?

Otherwise perhaps simply int total_pages = dom->total_pages + NR... and
use throughout?

>          if ( allocsz > 1024*1024 )
>              allocsz = 1024*1024;
>          fprintf(stderr, "alloc %"PRI_xen_pfn" at offset %"PRI_xen_pfn" which is %"PRI_xen_pfn"\n",
> @@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>      fprintf(stderr, "%s: popl'd p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n",
>              __func__, dom->total_pages-1, dom->p2m_host[dom->total_pages-1]);

I really need to scrub the debug printfs from my patch...

>  
> +    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);
> +
> +    fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn" xenstore_pfn=%"PRI_xen_pfn"\n",
> +            __func__, console_pfn, store_pfn);

... and so do you ;-)

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

[Stefano Stabellini]

On Tue, 26 Jun 2012, Ian Campbell wrote:
> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > - the last argument of find_next_bit is the maximum number of bits, not
> > the maximum number of bytes;
> 
> I guess 8*sizeof(...) is pretty ugly too so just using the hardcoded
> numbers is acceptable in this case.

Yeah, that's what I thought.

Re: [PATCH 5/5] xcbuild: add console and xenstore support

[Stefano Stabellini]

On Tue, 26 Jun 2012, Ian Campbell wrote:
> Do you not have libxl and friends up and running sufficiently to use?
> This xcbuild was really just intended to tide us over until that stuff
> worked, not to be an actual thing...

xl/libxl works well enough for basic commands like "xl list", however
in order to support something like "xl create" I expect that some more
refactoring is needed as well as code motion to arch specific files.
Considering that we are in code freeze right now, I thought it wouldn't
be acceptable for 4.2, so I didn't even try.


> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > ---
> >  tools/xcutils/Makefile  |    4 +-
> >  tools/xcutils/xcbuild.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 59 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile
> > index 92c5a68..c88f286 100644
> > --- a/tools/xcutils/Makefile
> > +++ b/tools/xcutils/Makefile
> > @@ -20,7 +20,7 @@ CFLAGS_xc_save.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe
> >  CFLAGS_readnotes.o  := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
> >  CFLAGS_lsevtchn.o   := $(CFLAGS_libxenctrl)
> >  CFLAGS_xcversion.o  := $(CFLAGS_libxenctrl)
> > -CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
> > +CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore)
> >  
> >  .PHONY: all
> >  all: build
> > @@ -35,7 +35,7 @@ xc_save: xc_save.o
> >  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
> >  
> >  xcbuild: xcbuild.o
> > -	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> > +	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
> >  
> >  readnotes: readnotes.o
> >  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> > diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c
> > index a70c3ca..7b5c0f8 100644
> > --- a/tools/xcutils/xcbuild.c
> > +++ b/tools/xcutils/xcbuild.c
> > @@ -1,6 +1,7 @@
> >  #include <unistd.h>
> >  #include <stdio.h>
> >  #include <stdlib.h>
> > +#include <string.h>
> >  
> >  #include <errno.h>
> >  
> > @@ -8,6 +9,7 @@
> >  //#include <xenguest.h>
> >  #include <xentoollog.h>
> >  #include <xc_dom.h>
> > +#include <xenstore.h>
> >  
> >  int main(int argc, char **argv)
> >  {
> > @@ -15,11 +17,19 @@ int main(int argc, char **argv)
> >  	xc_interface *xch;
> >  	int rv;
> >  	const char *image;
> > -	uint32_t domid;
> > +	uint32_t domid = 1;
> >  	xen_domain_handle_t handle;
> >  	int maxmem = 128; /* MB */ //atoi(argv[2]);
> >  	int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */
> >  	struct xc_dom_image *dom;
> > +	unsigned long console_mfn;
> > +	unsigned long console_port;
> > +	unsigned long store_mfn;
> > +	unsigned long store_port;
> > +	struct xs_handle *xs;
> > +	char *dom_path;
> > +	char path[256];
> > +	char val[256];
> >  
> >  	image = (argc < 2) ? "guest.img" : argv[1];
> >  	printf("Image: %s\n", image);
> > @@ -103,6 +113,52 @@ int main(int argc, char **argv)
> >  	if (rv) return rv;
> >  	rv = xc_dom_build_image(dom);
> >  	if (rv) return rv;
> > +
> > +	xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn);
> > +	console_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> > +	xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port);
> > +
> > +	xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn);
> > +	store_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> > +	xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port);
> > +	xs = xs_daemon_open();
> > +	if (xs == NULL) {
> > +		printf("Could not contact XenStore");
> > +		return errno;
> > +	}
> > +	dom_path = xs_get_domain_path(xs, domid);
> > +
> > +	snprintf(path, sizeof(path), "%s/console/port", dom_path);
> > +	snprintf(val, sizeof(val), "%lu", console_port);
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path);
> > +	snprintf(val, sizeof(val), "%lu", console_mfn);
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/console/type", dom_path);
> > +	snprintf(val, sizeof(val), "xenconsoled");
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/console/output", dom_path);
> > +	snprintf(val, sizeof(val), "pty");
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/console/limit", dom_path);
> > +	snprintf(val, sizeof(val), "%d", 1048576);
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/store/port", dom_path);
> > +	snprintf(val, sizeof(val), "%lu", store_port);
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +
> > +	snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path);
> > +	snprintf(val, sizeof(val), "%lu", store_mfn);
> > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > +	xs_introduce_domain(xs, domid, store_mfn, store_port);
> > +
> > +	xs_daemon_close(xs);
> > +
> >  	rv = xc_dom_boot_image(dom);
> >  	if (rv) return rv;
> >  
> 
> 
> 

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

[Stefano Stabellini]

On Tue, 26 Jun 2012, Ian Campbell wrote:
> On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > 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 |   32 ++++++++++++++++++++++----------
> >  1 files changed, 22 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > index bb86139..df2eefe 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
> > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
> >  static int alloc_magic_pages(struct xc_dom_image *dom)
> >  {
> >      DOMPRINTF_CALLED(dom->xch);
> > -    /* XXX
> > -     *   dom->p2m_guest
> > -     *   dom->start_info_pfn
> > -     *   dom->xenstore_pfn
> > -     *   dom->console_pfn
> > -     */
> >      return 0;
> >  }
> >  
> > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> >  {
> >      int rc;
> >      xen_pfn_t pfn, allocsz, i;
> > +    xen_pfn_t store_pfn, console_pfn;
> >  
> >      fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__,
> >              dom->total_pages, dom->rambase_pfn);
> >  
> >      dom->shadow_enabled = 1;
> >  
> > -    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
> > +    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES));
> >  
> >      fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__,
> >              dom->rambase_pfn, dom->total_pages );
> >      /* setup initial p2m */
> > -    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
> > +    for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
> >          dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
> >  
> >      fprintf(stderr, "%s: init'd p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
> > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> >  
> >      /* allocate guest memory */
> >      for ( i = rc = allocsz = 0;
> > -          (i < dom->total_pages) && !rc;
> > +          (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
> >            i += allocsz )
> >      {
> > -        allocsz = dom->total_pages - i;
> > +        allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;
> 
> All these "+ NR_MAGIC_PAGES" are a bit troublesome looking.
> 
> Do these pages need to be in p2m_host or would it be fine to just insert
> them into the guest p2m individually outside the main allocation logic?

I think it makes sense for them to be in p2m_host. In fact if we try to
allocate them later, wouldn't we have the problem of having to extend
the guest p2m? We might as well do it here.


> Otherwise perhaps simply int total_pages = dom->total_pages + NR... and
> use throughout?

Yes, I can do that.


> >          if ( allocsz > 1024*1024 )
> >              allocsz = 1024*1024;
> >          fprintf(stderr, "alloc %"PRI_xen_pfn" at offset %"PRI_xen_pfn" which is %"PRI_xen_pfn"\n",
> > @@ -168,6 +167,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> >      fprintf(stderr, "%s: popl'd p2m_host[%"PRI_xen_pfn"] = %"PRI_xen_pfn"\n",
> >              __func__, dom->total_pages-1, dom->p2m_host[dom->total_pages-1]);
> 
> I really need to scrub the debug printfs from my patch...
> 
> >  
> > +    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);
> > +
> > +    fprintf(stderr, "%s: console_pfn=%"PRI_xen_pfn" xenstore_pfn=%"PRI_xen_pfn"\n",
> > +            __func__, console_pfn, store_pfn);
> 
> ... and so do you ;-)
 
OK :)

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

[Stefano Stabellini]

On Tue, 26 Jun 2012, Ian Campbell wrote:
> We need to device how hybrid our hybrid arm guests are going to be.

Right.


> The particular hvm params you are using here (evtchn port etc) typically
> live in start_info for a PV guest. In principal we could define a start
> info for ARM too but that leaves the question of how the guest can find
> it (which loops up back to hvm_params...).

One way would be to introduce a new XENMAPSPACE, like we have done for
XENMAPSPACE_shared_info. Then the guest would use a
XENMEM_add_to_physmap to map it.


> Looking at the other stuff in start_info, it has stuff like modules (aka
> ramdisks) and command lines which ARM guest get via the normal ARM boot
> protocol stuff (i.e. the domain builder does it) and a bunch of stuff
> which seems to only make sense for proper-PV guests.
> 
> So maybe HVM PARAM is the right answer?

Yes, that is one of the reasons why I preferred introducing hvm_op
rather than a new XENMAPSPACE: we don't need anything else from
start_info aside from the parameters already provided through
HVMOP_get_param.
On the other hand hvm_op can be useful for other things, for example one
day we might use the HVM_PARAM_IOREQ_PFN parameter.
Most importantly, from the Linux side of things, acting like a PV on HVM
kernel is a perfect fit, the changes required are down to a minimum.


> sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN.
> Might we want that?

We don't need it thanks to XENFEAT_dom0, see 
1340381685-22529-3-git-send-email-stefano.stabellini@eu.citrix.com

Re: [PATCH 5/5] xcbuild: add console and xenstore support

[Ian Campbell]

On Tue, 2012-06-26 at 18:58 +0100, Stefano Stabellini wrote:
> On Tue, 26 Jun 2012, Ian Campbell wrote:
> > Do you not have libxl and friends up and running sufficiently to use?
> > This xcbuild was really just intended to tide us over until that stuff
> > worked, not to be an actual thing...
> 
> xl/libxl works well enough for basic commands like "xl list", however
> in order to support something like "xl create" I expect that some more
> refactoring is needed as well as code motion to arch specific files.
> Considering that we are in code freeze right now, I thought it wouldn't
> be acceptable for 4.2, so I didn't even try.

Sounds reasonable.

It might still be interesting to run xl create and see what actually
breaks...

I don't plan to commit xcbuild, but I could maintain a dev branch
somewhere with such code in it for people to pull in to their local
tree, if that sounds useful?

> > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > ---
> > >  tools/xcutils/Makefile  |    4 +-
> > >  tools/xcutils/xcbuild.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++-
> > >  2 files changed, 59 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/xcutils/Makefile b/tools/xcutils/Makefile
> > > index 92c5a68..c88f286 100644
> > > --- a/tools/xcutils/Makefile
> > > +++ b/tools/xcutils/Makefile
> > > @@ -20,7 +20,7 @@ CFLAGS_xc_save.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxe
> > >  CFLAGS_readnotes.o  := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
> > >  CFLAGS_lsevtchn.o   := $(CFLAGS_libxenctrl)
> > >  CFLAGS_xcversion.o  := $(CFLAGS_libxenctrl)
> > > -CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
> > > +CFLAGS_xcbuild.o    := $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) $(CFLAGS_libxenstore)
> > >  
> > >  .PHONY: all
> > >  all: build
> > > @@ -35,7 +35,7 @@ xc_save: xc_save.o
> > >  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
> > >  
> > >  xcbuild: xcbuild.o
> > > -	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> > > +	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
> > >  
> > >  readnotes: readnotes.o
> > >  	$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
> > > diff --git a/tools/xcutils/xcbuild.c b/tools/xcutils/xcbuild.c
> > > index a70c3ca..7b5c0f8 100644
> > > --- a/tools/xcutils/xcbuild.c
> > > +++ b/tools/xcutils/xcbuild.c
> > > @@ -1,6 +1,7 @@
> > >  #include <unistd.h>
> > >  #include <stdio.h>
> > >  #include <stdlib.h>
> > > +#include <string.h>
> > >  
> > >  #include <errno.h>
> > >  
> > > @@ -8,6 +9,7 @@
> > >  //#include <xenguest.h>
> > >  #include <xentoollog.h>
> > >  #include <xc_dom.h>
> > > +#include <xenstore.h>
> > >  
> > >  int main(int argc, char **argv)
> > >  {
> > > @@ -15,11 +17,19 @@ int main(int argc, char **argv)
> > >  	xc_interface *xch;
> > >  	int rv;
> > >  	const char *image;
> > > -	uint32_t domid;
> > > +	uint32_t domid = 1;
> > >  	xen_domain_handle_t handle;
> > >  	int maxmem = 128; /* MB */ //atoi(argv[2]);
> > >  	int memory_kb = 2*(maxmem + 1)*1024; /* bit of slack... */
> > >  	struct xc_dom_image *dom;
> > > +	unsigned long console_mfn;
> > > +	unsigned long console_port;
> > > +	unsigned long store_mfn;
> > > +	unsigned long store_port;
> > > +	struct xs_handle *xs;
> > > +	char *dom_path;
> > > +	char path[256];
> > > +	char val[256];
> > >  
> > >  	image = (argc < 2) ? "guest.img" : argv[1];
> > >  	printf("Image: %s\n", image);
> > > @@ -103,6 +113,52 @@ int main(int argc, char **argv)
> > >  	if (rv) return rv;
> > >  	rv = xc_dom_build_image(dom);
> > >  	if (rv) return rv;
> > > +
> > > +	xc_get_hvm_param(xch, domid, HVM_PARAM_CONSOLE_PFN, &console_mfn);
> > > +	console_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> > > +	xc_set_hvm_param(xch, domid, HVM_PARAM_CONSOLE_EVTCHN, console_port);
> > > +
> > > +	xc_get_hvm_param(xch, domid, HVM_PARAM_STORE_PFN, &store_mfn);
> > > +	store_port = xc_evtchn_alloc_unbound(xch, domid, 0);
> > > +	xc_set_hvm_param(xch, domid, HVM_PARAM_STORE_EVTCHN, store_port);
> > > +	xs = xs_daemon_open();
> > > +	if (xs == NULL) {
> > > +		printf("Could not contact XenStore");
> > > +		return errno;
> > > +	}
> > > +	dom_path = xs_get_domain_path(xs, domid);
> > > +
> > > +	snprintf(path, sizeof(path), "%s/console/port", dom_path);
> > > +	snprintf(val, sizeof(val), "%lu", console_port);
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/console/ring-ref", dom_path);
> > > +	snprintf(val, sizeof(val), "%lu", console_mfn);
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/console/type", dom_path);
> > > +	snprintf(val, sizeof(val), "xenconsoled");
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/console/output", dom_path);
> > > +	snprintf(val, sizeof(val), "pty");
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/console/limit", dom_path);
> > > +	snprintf(val, sizeof(val), "%d", 1048576);
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/store/port", dom_path);
> > > +	snprintf(val, sizeof(val), "%lu", store_port);
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +
> > > +	snprintf(path, sizeof(path), "%s/store/ring-ref", dom_path);
> > > +	snprintf(val, sizeof(val), "%lu", store_mfn);
> > > +	xs_write(xs, XBT_NULL, path, val, strlen(val));
> > > +	xs_introduce_domain(xs, domid, store_mfn, store_port);
> > > +
> > > +	xs_daemon_close(xs);
> > > +
> > >  	rv = xc_dom_boot_image(dom);
> > >  	if (rv) return rv;
> > >  
> > 
> > 
> > 

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

[Ian Campbell]

On Tue, 2012-06-26 at 19:05 +0100, Stefano Stabellini wrote:
> On Tue, 26 Jun 2012, Ian Campbell wrote:
> > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > > 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 |   32 ++++++++++++++++++++++----------
> > >  1 files changed, 22 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > > index bb86139..df2eefe 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
> > > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
> > >  static int alloc_magic_pages(struct xc_dom_image *dom)
> > >  {
> > >      DOMPRINTF_CALLED(dom->xch);
> > > -    /* XXX
> > > -     *   dom->p2m_guest
> > > -     *   dom->start_info_pfn
> > > -     *   dom->xenstore_pfn
> > > -     *   dom->console_pfn
> > > -     */
> > >      return 0;
> > >  }
> > >  
> > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> > >  {
> > >      int rc;
> > >      xen_pfn_t pfn, allocsz, i;
> > > +    xen_pfn_t store_pfn, console_pfn;
> > >  
> > >      fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__,
> > >              dom->total_pages, dom->rambase_pfn);
> > >  
> > >      dom->shadow_enabled = 1;
> > >  
> > > -    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
> > > +    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES));
> > >  
> > >      fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__,
> > >              dom->rambase_pfn, dom->total_pages );
> > >      /* setup initial p2m */
> > > -    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
> > > +    for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
> > >          dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
> > >  
> > >      fprintf(stderr, "%s: init'd p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
> > > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> > >  
> > >      /* allocate guest memory */
> > >      for ( i = rc = allocsz = 0;
> > > -          (i < dom->total_pages) && !rc;
> > > +          (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
> > >            i += allocsz )
> > >      {
> > > -        allocsz = dom->total_pages - i;
> > > +        allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;
> > 
> > All these "+ NR_MAGIC_PAGES" are a bit troublesome looking.
> > 
> > Do these pages need to be in p2m_host or would it be fine to just insert
> > them into the guest p2m individually outside the main allocation logic?
> 
> I think it makes sense for them to be in p2m_host. In fact if we try to
> allocate them later, wouldn't we have the problem of having to extend
> the guest p2m? We might as well do it here.

The actual guest p2m is internal to the hypervisor so we never see it at
the tools layer.

I'm unsure if we need these magic pages in p2m_host. If we remember the
gfn of the magic pages that's just as useful as remembering the offset
in p2m_host and using p2m_host[offset]?


Ian.

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

[Ian Campbell]

On Tue, 2012-06-26 at 19:29 +0100, Stefano Stabellini wrote:
> On Tue, 26 Jun 2012, Ian Campbell wrote:
> > We need to device how hybrid our hybrid arm guests are going to be.
> 
> Right.
> 
> 
> > The particular hvm params you are using here (evtchn port etc) typically
> > live in start_info for a PV guest. In principal we could define a start
> > info for ARM too but that leaves the question of how the guest can find
> > it (which loops up back to hvm_params...).
> 
> One way would be to introduce a new XENMAPSPACE, like we have done for
> XENMAPSPACE_shared_info. Then the guest would use a
> XENMEM_add_to_physmap to map it.
> 
> 
> > Looking at the other stuff in start_info, it has stuff like modules (aka
> > ramdisks) and command lines which ARM guest get via the normal ARM boot
> > protocol stuff (i.e. the domain builder does it) and a bunch of stuff
> > which seems to only make sense for proper-PV guests.
> > 
> > So maybe HVM PARAM is the right answer?
> 
> Yes, that is one of the reasons why I preferred introducing hvm_op
> rather than a new XENMAPSPACE: we don't need anything else from
> start_info aside from the parameters already provided through
> HVMOP_get_param.
> On the other hand hvm_op can be useful for other things, for example one
> day we might use the HVM_PARAM_IOREQ_PFN parameter.

That would be in future if/when we support proper "HVM" (or "PVHVM") ARM
guests i.e. ones which appear to the guest like a particular physical
platform, rather than our virtual hybrid platform, and therefore have a
QEMU providing a DM for that hardware?

> Most importantly, from the Linux side of things, acting like a PV on HVM
> kernel is a perfect fit, the changes required are down to a minimum.

OK.

> > sinfo does have flags which contains stuff like SIF_INITIAL_DOMAIN.
> > Might we want that?
> 
> We don't need it thanks to XENFEAT_dom0, see 
> 1340381685-22529-3-git-send-email-stefano.stabellini@eu.citrix.com

Great.

Re: [PATCH 5/5] xcbuild: add console and xenstore support

[Stefano Stabellini]

On Wed, 27 Jun 2012, Ian Campbell wrote:
> On Tue, 2012-06-26 at 18:58 +0100, Stefano Stabellini wrote:
> > On Tue, 26 Jun 2012, Ian Campbell wrote:
> > > Do you not have libxl and friends up and running sufficiently to use?
> > > This xcbuild was really just intended to tide us over until that stuff
> > > worked, not to be an actual thing...
> > 
> > xl/libxl works well enough for basic commands like "xl list", however
> > in order to support something like "xl create" I expect that some more
> > refactoring is needed as well as code motion to arch specific files.
> > Considering that we are in code freeze right now, I thought it wouldn't
> > be acceptable for 4.2, so I didn't even try.
> 
> Sounds reasonable.
> 
> It might still be interesting to run xl create and see what actually
> breaks...
> 
> I don't plan to commit xcbuild, but I could maintain a dev branch
> somewhere with such code in it for people to pull in to their local
> tree, if that sounds useful?

Yep.

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

[Stefano Stabellini]

On Wed, 27 Jun 2012, Ian Campbell wrote:
> On Tue, 2012-06-26 at 19:29 +0100, Stefano Stabellini wrote:
> > On Tue, 26 Jun 2012, Ian Campbell wrote:
> > > We need to device how hybrid our hybrid arm guests are going to be.
> > 
> > Right.
> > 
> > 
> > > The particular hvm params you are using here (evtchn port etc) typically
> > > live in start_info for a PV guest. In principal we could define a start
> > > info for ARM too but that leaves the question of how the guest can find
> > > it (which loops up back to hvm_params...).
> > 
> > One way would be to introduce a new XENMAPSPACE, like we have done for
> > XENMAPSPACE_shared_info. Then the guest would use a
> > XENMEM_add_to_physmap to map it.
> > 
> > 
> > > Looking at the other stuff in start_info, it has stuff like modules (aka
> > > ramdisks) and command lines which ARM guest get via the normal ARM boot
> > > protocol stuff (i.e. the domain builder does it) and a bunch of stuff
> > > which seems to only make sense for proper-PV guests.
> > > 
> > > So maybe HVM PARAM is the right answer?
> > 
> > Yes, that is one of the reasons why I preferred introducing hvm_op
> > rather than a new XENMAPSPACE: we don't need anything else from
> > start_info aside from the parameters already provided through
> > HVMOP_get_param.
> > On the other hand hvm_op can be useful for other things, for example one
> > day we might use the HVM_PARAM_IOREQ_PFN parameter.
> 
> That would be in future if/when we support proper "HVM" (or "PVHVM") ARM
> guests i.e. ones which appear to the guest like a particular physical
> platform, rather than our virtual hybrid platform, and therefore have a
> QEMU providing a DM for that hardware?

Yes, that is what HVM_PARAM_IOREQ_PFN would be for.

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

[Stefano Stabellini]

On Wed, 27 Jun 2012, Ian Campbell wrote:
> On Tue, 2012-06-26 at 19:05 +0100, Stefano Stabellini wrote:
> > On Tue, 26 Jun 2012, Ian Campbell wrote:
> > > On Fri, 2012-06-22 at 17:09 +0100, Stefano Stabellini wrote:
> > > > 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 |   32 ++++++++++++++++++++++----------
> > > >  1 files changed, 22 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> > > > index bb86139..df2eefe 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
> > > > @@ -47,12 +51,6 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
> > > >  static int alloc_magic_pages(struct xc_dom_image *dom)
> > > >  {
> > > >      DOMPRINTF_CALLED(dom->xch);
> > > > -    /* XXX
> > > > -     *   dom->p2m_guest
> > > > -     *   dom->start_info_pfn
> > > > -     *   dom->xenstore_pfn
> > > > -     *   dom->console_pfn
> > > > -     */
> > > >      return 0;
> > > >  }
> > > >  
> > > > @@ -127,18 +125,19 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> > > >  {
> > > >      int rc;
> > > >      xen_pfn_t pfn, allocsz, i;
> > > > +    xen_pfn_t store_pfn, console_pfn;
> > > >  
> > > >      fprintf(stderr, "%s: tot pages %"PRI_xen_pfn" rambase %"PRI_xen_pfn"\n", __func__,
> > > >              dom->total_pages, dom->rambase_pfn);
> > > >  
> > > >      dom->shadow_enabled = 1;
> > > >  
> > > > -    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
> > > > +    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * (dom->total_pages + NR_MAGIC_PAGES));
> > > >  
> > > >      fprintf(stderr, "%s: setup p2m from %"PRI_xen_pfn" for %"PRI_xen_pfn" pages\n", __func__,
> > > >              dom->rambase_pfn, dom->total_pages );
> > > >      /* setup initial p2m */
> > > > -    for ( pfn = 0; pfn < dom->total_pages; pfn++ )
> > > > +    for ( pfn = 0; pfn < (dom->total_pages + NR_MAGIC_PAGES); pfn++ )
> > > >          dom->p2m_host[pfn] = pfn + dom->rambase_pfn;
> > > >  
> > > >      fprintf(stderr, "%s: init'd p2m_host[0] = %"PRI_xen_pfn"\n", __func__, dom->p2m_host[0]);
> > > > @@ -148,10 +147,10 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> > > >  
> > > >      /* allocate guest memory */
> > > >      for ( i = rc = allocsz = 0;
> > > > -          (i < dom->total_pages) && !rc;
> > > > +          (i < dom->total_pages + NR_MAGIC_PAGES) && !rc;
> > > >            i += allocsz )
> > > >      {
> > > > -        allocsz = dom->total_pages - i;
> > > > +        allocsz = (dom->total_pages + NR_MAGIC_PAGES) - i;
> > > 
> > > All these "+ NR_MAGIC_PAGES" are a bit troublesome looking.
> > > 
> > > Do these pages need to be in p2m_host or would it be fine to just insert
> > > them into the guest p2m individually outside the main allocation logic?
> > 
> > I think it makes sense for them to be in p2m_host. In fact if we try to
> > allocate them later, wouldn't we have the problem of having to extend
> > the guest p2m? We might as well do it here.
> 
> The actual guest p2m is internal to the hypervisor so we never see it at
> the tools layer.
> 
> I'm unsure if we need these magic pages in p2m_host. If we remember the
> gfn of the magic pages that's just as useful as remembering the offset
> in p2m_host and using p2m_host[offset]?

I think that you are right: it is better not to add them to p2m_host.

---

libxc/arm: allocate xenstore and console pages

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>

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;
 }