Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the user-level fault handler #376

Open
wants to merge 28 commits into
base: ppos
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
762283a
update the implementation of fault handlers.
WenyuanShao Jun 16, 2018
e0579b1
Format change.
WenyuanShao Jun 16, 2018
0f95309
bring back the fourth arguement
WenyuanShao Jun 21, 2018
41be7b0
Fix a bug happened in the sret_ret
WenyuanShao Jun 26, 2018
370a243
fix the lack of the code caused by VM crash
WenyuanShao Jun 26, 2018
56feb7c
change name of the functions and variables
WenyuanShao Jun 29, 2018
f9ef836
update the introspect of the thread registers
WenyuanShao Jul 3, 2018
0d3752a
make the fault_handler an interface
WenyuanShao Jul 6, 2018
e36d46f
update the interface and add a new API
WenyuanShao Jul 8, 2018
716103b
fix some style issue.
WenyuanShao Jul 9, 2018
b08e377
achitecture based optimization: sp & in_fault now conbined to sp_faul…
WenyuanShao Jul 11, 2018
80c2ac8
roll back and separate sp and in_fault.
WenyuanShao Jul 12, 2018
f9863a6
optimize the invstk_entry by combine the in_fault bit with captbl of …
WenyuanShao Jul 20, 2018
ad4a0d8
set a bit into the comp_invstk_info of the invstk_entry to indicate w…
WenyuanShao Jul 23, 2018
b1da7b2
fix some style, add some comments
WenyuanShao Jul 23, 2018
99a5be5
Add more comments and delete redundant code.
WenyuanShao Jul 23, 2018
e200a66
slove some of the problem rasied in the PR #366.
WenyuanShao Aug 28, 2018
41be2d5
Modify some most of the comments in the thd.h.
WenyuanShao Aug 29, 2018
964af00
fix one specific comment in thd.h
WenyuanShao Aug 31, 2018
9b63e45
modify the thd_introspect and comments
WenyuanShao Sep 10, 2018
4fb83c3
update before pull
WenyuanShao Sep 10, 2018
f8b5104
commit before merge
WenyuanShao Sep 18, 2018
e87fc1b
polish the comments and clean the code.
WenyuanShao Sep 18, 2018
602ac89
delete several unnessary space because of the merge
WenyuanShao Sep 18, 2018
c8b3dcf
delete few tabs in boot_deps.h
WenyuanShao Sep 18, 2018
d7357f7
Merge remote-tracking branch 'upstream/ppos' into ppos
WenyuanShao May 20, 2019
4ab36d2
merge with main branch
WenyuanShao May 20, 2019
2edf3a5
add some TODO
WenyuanShao May 21, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
COMPONENT=llbooter.o
ASM_OBJS=s_stub.o
COMPONENT=llboot_comp.o
INTERFACES=
INTERFACES=fault_handler
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name choices: faults, faultmgr and of course fault_handler here. I actually like having the API name contain interface name as a prefix but again that's a personal preference, just wanted to mention that.

DEPENDENCIES=
IF_LIB=
ADDITIONAL_LIBS=-lcobj_format -lcos_kernel_api -lcos_defkernel_api
Expand Down
129 changes: 129 additions & 0 deletions src/components/implementation/no_interface/llbooter/boot_deps.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,20 @@
#include <res_spec.h>
#include <ps.h>
#include <bitmap.h>
#include <fault_handler.h>

/* Assembly function for sinv from new component */
extern word_t hypercall_entry_rets_inv(spdid_t cur, int op, word_t arg1, word_t arg2, word_t arg3, word_t *ret2, word_t *ret3);

/* Assembly function for sinv for the fault handlers */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider defining unsigned longs as something else in the future, perhaps ptrint_t. This is for the long term.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why aren't they just word_t. That is pretty architecture specific, isn't? You're also using that for registers among others so!

extern word_t fault_div_by_zero_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_memory_access_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_breakpoint_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_invalid_inst_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_invstk_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_comp_not_exist_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
extern word_t fault_handler_not_exist_inv(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);

extern int num_cobj;
extern spdid_t capmgr_spdid;
extern spdid_t root_spdid[];
Expand Down Expand Up @@ -114,6 +124,96 @@ boot_spd_initaep_get(spdid_t spdid)
return cos_sched_aep_get(boot_spd_defcompinfo_get(spdid));
}

void
fault_reg_print(spdid_t spdid)
{
struct pt_regs fault_regs;

struct cos_aep_info *child_aep = boot_spd_initaep_get(spdid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very interesting! It looks like the handler assumes the fault is always in the INITTHD of that component?

struct cos_compinfo *boot_info = boot_spd_compinfo_curr_get();

/*
* TODO: make it more portable by making the register an int that goes through a loop
* which is bounded by architecture-specific variables.
*/
fault_regs.ax = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems hard to make portable. I wonder if we should simply make the register an int that goes through a loop with the loop bound defined by an architecture-specific variable. IP/SP/FP are all exceptions, I'm sure.

I don't think this requires changes now, but should be considered in future iterations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are FAULT_REGXXs? Are they different from the regular REGXXs?

fault_regs.bx = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG1);
fault_regs.cx = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG2);
fault_regs.dx = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG3);
fault_regs.si = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG10);
fault_regs.di = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG11);
fault_regs.ip = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG12);
fault_regs.sp = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG13);
fault_regs.bp = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG14);
fault_regs.flags = cos_introspect(boot_info, child_aep->thd, THD_GET_FAULT_REG15);

printc("registers:\n");
printc("General registers-> EAX: %x, EBX: %x, ECX: %x, EDX: %x, SI: %x, DI: %x\n", (unsigned int)fault_regs.ax,
(unsigned int)fault_regs.bx, (unsigned int)fault_regs.cx, (unsigned int)fault_regs.dx, (unsigned int)fault_regs.si,
(unsigned int)fault_regs.di);
printc("Index registers-> IP: %x, SP: %x, BP: %x\n", (unsigned int)fault_regs.ip, (unsigned int)fault_regs.sp,
(unsigned int)fault_regs.bp);
printc("Indicator->EFLAGS: %x\n", (unsigned int)fault_regs.flags);
}

/* The fault handlers*/
void
fault_div_by_zero(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in div by zero error fault handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a memory address fault? I know general protection faults, and page-faults.

fault_memory_access(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in memory access handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

void
fault_breakpoint(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in breakpoint trap handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

void
fault_invalid_inst(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in invalid instruction trap handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

/* TODO: needs to separate overflow and underflow into two fault handlers. */
void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a future iteration, I think we want two fault handlers for overflow and underflow. I believe the separate components might want to implement each. TODO in the future.

fault_invstk(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in invstk overflow and underflow fault handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

void
fault_comp_not_exist(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in component does not exist fault handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
Copy link
Member

@phanikishoreg phanikishoreg Jun 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to all PRINT statements, grammar doesn't seem right!
I'd rephrase it as: \"Component Does Not Exist\" fault happened in component [%u] or something like that.

fault_reg_print(cos_inv_token());
return;
}

void
fault_handler_not_exist(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type)
{
PRINTLOG(PRINT_ERROR, "in fault handler does not exist fault handler, fault happens in component:%u\n\n", (unsigned int)cos_inv_token());
fault_reg_print(cos_inv_token());
return;
}

static vaddr_t
boot_deps_map_sect(spdid_t spdid, vaddr_t *mapaddr)
{
Expand Down Expand Up @@ -144,6 +244,35 @@ boot_capmgr_mem_alloc(void)
cos_meminfo_alloc(capmgr_info, BOOT_MEM_KM_BASE, mem_sz);
}

static void
boot_fault_handler_sinv_alloc(spdid_t spdid)
{
invtoken_t token = (invtoken_t)spdid;
int ret;

struct cos_compinfo *comp_info = boot_spd_compinfo_get(spdid);
struct cos_compinfo *boot_info = boot_spd_compinfo_curr_get();

/*
* All sinv caps created here are CAP_FLT,
* a different capability type to avoid an extra branch in fast-path.
*/
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_DIVZERO, boot_info->comp_cap, (vaddr_t)fault_div_by_zero_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_MEM_ACCESS, boot_info->comp_cap, (vaddr_t)fault_memory_access_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_BRKPT, boot_info->comp_cap, (vaddr_t)fault_breakpoint_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_INVLD_INS, boot_info->comp_cap, (vaddr_t)fault_invalid_inst_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_INVSTK, boot_info->comp_cap, (vaddr_t)fault_invstk_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_COMP_NOT_EXIST, boot_info->comp_cap, (vaddr_t)fault_comp_not_exist_inv, token);
assert(ret == 0);
ret = cos_fault_sinv_alloc_at(comp_info, COMP_CAPTBL_FLT_HAND_NOT_EXIST, boot_info->comp_cap, (vaddr_t)fault_handler_not_exist_inv, token);
assert(ret == 0);
}

void
boot_comp_mem_alloc(spdid_t spdid)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ boot_create_cap_system(void)
if (boot_comp_map(h, spdid, ci)) BUG();

boot_newcomp_create(spdid, boot_spd_compinfo_get(spdid));
boot_fault_handler_sinv_alloc(spdid);
PRINTLOG(PRINT_DEBUG, "Comp %d (%s) created @ %x!\n", h->id, h->name, sect->vaddr);
}

Expand Down
1 change: 1 addition & 0 deletions src/components/include/cos_kernel_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ thdcap_t cos_thd_alloc_ext(struct cos_compinfo *ci, compcap_t comp, thdclosure_i
/* Create the initial (cos_init) thread */
thdcap_t cos_initthd_alloc(struct cos_compinfo *ci, compcap_t comp);
sinvcap_t cos_sinv_alloc(struct cos_compinfo *srcci, compcap_t dstcomp, vaddr_t entry, invtoken_t token);
int cos_fault_sinv_alloc_at(struct cos_compinfo *srcci, capid_t cap, compcap_t dstcomp, vaddr_t entry, invtoken_t token);
arcvcap_t cos_arcv_alloc(struct cos_compinfo *ci, thdcap_t thdcap, tcap_t tcapcap, compcap_t compcap, arcvcap_t enotif);
asndcap_t cos_asnd_alloc(struct cos_compinfo *ci, arcvcap_t arcvcap, captblcap_t ctcap);

Expand Down
5 changes: 5 additions & 0 deletions src/components/interface/fault_handler/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
LIB_OBJS=
LIBS=$(LIB_OBJS:%.o=%.a)

include ../Makefile.subdir

12 changes: 12 additions & 0 deletions src/components/interface/fault_handler/fault_handler.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#ifndef FAULT_HANDLER_H
#define FAULT_HANDLER_H

void fault_div_by_zero(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'd use word_t where possible.

void fault_memory_access(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
void fault_breakpoint(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
void fault_invalid_inst(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
void fault_invstk(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
void fault_comp_not_exist(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);
void fault_handler_not_exist(unsigned long sp, unsigned long ip, unsigned long fault_addr, unsigned long fault_type);

#endif /* FAULT_HANDLER_H */
11 changes: 11 additions & 0 deletions src/components/interface/fault_handler/stubs/s_stub.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#include <cos_asm_server_stub_simple_stack.h>

.text

cos_asm_server_stub(fault_div_by_zero)
cos_asm_server_stub(fault_breakpoint)
cos_asm_server_stub(fault_memory_access)
cos_asm_server_stub(fault_invalid_inst)
cos_asm_server_stub(fault_comp_not_exist)
cos_asm_server_stub(fault_invstk)
cos_asm_server_stub(fault_handler_not_exist)
13 changes: 13 additions & 0 deletions src/components/lib/cos_kernel_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,19 @@ cos_sinv_alloc(struct cos_compinfo *srcci, compcap_t dstcomp, vaddr_t entry, inv
return cap;
}

int
cos_fault_sinv_alloc_at(struct cos_compinfo *srcci, capid_t cap, compcap_t dstcomp, vaddr_t entry, invtoken_t token)
{
printd("cos_fault_sinv_alloc_at\n");

assert(srcci && dstcomp);

if (!cap) return 0;
if (call_cap_op(srcci->captbl_cap, CAPTBL_OP_FAULTACTIVATE, cap, dstcomp, entry, token)) BUG();

return 0;
}

int
cos_sinv(sinvcap_t sinv, word_t arg1, word_t arg2, word_t arg3, word_t arg4)
{
Expand Down
31 changes: 19 additions & 12 deletions src/kernel/capinv.c
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,8 @@ cap_thd_switch(struct pt_regs *regs, struct thread *curr, struct thread *next, s
struct cos_cpu_local_info *cos_info)
{
struct next_thdinfo *nti = &cos_info->next_ti;
struct comp_info * next_ci = &(next->invstk[next->invstk_top].comp_info);
/* invstk_top never contains in_fault flag in its captbl. So we can use it as comp_info */
struct comp_info * next_ci = (struct comp_info *)&(next->invstk[next->invstk_top].comp_invstk_info);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a function to get this? Shouldn't there be?

int preempt = 0;

assert(next_ci && curr && next);
Expand Down Expand Up @@ -685,7 +686,7 @@ cap_ipi_process(struct pt_regs *regs)
thd_curr = thd_next = thd_current(cos_info);
receiver_rings = &IPI_cap_dest[get_cpuid()];
tcap_curr = tcap_next = tcap_current(cos_info);
ci = thd_invstk_current(thd_curr, &ip, &sp, cos_info);
ci = thd_invstk_current(thd_curr, cos_info, &ip, &sp);
assert(ci && ci->captbl);

scan_base = receiver_rings->start;
Expand Down Expand Up @@ -818,7 +819,7 @@ cap_hw_asnd(struct cap_asnd *asnd, struct pt_regs *regs)
thd = thd_current(cos_info);
tcap = tcap_current(cos_info);
assert(thd);
ci = thd_invstk_current(thd, &ip, &sp, cos_info);
ci = thd_invstk_current(thd, cos_info, &ip, &sp);
assert(ci && ci->captbl);
assert(!(thd->state & THD_STATE_PREEMPTED));
rcv_thd = arcv->thd;
Expand Down Expand Up @@ -872,7 +873,7 @@ timer_process(struct pt_regs *regs)
assert(cos_info);
thd_curr = thd_current(cos_info);
assert(thd_curr && thd_curr->cpuid == get_cpuid());
comp = thd_invstk_current(thd_curr, &ip, &sp, cos_info);
comp = thd_invstk_current(thd_curr, cos_info, &ip, &sp);
assert(comp);

return expended_process(regs, thd_curr, comp, cos_info, 1);
Expand Down Expand Up @@ -997,8 +998,7 @@ composite_syscall_handler(struct pt_regs *regs)
/* fast path: invocation return (avoiding captbl accesses) */
if (cap == COS_DEFAULT_RET_CAP) {
/* No need to lookup captbl */
sret_ret(thd, regs, cos_info);
return 0;
return sret_ret(thd, regs, cos_info);
}

/* FIXME: use a cap for print */
Expand All @@ -1007,7 +1007,7 @@ composite_syscall_handler(struct pt_regs *regs)
return 0;
}

ci = thd_invstk_current(thd, &ip, &sp, cos_info);
ci = thd_invstk_current(thd, cos_info, &ip, &sp);
assert(ci && ci->captbl);

/*
Expand All @@ -1022,7 +1022,7 @@ composite_syscall_handler(struct pt_regs *regs)
}
/* fastpath: invocation */
if (likely(ch->type == CAP_SINV)) {
sinv_call(thd, (struct cap_sinv *)ch, regs, cos_info);
sinv_call(thd, (struct cap_sinv *)ch, regs, cos_info, 0);
return 0;
}

Expand Down Expand Up @@ -1094,7 +1094,7 @@ static int __attribute__((noinline)) composite_syscall_slowpath(struct pt_regs *
cap = __userregs_getcap(regs);
capin = __userregs_get1(regs);

ci = thd_invstk_current(thd, &ip, &sp, cos_info);
ci = thd_invstk_current(thd, cos_info, &ip, &sp);
assert(ci && ci->captbl);
ct = ci->captbl;

Expand Down Expand Up @@ -1294,7 +1294,7 @@ static int __attribute__((noinline)) composite_syscall_slowpath(struct pt_regs *
vaddr_t entry_addr = __userregs_get3(regs);
invtoken_t token = __userregs_get4(regs);

ret = sinv_activate(ct, cap, capin, dest_comp_cap, entry_addr, token);
ret = sinv_activate(ct, cap, capin, dest_comp_cap, CAP_SINV, entry_addr, token);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sinv_activate with a type? I think you should leave sinv_activate as is. In the FAULTACTIVATE, use faultinv_activate or something and call an internal function that would be sinv_activate_type within inv.h

break;
}
case CAPTBL_OP_SINVDEACTIVATE: {
Expand All @@ -1303,6 +1303,14 @@ static int __attribute__((noinline)) composite_syscall_slowpath(struct pt_regs *
ret = sinv_deactivate(op_cap, capin, lid);
break;
}
case CAPTBL_OP_FAULTACTIVATE: {
capid_t dest_comp_cap = __userregs_get2(regs);
vaddr_t entry_addr = __userregs_get3(regs);
invtoken_t token = __userregs_get4(regs);

ret = sinv_activate(ct, cap, capin, dest_comp_cap, CAP_FLT, entry_addr, token);
break;
}
case CAPTBL_OP_SRETACTIVATE: {
ret = -EINVAL;
break;
Expand Down Expand Up @@ -1517,8 +1525,7 @@ static int __attribute__((noinline)) composite_syscall_slowpath(struct pt_regs *
* We usually don't have sret cap as we have 0 as the
* default return cap.
*/
sret_ret(thd, regs, cos_info);
return 0;
return sret_ret(thd, regs, cos_info);
}
case CAP_TCAP: {
/* TODO: Validate that all tcaps are on the same core */
Expand Down
Loading