Message ID | f4c4b711106283e26536105105892b93bb39ea3e.1685359848.git.oleksii.kurochko@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | RISCV basic exception handling implementation | expand |
On 29.05.2023 14:13, Oleksii Kurochko wrote: > --- a/xen/arch/riscv/traps.c > +++ b/xen/arch/riscv/traps.c > @@ -12,6 +12,31 @@ > #include <asm/processor.h> > #include <asm/traps.h> > > +#define cast_to_bug_frame(addr) \ > + (const struct bug_frame *)(addr) I can't find a use for this; should it be dropped or moved to some later patch? In any event, if ti's intended to survive, it needs yet another pair of parentheses. > +/* > + * Initialize the trap handling. > + * > + * The function is called after MMU is enabled. > + */ > +void trap_init(void) Is this going to be used for secondary processors as well? If not, it will want to be __init. > +{ > + /* > + * When the MMU is off, addr varialbe will be a physical address otherwise > + * it would be a virtual address. > + * > + * It will work fine as: > + * - access to addr is PC-relative. > + * - -nopie is used. -nopie really suppresses the compiler emitting > + * code going through .got (which then indeed would mean using absolute > + * addresses). > + */ Is all of this comment still relevant not that you're running with the MMU already enabled. Jan > + unsigned long addr = (unsigned long)&handle_trap; > + > + csr_write(CSR_STVEC, addr); > +} > + > static const char *decode_trap_cause(unsigned long cause) > { > static const char *const trap_causes[] = {
On Tue, 2023-05-30 at 17:44 +0200, Jan Beulich wrote: > On 29.05.2023 14:13, Oleksii Kurochko wrote: > > --- a/xen/arch/riscv/traps.c > > +++ b/xen/arch/riscv/traps.c > > @@ -12,6 +12,31 @@ > > #include <asm/processor.h> > > #include <asm/traps.h> > > > > +#define cast_to_bug_frame(addr) \ > > + (const struct bug_frame *)(addr) > > I can't find a use for this; should it be dropped or moved to some > later patch? In any event, if ti's intended to survive, it needs yet > another pair of parentheses. You are right. It should be a part of the next patch. Thanks. > > > +/* > > + * Initialize the trap handling. > > + * > > + * The function is called after MMU is enabled. > > + */ > > +void trap_init(void) > > Is this going to be used for secondary processors as well? If not, > it will want to be __init. I think I'll use it for secondary processors. > > > +{ > > + /* > > + * When the MMU is off, addr varialbe will be a physical > > address otherwise > > + * it would be a virtual address. > > + * > > + * It will work fine as: > > + * - access to addr is PC-relative. > > + * - -nopie is used. -nopie really suppresses the compiler > > emitting > > + * code going through .got (which then indeed would mean > > using absolute > > + * addresses). > > + */ > > Is all of this comment still relevant not that you're running with > the MMU already enabled. Not really. I think comment above trap_init() function will be enough. I'll remove this comment. ~ Oleksii
diff --git a/xen/arch/riscv/include/asm/traps.h b/xen/arch/riscv/include/asm/traps.h index f3fb6b25d1..f1879294ef 100644 --- a/xen/arch/riscv/include/asm/traps.h +++ b/xen/arch/riscv/include/asm/traps.h @@ -7,6 +7,7 @@ void do_trap(struct cpu_user_regs *cpu_regs); void handle_trap(void); +void trap_init(void); #endif /* __ASSEMBLY__ */ diff --git a/xen/arch/riscv/setup.c b/xen/arch/riscv/setup.c index 845d18d86f..1cae0e5ccc 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -3,6 +3,7 @@ #include <asm/early_printk.h> #include <asm/mm.h> +#include <asm/traps.h> /* Xen stack for bringing up the first CPU. */ unsigned char __initdata cpu0_boot_stack[STACK_SIZE] @@ -25,6 +26,8 @@ void __init noreturn start_xen(unsigned long bootcpu_id, void __init noreturn cont_after_mmu_is_enabled(void) { + trap_init(); + early_printk("All set up\n"); for ( ;; ) diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c index ea1012e83e..48c1059954 100644 --- a/xen/arch/riscv/traps.c +++ b/xen/arch/riscv/traps.c @@ -12,6 +12,31 @@ #include <asm/processor.h> #include <asm/traps.h> +#define cast_to_bug_frame(addr) \ + (const struct bug_frame *)(addr) + +/* + * Initialize the trap handling. + * + * The function is called after MMU is enabled. + */ +void trap_init(void) +{ + /* + * When the MMU is off, addr varialbe will be a physical address otherwise + * it would be a virtual address. + * + * It will work fine as: + * - access to addr is PC-relative. + * - -nopie is used. -nopie really suppresses the compiler emitting + * code going through .got (which then indeed would mean using absolute + * addresses). + */ + unsigned long addr = (unsigned long)&handle_trap; + + csr_write(CSR_STVEC, addr); +} + static const char *decode_trap_cause(unsigned long cause) { static const char *const trap_causes[] = {