mini-os: initial ARM support

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/gnttab.c     |  2 +-
 extras/mini-os/hypervisor.c |  2 +-
 extras/mini-os/sched.c      | 26 +++++++++++++-------------
 3 files changed, 15 insertions(+), 15 deletions(-)

extras/mini-os/gnttab.c +1 -1

index 2f1b3d7..6f25006 100644
@@ -164,7 +164,7 @@ gnttabop_error(int16_t status)
 {
     status = -status;
     if (status < 0 || status >= ARRAY_SIZE(gnttabop_error_msgs))
-	return "bad status";
+        return "bad status";
     else
         return gnttabop_error_msgs[status];
 }

extras/mini-os/hypervisor.c +1 -1

index b4688a0..c5de872 100644
@@ -64,7 +64,7 @@ void do_hypervisor_callback(struct pt_regs *regs)
             l2 &= ~(1UL << l2i);
 
             port = (l1i * (sizeof(unsigned long) * 8)) + l2i;
-			do_event(port, regs);
+            do_event(port, regs);
         }
     }
 

extras/mini-os/sched.c +13 -13

index 174945e..99d87b6 100644
@@ -165,28 +165,28 @@ struct _reent *__getreent(void)
     struct _reent *_reent;
 
     if (!threads_started)
-	_reent = _impure_ptr;
+        _reent = _impure_ptr;
     else if (in_callback)
-	_reent = &callback_reent;
+        _reent = &callback_reent;
     else
-	_reent = &get_current()->reent;
+        _reent = &get_current()->reent;
 
 #ifndef NDEBUG
 #if defined(__x86_64__) || defined(__x86__)
     {
 #ifdef __x86_64__
-	register unsigned long sp asm ("rsp");
+        register unsigned long sp asm ("rsp");
 #else
-	register unsigned long sp asm ("esp");
+        register unsigned long sp asm ("esp");
 #endif
-	if ((sp & (STACK_SIZE-1)) < STACK_SIZE / 16) {
-	    static int overflowing;
-	    if (!overflowing) {
-		overflowing = 1;
-		printk("stack overflow\n");
-		BUG();
-	    }
-	}
+        if ((sp & (STACK_SIZE-1)) < STACK_SIZE / 16) {
+            static int overflowing;
+            if (!overflowing) {
+                overflowing = 1;
+                printk("stack overflow\n");
+                BUG();
+            }
+        }
     }
 #endif
 #else

Signature · 1 line

2.0.0

From: Karim Raslan <[email protected]>

Moves some x86-specific code into arch/x86.

Signed-off-by Karim Allah Ahmed <[email protected]>

[[email protected]: split into multiple patches]

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/arch/x86/mm.c    | 13 +++++++++++++
 extras/mini-os/gnttab.c         |  9 +--------
 extras/mini-os/include/gnttab.h |  1 +
 3 files changed, 15 insertions(+), 8 deletions(-)

extras/mini-os/arch/x86/mm.c +13 -0

index 35df15b..9c6d1b8 100644
@@ -942,3 +942,16 @@ void arch_init_mm(unsigned long* start_pfn_p, unsigned long* max_pfn_p)
     *start_pfn_p = start_pfn;
     *max_pfn_p = max_pfn;
 }
+
+grant_entry_t *arch_init_gnttab(int nr_grant_frames)
+{
+    struct gnttab_setup_table setup;
+    unsigned long frames[nr_grant_frames];
+
+    setup.dom = DOMID_SELF;
+    setup.nr_frames = nr_grant_frames;
+    set_xen_guest_handle(setup.frame_list, frames);
+
+    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
+    return map_frames(frames, nr_grant_frames);
+}

extras/mini-os/gnttab.c +1 -8

index 6f25006..f395d12 100644
@@ -172,8 +172,6 @@ gnttabop_error(int16_t status)
 void
 init_gnttab(void)
 {
-    struct gnttab_setup_table setup;
-    unsigned long frames[NR_GRANT_FRAMES];
     int i;
 
 #ifdef GNT_DEBUG

@@ -182,12 +180,7 @@ init_gnttab(void)
     for (i = NR_RESERVED_ENTRIES; i < NR_GRANT_ENTRIES; i++)
         put_free_entry(i);
 
-    setup.dom = DOMID_SELF;
-    setup.nr_frames = NR_GRANT_FRAMES;
-    set_xen_guest_handle(setup.frame_list, frames);
-
-    HYPERVISOR_grant_table_op(GNTTABOP_setup_table, &setup, 1);
-    gnttab_table = map_frames(frames, NR_GRANT_FRAMES);
+    gnttab_table = arch_init_gnttab(NR_GRANT_FRAMES);
     printk("gnttab_table mapped at %p.\n", gnttab_table);
 }
 

extras/mini-os/include/gnttab.h +1 -0

index acd6c39..c43ad42 100644
@@ -12,5 +12,6 @@ unsigned long gnttab_end_transfer(grant_ref_t gref);
 int gnttab_end_access(grant_ref_t ref);
 const char *gnttabop_error(int16_t status);
 void fini_gnttab(void);
+grant_entry_t *arch_init_gnttab(int nr_grant_frames);
 
 #endif /* !__GNTTAB_H__ */

Signature · 1 line

2.0.0

This isn't available on ARM.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/hypervisor.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

extras/mini-os/hypervisor.c +10 -0

index c5de872..1b61d9b 100644
@@ -73,18 +73,26 @@ void do_hypervisor_callback(struct pt_regs *regs)
 
 void force_evtchn_callback(void)
 {
+#ifdef XEN_HAVE_PV_UPCALL_MASK
     int save;
+#endif
     vcpu_info_t *vcpu;
     vcpu = &HYPERVISOR_shared_info->vcpu_info[smp_processor_id()];
+#ifdef XEN_HAVE_PV_UPCALL_MASK
     save = vcpu->evtchn_upcall_mask;
+#endif
 
     while (vcpu->evtchn_upcall_pending) {
+#ifdef XEN_HAVE_PV_UPCALL_MASK
         vcpu->evtchn_upcall_mask = 1;
+#endif
         barrier();
         do_hypervisor_callback(NULL);
         barrier();
+#ifdef XEN_HAVE_PV_UPCALL_MASK
         vcpu->evtchn_upcall_mask = save;
         barrier();
+#endif
     };
 }
 

@@ -110,7 +118,9 @@ inline void unmask_evtchn(uint32_t port)
               &vcpu_info->evtchn_pending_sel) )
     {
         vcpu_info->evtchn_upcall_pending = 1;
+#ifdef XEN_HAVE_PV_UPCALL_MASK
         if ( !vcpu_info->evtchn_upcall_mask )
+#endif
             force_evtchn_callback();
     }
 }

Signature · 1 line

2.0.0

The code previously assumed that a long was 64-bits.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/mm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

extras/mini-os/mm.c +2 -2

index d2d5264..d31ef97 100644
@@ -409,8 +409,8 @@ void init_mm(void)
      * now we can initialise the page allocator
      */
     printk("MM: Initialise page allocator for %lx(%lx)-%lx(%lx)\n",
-           (u_long)to_virt(PFN_PHYS(start_pfn)), PFN_PHYS(start_pfn), 
-           (u_long)to_virt(PFN_PHYS(max_pfn)), PFN_PHYS(max_pfn));
+           (u_long)to_virt(PFN_PHYS(start_pfn)), (u_long)PFN_PHYS(start_pfn), 
+           (u_long)to_virt(PFN_PHYS(max_pfn)), (u_long)PFN_PHYS(max_pfn));
     init_page_allocator(PFN_PHYS(start_pfn), PFN_PHYS(max_pfn));
     printk("MM: done\n");
 

Signature · 1 line

2.0.0

Avoids using _hypercall1 in events.c.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/events.c         | 4 ++--
 extras/mini-os/include/x86/os.h | 7 +++++++
 2 files changed, 9 insertions(+), 2 deletions(-)

extras/mini-os/events.c +2 -2

index 3c92d82..d07dbfb 100644
@@ -244,7 +244,7 @@ int evtchn_get_peercontext(evtchn_port_t local_port, char *ctx, int size)
     op.cmd = FLASK_GET_PEER_SID;
     op.interface_version = XEN_FLASK_INTERFACE_VERSION;
     op.u.peersid.evtchn = local_port;
-    rc = _hypercall1(int, xsm_op, &op);
+    rc = HYPERVISOR_xsm_op(&op);
     if (rc)
         return rc;
     sid = op.u.peersid.sid;

@@ -252,7 +252,7 @@ int evtchn_get_peercontext(evtchn_port_t local_port, char *ctx, int size)
     op.u.sid_context.sid = sid;
     op.u.sid_context.size = size;
     set_xen_guest_handle(op.u.sid_context.context, ctx);
-    rc = _hypercall1(int, xsm_op, &op);
+    rc = HYPERVISOR_xsm_op(&op);
     return rc;
 }
 

extras/mini-os/include/x86/os.h +7 -0

index 73b8297..9e3f9bf 100644
@@ -20,6 +20,7 @@
 #include <mini-os/types.h>
 #include <mini-os/hypervisor.h>
 #include <mini-os/kernel.h>
+#include <xen/xsm/flask_op.h>
 
 #define USED    __attribute__ ((used))
 

@@ -563,6 +564,12 @@ static __inline__ int synch_var_test_bit(int nr, volatile void * addr)
  synch_const_test_bit((nr),(addr)) : \
  synch_var_test_bit((nr),(addr)))
 
+static inline int
+HYPERVISOR_xsm_op(
+	struct xen_flask_op *op)
+{
+	return _hypercall1(int, xsm_op, op);
+}
 
 #undef ADDR
 

Signature · 1 line

2.0.0

Quoted 3 lines · Thomas Leonard, le Thu 26 Jun 2014 12:28:28 +0100, a écrit :

Avoids using _hypercall1 in events.c.

Signed-off-by Thomas Leonard <[email protected]>

Acked-by Samuel Thibault <[email protected]>

Quoted 6 lines · Samuel Thibault

Quoted 3 lines · Thomas Leonard, le Thu 26 Jun 2014 12:28:28 +0100, a écrit :

Avoids using _hypercall1 in events.c.

Signed-off-by Thomas Leonard <[email protected]>

Acked-by Samuel Thibault <[email protected]>

On x86 this results in

In file included from stubdom/../extras/mini-os/include/mini-os/gntmap.h:4:0,

                 from stubdom/../extras/mini-os/include/mini-os/lib.h:55,
                 from stubdom/../extras/mini-os/include/mini-os/mm.h:36,
                 from stubdom/../extras/mini-os/include/x86/x86_64/hypercall-x86_64.h:39,
                 from stubdom/../extras/mini-os/include/hypervisor.h:20,
                 from mini-os.c:13:

stubdom/../extras/mini-os/include/x86/os.h: In function ‘HYPERVISOR_xsm_op’:
stubdom/../extras/mini-os/include/x86/os.h:566:21: error: expected expression before ‘int’

I'm not sure why.

Quoted 19 lines · Ian Campbell

Quoted 6 lines · Samuel Thibault

Quoted 3 lines · Thomas Leonard, le Thu 26 Jun 2014 12:28:28 +0100, a écrit :

Avoids using _hypercall1 in events.c.

Signed-off-by Thomas Leonard <[email protected]>

Acked-by Samuel Thibault <[email protected]>

On x86 this results in

In file included from stubdom/../extras/mini-os/include/mini-os/gntmap.h:4:0,

                 from stubdom/../extras/mini-os/include/mini-os/lib.h:55,
                 from stubdom/../extras/mini-os/include/mini-os/mm.h:36,
                 from stubdom/../extras/mini-os/include/x86/x86_64/hypercall-x86_64.h:39,
                 from stubdom/../extras/mini-os/include/hypervisor.h:20,
                 from mini-os.c:13:

stubdom/../extras/mini-os/include/x86/os.h: In function ‘HYPERVISOR_xsm_op’:
stubdom/../extras/mini-os/include/x86/os.h:566:21: error: expected expression before ‘int’

Quoted 1 line

I'm not sure why.

That's strange. It compiles for me on x86_64 (running "make" in the mini-os directory and also running "make" in "stubdom").

If you tell me the compiler/OS/arch you're on I can try to reproduce it.

I could break it at the mm.h -> lib.h link. I can't see why the low-level MM code needs libc,

and it seems to compile without it (adding some lib.h includes in various C files instead). Any objections?

Quoted 19 lines · Ian Campbell

Quoted 6 lines · Samuel Thibault

Quoted 3 lines · Thomas Leonard, le Thu 26 Jun 2014 12:28:28 +0100, a écrit :

Avoids using _hypercall1 in events.c.

Signed-off-by Thomas Leonard <[email protected]>

Acked-by Samuel Thibault <[email protected]>

On x86 this results in

In file included from stubdom/../extras/mini-os/include/mini-os/gntmap.h:4:0,

                 from stubdom/../extras/mini-os/include/mini-os/lib.h:55,
                 from stubdom/../extras/mini-os/include/mini-os/mm.h:36,
                 from stubdom/../extras/mini-os/include/x86/x86_64/hypercall-x86_64.h:39,
                 from stubdom/../extras/mini-os/include/hypervisor.h:20,
                 from mini-os.c:13:

stubdom/../extras/mini-os/include/x86/os.h: In function ‘HYPERVISOR_xsm_op’:
stubdom/../extras/mini-os/include/x86/os.h:566:21: error: expected expression before ‘int’

Quoted 1 line

I'm not sure why.

That's strange. It compiles for me on x86_64 (running "make" in the mini-os directory and also running "make" in "stubdom").

If you tell me the compiler/OS/arch you're on I can try to reproduce it.

This allows closing any ports opened by the arch-specific code. ARM will use it for the debug port.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/arch/x86/events.c | 4 ++++
 extras/mini-os/events.c          | 1 +
 extras/mini-os/include/events.h  | 4 ++++
 3 files changed, 9 insertions(+)

extras/mini-os/arch/x86/events.c +4 -0

index e420a98..5198cf3 100644
@@ -23,6 +23,10 @@ void arch_init_events(void)
 #endif
 }
 
+void arch_unbind_ports(void)
+{
+}
+
 void arch_fini_events(void)
 {
 #if defined(__x86_64__)

extras/mini-os/events.c +1 -0

index d07dbfb..2a23042 100644
@@ -179,6 +179,7 @@ void init_events(void)
 void fini_events(void)
 {
     /* Dealloc all events */
+    arch_unbind_ports();
     unbind_all_ports();
     arch_fini_events();
 }

extras/mini-os/include/events.h +4 -0

index 0452d21..89b5997 100644
@@ -26,6 +26,10 @@ typedef void (*evtchn_handler_t)(evtchn_port_t, struct pt_regs *, void *);
 
 /* prototypes */
 void arch_init_events(void);
+
+/* Called by fini_events to close any ports opened by arch-specific code. */
+void arch_unbind_ports(void);
+
 void arch_fini_events(void);
 
 int do_event(evtchn_port_t port, struct pt_regs *regs);

Signature · 1 line

2.0.0

Quelqu'un aurait-il une solution pour réinitialiser un MBR

We don't need to define this on ARM. Suggested by Julien Grall.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/include/types.h                       | 12 ------------
 extras/mini-os/include/x86/x86_32/hypercall-x86_32.h |  5 +++++
 extras/mini-os/include/x86/x86_64/hypercall-x86_64.h |  4 ++++
 3 files changed, 9 insertions(+), 12 deletions(-)

extras/mini-os/include/types.h +0 -12

index de356e8..93356fe 100644
@@ -30,23 +30,11 @@ typedef unsigned long       u_long;
 #ifdef __i386__
 typedef long long           quad_t;
 typedef unsigned long long  u_quad_t;
-
-typedef struct { unsigned long pte_low, pte_high; } pte_t;
-
 #elif defined(__x86_64__)
 typedef long                quad_t;
 typedef unsigned long       u_quad_t;
-
-typedef struct { unsigned long pte; } pte_t;
 #endif /* __i386__ || __x86_64__ */
 
-#ifdef __x86_64__
-#define __pte(x) ((pte_t) { (x) } )
-#else
-#define __pte(x) ({ unsigned long long _x = (x);        \
-    ((pte_t) {(unsigned long)(_x), (unsigned long)(_x>>32)}); })
-#endif
-
 #ifdef HAVE_LIBC
 #include <limits.h>
 #include <stdint.h>

extras/mini-os/include/x86/x86_32/hypercall-x86_32.h +5 -0

index dcfbe41..99a4ee3 100644
@@ -35,6 +35,11 @@
 #include <xen/nmi.h>
 #include <mini-os/mm.h>
 
+typedef struct { unsigned long pte_low, pte_high; } pte_t;
+
+#define __pte(x) ({ unsigned long long _x = (x);        \
+    ((pte_t) {(unsigned long)(_x), (unsigned long)(_x>>32)}); })
+
 #define __STR(x) #x
 #define STR(x) __STR(x)
 

extras/mini-os/include/x86/x86_64/hypercall-x86_64.h +4 -0

index 7083763..e00b3bd 100644
@@ -38,6 +38,10 @@
 #include <xen/sched.h>
 #include <mini-os/mm.h>
 
+typedef struct { unsigned long pte; } pte_t;
+
+#define __pte(x) ((pte_t) { (x) } )
+
 #define __STR(x) #x
 #define STR(x) __STR(x)
 

Signature · 1 line

2.0.0

Requested by Julien Grall.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/include/compiler.h | 5 +++++
 extras/mini-os/include/x86/os.h   | 7 +------
 2 files changed, 6 insertions(+), 6 deletions(-)
 create mode 100644 extras/mini-os/include/compiler.h

extras/mini-os/include/compiler.h new file +5 -0

new file mode 100644
index 0000000..e35c9d5
@@ -0,0 +1,5 @@
+#if __GNUC__ == 2 && __GNUC_MINOR__ < 96
+#define __builtin_expect(x, expected_value) (x)
+#endif
+#define unlikely(x)  __builtin_expect((x),0)
+#define likely(x)  __builtin_expect((x),1)

extras/mini-os/include/x86/os.h +1 -6

index 9e3f9bf..7a1bdc5 100644
@@ -7,16 +7,11 @@
 #ifndef _OS_H_
 #define _OS_H_
 
-#if __GNUC__ == 2 && __GNUC_MINOR__ < 96
-#define __builtin_expect(x, expected_value) (x)
-#endif
-#define unlikely(x)  __builtin_expect((x),0)
-#define likely(x)  __builtin_expect((x),1)
-
 #define smp_processor_id() 0
 
 
 #ifndef __ASSEMBLY__
+#include <mini-os/compiler.h>
 #include <mini-os/types.h>
 #include <mini-os/hypervisor.h>
 #include <mini-os/kernel.h>

Signature · 1 line

2.0.0

The old code said "Xenbus tests disabled, because of a Xend bug". Since xend doesn't exist any longer, we can enable these again.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

extras/mini-os/test.c +1 -2

index bbfc613..20d372b 100644
@@ -56,8 +56,7 @@ void test_xenbus(void);
 
 static void xenbus_tester(void *p)
 {
-    printk("Xenbus tests disabled, because of a Xend bug.\n");
-    /* test_xenbus(); */
+    test_xenbus();
 }
 #endif
 

Signature · 1 line

2.0.0

The old code said "Xenbus tests disabled, because of a Xend bug". Since xend doesn't exist any longer, we can enable these again.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

extras/mini-os/test.c +1 -2

index bbfc613..20d372b 100644
@@ -56,8 +56,7 @@ void test_xenbus(void);
 
 static void xenbus_tester(void *p)
 {
-    printk("Xenbus tests disabled, because of a Xend bug.\n");
-    /* test_xenbus(); */
+    test_xenbus();
 }
 #endif
 

Quoted 2 lines · Thomas Leonard

The old code said "Xenbus tests disabled, because of a Xend bug". Since xend doesn't exist any longer, we can enable these again.

Out of curiosity, do you know if they work now? (I don't think the answer should block this patch either way).

Quoted 19 lines

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

extras/mini-os/test.c +1 -2

index bbfc613..20d372b 100644
@@ -56,8 +56,7 @@ void test_xenbus(void);
 
 static void xenbus_tester(void *p)
 {
-    printk("Xenbus tests disabled, because of a Xend bug.\n");
-    /* test_xenbus(); */
+    test_xenbus();
 }
 #endif
 

Quoted 6 lines · Ian Campbell

Quoted 2 lines · Thomas Leonard

The old code said "Xenbus tests disabled, because of a Xend bug". Since xend doesn't exist any longer, we can enable these again.

Out of curiosity, do you know if they work now? (I don't think the answer should block this patch either way).

On ARM, with all the patches I sent applied, the output is:

[...]
Doing xenbus test.
Periodic thread started.
T(s=0 us=1253)
Got a reply, type 16, id 1, len 7.
Doing ls test.
ls device...
ls device[0] -> suspend
ls device[1] -> vbd
ls device[2] -> vif
ls device/vif...
ls device/vif[0] -> 0
ls device/vif/0...
ls device/vif/0[0] -> backend
ls device/vif/0[1] -> backend-id
ls device/vif/0[2] -> state
ls device/vif/0[3] -> handle
ls device/vif/0[4] -> mac
Doing read test.
Read device/vif/0/mac...
Read device/vif/0/mac -> 00:16:3e:7d:69:0d.
Read device/vif/0/backend...
Read device/vif/0/backend -> /local/domain/0/backend/vif/1/0. Doing write test.
Write flobble to device/vif/0/flibble...
Success.
Read device/vif/0/flibble...
Read device/vif/0/flibble -> flobble.
Write widget to device/vif/0/flibble...
Success.
Read device/vif/0/flibble...
Read device/vif/0/flibble -> widget.
Doing rm test.
rm device/vif/0/flibble...
Success.
Read device/vif/0/flibble...
Error in xenbus read: ENOENT
(Should have said ENOENT)
Thread "xenbus_tester" exited.
T(s=1 us=1543)
[...]

Quoted 21 lines

Quoted 19 lines

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/test.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

extras/mini-os/test.c +1 -2

index bbfc613..20d372b 100644
@@ -56,8 +56,7 @@ void test_xenbus(void);
 
 static void xenbus_tester(void *p)
 {
-    printk("Xenbus tests disabled, because of a Xend bug.\n");
-    /* test_xenbus(); */
+    test_xenbus();
 }
 #endif
 

Testing whether the result of local_irq_save is zero doesn't work on ARM.

Suggested by Julien Grall.

Signed-off-by Thomas Leonard <[email protected]>

---
 extras/mini-os/sched.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

extras/mini-os/sched.c +5 -5

index 99d87b6..d0c607e 100644
@@ -78,6 +78,11 @@ void schedule(void)
     struct thread *prev, *next, *thread, *tmp;
     unsigned long flags;
 
+    if (irqs_disabled()) {
+        printk("Must not call schedule() with IRQs disabled\n");
+        BUG();
+    }
+
     prev = current;
     local_irq_save(flags); 
 

@@ -86,11 +91,6 @@ void schedule(void)
         BUG();
     }
 
-    if (flags) {
-        printk("Must not call schedule() with IRQs disabled\n");
-        BUG();
-    }
-
     do {
         /* Examine all threads.
            Find a runnable thread, but also wake up expired ones and find the

Signature · 1 line

2.0.0

extras/mini-os/include/arm/os.h new file +0 -0

new file mode 100644
index 0000000..bde8796

+#define BUG() while(1){}

Hi Thomas,

extras/mini-os/include/arm/hypercall-arm.h line 40

Quoted 12 lines · Thomas Leonard

extras/mini-os/include/arm/hypercall-arm.h new file +7 -0

new file mode 100644
index 0000000..6d862ff
+#ifndef __HYPERCALL_ARM_H__
+#define __HYPERCALL_ARM_H__
+
+#include <xen/xen.h>
+#include <xen/sched.h>
+#include <xen/xsm/flask_op.h>
+#include <mini-os/mm.h>

Does all those includes relevant for hypercall-arm?

Quoted 5 lines

extras/mini-os/include/arm/os.h new file +0 -0

new file mode 100644
index 0000000..bde8796

[..]

Quoted 1 line

+#define BUG() while(1){}

It looks odd to use onely while (1). Shouldn't you call do_exit as x86 does?

Quoted 3 lines

Quoted 1 line

+#define BUG() while(1){}

It looks odd to use onely while (1). Shouldn't you call do_exit as x86 does?

Note that patch #20 changes this to dump the registers and stack.

[ also, exiting on BUG is annoying because you may lose the console output if the VM dies before xl attaches to it. ]

From: Karim Raslan <[email protected]>

Looks like this is revision v1.3.0-47-gbe60268 from
http://git.jdl.com/gitweb/?p=dtc.git

The memmove implementation is from FreeBSD's
contrib/ldns/compat/memmove.c

Hi Thomas,

Quoted 1 line · Thomas Leonard

Quoted 4 lines

From: Karim Raslan <[email protected]>

Looks like this is revision v1.3.0-47-gbe60268 from
http://git.jdl.com/gitweb/?p=dtc.git

Unfortunately I can find this commit in the git tree. Do you have the full sha1?

AFAIU, you are using the version 1.3.0, right? If so, why didn't you get the latest version (i.e 1.4.0)?

Quoted 2 lines

The memmove implementation is from FreeBSD's
contrib/ldns/compat/memmove.c

Can you add a commit number just for tracking purpose?

Quoted 7 lines · Julien Grall

Hi Thomas,

Quoted 3 lines · Thomas Leonard

From: Karim Raslan <[email protected]>

Quoted 6 lines

Quoted 2 lines

Looks like this is revision v1.3.0-47-gbe60268 from
http://git.jdl.com/gitweb/?p=dtc.git

Unfortunately I can find this commit in the git tree. Do you have the full sha1?

It works for me:

$ git clone git://git.jdl.com/software/dtc.git
$ cd dtc
$ git show be60268
commit be6026838e45b67800ac803f4ad8cca3cde57d6d
Author: David Gibson <[email protected]>
Date:   Fri Jun 1 14:12:38 2012 +1000

libfdt: Add helper function to create a trivial, empty tree

Quoted 2 lines

AFAIU, you are using the version 1.3.0, right? If so, why didn't you get the latest version (i.e 1.4.0)?

I don't know why this particular version was chosen. I could upgrade it if you want. I don't think there are any functional changes (uint32_t has been replaced by fdt32_t all over the place, and there are a couple of extra functions, which we don't need).

Quoted 12 lines · Thomas Leonard

Quoted 8 lines · Julien Grall

Hi Thomas,

Quoted 4 lines · Thomas Leonard

From: Karim Raslan <[email protected]>

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Looks like this is revision v1.3.0-47-gbe60268 from
http://git.jdl.com/gitweb/?p=dtc.git

Unfortunately I can find this commit in the git tree. Do you have the full sha1?

It works for me:

Hmmm right, sorry for the noise. Even though, you shell command below show a tag but a commit ;).

Quoted 12 lines

$ git clone git://git.jdl.com/software/dtc.git
$ cd dtc
$ git show be60268
commit be6026838e45b67800ac803f4ad8cca3cde57d6d
Author: David Gibson <[email protected]>
Date:   Fri Jun 1 14:12:38 2012 +1000

libfdt: Add helper function to create a trivial, empty tree

Quoted 2 lines

AFAIU, you are using the version 1.3.0, right? If so, why didn't you get the latest version (i.e 1.4.0)?

Quoted 4 lines

I don't know why this particular version was chosen. I could upgrade it if you want. I don't think there are any functional changes (uint32_t has been replaced by fdt32_t all over the place, and there are a couple of extra functions, which we don't need).

Just looking to the libfdt already embedded in xen (xen/common/libfdt) and it looks like we are using the same version.

I'm wondering if you could share the same code and avoid to have the libfdt imported twice in Xen. Any thoughts?

Quoted 8 lines · Julien Grall

Quoted 6 lines · Thomas Leonard

Quoted 1 line · Julien Grall

I don't know why this particular version was chosen. I could upgrade it if you want. I don't think there are any functional changes (uint32_t has been replaced by fdt32_t all over the place, and there are a couple of extra functions, which we don't need).

Quoted 3 lines

Just looking to the libfdt already embedded in xen (xen/common/libfdt) and it looks like we are using the same version.

I'm wondering if you could share the same code and avoid to have the libfdt imported twice in Xen. Any thoughts?

MiniOS really needs to be able to compile as a standalone repository and not depend on Xen's source code if at all possible. This also came up at the recent hackathon in the context of splitting it out of the main repository in the future.

On ARM, Mini-OS will boot and display some output on the console. Tested with:

make XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- \
	CONFIG_TEST=y CONFIG_START_NETWORK=n CONFIG_BLKFRONT=n \
	CONFIG_NETFRONT=n CONFIG_FBFRONT=n CONFIG_KBDFRONT=n \
	CONFIG_CONSFRONT=n CONFIG_XC=n -j4

Signed-off-by Karim Allah Ahmed <[email protected]>

[[email protected]: made x86_64 support work again]

[[email protected]: split into multiple patches]

[[email protected]: re-enabled force_evtchn_callback]

[[email protected]: enable regular console]

[[email protected]: fixed initialisation code:
- Configure write-back caching in page table. This is needed for reliable hypercalls to Xen (thanks to Julien Grall).
- Use "client mode" for access control (domains are deprecated, according to ARM Cortex-A Series Programmer’s Guide version 4.0, section 9.6.4).
- Enable more SCTLR features (icache, branch prediction)] [[email protected]: use Virtual Count register for monotonic time] [[email protected]: fixed HYPERVISOR_shutdown] [[email protected]: get xenstore details from hypervisor] [[email protected]: use GCC implementation of division] [[email protected]: cleaned up interrupt handlers and threading] [[email protected]: call exit_thread when a thread returns] [[email protected]: implemented block_domain for ARM] [[email protected]: fixed hang when enabling interrupts] [[email protected]: added -march=armv7-a to flags] [[email protected]: CLREX after handling IRQs] [[email protected]: unbind debug port at shutdown] [[email protected]: allow unaligned accesses] [[email protected]: fix zImage header for XSA-95] [[email protected]: get RAM base and size from the FDT] [[email protected]: get GIC addresses from FDT] [[email protected]: added ARM grant table initialisation] [[email protected]: added missing copyright header to hypercalls32.S] [[email protected]: moved GIC driver to arm directory] [[email protected]: fixes suggested by Julien Grall:

- Removed unnecessary isb.
- Renamed GICD_PRIORITY to GICD_IPRIORITYR.
- Change IRQ number type from unsigned char to int.
- Added volatile to {set,clear}_bit_non_atomic.
- Fixed some comments.
- Check compatible properties in DTB.] [[email protected]: made image relocatable] [[email protected]: added mfn_to_pfn and pfn_to_mfn]

Signed-off-by Thomas Leonard <[email protected]>

---
  extras/mini-os/ARM-TODO.txt              |   6 +
  extras/mini-os/Config.mk                 |   2 +
  extras/mini-os/Makefile                  |   9 ++
  extras/mini-os/arch/arm/Makefile         |  32 ++++
  extras/mini-os/arch/arm/arch.mk          |   7 +
  extras/mini-os/arch/arm/arm32.S          | 266 +++++++++++++++++++++++++++++++
  extras/mini-os/arch/arm/events.c         |  30 ++++
  extras/mini-os/arch/arm/gic.c            | 222 ++++++++++++++++++++++++++
  extras/mini-os/arch/arm/hypercalls32.S   |  75 +++++++++
  extras/mini-os/arch/arm/minios-arm32.lds |  75 +++++++++
  extras/mini-os/arch/arm/mm.c             | 134 ++++++++++++++++
  extras/mini-os/arch/arm/sched.c          |  37 +++++
  extras/mini-os/arch/arm/setup.c          | 116 ++++++++++++++
  extras/mini-os/arch/arm/time.c           | 202 +++++++++++++++++++++++
  extras/mini-os/kernel.c                  |   2 +-
  15 files changed, 1214 insertions(+), 1 deletion(-)

create mode 100644 extras/mini-os/ARM-TODO.txt create mode 100755 extras/mini-os/arch/arm/Makefile create mode 100644 extras/mini-os/arch/arm/arch.mk create mode 100644 extras/mini-os/arch/arm/arm32.S create mode 100644 extras/mini-os/arch/arm/events.c create mode 100644 extras/mini-os/arch/arm/gic.c create mode 100644 extras/mini-os/arch/arm/hypercalls32.S create mode 100755 extras/mini-os/arch/arm/minios-arm32.lds create mode 100644 extras/mini-os/arch/arm/mm.c create mode 100644 extras/mini-os/arch/arm/sched.c create mode 100644 extras/mini-os/arch/arm/setup.c create mode 100644 extras/mini-os/arch/arm/time.c

extras/mini-os/Config.mk +2 -0

index d61877b..4ecde54 100644
@@ -12,6 +12,8 @@ export XEN_INTERFACE_VERSION
  # If not x86 then use $(XEN_TARGET_ARCH)
  ifeq ($(findstring x86_,$(XEN_TARGET_ARCH)),x86_)
  TARGET_ARCH_FAM = x86
+else ifeq ($(findstring arm,$(XEN_TARGET_ARCH)),arm)
+TARGET_ARCH_FAM = arm
  else
  TARGET_ARCH_FAM = $(XEN_TARGET_ARCH)
  endif

extras/mini-os/Makefile +0 -0

index 931cd05..01d8af0 100644

extras/mini-os/arch/arm/Makefile new file +32 -0

new file mode 100755
index 0000000..8b78651
@@ -0,0 +1,32 @@
+#
+# ARM architecture specific makefiles.
+#
+
+XEN_ROOT = $(CURDIR)/../../../..
+include $(XEN_ROOT)/Config.mk
+include ../../Config.mk
+
+# include arch.mk has to be before minios.mk!
+
+include arch.mk
+include ../../minios.mk
+
+# Sources here are all *.c (without $(XEN_TARGET_ARCH).S)
+# This is handled in $(HEAD_ARCH_OBJ)
+ARCH_SRCS := $(wildcard *.c)
+
+# The objects built from the sources.
+ARCH_OBJS := $(patsubst %.c,$(OBJ_DIR)/%.o,$(ARCH_SRCS))
+
+ARCH_OBJS += hypercalls32.o
+
+all: $(OBJ_DIR)/$(ARCH_LIB)
+
+# $(HEAD_ARCH_OBJ) is only built here, needed on linking
+# in ../../Makefile.
+$(OBJ_DIR)/$(ARCH_LIB): $(ARCH_OBJS) $(OBJ_DIR)/$(HEAD_ARCH_OBJ)
+	$(AR) rv $(OBJ_DIR)/$(ARCH_LIB) $(ARCH_OBJS)
+
+clean:
+	rm -f $(OBJ_DIR)/$(ARCH_LIB) $(ARCH_OBJS) $(OBJ_DIR)/$(HEAD_ARCH_OBJ)
+

extras/mini-os/arch/arm/arch.mk new file +7 -0

new file mode 100644
index 0000000..ab20d99
@@ -0,0 +1,7 @@
+ifeq ($(XEN_TARGET_ARCH),arm32)
+DEF_ASFLAGS += -march=armv7-a
+ARCH_CFLAGS  := -march=armv7-a -marm -fms-extensions -D__arm__ -DXEN_HAVE_PV_GUEST_ENTRY #-DCPU_EXCLUSIVE_LDST
+EXTRA_INC += $(TARGET_ARCH_FAM)/$(XEN_TARGET_ARCH)
+EXTRA_SRC += arch/$(EXTRA_INC)
+endif
+

extras/mini-os/arch/arm/arm32.S new file +0 -0

new file mode 100644
index 0000000..de74ed9

extras/mini-os/arch/arm/arm32.S new file +19 -0

+.pushsection .data
+_data:
+.align	14
+.globl page_dir
+@ Each word maps one 1MB virtual section to a physical section
+page_dir:
+	.fill (4*1024), 4, 0x0
+
+.align 12
+.globl shared_info_page
+shared_info_page:
+	.fill (1024), 4, 0x0
+
+.align 3
+.globl stack
+stack:
+	.fill (4*1024), 4, 0x0
+stack_end:
+

extras/mini-os/arch/arm/gic.c new file +0 -0

new file mode 100644
index 0000000..5641eb0

extras/mini-os/arch/arm/gic.c new file +5 -0

+void gic_init(void) {
+    gic.gicd_base = NULL;
+    int node = 0;
+    int depth = 0;
+    for (;;)

extras/mini-os/arch/arm/hypercalls32.S new file +0 -0

new file mode 100644
index 0000000..0d7662d

extras/mini-os/arch/arm/mm.c +0 -0

+void arch_init_mm(unsigned long *start_pfn_p, unsigned long *max_pfn_p)

extras/mini-os/arch/arm/mm.c +4 -0

+/* Get Xen's suggested physical page assignments for the grant table. */
+static unsigned long get_gnttab_base(void)
+{
+    int hypervisor;

Quoted 40 lines · Thomas Leonard

On ARM, Mini-OS will boot and display some output on the console. Tested with:

make XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- \
        CONFIG_TEST=y CONFIG_START_NETWORK=n CONFIG_BLKFRONT=n \
        CONFIG_NETFRONT=n CONFIG_FBFRONT=n CONFIG_KBDFRONT=n \
        CONFIG_CONSFRONT=n CONFIG_XC=n -j4

Signed-off-by Karim Allah Ahmed <[email protected]>

[[email protected]: made x86_64 support work again]

[[email protected]: split into multiple patches]

[[email protected]: re-enabled force_evtchn_callback]

[[email protected]: enable regular console]

[[email protected]: fixed initialisation code:
- Configure write-back caching in page table. This is needed for reliable hypercalls to Xen (thanks to Julien Grall).
- Use "client mode" for access control (domains are deprecated, according to ARM Cortex-A Series Programmer’s Guide version 4.0, section 9.6.4).
- Enable more SCTLR features (icache, branch prediction)] [[email protected]: use Virtual Count register for monotonic time] [[email protected]: fixed HYPERVISOR_shutdown] [[email protected]: get xenstore details from hypervisor] [[email protected]: use GCC implementation of division] [[email protected]: cleaned up interrupt handlers and threading] [[email protected]: call exit_thread when a thread returns] [[email protected]: implemented block_domain for ARM] [[email protected]: fixed hang when enabling interrupts] [[email protected]: added -march=armv7-a to flags] [[email protected]: CLREX after handling IRQs] [[email protected]: unbind debug port at shutdown] [[email protected]: allow unaligned accesses] [[email protected]: fix zImage header for XSA-95] [[email protected]: get RAM base and size from the FDT] [[email protected]: get GIC addresses from FDT] [[email protected]: added ARM grant table initialisation] [[email protected]: added missing copyright header to hypercalls32.S] [[email protected]: moved GIC driver to arm directory] [[email protected]: fixes suggested by Julien Grall:

Quoted 8 lines

- Removed unnecessary isb.
- Renamed GICD_PRIORITY to GICD_IPRIORITYR.
- Change IRQ number type from unsigned char to int.
- Added volatile to {set,clear}_bit_non_atomic.
- Fixed some comments.
- Check compatible properties in DTB.] [[email protected]: made image relocatable] [[email protected]: added mfn_to_pfn and pfn_to_mfn]

IHMO all these changes doesn't belong to the commit message, i.e should not appear in the commit message when Ian will apply your commit message.

I would move them in a changelog. You can separate the commit message and the changelog by --- and a newline. Everything after the --- will be removed by git am.

Quoted 53 lines

Signed-off-by Thomas Leonard <[email protected]>

---
  extras/mini-os/ARM-TODO.txt              |   6 +
  extras/mini-os/Config.mk                 |   2 +
  extras/mini-os/Makefile                  |   9 ++
  extras/mini-os/arch/arm/Makefile         |  32 ++++
  extras/mini-os/arch/arm/arch.mk          |   7 +
  extras/mini-os/arch/arm/arm32.S          | 266
+++++++++++++++++++++++++++++++
  extras/mini-os/arch/arm/events.c         |  30 ++++
  extras/mini-os/arch/arm/gic.c            | 222
++++++++++++++++++++++++++
  extras/mini-os/arch/arm/hypercalls32.S   |  75 +++++++++
  extras/mini-os/arch/arm/minios-arm32.lds |  75 +++++++++
  extras/mini-os/arch/arm/mm.c             | 134 ++++++++++++++++
  extras/mini-os/arch/arm/sched.c          |  37 +++++
  extras/mini-os/arch/arm/setup.c          | 116 ++++++++++++++
  extras/mini-os/arch/arm/time.c           | 202 +++++++++++++++++++++++
  extras/mini-os/kernel.c                  |   2 +-
  15 files changed, 1214 insertions(+), 1 deletion(-)

create mode 100644 extras/mini-os/ARM-TODO.txt create mode 100755 extras/mini-os/arch/arm/Makefile create mode 100644 extras/mini-os/arch/arm/arch.mk create mode 100644 extras/mini-os/arch/arm/arm32.S create mode 100644 extras/mini-os/arch/arm/events.c create mode 100644 extras/mini-os/arch/arm/gic.c create mode 100644 extras/mini-os/arch/arm/hypercalls32.S create mode 100755 extras/mini-os/arch/arm/minios-arm32.lds create mode 100644 extras/mini-os/arch/arm/mm.c create mode 100644 extras/mini-os/arch/arm/sched.c create mode 100644 extras/mini-os/arch/arm/setup.c create mode 100644 extras/mini-os/arch/arm/time.c

extras/mini-os/Config.mk +2 -0

index d61877b..4ecde54 100644
@@ -12,6 +12,8 @@ export XEN_INTERFACE_VERSION
  # If not x86 then use $(XEN_TARGET_ARCH)
  ifeq ($(findstring x86_,$(XEN_TARGET_ARCH)),x86_)
  TARGET_ARCH_FAM = x86
+else ifeq ($(findstring arm,$(XEN_TARGET_ARCH)),arm)
+TARGET_ARCH_FAM = arm
  else
  TARGET_ARCH_FAM = $(XEN_TARGET_ARCH)
  endif

extras/mini-os/Makefile +0 -0

index 931cd05..01d8af0 100644
@@ -78,6 +78,9 @@ TARGET := mini-os
  SUBDIRS := lib xenbus console
 

extras/mini-os/Makefile TARGET := mini-os · line 82

Quoted 3 lines

extras/mini-os/Makefile +2 -0

  ifeq ($(XEN_TARGET_ARCH),arm32)
+# Need libgcc.a for division helpers
+LDLIBS += `$(CC) -print-libgcc-file-name`

OOI, how much code does add libgcc for the division helpers?

Quoted 66 lines

extras/mini-os/arch/arm/Makefile new file +31 -0

new file mode 100755
index 0000000..8b78651
@@ -0,0 +1,32 @@
+#
+# ARM architecture specific makefiles.
+#
+
+XEN_ROOT = $(CURDIR)/../../../..
+include $(XEN_ROOT)/Config.mk
+include ../../Config.mk
+
+# include arch.mk has to be before minios.mk!
+
+include arch.mk
+include ../../minios.mk
+
+# Sources here are all *.c (without $(XEN_TARGET_ARCH).S)
+# This is handled in $(HEAD_ARCH_OBJ)
+ARCH_SRCS := $(wildcard *.c)
+
+# The objects built from the sources.
+ARCH_OBJS := $(patsubst %.c,$(OBJ_DIR)/%.o,$(ARCH_SRCS))
+
+ARCH_OBJS += hypercalls32.o
+
+all: $(OBJ_DIR)/$(ARCH_LIB)
+
+# $(HEAD_ARCH_OBJ) is only built here, needed on linking
+# in ../../Makefile.
+$(OBJ_DIR)/$(ARCH_LIB): $(ARCH_OBJS) $(OBJ_DIR)/$(HEAD_ARCH_OBJ)
+       $(AR) rv $(OBJ_DIR)/$(ARCH_LIB) $(ARCH_OBJS)
+
+clean:
+       rm -f $(OBJ_DIR)/$(ARCH_LIB) $(ARCH_OBJS)

$(OBJ_DIR)/$(HEAD_ARCH_OBJ)

extras/mini-os/arch/arm/Makefile new file +1 -0

+

extras/mini-os/arch/arm/arch.mk new file +7 -1

new file mode 100644
index 0000000..ab20d99
@@ -0,0 +1,7 @@
+ifeq ($(XEN_TARGET_ARCH),arm32)
+DEF_ASFLAGS += -march=armv7-a
+ARCH_CFLAGS  := -march=armv7-a -marm -fms-extensions -D__arm__
-DXEN_HAVE_PV_GUEST_ENTRY #-DCPU_EXCLUSIVE_LDST
+EXTRA_INC += $(TARGET_ARCH_FAM)/$(XEN_TARGET_ARCH)
+EXTRA_SRC += arch/$(EXTRA_INC)
+endif
+

extras/mini-os/arch/arm/arm32.S new file +4 -0

new file mode 100644
index 0000000..de74ed9
@@ -0,0 +1,266 @@
+@ Virtual address of the start of RAM (any value will do, but it must be
+@ section-aligned). Update the lds script if changed.
+#define VIRT_BASE 0x400000
+

Quoted 4 lines

extras/mini-os/arch/arm/arm32.S new file +1 -0

+@ Offset of the kernel within the RAM. This is a zImage convention which

we
+@ rely on.
+#define ZIMAGE_KERNEL_OFFSET 0x8000

Hmmm... this is not a zImage convention... IIRC Linux is using this offset to have enough space to create startup page table during boot.

The Linux zImage is able to relocate itself in the memory to respect this convention. But the zImage itself can be loaded anywhere in the memory.

By looking to your code below your are relying that the kernel will be loaded at 0xXXXX8000 which is incorrect. This offset is odd and make other kernel (such as FreeBSD) requiring the same trick which is not part of the Linux boot protocol.

I plan to send a patch to require the start address to be 2MB aligned, so kernels will be able to use 2MB (for LPAE) and 1MB section for there early page table.

If the kernel wants another alignment, then you will have to relocate yourself. Even though in your case, you don't need this odd 0xXXXX8000.

[..]

extras/mini-os/arch/arm/arm32.S line 145

Quoted 13 lines

extras/mini-os/arch/arm/arm32.S new file +5 -0

+       @ Fill in the whole top-level translation table (at page_dir).
+       @ Populate the whole pagedir with 1MB section descriptors.
+
+       mov     r1, r7                  @ r1 -> first section entry
+       add     r3, r1, #4*4*1024       @ limit (4 GB address space, 4

byte entries)
+ orr r0, r8, r9 @ r0 = entry mapping section zero
to start of physical RAM
+1:

extras/mini-os/arch/arm/arm32.S new file +4 -0

+       str     r0, [r1],#4             @ write the section entry
+       add     r0, r0, #1 << 20        @ next physical page (wraps)
+       cmp     r1, r3
+       bne     1b

I would document a bit more this part. It took me a bit of time to understand that you mapping the whole address space in an odd manner.

While it's fine for a first version of Mini-OS support for ARM. It think at long term you want to map only necessary bank rank. It will be easier to catch programming error and avoid to trap in the hypervisor because the physical address doesn't exist.

Futhermore, the data abort sent by the hypervisor is odd (debug smth).

Quoted 24 lines

+.pushsection .data

extras/mini-os/arch/arm/arm32.S new file +23 -0

+_data:
+.align 14
+.globl page_dir
+@ Each word maps one 1MB virtual section to a physical section
+page_dir:
+       .fill (4*1024), 4, 0x0
+
+.align 12
+.globl shared_info_page
+shared_info_page:
+       .fill (1024), 4, 0x0
+
+.align 3
+.globl stack
+stack:
+       .fill (4*1024), 4, 0x0
+stack_end:
+
+.align 3
+irqstack:
+       .fill (1024), 4, 0x0
+irqstack_end:
+

extras/mini-os/arch/arm/arm32.S line 202

Quoted 5 lines

+.globl physical_address_offset

extras/mini-os/arch/arm/arm32.S new file +4 -0

+physical_address_offset:
+       .long   0
+
+.popsection

Any reason to define theses variables in ASM rather than C?

Quoted 11 lines

extras/mini-os/arch/arm/gic.c new file +5 -0

new file mode 100644
index 0000000..5641eb0
@@ -0,0 +1,222 @@
+// ARM GIC implementation
+
+#include <mini-os/os.h>
+#include <mini-os/hypervisor.h>
+#include <libfdt.h>

extras/mini-os/arch/arm/gic.c line 10

Quoted 5 lines

extras/mini-os/arch/arm/gic.c new file +5 -0

+
+//#define VGIC_DEBUG
+#ifdef VGIC_DEBUG
+#define DEBUG(_f, _a...) \
+    DEBUG("MINI_OS(file=vgic.c, line=%d) " _f , __LINE__, ## _a)

Did you intend to use printk here?

[..]

Quoted 14 lines

extras/mini-os/arch/arm/gic.c new file +11 -0

+void gic_init(void) {
+    gic.gicd_base = NULL;
+    int node = 0;
+    int depth = 0;
+    for (;;)
+    {
+        node = fdt_next_node(device_tree, node, &depth);
+        if (node <= 0 || depth < 0)
+            break;
+
+        if (fdt_getprop(device_tree, node, "interrupt-controller", NULL))

{

extras/mini-os/arch/arm/gic.c new file +2 -0

+            int len = 0;
+

Quoted 4 lines

+ if (fdt_node_check_compatible(device_tree, node,
"arm,cortex-a15-gic") &&
+ fdt_node_check_compatible(device_tree, node,
"arm,cortex-a9-gic") &&

I don't think this compatible is necessary. Cortex A9 doesn't support virtualisation.

Quoted 10 lines

+ fdt_node_check_compatible(device_tree, node,
"arm,cortex-a7-gic")) {
+ printk("Skipping incompatible interrupt-controller
node\n");

extras/mini-os/arch/arm/gic.c new file +4 -0

+                continue;
+            }
+
+            const uint64_t *reg = fdt_getprop(device_tree, node, "reg",

&len);
+ if (reg == NULL || len != 32) {

As asked on the previous version, if you plan to assume specific range size for the time-being, please explain the 32.

extras/mini-os/arch/arm/gic.c line 200

Quoted 6 lines

extras/mini-os/arch/arm/gic.c new file +6 -0

+                /* TODO: support other formats */
+                printk("Bad 'reg' property: %p %d\n", reg, len);
+                continue;
+            }
+            gic.gicd_base = to_virt((long) fdt64_to_cpu(reg[0]));
+            gic.gicc_base = to_virt((long) fdt64_to_cpu(reg[2]));

AFAIU, your are mapping the GIC region with normal attribute. With this attribute, the processor may reorder the write in the device memory, preload memory, caching.... Which is completely wrong in this case. I'm sa bit surprised that it works correctly. You have to map thoses regions as device memory.

[..]

Quoted 6 lines

extras/mini-os/arch/arm/hypercalls32.S new file +0 -0

new file mode 100644
index 0000000..0d7662d

[..]

extras/mini-os/arch/arm/hypercalls32.S line 44

Quoted 12 lines

extras/mini-os/arch/arm/hypercalls32.S new file +12 -0

+#define __HYPERVISOR_memory_op            12
+#define __HYPERVISOR_xen_version          17
+#define __HYPERVISOR_console_io           18
+#define __HYPERVISOR_grant_table_op       20
+#define __HYPERVISOR_vcpu_op              24
+#define __HYPERVISOR_xsm_op               27
+#define __HYPERVISOR_sched_op             29
+#define __HYPERVISOR_event_channel_op     32
+#define __HYPERVISOR_physdev_op           33
+#define __HYPERVISOR_hvm_op               34
+#define __HYPERVISOR_sysctl               35
+#define __HYPERVISOR_domctl               36

Hmmm... why do you hardcode those numbers here? Can't you include the correct header?

[..]

extras/mini-os/arch/arm/minios-arm32.lds line 5

Quoted 12 lines

extras/mini-os/arch/arm/minios-arm32.lds new file +5 -0

new file mode 100755
index 0000000..b18ca55
@@ -0,0 +1,75 @@
+OUTPUT_ARCH(arm)
+ENTRY(_start)
+SECTIONS
+{
+  . = 0x408000;

I would use the preprocessor to avoid hardcoding address here. It will help later if someone wants to change the virtual address of the kernel.

[..]

Quoted 1 line

extras/mini-os/arch/arm/mm.c +0 -0

[..]

Quoted 1 line

+void arch_init_mm(unsigned long *start_pfn_p, unsigned long *max_pfn_p)

[..]

extras/mini-os/arch/arm/mm.c line 39

Quoted 5 lines

extras/mini-os/arch/arm/mm.c +5 -0

+
+    /* Xen will always provide us at least one bank of memory.
+     * Mini-OS will use the first bank for the time-being. */
+    regs = fdt_getprop(device_tree, memory, "reg", &prop_len);
+    if (regs == NULL || prop_len != 16) {

Same remark as in gic_init. Please explain where does come from the 16.

[..]

extras/mini-os/arch/arm/mm.c line 47

Quoted 8 lines

extras/mini-os/arch/arm/mm.c +8 -0

+        /* TODO: support other formats */
+        printk("Bad 'reg' property: %p %d\n", regs, prop_len);
+        BUG();
+    }
+
+    unsigned int end = (unsigned int) &_end;
+    unsigned int mem_base = fdt64_to_cpu(regs[0]);
+    unsigned int mem_size = fdt64_to_cpu(regs[1]);

In general if you have to cast/create a variable to contain a physical address. Please make create a type paddr_t and use it.

When someone will implement arm64 support it will have less trouble because we cast correctly cast the value.

IIRC, there is few other place where it's the same problem.

Quoted 10 lines

extras/mini-os/arch/arm/mm.c +9 -0

+/* Get Xen's suggested physical page assignments for the grant table. */
+static unsigned long get_gnttab_base(void)
+{
+    int hypervisor;
+    int len = 0;
+    const uint64_t *regs;
+    unsigned int gnttab_base;
+
+    hypervisor = fdt_node_offset_by_compatible(device_tree, -1,

"xen,xen");

extras/mini-os/arch/arm/mm.c line 90

Quoted 5 lines

extras/mini-os/arch/arm/mm.c +5 -0

+    BUG_ON(hypervisor < 0);
+
+    regs = fdt_getprop(device_tree, hypervisor, "reg", &len);
+    if (regs == NULL || len != 16) {
+        /* TODO: support other formats */

Please explain why the 16.

extras/mini-os/arch/arm/mm.c line 95

Quoted 5 lines

extras/mini-os/arch/arm/mm.c +5 -0

+        printk("Bad 'reg' property: %p %d\n", regs, len);
+        BUG();
+    }
+
+    gnttab_base = fdt64_to_cpu(regs[0]);

This look odd... you are using unsigned int for the variable but the function is return unsigned long.