xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] xen/arm: event channels and shared_info page
@ 2012-02-23 18:20 Stefano Stabellini
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:20 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com
  Cc: Tim Deegan (3P), Stefano Stabellini, Ian Campbell, David Vrabel

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

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

With this the corresponding Linux patch series applied to both Linux and
Xen, I am able to boot dom0, start xenstored and run basic xl commands,
like "xl list" and "xl uptime".



Changes in v2:

- fixed tabs/spaces problem.



Stefano Stabellini (5):
      arm: shared_info page allocation and mapping
      arm: implement vcpu_mark_events_pending
      arm: set the default dom0 max_vcpus value to 1 (currently is 0)
      arm: use r12 to pass the hypercall number
      arm: introduce more hypercalls

 xen/arch/arm/Makefile           |    1 +
 xen/arch/arm/domain.c           |   19 ++++++++
 xen/arch/arm/domain_build.c     |    2 +-
 xen/arch/arm/dummy.S            |    1 -
 xen/arch/arm/gic.h              |    3 +
 xen/arch/arm/mm.c               |   98 +++++++++++++++++++++++++++++++++++++--
 xen/arch/arm/p2m.c              |   15 ++++++-
 xen/arch/arm/physdev.c          |   27 +++++++++++
 xen/arch/arm/traps.c            |   21 +++++---
 xen/include/asm-arm/hypercall.h |    3 +
 xen/include/asm-arm/mm.h        |    4 ++
 xen/include/asm-arm/p2m.h       |    2 +
 12 files changed, 181 insertions(+), 15 deletions(-)


A git branch based on my previous gic and tools patches is available
here:

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

Cheers,

Stefano

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
@ 2012-02-23 18:21 ` Stefano Stabellini
  2012-02-24 10:22   ` Ian Campbell
                     ` (2 more replies)
  2012-02-23 18:21 ` [PATCH v2 2/5] arm: implement vcpu_mark_events_pending Stefano Stabellini
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim.Deegan, Stefano Stabellini, Ian.Campbell, david.vrabel

Allocate the shared_info page at domain creation.

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

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen/arch/arm/domain.c     |    8 ++++
 xen/arch/arm/mm.c         |   98 +++++++++++++++++++++++++++++++++++++++++++--
 xen/arch/arm/p2m.c        |   15 ++++++-
 xen/include/asm-arm/mm.h  |    4 ++
 xen/include/asm-arm/p2m.h |    2 +
 5 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 0b55934..0ee96b9 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -235,6 +235,14 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
     if ( (rc = p2m_init(d)) != 0 )
         goto fail;
 
+    rc = -ENOMEM;
+    if ( (d->shared_info = alloc_xenheap_pages(0, MEMF_bits(32))) == NULL )
+        goto fail;
+
+    clear_page(d->shared_info);
+    share_xen_page_with_guest(
+            virt_to_page(d->shared_info), d, XENSHARE_writable);
+
     d->max_vcpus = 8;
 
     if ( (rc = domain_vgic_init(d)) != 0 )
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a0f39eb..0fe0398 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -25,8 +25,11 @@
 #include <xen/mm.h>
 #include <xen/preempt.h>
 #include <xen/errno.h>
+#include <xen/guest_access.h>
 #include <asm/page.h>
 #include <asm/current.h>
+#include <public/memory.h>
+#include <xen/sched.h>
 
 struct domain *dom_xen, *dom_io;
 
@@ -323,17 +326,104 @@ void arch_dump_shared_mem_info(void)
 {
 }
 
-long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+int donate_page(struct domain *d, struct page_info *page, unsigned int memflags)
 {
+    ASSERT(0);
     return -ENOSYS;
 }
 
-int donate_page(struct domain *d, struct page_info *page, unsigned int memflags)
+void share_xen_page_with_guest(struct page_info *page,
+                          struct domain *d, int readonly)
 {
-    ASSERT(0);
-    return -ENOSYS;
+    if ( page_get_owner(page) == d )
+        return;
+
+    spin_lock(&d->page_alloc_lock);
+
+    /* The incremented type count pins as writable or read-only. */
+    page->u.inuse.type_info  = (readonly ? PGT_none : PGT_writable_page);
+    page->u.inuse.type_info |= PGT_validated | 1;
+
+    page_set_owner(page, d);
+    wmb(); /* install valid domain ptr before updating refcnt. */
+    ASSERT((page->count_info & ~PGC_xen_heap) == 0);
+
+    /* Only add to the allocation list if the domain isn't dying. */
+    if ( !d->is_dying )
+    {
+        page->count_info |= PGC_allocated | 1;
+        if ( unlikely(d->xenheap_pages++ == 0) )
+            get_knownalive_domain(d);
+        page_list_add_tail(page, &d->xenpage_list);
+    }
+
+    spin_unlock(&d->page_alloc_lock);
+}
+
+static int xenmem_add_to_physmap_once(
+    struct domain *d,
+    const struct xen_add_to_physmap *xatp)
+{
+    unsigned long mfn = 0;
+    int rc;
+
+    switch ( xatp->space )
+    {
+        case XENMAPSPACE_shared_info:
+            if ( xatp->idx == 0 )
+                mfn = virt_to_mfn(d->shared_info);
+            break;
+        default:
+            return -ENOSYS;
+    }
+
+    domain_lock(d);
+
+    /* Map at new location. */
+    rc = guest_physmap_add_page(d, xatp->gpfn, mfn);
+
+    domain_unlock(d);
+
+    return rc;
+}
+
+static int xenmem_add_to_physmap(struct domain *d,
+                                 struct xen_add_to_physmap *xatp)
+{
+    return xenmem_add_to_physmap_once(d, xatp);
 }
 
+long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
+{
+    int rc;
+
+    switch ( op )
+    {
+    case XENMEM_add_to_physmap:
+    {
+        struct xen_add_to_physmap xatp;
+        struct domain *d;
+
+        if ( copy_from_guest(&xatp, arg, 1) )
+            return -EFAULT;
+
+        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
+        if ( rc != 0 )
+            return rc;
+
+        rc = xenmem_add_to_physmap(d, &xatp);
+
+        rcu_unlock_domain(d);
+
+        return rc;
+    }
+
+    default:
+        return -ENOSYS;
+    }
+
+    return 0;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 14614fd..1a00a50 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -118,7 +118,12 @@ static int create_p2m_entries(struct domain *d,
         }
         /* else: third already valid */
 
-        BUG_ON(third[third_table_offset(addr)].p2m.valid);
+        if ( third[third_table_offset(addr)].p2m.valid )
+        {
+            /* p2m entry already present */
+            free_domheap_page(
+                    mfn_to_page(third[third_table_offset(addr)].p2m.base));
+        }
 
         /* Allocate a new RAM page and attach */
         if (alloc)
@@ -172,6 +177,14 @@ int map_mmio_regions(struct domain *d,
     return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
 }
 
+int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
+        unsigned long mfn)
+{
+    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
+                              (gpfn + 1) << PAGE_SHIFT,
+                              mfn << PAGE_SHIFT);
+}
+
 int p2m_alloc_table(struct domain *d)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index bfc0f76..56ab9415 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -78,6 +78,10 @@ struct page_info
 #define _PGT_pinned       PG_shift(5)
 #define PGT_pinned        PG_mask(1, 5)
 
+ /* Has this page been validated for use as its current type? */
+#define _PGT_validated    PG_shift(6)
+#define PGT_validated     PG_mask(1, 6)
+
  /* Count of uses of this frame as its current type. */
 #define PGT_count_width   PG_shift(9)
 #define PGT_count_mask    ((1UL<<PGT_count_width)-1)
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index aec52f7..c0f2995 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -39,6 +39,8 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
  * address maddr. */
 int map_mmio_regions(struct domain *d, paddr_t start_gaddr,
                      paddr_t end_gaddr, paddr_t maddr);
+int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
+        unsigned long mfn);
 
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
@ 2012-02-23 18:21 ` Stefano Stabellini
  2012-02-24 10:33   ` Ian Campbell
  2012-02-23 18:21 ` [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0) Stefano Stabellini
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim.Deegan, Stefano Stabellini, Ian.Campbell, david.vrabel

Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
we reserve for Xen usage.
In the future the interrupt used for event injection might be dynamic
and could be written into the device tree.

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

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 0ee96b9..4752aaa 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -272,6 +272,17 @@ void arch_dump_vcpu_info(struct vcpu *v)
 {
 }
 
+void vcpu_mark_events_pending(struct vcpu *v)
+{
+    int already_pending = test_and_set_bit(
+        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
+
+    if ( already_pending )
+        return;
+
+    vgic_vcpu_inject_irq(v, VGIC_IRQ_EVTCHN_CALLBACK, 1);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
index 3f2cc4b..93f6f53 100644
--- a/xen/arch/arm/dummy.S
+++ b/xen/arch/arm/dummy.S
@@ -31,7 +31,6 @@ DUMMY(arch_vcpu_reset);
 DUMMY(free_vcpu_guest_context);
 DUMMY(sync_vcpu_execstate);
 NOP(update_vcpu_system_time);
-DUMMY(vcpu_mark_events_pending);
 DUMMY(vcpu_show_execution_state);
 
 /* Page Reference & Type Maintenance */
diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
index 81c388d..36fdea1 100644
--- a/xen/arch/arm/gic.h
+++ b/xen/arch/arm/gic.h
@@ -121,6 +121,9 @@
 #define GICH_LR_CPUID_SHIFT     9
 #define GICH_VTR_NRLRGS         0x3f
 
+/* XXX: write this into the DT */
+#define VGIC_IRQ_EVTCHN_CALLBACK 63
+
 extern int domain_vgic_init(struct domain *d);
 extern int vcpu_vgic_init(struct vcpu *v);
 extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int virtual);
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0)
  2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
  2012-02-23 18:21 ` [PATCH v2 2/5] arm: implement vcpu_mark_events_pending Stefano Stabellini
@ 2012-02-23 18:21 ` Stefano Stabellini
  2012-02-24 10:40   ` Ian Campbell
  2012-02-23 18:21 ` [PATCH v2 4/5] arm: use r12 to pass the hypercall number Stefano Stabellini
  2012-02-23 18:21 ` [PATCH v2 5/5] arm: introduce more hypercalls Stefano Stabellini
  4 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim.Deegan, Stefano Stabellini, Ian.Campbell, david.vrabel

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

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index cbbc0b9..06ba285 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -9,7 +9,7 @@
 #include "gic.h"
 #include "kernel.h"
 
-static unsigned int __initdata opt_dom0_max_vcpus;
+static unsigned int __initdata opt_dom0_max_vcpus = 1;
 integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
 
 struct vcpu *__init alloc_dom0_vcpu0(void)
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 4/5] arm: use r12 to pass the hypercall number
  2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
                   ` (2 preceding siblings ...)
  2012-02-23 18:21 ` [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0) Stefano Stabellini
@ 2012-02-23 18:21 ` Stefano Stabellini
  2012-02-23 19:29   ` Ian Campbell
  2012-02-23 18:21 ` [PATCH v2 5/5] arm: introduce more hypercalls Stefano Stabellini
  4 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim.Deegan, Stefano Stabellini, Ian.Campbell, david.vrabel

Use r12 to pass the hypercall number and r0-r4 for the hypercall
arguments as usual.

Use the ISS to pass an hypervisor specific tag.

Remove passing unused registers to arm_hypercall_table: we don't have 6
arguments hypercalls and we never use 64 bit values as hypercall
arguments, 64 bit values are only contained within structs passed as
arguments.

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

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 395d0af..44d19ec 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -367,7 +367,6 @@ unsigned long do_arch_0(unsigned int cmd, unsigned long long value)
 }
 
 typedef unsigned long arm_hypercall_t(
-    unsigned int, unsigned int, unsigned int, unsigned int, unsigned int,
     unsigned int, unsigned int, unsigned int, unsigned int, unsigned int);
 
 #define HYPERCALL(x)                                        \
@@ -409,16 +408,17 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
 {
     local_irq_enable();
 
-    regs->r0 = arm_hypercall_table[iss](regs->r0,
+    if ( iss != XEN_HYPERCALL_TAG  ) {
+        printk("%s %d: received an alien hypercall iss=%lx\n", __func__ ,
+                __LINE__ , iss);
+        return;
+    }
+
+    regs->r0 = arm_hypercall_table[regs->r12](regs->r0,
                              regs->r1,
                              regs->r2,
                              regs->r3,
-                             regs->r4,
-                             regs->r5,
-                             regs->r6,
-                             regs->r7,
-                             regs->r8,
-                             regs->r9);
+                             regs->r4);
 }
 
 static void do_cp15_32(struct cpu_user_regs *regs,
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index e840507..d517542 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -3,6 +3,8 @@
 
 #include <public/domctl.h> /* for arch_do_domctl */
 
+#define XEN_HYPERCALL_TAG   0XEA1
+
 #endif /* __ASM_ARM_HYPERCALL_H__ */
 /*
  * Local variables:
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v2 5/5] arm: introduce more hypercalls
  2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
                   ` (3 preceding siblings ...)
  2012-02-23 18:21 ` [PATCH v2 4/5] arm: use r12 to pass the hypercall number Stefano Stabellini
@ 2012-02-23 18:21 ` Stefano Stabellini
  2012-02-24 10:41   ` Ian Campbell
  4 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-23 18:21 UTC (permalink / raw)
  To: xen-devel; +Cc: Tim.Deegan, Stefano Stabellini, Ian.Campbell, david.vrabel

Implement xen_version, event_channel_op, memory_op sysctl and physdev_op
hypercalls.

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

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 49b64fe..619430c 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -14,6 +14,7 @@ obj-y += kernel.o
 obj-y += mm.o
 obj-y += p2m.o
 obj-y += guestcopy.o
+obj-y += physdev.o
 obj-y += setup.o
 obj-y += time.o
 obj-y += smpboot.o
diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
new file mode 100644
index 0000000..bcf4337
--- /dev/null
+++ b/xen/arch/arm/physdev.c
@@ -0,0 +1,27 @@
+/******************************************************************************
+ * Arch-specific physdev.c
+ *
+ * Copyright (c) 2012, Citrix Systems
+ */
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <asm/hypercall.h>
+
+
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
+{
+    printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
+    return -ENOSYS;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 44d19ec..b266c5e 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -376,6 +376,11 @@ static arm_hypercall_t *arm_hypercall_table[] = {
     HYPERCALL(arch_0),
     HYPERCALL(sched_op),
     HYPERCALL(console_io),
+    HYPERCALL(xen_version),
+    HYPERCALL(event_channel_op),
+    HYPERCALL(memory_op),
+    HYPERCALL(physdev_op),
+    HYPERCALL(sysctl),
 };
 
 static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
index d517542..b91f8b6 100644
--- a/xen/include/asm-arm/hypercall.h
+++ b/xen/include/asm-arm/hypercall.h
@@ -2,6 +2,7 @@
 #define __ASM_ARM_HYPERCALL_H__
 
 #include <public/domctl.h> /* for arch_do_domctl */
+int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg);
 
 #define XEN_HYPERCALL_TAG   0XEA1
 
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/5] arm: use r12 to pass the hypercall number
  2012-02-23 18:21 ` [PATCH v2 4/5] arm: use r12 to pass the hypercall number Stefano Stabellini
@ 2012-02-23 19:29   ` Ian Campbell
  2012-02-24 12:21     ` Stefano Stabellini
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-23 19:29 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Use r12 to pass the hypercall number and r0-r4 for the hypercall
> arguments as usual.

Strictly speaking "as usual" in the ARM calling convention would be args
in r0-r3 and the fifth (and subsequent) arguments on the stack. However
we are free to implement our own convention for hypercalls and avoiding
arguments on the stack is a good idea.

Please could you add a comment describing the interface. X86 documents
this in xen/include/public/arch-x86/xen-x86_{32,64}.h so I suppose
xen/include/public/arch-arm.h is appropriate

We should define precisely which registers are clobbered by a hypercall.
X86 clobbers exactly those arguments registers which are used for that
hypercall but perhaps we can simplify and always clobber r0..r4,r12
(plus any other caller saved registers in the usual calling convention,
just for sanity). r0..r3,r12 are already clobbered in the normal calling
convention so the only difference would be clobbering r4 which is
normally callee saved (but is also not normally used to pass arguments).

We should explicitly clobber whatever we say we will too in a debug
build.

I've trimmed the quote already so I'll mention it here instead:
XEN_HYPERCALL_TAG should be defined in the public interface too not in
the private asm headers.

> @@ -409,16 +408,17 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
>  {
>      local_irq_enable();
>  
> -    regs->r0 = arm_hypercall_table[iss](regs->r0,
> +    if ( iss != XEN_HYPERCALL_TAG  ) {
> +        printk("%s %d: received an alien hypercall iss=%lx\n", __func__ ,
> +                __LINE__ , iss);

	regs->r0 = -EINVAL;

here I think.

> +        return;
> +    }
> +
> +    regs->r0 = arm_hypercall_table[regs->r12](regs->r0,

It's an existing issue but we need to check that
arm_hypercall_table[regs->r12] is non-NULL here and return -ENOSYS (by
setting r0) if it is.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
@ 2012-02-24 10:22   ` Ian Campbell
  2012-02-24 12:46     ` Ian Campbell
  2012-02-24 13:04   ` Ian Campbell
  2012-02-24 16:50   ` Ian Campbell
  2 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 10:22 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Allocate the shared_info page at domain creation.
> 
> Implement arch_memory_op, only for XENMEM_add_to_physmap with space ==
> XENMAPSPACE_shared_info, so that the guest can map the shared_info page.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/arch/arm/domain.c     |    8 ++++
>  xen/arch/arm/mm.c         |   98 +++++++++++++++++++++++++++++++++++++++++++--
>  xen/arch/arm/p2m.c        |   15 ++++++-
>  xen/include/asm-arm/mm.h  |    4 ++
>  xen/include/asm-arm/p2m.h |    2 +
>  5 files changed, 122 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 0b55934..0ee96b9 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -235,6 +235,14 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>      if ( (rc = p2m_init(d)) != 0 )
>          goto fail;
>  
> +    rc = -ENOMEM;
> +    if ( (d->shared_info = alloc_xenheap_pages(0, MEMF_bits(32))) == NULL )
> +        goto fail;
> +
> +    clear_page(d->shared_info);
> +    share_xen_page_with_guest(
> +            virt_to_page(d->shared_info), d, XENSHARE_writable);
> +
>      d->max_vcpus = 8;
>  
>      if ( (rc = domain_vgic_init(d)) != 0 )
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index a0f39eb..0fe0398 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -25,8 +25,11 @@
>  #include <xen/mm.h>
>  #include <xen/preempt.h>
>  #include <xen/errno.h>
> +#include <xen/guest_access.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
> +#include <public/memory.h>
> +#include <xen/sched.h>
>  
>  struct domain *dom_xen, *dom_io;
>  
> @@ -323,17 +326,104 @@ void arch_dump_shared_mem_info(void)
>  {
>  }
>  
> -long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
> +int donate_page(struct domain *d, struct page_info *page, unsigned int memflags)
>  {
> +    ASSERT(0);
>      return -ENOSYS;
>  }
>  
> -int donate_page(struct domain *d, struct page_info *page, unsigned int memflags)
> +void share_xen_page_with_guest(struct page_info *page,
> +                          struct domain *d, int readonly)
>  {
> -    ASSERT(0);
> -    return -ENOSYS;
> +    if ( page_get_owner(page) == d )
> +        return;
> +
> +    spin_lock(&d->page_alloc_lock);
> +
> +    /* The incremented type count pins as writable or read-only. */
> +    page->u.inuse.type_info  = (readonly ? PGT_none : PGT_writable_page);
> +    page->u.inuse.type_info |= PGT_validated | 1;
> +
> +    page_set_owner(page, d);
> +    wmb(); /* install valid domain ptr before updating refcnt. */
> +    ASSERT((page->count_info & ~PGC_xen_heap) == 0);
> +
> +    /* Only add to the allocation list if the domain isn't dying. */
> +    if ( !d->is_dying )
> +    {
> +        page->count_info |= PGC_allocated | 1;
> +        if ( unlikely(d->xenheap_pages++ == 0) )
> +            get_knownalive_domain(d);
> +        page_list_add_tail(page, &d->xenpage_list);
> +    }
> +
> +    spin_unlock(&d->page_alloc_lock);
> +}
> +
> +static int xenmem_add_to_physmap_once(
> +    struct domain *d,
> +    const struct xen_add_to_physmap *xatp)
> +{
> +    unsigned long mfn = 0;
> +    int rc;
> +
> +    switch ( xatp->space )
> +    {
> +        case XENMAPSPACE_shared_info:
> +            if ( xatp->idx == 0 )
> +                mfn = virt_to_mfn(d->shared_info);
> +            break;
> +        default:
> +            return -ENOSYS;
> +    }
> +
> +    domain_lock(d);
> +
> +    /* Map at new location. */
> +    rc = guest_physmap_add_page(d, xatp->gpfn, mfn);
> +
> +    domain_unlock(d);
> +
> +    return rc;
> +}
> +
> +static int xenmem_add_to_physmap(struct domain *d,
> +                                 struct xen_add_to_physmap *xatp)
> +{
> +    return xenmem_add_to_physmap_once(d, xatp);
>  }
>  
> +long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg)
> +{
> +    int rc;
> +
> +    switch ( op )
> +    {
> +    case XENMEM_add_to_physmap:
> +    {
> +        struct xen_add_to_physmap xatp;
> +        struct domain *d;
> +
> +        if ( copy_from_guest(&xatp, arg, 1) )
> +            return -EFAULT;
> +
> +        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
> +        if ( rc != 0 )
> +            return rc;
> +
> +        rc = xenmem_add_to_physmap(d, &xatp);
> +
> +        rcu_unlock_domain(d);
> +
> +        return rc;
> +    }
> +
> +    default:
> +        return -ENOSYS;
> +    }
> +
> +    return 0;
> +}
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 14614fd..1a00a50 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -118,7 +118,12 @@ static int create_p2m_entries(struct domain *d,
>          }
>          /* else: third already valid */
>  
> -        BUG_ON(third[third_table_offset(addr)].p2m.valid);
> +        if ( third[third_table_offset(addr)].p2m.valid )
> +        {
> +            /* p2m entry already present */
> +            free_domheap_page(
> +                    mfn_to_page(third[third_table_offset(addr)].p2m.base));
> +        }
>  
>          /* Allocate a new RAM page and attach */
>          if (alloc)
> @@ -172,6 +177,14 @@ int map_mmio_regions(struct domain *d,
>      return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
>  }
>  
> +int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
> +        unsigned long mfn)
> +{
> +    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
> +                              (gpfn + 1) << PAGE_SHIFT,
> +                              mfn << PAGE_SHIFT);
> +}
> +
>  int p2m_alloc_table(struct domain *d)
>  {
>      struct p2m_domain *p2m = &d->arch.p2m;
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index bfc0f76..56ab9415 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -78,6 +78,10 @@ struct page_info
>  #define _PGT_pinned       PG_shift(5)
>  #define PGT_pinned        PG_mask(1, 5)
>  
> + /* Has this page been validated for use as its current type? */
> +#define _PGT_validated    PG_shift(6)
> +#define PGT_validated     PG_mask(1, 6)
> +
>   /* Count of uses of this frame as its current type. */
>  #define PGT_count_width   PG_shift(9)
>  #define PGT_count_mask    ((1UL<<PGT_count_width)-1)
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index aec52f7..c0f2995 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -39,6 +39,8 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
>   * address maddr. */
>  int map_mmio_regions(struct domain *d, paddr_t start_gaddr,
>                       paddr_t end_gaddr, paddr_t maddr);
> +int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
> +        unsigned long mfn);
>  
>  unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
>  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-23 18:21 ` [PATCH v2 2/5] arm: implement vcpu_mark_events_pending Stefano Stabellini
@ 2012-02-24 10:33   ` Ian Campbell
  2012-02-24 10:53     ` David Vrabel
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 10:33 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
> we reserve for Xen usage.

Does this require a trap when the guest acks or EOIs this interrupt?
What about maintenance interrupts arising from injecting this?

Might we want to instead inject this interrupt as an SGI, so that it
appears as a per-VCPU interrupt?

> In the future the interrupt used for event injection might be dynamic
> and could be written into the device tree.

I think this is required, isn't it? At least for the privileged domain
which also sees physical interrupts, unless we want to get into playing
tricks with non 1:1 interrupts.

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/domain.c |   11 +++++++++++
>  xen/arch/arm/dummy.S  |    1 -
>  xen/arch/arm/gic.h    |    3 +++
>  3 files changed, 14 insertions(+), 1 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 0ee96b9..4752aaa 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -272,6 +272,17 @@ void arch_dump_vcpu_info(struct vcpu *v)
>  {
>  }
>  
> +void vcpu_mark_events_pending(struct vcpu *v)
> +{
> +    int already_pending = test_and_set_bit(
> +        0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
> +
> +    if ( already_pending )
> +        return;
> +
> +    vgic_vcpu_inject_irq(v, VGIC_IRQ_EVTCHN_CALLBACK, 1);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/dummy.S b/xen/arch/arm/dummy.S
> index 3f2cc4b..93f6f53 100644
> --- a/xen/arch/arm/dummy.S
> +++ b/xen/arch/arm/dummy.S
> @@ -31,7 +31,6 @@ DUMMY(arch_vcpu_reset);
>  DUMMY(free_vcpu_guest_context);
>  DUMMY(sync_vcpu_execstate);
>  NOP(update_vcpu_system_time);
> -DUMMY(vcpu_mark_events_pending);
>  DUMMY(vcpu_show_execution_state);
>  
>  /* Page Reference & Type Maintenance */
> diff --git a/xen/arch/arm/gic.h b/xen/arch/arm/gic.h
> index 81c388d..36fdea1 100644
> --- a/xen/arch/arm/gic.h
> +++ b/xen/arch/arm/gic.h
> @@ -121,6 +121,9 @@
>  #define GICH_LR_CPUID_SHIFT     9
>  #define GICH_VTR_NRLRGS         0x3f
>  
> +/* XXX: write this into the DT */
> +#define VGIC_IRQ_EVTCHN_CALLBACK 63
> +
>  extern int domain_vgic_init(struct domain *d);
>  extern int vcpu_vgic_init(struct vcpu *v);
>  extern void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq,int virtual);

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0)
  2012-02-23 18:21 ` [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0) Stefano Stabellini
@ 2012-02-24 10:40   ` Ian Campbell
  2012-02-24 12:36     ` Stefano Stabellini
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 10:40 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Why is this needed? It makes it impossible to distinguish the "use all
CPUs" default from the "only use 1 CPU" case.

I think we want this instead, it's approx what x86 does, modulo not
using cpupool0:

8<----------------------------------------------

>From 7c66c893c5fbf485bc9071b9ef14680475b91670 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Fri, 24 Feb 2012 10:36:42 +0000
Subject: [PATCH] arm: handle dom0_max_vcpus=0 case properly

Also use xzalloc_array.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/domain_build.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ec5a9f3..4efe07d 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -15,13 +15,14 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
 
 struct vcpu *__init alloc_dom0_vcpu0(void)
 {
-    dom0->vcpu = xmalloc_array(struct vcpu *, opt_dom0_max_vcpus);
+    if ( opt_dom0_max_vcpus == 0 )
+        opt_dom0_max_vcpus = num_online_cpus();
+    if ( opt_dom0_max_vcpus > MAX_VIRT_CPUS )
+        opt_dom0_max_vcpus = MAX_VIRT_CPUS;
+
+    dom0->vcpu = xzalloc_array(struct vcpu *, opt_dom0_max_vcpus);
     if ( !dom0->vcpu )
-    {
-            printk("failed to alloc dom0->vccpu\n");
         return NULL;
-    }
-    memset(dom0->vcpu, 0, opt_dom0_max_vcpus * sizeof(*dom0->vcpu));
     dom0->max_vcpus = opt_dom0_max_vcpus;
 
     return alloc_vcpu(dom0, 0, 0);
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 5/5] arm: introduce more hypercalls
  2012-02-23 18:21 ` [PATCH v2 5/5] arm: introduce more hypercalls Stefano Stabellini
@ 2012-02-24 10:41   ` Ian Campbell
  0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 10:41 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Implement xen_version, event_channel_op, memory_op sysctl and physdev_op
> hypercalls.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> ---
>  xen/arch/arm/Makefile           |    1 +
>  xen/arch/arm/physdev.c          |   27 +++++++++++++++++++++++++++
>  xen/arch/arm/traps.c            |    5 +++++
>  xen/include/asm-arm/hypercall.h |    1 +
>  4 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 xen/arch/arm/physdev.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 49b64fe..619430c 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -14,6 +14,7 @@ obj-y += kernel.o
>  obj-y += mm.o
>  obj-y += p2m.o
>  obj-y += guestcopy.o
> +obj-y += physdev.o
>  obj-y += setup.o
>  obj-y += time.o
>  obj-y += smpboot.o
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> new file mode 100644
> index 0000000..bcf4337
> --- /dev/null
> +++ b/xen/arch/arm/physdev.c
> @@ -0,0 +1,27 @@
> +/******************************************************************************
> + * Arch-specific physdev.c
> + *
> + * Copyright (c) 2012, Citrix Systems
> + */
> +
> +#include <xen/config.h>
> +#include <xen/types.h>
> +#include <xen/lib.h>
> +#include <xen/errno.h>
> +#include <asm/hypercall.h>
> +
> +
> +int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> +{
> +    printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
> +    return -ENOSYS;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-set-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 44d19ec..b266c5e 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -376,6 +376,11 @@ static arm_hypercall_t *arm_hypercall_table[] = {
>      HYPERCALL(arch_0),
>      HYPERCALL(sched_op),
>      HYPERCALL(console_io),
> +    HYPERCALL(xen_version),
> +    HYPERCALL(event_channel_op),
> +    HYPERCALL(memory_op),
> +    HYPERCALL(physdev_op),
> +    HYPERCALL(sysctl),
>  };
>  
>  static void do_debug_trap(struct cpu_user_regs *regs, unsigned int code)
> diff --git a/xen/include/asm-arm/hypercall.h b/xen/include/asm-arm/hypercall.h
> index d517542..b91f8b6 100644
> --- a/xen/include/asm-arm/hypercall.h
> +++ b/xen/include/asm-arm/hypercall.h
> @@ -2,6 +2,7 @@
>  #define __ASM_ARM_HYPERCALL_H__
>  
>  #include <public/domctl.h> /* for arch_do_domctl */
> +int do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg);
>  
>  #define XEN_HYPERCALL_TAG   0XEA1
>  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-24 10:33   ` Ian Campbell
@ 2012-02-24 10:53     ` David Vrabel
  2012-02-24 14:15       ` Stefano Stabellini
  0 siblings, 1 reply; 21+ messages in thread
From: David Vrabel @ 2012-02-24 10:53 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel@lists.xensource.com, Tim Deegan (3P),
	Stefano Stabellini

On 24/02/12 10:33, Ian Campbell wrote:
> On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
>> Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
>> we reserve for Xen usage.
> 
> Does this require a trap when the guest acks or EOIs this interrupt?
> What about maintenance interrupts arising from injecting this?
> 
> Might we want to instead inject this interrupt as an SGI, so that it
> appears as a per-VCPU interrupt?

I don't think it's possible to register a SGI handler in Linux
currently.  The mapping of IRQ numbers to GIC interrupts skips over the
SGIs.  This would be easy to fix I think.

David

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 4/5] arm: use r12 to pass the hypercall number
  2012-02-23 19:29   ` Ian Campbell
@ 2012-02-24 12:21     ` Stefano Stabellini
  0 siblings, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-24 12:21 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Tim Deegan (3P), xen-devel@lists.xensource.com, David Vrabel,
	Stefano Stabellini

On Thu, 23 Feb 2012, Ian Campbell wrote:
> On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> > Use r12 to pass the hypercall number and r0-r4 for the hypercall
> > arguments as usual.
> 
> Strictly speaking "as usual" in the ARM calling convention would be args
> in r0-r3 and the fifth (and subsequent) arguments on the stack. However
> we are free to implement our own convention for hypercalls and avoiding
> arguments on the stack is a good idea.

Yes, you are right. I meant "as before" here.


> Please could you add a comment describing the interface. X86 documents
> this in xen/include/public/arch-x86/xen-x86_{32,64}.h so I suppose
> xen/include/public/arch-arm.h is appropriate

Good idea.


> We should define precisely which registers are clobbered by a hypercall.
> X86 clobbers exactly those arguments registers which are used for that
> hypercall but perhaps we can simplify and always clobber r0..r4,r12
> (plus any other caller saved registers in the usual calling convention,
> just for sanity). r0..r3,r12 are already clobbered in the normal calling
> convention so the only difference would be clobbering r4 which is
> normally callee saved (but is also not normally used to pass arguments).
> 
> We should explicitly clobber whatever we say we will too in a debug
> build.

OK

> I've trimmed the quote already so I'll mention it here instead:
> XEN_HYPERCALL_TAG should be defined in the public interface too not in
> the private asm headers.

Sure


> > @@ -409,16 +408,17 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, unsigned long iss)
> >  {
> >      local_irq_enable();
> >  
> > -    regs->r0 = arm_hypercall_table[iss](regs->r0,
> > +    if ( iss != XEN_HYPERCALL_TAG  ) {
> > +        printk("%s %d: received an alien hypercall iss=%lx\n", __func__ ,
> > +                __LINE__ , iss);
> 
> 	regs->r0 = -EINVAL;
> 
> here I think.

yes, we need that


> > +        return;
> > +    }
> > +
> > +    regs->r0 = arm_hypercall_table[regs->r12](regs->r0,
> 
> It's an existing issue but we need to check that
> arm_hypercall_table[regs->r12] is non-NULL here and return -ENOSYS (by
> setting r0) if it is.

Yes, you are right.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0)
  2012-02-24 10:40   ` Ian Campbell
@ 2012-02-24 12:36     ` Stefano Stabellini
  0 siblings, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-24 12:36 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Tim Deegan (3P), xen-devel@lists.xensource.com, David Vrabel,
	Stefano Stabellini

On Fri, 24 Feb 2012, Ian Campbell wrote:
> From 7c66c893c5fbf485bc9071b9ef14680475b91670 Mon Sep 17 00:00:00 2001
> From: Ian Campbell <ian.campbell@citrix.com>
> Date: Fri, 24 Feb 2012 10:36:42 +0000
> Subject: [PATCH] arm: handle dom0_max_vcpus=0 case properly
> 
> Also use xzalloc_array.

ack

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  xen/arch/arm/domain_build.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index ec5a9f3..4efe07d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -15,13 +15,14 @@ integer_param("dom0_max_vcpus", opt_dom0_max_vcpus);
>  
>  struct vcpu *__init alloc_dom0_vcpu0(void)
>  {
> -    dom0->vcpu = xmalloc_array(struct vcpu *, opt_dom0_max_vcpus);
> +    if ( opt_dom0_max_vcpus == 0 )
> +        opt_dom0_max_vcpus = num_online_cpus();
> +    if ( opt_dom0_max_vcpus > MAX_VIRT_CPUS )
> +        opt_dom0_max_vcpus = MAX_VIRT_CPUS;
> +
> +    dom0->vcpu = xzalloc_array(struct vcpu *, opt_dom0_max_vcpus);
>      if ( !dom0->vcpu )
> -    {
> -            printk("failed to alloc dom0->vccpu\n");
>          return NULL;
> -    }
> -    memset(dom0->vcpu, 0, opt_dom0_max_vcpus * sizeof(*dom0->vcpu));
>      dom0->max_vcpus = opt_dom0_max_vcpus;
>  
>      return alloc_vcpu(dom0, 0, 0);
> -- 
> 1.7.2.5
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-24 10:22   ` Ian Campbell
@ 2012-02-24 12:46     ` Ian Campbell
  0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 12:46 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

> > @@ -172,6 +177,14 @@ int map_mmio_regions(struct domain *d,
> >      return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
> >  }
> >  
> > +int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
> > +        unsigned long mfn)

Should these be either paddr_t or xen_pfn_t?

> > +{
> > +    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
> > +                              (gpfn + 1) << PAGE_SHIFT,
> > +                              mfn << PAGE_SHIFT);

As it stands you need to cast to paddr_t first so the shift won't
overflow.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
  2012-02-24 10:22   ` Ian Campbell
@ 2012-02-24 13:04   ` Ian Campbell
  2012-02-24 16:50   ` Ian Campbell
  2 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 13:04 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> Allocate the shared_info page at domain creation.
> 
> Implement arch_memory_op, only for XENMEM_add_to_physmap with space ==
> XENMAPSPACE_shared_info, so that the guest can map the shared_info page.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  xen/arch/arm/domain.c     |    8 ++++
>  xen/arch/arm/mm.c         |   98 +++++++++++++++++++++++++++++++++++++++++++--
>  xen/arch/arm/p2m.c        |   15 ++++++-
>  xen/include/asm-arm/mm.h  |    4 ++
>  xen/include/asm-arm/p2m.h |    2 +
>  5 files changed, 122 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 0b55934..0ee96b9 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -235,6 +235,14 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>      if ( (rc = p2m_init(d)) != 0 )
>          goto fail;
>  
> +    rc = -ENOMEM;
> +    if ( (d->shared_info = alloc_xenheap_pages(0, MEMF_bits(32))) == NULL )
> +        goto fail;

No reason to limit this to 32 bit (I think).

Also you can avoid it in the idle domain case.

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-24 10:53     ` David Vrabel
@ 2012-02-24 14:15       ` Stefano Stabellini
  2012-02-24 17:03         ` Ian Campbell
  0 siblings, 1 reply; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-24 14:15 UTC (permalink / raw)
  To: David Vrabel
  Cc: Tim Deegan (3P), xen-devel@lists.xensource.com, Ian Campbell,
	Stefano Stabellini

On Fri, 24 Feb 2012, David Vrabel wrote:
> On 24/02/12 10:33, Ian Campbell wrote:
> > On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> >> Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
> >> we reserve for Xen usage.
> > 
> > Does this require a trap when the guest acks or EOIs this interrupt?
> > What about maintenance interrupts arising from injecting this?

Yes, we would get a maintenance interrupt in the hypervisor every time
the guest EOI's it.


> > Might we want to instead inject this interrupt as an SGI, so that it
> > appears as a per-VCPU interrupt?
> 
> I don't think it's possible to register a SGI handler in Linux
> currently.  The mapping of IRQ numbers to GIC interrupts skips over the
> SGIs.  This would be easy to fix I think.

A per-vcpu interrupt that doesn't require an EOI would be ideal, however
like David wrote, it is not possible to register an SGI handler in Linux
at the moment. I'll give it a look though.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
  2012-02-24 10:22   ` Ian Campbell
  2012-02-24 13:04   ` Ian Campbell
@ 2012-02-24 16:50   ` Ian Campbell
  2012-02-27 14:19     ` Stefano Stabellini
  2 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 16:50 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> @@ -172,6 +177,14 @@ int map_mmio_regions(struct domain *d,
>      return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
>  }
>  
> +int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
> +        unsigned long mfn)
> +{
> +    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
> +                              (gpfn + 1) << PAGE_SHIFT,
> +                              mfn << PAGE_SHIFT);
> +} 

This doesn't actually end up getting called from the common code bits
which should call it. Think we need this on top.

You can fold into your patch if you prefer.

8<--------------------------------------------

>From 7d26c508e3ef52dcddf562ba506b384a77423919 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Fri, 24 Feb 2012 16:43:11 +0000
Subject: [PATCH] arm: define CONFIG_PAGING_ASSISTANCE

This is needed for common code (e.g. XENMEM_populate_physmap) to actually call
guest_phys_map. Adjust the API to match what the common code expects.
Implement a dummy guest_physmap_remove_page.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 xen/arch/arm/mm.c            |    2 +-
 xen/arch/arm/p2m.c           |   15 ++++++++++++---
 xen/include/asm-arm/config.h |    2 ++
 xen/include/asm-arm/p2m.h    |   11 +++++++++--
 xen/include/asm-arm/paging.h |    3 +++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 0fe0398..cf3de8a 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -380,7 +380,7 @@ static int xenmem_add_to_physmap_once(
     domain_lock(d);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, xatp->gpfn, mfn);
+    rc = guest_physmap_add_page(d, xatp->gpfn, mfn, 0);
 
     domain_unlock(d);
 
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 1a00a50..4c94ef0 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -177,14 +177,23 @@ int map_mmio_regions(struct domain *d,
     return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
 }
 
-int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
-        unsigned long mfn)
+int guest_physmap_add_page(struct domain *d,
+                           unsigned long gpfn,
+                           unsigned long mfn,
+                           unsigned int page_order)
 {
     return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
-                              (gpfn + 1) << PAGE_SHIFT,
+                              (gpfn + (1<<page_order)) << PAGE_SHIFT,
                               mfn << PAGE_SHIFT);
 }
 
+void guest_physmap_remove_page(struct domain *d,
+                               unsigned long gpfn,
+                               unsigned long mfn, unsigned int page_order)
+{
+    ASSERT(0);
+}
+
 int p2m_alloc_table(struct domain *d)
 {
     struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index c2ab0a2..63fb48e 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -7,6 +7,8 @@
 #ifndef __ARM_CONFIG_H__
 #define __ARM_CONFIG_H__
 
+#define CONFIG_PAGING_ASSISTANCE 1
+
 #define CONFIG_PAGING_LEVELS 3
 
 #define CONFIG_ARM 1
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c0f2995..d8e8dc8 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -39,8 +39,15 @@ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
  * address maddr. */
 int map_mmio_regions(struct domain *d, paddr_t start_gaddr,
                      paddr_t end_gaddr, paddr_t maddr);
-int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
-        unsigned long mfn);
+
+/* Untyped version for RAM only, for compatibility */
+int guest_physmap_add_page(struct domain *d,
+                           unsigned long gfn,
+                           unsigned long mfn,
+                           unsigned int page_order);
+void guest_physmap_remove_page(struct domain *d,
+                               unsigned long gpfn,
+                               unsigned long mfn, unsigned int page_order);
 
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
diff --git a/xen/include/asm-arm/paging.h b/xen/include/asm-arm/paging.h
index 4dc340f..3d7dd95 100644
--- a/xen/include/asm-arm/paging.h
+++ b/xen/include/asm-arm/paging.h
@@ -1,6 +1,9 @@
 #ifndef _XEN_PAGING_H
 #define _XEN_PAGING_H
 
+#define paging_mode_translate(d)              (0)
+#define paging_mode_external(d)               (0)
+
 #endif /* XEN_PAGING_H */
 
 /*
-- 
1.7.2.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-24 14:15       ` Stefano Stabellini
@ 2012-02-24 17:03         ` Ian Campbell
  2012-02-29 17:14           ` Stefano Stabellini
  0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2012-02-24 17:03 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel@lists.xensource.com, David Vrabel, Tim Deegan (3P)

On Fri, 2012-02-24 at 14:15 +0000, Stefano Stabellini wrote:
> On Fri, 24 Feb 2012, David Vrabel wrote:
> > On 24/02/12 10:33, Ian Campbell wrote:
> > > On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> > >> Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
> > >> we reserve for Xen usage.
> > > 
> > > Does this require a trap when the guest acks or EOIs this interrupt?
> > > What about maintenance interrupts arising from injecting this?
> 
> Yes, we would get a maintenance interrupt in the hypervisor every time
> the guest EOI's it.

We really want to avoid those.

> > > Might we want to instead inject this interrupt as an SGI, so that it
> > > appears as a per-VCPU interrupt?
> > 
> > I don't think it's possible to register a SGI handler in Linux
> > currently.  The mapping of IRQ numbers to GIC interrupts skips over the
> > SGIs.  This would be easy to fix I think.
> 
> A per-vcpu interrupt that doesn't require an EOI would be ideal, however
> like David wrote, it is not possible to register an SGI handler in Linux
> at the moment. I'll give it a look though.

Do SGIs avoid the EOI and maintenance interrupt issues though?

Ian.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 1/5] arm: shared_info page allocation and mapping
  2012-02-24 16:50   ` Ian Campbell
@ 2012-02-27 14:19     ` Stefano Stabellini
  0 siblings, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-27 14:19 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Tim Deegan (3P), xen-devel@lists.xensource.com, David Vrabel,
	Stefano Stabellini

On Fri, 24 Feb 2012, Ian Campbell wrote:
> On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> > @@ -172,6 +177,14 @@ int map_mmio_regions(struct domain *d,
> >      return create_p2m_entries(d, 0, start_gaddr, end_gaddr, maddr);
> >  }
> >  
> > +int guest_physmap_add_page(struct domain *d, unsigned long gpfn,
> > +        unsigned long mfn)
> > +{
> > +    return create_p2m_entries(d, 0, gpfn << PAGE_SHIFT,
> > +                              (gpfn + 1) << PAGE_SHIFT,
> > +                              mfn << PAGE_SHIFT);
> > +} 
> 
> This doesn't actually end up getting called from the common code bits
> which should call it. Think we need this on top.
> 
> You can fold into your patch if you prefer.

I'll fold it into my patch.
Thanks!

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v2 2/5] arm: implement vcpu_mark_events_pending
  2012-02-24 17:03         ` Ian Campbell
@ 2012-02-29 17:14           ` Stefano Stabellini
  0 siblings, 0 replies; 21+ messages in thread
From: Stefano Stabellini @ 2012-02-29 17:14 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Tim Deegan (3P), xen-devel@lists.xensource.com, David Vrabel,
	Stefano Stabellini

On Fri, 24 Feb 2012, Ian Campbell wrote:
> On Fri, 2012-02-24 at 14:15 +0000, Stefano Stabellini wrote:
> > On Fri, 24 Feb 2012, David Vrabel wrote:
> > > On 24/02/12 10:33, Ian Campbell wrote:
> > > > On Thu, 2012-02-23 at 18:21 +0000, Stefano Stabellini wrote:
> > > >> Implement vcpu_mark_events_pending using the vgic to inject INT 63, that
> > > >> we reserve for Xen usage.
> > > > 
> > > > Does this require a trap when the guest acks or EOIs this interrupt?
> > > > What about maintenance interrupts arising from injecting this?
> > 
> > Yes, we would get a maintenance interrupt in the hypervisor every time
> > the guest EOI's it.
> 
> We really want to avoid those.

yep

> > > > Might we want to instead inject this interrupt as an SGI, so that it
> > > > appears as a per-VCPU interrupt?
> > > 
> > > I don't think it's possible to register a SGI handler in Linux
> > > currently.  The mapping of IRQ numbers to GIC interrupts skips over the
> > > SGIs.  This would be easy to fix I think.
> > 
> > A per-vcpu interrupt that doesn't require an EOI would be ideal, however
> > like David wrote, it is not possible to register an SGI handler in Linux
> > at the moment. I'll give it a look though.
> 
> Do SGIs avoid the EOI and maintenance interrupt issues though?
 
SGI (or PPI) would avoid maintenance interrupt issues if we don't ask
for any maintenance interrupts in the LR.

It is easy to determine if an event notification interrupt has already
been EOI'd by the guest just looking at the evtchn_upcall_pending bit in
the shared_info page.
Also we can safely assume that there is at most one event notification
interrupt pending at any time in any set of LR registers because we
never inject more than a single event notification interrupt in one pcpu
(see vcpu_mark_events_pending).
Therefore checking for event notification interrupts that need to be
cleaned whenever we receive a maintenance interrupt and whenever we want
to inject a new interrupt should be enough.

More details and actual code in the next patch series.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2012-02-29 17:14 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-23 18:20 [PATCH v2 0/5] xen/arm: event channels and shared_info page Stefano Stabellini
2012-02-23 18:21 ` [PATCH v2 1/5] arm: shared_info page allocation and mapping Stefano Stabellini
2012-02-24 10:22   ` Ian Campbell
2012-02-24 12:46     ` Ian Campbell
2012-02-24 13:04   ` Ian Campbell
2012-02-24 16:50   ` Ian Campbell
2012-02-27 14:19     ` Stefano Stabellini
2012-02-23 18:21 ` [PATCH v2 2/5] arm: implement vcpu_mark_events_pending Stefano Stabellini
2012-02-24 10:33   ` Ian Campbell
2012-02-24 10:53     ` David Vrabel
2012-02-24 14:15       ` Stefano Stabellini
2012-02-24 17:03         ` Ian Campbell
2012-02-29 17:14           ` Stefano Stabellini
2012-02-23 18:21 ` [PATCH v2 3/5] arm: set the default dom0 max_vcpus value to 1 (currently is 0) Stefano Stabellini
2012-02-24 10:40   ` Ian Campbell
2012-02-24 12:36     ` Stefano Stabellini
2012-02-23 18:21 ` [PATCH v2 4/5] arm: use r12 to pass the hypercall number Stefano Stabellini
2012-02-23 19:29   ` Ian Campbell
2012-02-24 12:21     ` Stefano Stabellini
2012-02-23 18:21 ` [PATCH v2 5/5] arm: introduce more hypercalls Stefano Stabellini
2012-02-24 10:41   ` Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).