Marked problematic code in ARM subtree.
authorDavid Cock <david.cock@inf.ethz.ch>
Wed, 8 Jul 2015 11:51:09 +0000 (13:51 +0200)
committerDavid Cock <david.cock@inf.ethz.ch>
Wed, 8 Jul 2015 11:51:09 +0000 (13:51 +0200)
Signed-off-by: David Cock <david.cock@inf.ethz.ch>

kernel/arch/arm/exec.c
kernel/arch/arm/exn.c
kernel/arch/arm/phys_mmap.c
kernel/arch/arm/syscall.c
kernel/arch/armv8/exceptions.S
kernel/arch/armv8/misc.c

index 13a4950..91a2758 100644 (file)
 #include <exec.h>
 #include <exceptions.h>
 #include <misc.h>
+/* XXX - not AArch64-compatible. */
 #include <cp15.h>   // for invalidating tlb and cache
 
 static arch_registers_state_t upcall_state;
 
 extern uint32_t ctr;
 static inline __attribute__((noreturn))
+/* XXX - not 64-bit clean, not AArch64-compatible. */
 void do_resume(uint32_t *regs)
 {
     STATIC_ASSERT(CPSR_REG ==  0, "");
@@ -108,6 +110,7 @@ execute(lvaddr_t entry)
     arch_registers_state_t *state = &upcall_state;
     assert(0 != disp_arm->got_base);
 
+    /* XXX - not AArch64-compatible. */
     state->named.r10 = disp_arm->got_base;
 
     struct dispatcher_shared_generic *disp_gen
@@ -142,6 +145,7 @@ void __attribute__ ((noreturn)) resume(arch_registers_state_t *state)
     do_resume(state->regs);
 }
 
+/* XXX - not AArch64-compatible. */
 void wait_for_interrupt(void)
 {
     // REVIEW: Timer interrupt could be masked here.
index 802bc6e..9f42554 100644 (file)
@@ -11,6 +11,7 @@
 #include <dispatch.h>
 #include <arm.h>
 #include <arm_hal.h>
+/* XXX - not AArch64-compatible. */
 #include <cp15.h>
 #include <exceptions.h>
 #include <exec.h>
@@ -124,6 +125,7 @@ void handle_user_undef(lvaddr_t fault_address,
     resume(&resume_area);
 }
 
+/* XXX - not 64-bit clean, not AArch64-compatible. */
 static int32_t bkpt_decode(lvaddr_t fault_address)
 {
     int32_t bkpt_id = -1;
@@ -139,6 +141,7 @@ static int32_t bkpt_decode(lvaddr_t fault_address)
     return bkpt_id;
 }
 
+/* XXX - not 64-bit clean, not AArch64-compatible. */
 void fatal_kernel_fault(uint32_t evector, lvaddr_t address, arch_registers_state_t* save_area
     )
 {
@@ -245,6 +248,7 @@ void fatal_kernel_fault(uint32_t evector, lvaddr_t address, arch_registers_state
 void handle_irq(arch_registers_state_t* save_area, uintptr_t fault_pc)
 {
     uint32_t irq = 0;
+/* XXX - not revision-independent. */
 #if defined(__ARM_ARCH_7A__)
     irq = gic_get_active_irq();
 #else
@@ -252,6 +256,7 @@ void handle_irq(arch_registers_state_t* save_area, uintptr_t fault_pc)
     irq = pic_get_active_irq();
 #endif
 
+/* XXX - not 64-bit clean */
     debug(SUBSYS_DISPATCH, "IRQ %"PRIu32" while %s\n", irq,
           dcb_current ? (dcb_current->disabled ? "disabled": "enabled") : "in kernel");
 
@@ -283,6 +288,7 @@ void handle_irq(arch_registers_state_t* save_area, uintptr_t fault_pc)
     // we just acknowledge it here
     else if(irq == 1)
     {
+/* XXX - not revision-independent. */
 #if defined(__ARM_ARCH_7A__)
        gic_ack_irq(irq);
 #else
@@ -292,6 +298,7 @@ void handle_irq(arch_registers_state_t* save_area, uintptr_t fault_pc)
        dispatch(schedule());
     }
     else {
+/* XXX - not revision-independent. */
 #if defined(__ARM_ARCH_7A__)
         gic_ack_irq(irq);
         send_user_interrupt(irq);
index 938717a..ab19a44 100644 (file)
@@ -2,6 +2,8 @@
  * \file
  * \brief Rudimentary physical memory map implementation.
  */
+/* XXX - why is this rudimentary? */
+/* XXX - why is this in ARM-specific code? */
 
 /*
  * Copyright (c) 2008, 2010, ETH Zurich.
index aed0f3a..ebbc0b2 100644 (file)
@@ -39,6 +39,7 @@ func( \
     assert(n == argc); \
     struct registers_arm_syscall_args* sa = &context->syscall_args
 
+/* XXX - not revision-independent. */
 #ifdef __ARCH_ARM_5__
 #define NYI(str) printf("armv5: %s\n", str)
 #elif __ARCH_ARM_7M__
@@ -105,6 +106,7 @@ handle_dispatcher_perfmon(
     int argc
     )
 {
+    /* XXX - implement this? */
     return SYSRET(SYS_ERR_PERFMON_NOT_AVAILABLE);
 }
 
@@ -622,6 +624,7 @@ monitor_create_cap(
 
     struct registers_arm_syscall_args* sa = &context->syscall_args;
 
+    /* XXX - not 64-bit clean */
     //printf("%d: %"PRIu32", %"PRIu32", %"PRIu32", %"PRIu32", %"PRIu32", %"PRIu32"\n",
     //        argc, sa->arg0, sa->arg1, sa->arg2, sa->arg3, sa->arg4, sa->arg5);
 
@@ -660,6 +663,7 @@ monitor_spawn_core(
     arch_registers_state_t* context,
     int argc)
 {
+    /* XXX - Why is this commented out? */
        //assert(3 == argc);
 
        struct registers_arm_syscall_args* sa = &context->syscall_args;
@@ -688,6 +692,7 @@ monitor_identify_cap(
 
 INVOCATION_HANDLER(monitor_identify_domains_cap)
 {
+    /* XXX - why is this not used consistently? */
     INVOCATION_PRELUDE(7);
     errval_t err;
 
@@ -712,6 +717,8 @@ static struct sysret handle_irq_table_set( struct capability* to,
         int argc
         )
 {
+/* XXX - not revision-independent. */
+/* XXX - this seems like a pretty serious limitation. */
 #if defined(__ARM_ARCH_7M__) || defined(__ARM_ARCH_5__)
     NYI("can not handle userspace IRQs yet");
     return SYSRET(SYS_ERR_IRQ_INVALID);
@@ -728,6 +735,8 @@ static struct sysret handle_irq_table_delete( struct capability* to,
         int argc
         )
 {
+/* XXX - not revision-independent. */
+/* XXX - this seems like a pretty serious limitation. */
 #if defined(__ARM_ARCH_7M__) || defined(__ARM_ARCH_5__)
     NYI("can not handle userspace IRQs yet");
     return SYSRET(SYS_ERR_IRQ_INVALID);
@@ -847,6 +856,7 @@ static invocation_t invocations[ObjType_Num][CAP_MAX_CMD] = {
         [KernelCmd_Revoke_mark_relations] = monitor_handle_revoke_mark_rels,
         [KernelCmd_Revoke_mark_target] = monitor_handle_revoke_mark_tgt,
         [KernelCmd_Set_cap_owner]     = monitor_set_cap_owner,
+        /* XXX - why is this commented out? */
         //[KernelCmd_Setup_trace]       = handle_trace_setup,
         [KernelCmd_Spawn_core]        = monitor_spawn_core,
         [KernelCmd_Unlock_cap]        = monitor_unlock_cap,
@@ -864,6 +874,7 @@ handle_invoke(arch_registers_state_t *context, int argc)
 {
     struct registers_arm_syscall_args* sa = &context->syscall_args;
 
+    /* XXX - can we generate them from the same source? */
     //
     // Must match lib/barrelfish/include/arch/arm/arch/invocations.h
     //
@@ -891,6 +902,7 @@ handle_invoke(arch_registers_state_t *context, int argc)
             assert(listener != NULL);
 
             if (listener->disp) {
+                /* XXX - not 64-bit clean */
                 uint8_t length_words = (sa->arg0 >> 28) & 0xff;
                 uint8_t send_bits = (sa->arg0 >> 8) & 0xff;
                 capaddr_t send_cptr = sa->arg2;
@@ -1015,6 +1027,7 @@ static struct sysret handle_debug_syscall(int msg)
             retval.value = tsc_get_hz();
             break;
 
+        /* XXX - not revision-independent. */
         #if defined(__pandaboard__)
         case DEBUG_HARDWARE_GLOBAL_TIMER_LOW:
             retval.value = gt_read_low();
@@ -1032,11 +1045,13 @@ static struct sysret handle_debug_syscall(int msg)
     return retval;
 }
 
+/* XXX - function documentation is inconsistent. */
 /**
  * System call dispatch routine.
  *
  * @return struct sysret for all calls except yield / invoke.
  */
+/* XXX - why is this commented out? */
 //__attribute__((noreturn))
 void sys_syscall(arch_registers_state_t* context)
 {
@@ -1078,6 +1093,7 @@ void sys_syscall(arch_registers_state_t* context)
             }
             break;
             
+/* XXX - not revision-independent. */
 #ifdef  __ARM_ARCH_7M__
     //help the dispatcher resume a context that can not be restored whithout a mode change
         case SYSCALL_RESUME_CONTEXT:
@@ -1093,6 +1109,7 @@ void sys_syscall(arch_registers_state_t* context)
     }
 
     if (r.error) {
+        /* XXX - not 64-bit clean, not AArch64-compatible. */
         debug(SUBSYS_SYSCALL, "syscall failed %08"PRIx32" => %08"PRIxERRV"\n",
               sa->arg0, r.error);
     }
@@ -1103,6 +1120,7 @@ void sys_syscall(arch_registers_state_t* context)
     resume(context);
 }
 
+/* XXX - this looks like a big hack. */
 #ifdef __ARM_ARCH_7M__    //armv7-m: cortex-m3 on pandaboard
 /*
     needed because to resume an interrupted IT block, there literally is only one way:
index d80aec4..c2a6be0 100644 (file)
@@ -29,6 +29,7 @@
     ubfm    \reg, \reg, #2, #3 // extract currentel[3:2]
 .endm
 
+/* XXX - add a cross-reference. */
 // macro for saving exception frame: needs to match with struct layout in C
 // code
 .macro push_state_to_exn_frame
@@ -110,7 +111,7 @@ pop_state_cont_\@:
 .endm
 
 .global exception_vectors
-// why?
+// XXX - why?
 .type exception_vectors @function
 // page-align exception vector table, cf. ARM reference manual, p.D7-2138
 .align 12
index f6a2a13..1687585 100644 (file)
@@ -4,6 +4,7 @@
  * This file contains miscellaneous architecture-independent kernel support
  * code that doesn't belong anywhere else.
  */
+/* XXX - why is this in an arch-specific directory? */
 
 /*
  * Copyright (c) 2007, 2008, ETH Zurich.