| Message ID | 91b0284d20f530f2795a119ccb7436ee0b800256.1678976127.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RISCV basic exception handling implementation | expand |
Hi Oleksii, On 16/03/2023 14:39, Oleksii Kurochko wrote: > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > --- > Changes in V5: > - Nothing changed > --- > Changes in V4: > - Nothing changed > --- > Changes in V3: > - Nothing changed > --- > Changes in V2: > - Rename setup_trap_handler() to trap_init(). > - Add Reviewed-by to the commit message. > --- > xen/arch/riscv/include/asm/traps.h | 1 + > xen/arch/riscv/setup.c | 5 +++++ > xen/arch/riscv/traps.c | 7 +++++++ > 3 files changed, 13 insertions(+) > > 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 36556eb779..b44d105b5f 100644 > --- a/xen/arch/riscv/setup.c > +++ b/xen/arch/riscv/setup.c > @@ -3,7 +3,9 @@ > #include <xen/kernel.h> > > #include <asm/boot-info.h> > +#include <asm/csr.h> > #include <asm/early_printk.h> > +#include <asm/traps.h> > > /* Xen stack for bringing up the first CPU. */ > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long bootcpu_id, > > fill_boot_info(); > > + trap_init(); > + > early_printk("All set up\n"); > + > for ( ;; ) > asm volatile ("wfi"); > > diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c > index 8a1529e0c5..581f34efbc 100644 > --- a/xen/arch/riscv/traps.c > +++ b/xen/arch/riscv/traps.c > @@ -13,6 +13,13 @@ > #include <asm/processor.h> > #include <asm/traps.h> > > +void trap_init(void) > +{ > + unsigned long addr = (unsigned long)&handle_trap; It is not super clear to me whether this is going to store the virtual or physical address. Depending on the answer, the next would be whether the value would still be valid after the MMU is turned on? > + > + csr_write(CSR_STVEC, addr); > +} > + > static const char *decode_trap_cause(unsigned long cause) > { > static const char *const trap_causes[] = { Cheers,
Hi Julien, On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote: > Hi Oleksii, > > On 16/03/2023 14:39, Oleksii Kurochko wrote: > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > --- > > Changes in V5: > > - Nothing changed > > --- > > Changes in V4: > > - Nothing changed > > --- > > Changes in V3: > > - Nothing changed > > --- > > Changes in V2: > > - Rename setup_trap_handler() to trap_init(). > > - Add Reviewed-by to the commit message. > > --- > > xen/arch/riscv/include/asm/traps.h | 1 + > > xen/arch/riscv/setup.c | 5 +++++ > > xen/arch/riscv/traps.c | 7 +++++++ > > 3 files changed, 13 insertions(+) > > > > 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 36556eb779..b44d105b5f 100644 > > --- a/xen/arch/riscv/setup.c > > +++ b/xen/arch/riscv/setup.c > > @@ -3,7 +3,9 @@ > > #include <xen/kernel.h> > > > > #include <asm/boot-info.h> > > +#include <asm/csr.h> > > #include <asm/early_printk.h> > > +#include <asm/traps.h> > > > > /* Xen stack for bringing up the first CPU. */ > > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > > @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long > > bootcpu_id, > > > > fill_boot_info(); > > > > + trap_init(); > > + > > early_printk("All set up\n"); > > + > > for ( ;; ) > > asm volatile ("wfi"); > > > > diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c > > index 8a1529e0c5..581f34efbc 100644 > > --- a/xen/arch/riscv/traps.c > > +++ b/xen/arch/riscv/traps.c > > @@ -13,6 +13,13 @@ > > #include <asm/processor.h> > > #include <asm/traps.h> > > > > +void trap_init(void) > > +{ > > + unsigned long addr = (unsigned long)&handle_trap; > > It is not super clear to me whether this is going to store the > virtual > or physical address. Actually it is going to store both the virtual and physical address. Depending on if MMU is enabled or not. > > Depending on the answer, the next would be whether the value would > still > be valid after the MMU is turned on? It would still be valid because for addr will be generated PC-relative address. > > > + > > + csr_write(CSR_STVEC, addr); > > +} > > + > > static const char *decode_trap_cause(unsigned long cause) > > { > > static const char *const trap_causes[] = { > > Cheers, > > ~ Oleksii
On 22/03/2023 11:33, Oleksii wrote: > Hi Julien, Hi Oleksii, > > On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote: >> Hi Oleksii, >> >> On 16/03/2023 14:39, Oleksii Kurochko wrote: >>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> >>> --- >>> Changes in V5: >>> - Nothing changed >>> --- >>> Changes in V4: >>> - Nothing changed >>> --- >>> Changes in V3: >>> - Nothing changed >>> --- >>> Changes in V2: >>> - Rename setup_trap_handler() to trap_init(). >>> - Add Reviewed-by to the commit message. >>> --- >>> xen/arch/riscv/include/asm/traps.h | 1 + >>> xen/arch/riscv/setup.c | 5 +++++ >>> xen/arch/riscv/traps.c | 7 +++++++ >>> 3 files changed, 13 insertions(+) >>> >>> 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 36556eb779..b44d105b5f 100644 >>> --- a/xen/arch/riscv/setup.c >>> +++ b/xen/arch/riscv/setup.c >>> @@ -3,7 +3,9 @@ >>> #include <xen/kernel.h> >>> >>> #include <asm/boot-info.h> >>> +#include <asm/csr.h> >>> #include <asm/early_printk.h> >>> +#include <asm/traps.h> >>> >>> /* Xen stack for bringing up the first CPU. */ >>> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] >>> @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long >>> bootcpu_id, >>> >>> fill_boot_info(); >>> >>> + trap_init(); >>> + >>> early_printk("All set up\n"); >>> + >>> for ( ;; ) >>> asm volatile ("wfi"); >>> >>> diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c >>> index 8a1529e0c5..581f34efbc 100644 >>> --- a/xen/arch/riscv/traps.c >>> +++ b/xen/arch/riscv/traps.c >>> @@ -13,6 +13,13 @@ >>> #include <asm/processor.h> >>> #include <asm/traps.h> >>> >>> +void trap_init(void) >>> +{ >>> + unsigned long addr = (unsigned long)&handle_trap; >> >> It is not super clear to me whether this is going to store the >> virtual >> or physical address. > Actually it is going to store both the virtual and physical address. > Depending on if MMU is enabled or not. I think some comment in the code would be really good because this is... >> >> Depending on the answer, the next would be whether the value would >> still >> be valid after the MMU is turned on? > It would still be valid because for addr will be generated PC-relative > address. ... not clear to me what would guarantee that Xen is compiled with -noPIE. Is the cmodel? A suggestion for the top of the function: "Initialize the trap handling. This is called twice (before and after the MMU)." And for on top of 'addr', I would add: "When the MMU is off, this will be a physical address otherwise it would be a virtual address. This is guarantee because [fill the blank]". Cheers,
On Wed, 2023-03-22 at 12:14 +0000, Julien Grall wrote: > > > On 22/03/2023 11:33, Oleksii wrote: > > Hi Julien, > > Hi Oleksii, > > > > > On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote: > > > Hi Oleksii, > > > > > > On 16/03/2023 14:39, Oleksii Kurochko wrote: > > > > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > --- > > > > Changes in V5: > > > > - Nothing changed > > > > --- > > > > Changes in V4: > > > > - Nothing changed > > > > --- > > > > Changes in V3: > > > > - Nothing changed > > > > --- > > > > Changes in V2: > > > > - Rename setup_trap_handler() to trap_init(). > > > > - Add Reviewed-by to the commit message. > > > > --- > > > > xen/arch/riscv/include/asm/traps.h | 1 + > > > > xen/arch/riscv/setup.c | 5 +++++ > > > > xen/arch/riscv/traps.c | 7 +++++++ > > > > 3 files changed, 13 insertions(+) > > > > > > > > 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 36556eb779..b44d105b5f 100644 > > > > --- a/xen/arch/riscv/setup.c > > > > +++ b/xen/arch/riscv/setup.c > > > > @@ -3,7 +3,9 @@ > > > > #include <xen/kernel.h> > > > > > > > > #include <asm/boot-info.h> > > > > +#include <asm/csr.h> > > > > #include <asm/early_printk.h> > > > > +#include <asm/traps.h> > > > > > > > > /* Xen stack for bringing up the first CPU. */ > > > > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > > > > @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long > > > > bootcpu_id, > > > > > > > > fill_boot_info(); > > > > > > > > + trap_init(); > > > > + > > > > early_printk("All set up\n"); > > > > + > > > > for ( ;; ) > > > > asm volatile ("wfi"); > > > > > > > > diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c > > > > index 8a1529e0c5..581f34efbc 100644 > > > > --- a/xen/arch/riscv/traps.c > > > > +++ b/xen/arch/riscv/traps.c > > > > @@ -13,6 +13,13 @@ > > > > #include <asm/processor.h> > > > > #include <asm/traps.h> > > > > > > > > +void trap_init(void) > > > > +{ > > > > + unsigned long addr = (unsigned long)&handle_trap; > > > > > > It is not super clear to me whether this is going to store the > > > virtual > > > or physical address. > > Actually it is going to store both the virtual and physical > > address. > > Depending on if MMU is enabled or not. > > I think some comment in the code would be really good because this > is... > > > > > > > Depending on the answer, the next would be whether the value > > > would > > > still > > > be valid after the MMU is turned on? > > It would still be valid because for addr will be generated PC- > > relative > > address. > > ... not clear to me what would guarantee that Xen is compiled with > -noPIE. Is the cmodel? There is a patch: https://lore.kernel.org/xen-devel/2785518800dce64fafb3096480a5ae4c4e026bcb.1678970065.git.oleksii.kurochko@gmail.com/ Which guarantees that Xen is complied with -noPIE. The cmodel determines which software addressing mode is used, and, therefore, what constraints are enforced on the linked program. > > A suggestion for the top of the function: > > "Initialize the trap handling. This is called twice (before and after > the MMU)." > > And for on top of 'addr', I would add: > > "When the MMU is off, this will be a physical address otherwise it > would > be a virtual address. This is guarantee because [fill the blank]". Thanks for the recommendations. I will take them into account. ~ Oleksii
Hi Oleksii, On 22/03/2023 13:40, Oleksii wrote: > On Wed, 2023-03-22 at 12:14 +0000, Julien Grall wrote: >> >> >> On 22/03/2023 11:33, Oleksii wrote: >>> Hi Julien, >> >> Hi Oleksii, >> >>> >>> On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote: >>>> Hi Oleksii, >>>> >>>> On 16/03/2023 14:39, Oleksii Kurochko wrote: >>>>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> >>>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> >>>>> --- >>>>> Changes in V5: >>>>> - Nothing changed >>>>> --- >>>>> Changes in V4: >>>>> - Nothing changed >>>>> --- >>>>> Changes in V3: >>>>> - Nothing changed >>>>> --- >>>>> Changes in V2: >>>>> - Rename setup_trap_handler() to trap_init(). >>>>> - Add Reviewed-by to the commit message. >>>>> --- >>>>> xen/arch/riscv/include/asm/traps.h | 1 + >>>>> xen/arch/riscv/setup.c | 5 +++++ >>>>> xen/arch/riscv/traps.c | 7 +++++++ >>>>> 3 files changed, 13 insertions(+) >>>>> >>>>> 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 36556eb779..b44d105b5f 100644 >>>>> --- a/xen/arch/riscv/setup.c >>>>> +++ b/xen/arch/riscv/setup.c >>>>> @@ -3,7 +3,9 @@ >>>>> #include <xen/kernel.h> >>>>> >>>>> #include <asm/boot-info.h> >>>>> +#include <asm/csr.h> >>>>> #include <asm/early_printk.h> >>>>> +#include <asm/traps.h> >>>>> >>>>> /* Xen stack for bringing up the first CPU. */ >>>>> unsigned char __initdata cpu0_boot_stack[STACK_SIZE] >>>>> @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long >>>>> bootcpu_id, >>>>> >>>>> fill_boot_info(); >>>>> >>>>> + trap_init(); >>>>> + >>>>> early_printk("All set up\n"); >>>>> + >>>>> for ( ;; ) >>>>> asm volatile ("wfi"); >>>>> >>>>> diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c >>>>> index 8a1529e0c5..581f34efbc 100644 >>>>> --- a/xen/arch/riscv/traps.c >>>>> +++ b/xen/arch/riscv/traps.c >>>>> @@ -13,6 +13,13 @@ >>>>> #include <asm/processor.h> >>>>> #include <asm/traps.h> >>>>> >>>>> +void trap_init(void) >>>>> +{ >>>>> + unsigned long addr = (unsigned long)&handle_trap; >>>> >>>> It is not super clear to me whether this is going to store the >>>> virtual >>>> or physical address. >>> Actually it is going to store both the virtual and physical >>> address. >>> Depending on if MMU is enabled or not. >> >> I think some comment in the code would be really good because this >> is... >> >>>> >>>> Depending on the answer, the next would be whether the value >>>> would >>>> still >>>> be valid after the MMU is turned on? >>> It would still be valid because for addr will be generated PC- >>> relative >>> address. >> >> ... not clear to me what would guarantee that Xen is compiled with >> -noPIE. Is the cmodel? There is a missing "given" after "that". > There is a patch: > https://lore.kernel.org/xen-devel/2785518800dce64fafb3096480a5ae4c4e026bcb.1678970065.git.oleksii.kurochko@gmail.com/ > Which guarantees that Xen is complied with -noPIE. I am a bit puzzled with what you wrote. From my understanding, with -noPIE, the compiler would be free to use absolute address rather than pc-relative one. Do you have any pointer to documentation that would back your reasoning? I have already mentioned before, but I think it would really useful if you start adding more documentation (in particular of such behavior) in the code or docs/ (for more wide). This would help everyone to understand how everything is meant to work. Cheers,
On 22.03.2023 14:51, Julien Grall wrote: > I am a bit puzzled with what you wrote. From my understanding, with > -noPIE, the compiler would be free to use absolute address rather than > pc-relative one. Do you have any pointer to documentation that would > back your reasoning? It might for RV32 (using lui), but for 64-bit the ISA simply doesn't have any halfway efficient means to load addresses in other than a PC-relative manner [1]. -nopie really suppresses the compiler emitting code going through .got (which then indeed would mean using absolute addresses). I understand that at least for now RV32 isn't really of interest; if it were, I guess the concern would be more significant. Jan [1] There aren't even suitable relocations to express such in ELF, because you'd need to have a way to get at the top 32 bits of an address. The only alternative would be .got-like indirection without actually using .got.
On Wed, 2023-03-22 at 13:51 +0000, Julien Grall wrote: > Hi Oleksii, > > On 22/03/2023 13:40, Oleksii wrote: > > On Wed, 2023-03-22 at 12:14 +0000, Julien Grall wrote: > > > > > > > > > On 22/03/2023 11:33, Oleksii wrote: > > > > Hi Julien, > > > > > > Hi Oleksii, > > > > > > > > > > > On Tue, 2023-03-21 at 17:42 +0000, Julien Grall wrote: > > > > > Hi Oleksii, > > > > > > > > > > On 16/03/2023 14:39, Oleksii Kurochko wrote: > > > > > > Signed-off-by: Oleksii Kurochko > > > > > > <oleksii.kurochko@gmail.com> > > > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > > > --- > > > > > > Changes in V5: > > > > > > - Nothing changed > > > > > > --- > > > > > > Changes in V4: > > > > > > - Nothing changed > > > > > > --- > > > > > > Changes in V3: > > > > > > - Nothing changed > > > > > > --- > > > > > > Changes in V2: > > > > > > - Rename setup_trap_handler() to trap_init(). > > > > > > - Add Reviewed-by to the commit message. > > > > > > --- > > > > > > xen/arch/riscv/include/asm/traps.h | 1 + > > > > > > xen/arch/riscv/setup.c | 5 +++++ > > > > > > xen/arch/riscv/traps.c | 7 +++++++ > > > > > > 3 files changed, 13 insertions(+) > > > > > > > > > > > > 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 36556eb779..b44d105b5f 100644 > > > > > > --- a/xen/arch/riscv/setup.c > > > > > > +++ b/xen/arch/riscv/setup.c > > > > > > @@ -3,7 +3,9 @@ > > > > > > #include <xen/kernel.h> > > > > > > > > > > > > #include <asm/boot-info.h> > > > > > > +#include <asm/csr.h> > > > > > > #include <asm/early_printk.h> > > > > > > +#include <asm/traps.h> > > > > > > > > > > > > /* Xen stack for bringing up the first CPU. */ > > > > > > unsigned char __initdata cpu0_boot_stack[STACK_SIZE] > > > > > > @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned > > > > > > long > > > > > > bootcpu_id, > > > > > > > > > > > > fill_boot_info(); > > > > > > > > > > > > + trap_init(); > > > > > > + > > > > > > early_printk("All set up\n"); > > > > > > + > > > > > > for ( ;; ) > > > > > > asm volatile ("wfi"); > > > > > > > > > > > > diff --git a/xen/arch/riscv/traps.c > > > > > > b/xen/arch/riscv/traps.c > > > > > > index 8a1529e0c5..581f34efbc 100644 > > > > > > --- a/xen/arch/riscv/traps.c > > > > > > +++ b/xen/arch/riscv/traps.c > > > > > > @@ -13,6 +13,13 @@ > > > > > > #include <asm/processor.h> > > > > > > #include <asm/traps.h> > > > > > > > > > > > > +void trap_init(void) > > > > > > +{ > > > > > > + unsigned long addr = (unsigned long)&handle_trap; > > > > > > > > > > It is not super clear to me whether this is going to store > > > > > the > > > > > virtual > > > > > or physical address. > > > > Actually it is going to store both the virtual and physical > > > > address. > > > > Depending on if MMU is enabled or not. > > > > > > I think some comment in the code would be really good because > > > this > > > is... > > > > > > > > > > > > > Depending on the answer, the next would be whether the value > > > > > would > > > > > still > > > > > be valid after the MMU is turned on? > > > > It would still be valid because for addr will be generated PC- > > > > relative > > > > address. > > > > > > ... not clear to me what would guarantee that Xen is compiled > > > with > > > -noPIE. Is the cmodel? > > There is a missing "given" after "that". > > > There is a patch: > > https://lore.kernel.org/xen-devel/2785518800dce64fafb3096480a5ae4c4e026bcb.1678970065.git.oleksii.kurochko@gmail.com/ > > Which guarantees that Xen is complied with -noPIE. > I am a bit puzzled with what you wrote. From my understanding, with > -noPIE, the compiler would be free to use absolute address rather > than > pc-relative one. Do you have any pointer to documentation that would > back your reasoning? https://riscv.org/wp-content/uploads/2017/05/riscv-spec-v2.2.pdf If look at pseudoinstructions (p110 in pdf) which work with addresses they are always pc-relative ( they all use AUIPC instruction ). The only question is that if .got is used or not ( which depends on pie,pic, etc... ) > > I have already mentioned before, but I think it would really useful > if > you start adding more documentation (in particular of such behavior) > in > the code or docs/ (for more wide). This would help everyone to > understand how everything is meant to work. Sure. I will add more documentation for such kind of code. ~ 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 36556eb779..b44d105b5f 100644 --- a/xen/arch/riscv/setup.c +++ b/xen/arch/riscv/setup.c @@ -3,7 +3,9 @@ #include <xen/kernel.h> #include <asm/boot-info.h> +#include <asm/csr.h> #include <asm/early_printk.h> +#include <asm/traps.h> /* Xen stack for bringing up the first CPU. */ unsigned char __initdata cpu0_boot_stack[STACK_SIZE] @@ -32,7 +34,10 @@ void __init noreturn start_xen(unsigned long bootcpu_id, fill_boot_info(); + trap_init(); + early_printk("All set up\n"); + for ( ;; ) asm volatile ("wfi"); diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c index 8a1529e0c5..581f34efbc 100644 --- a/xen/arch/riscv/traps.c +++ b/xen/arch/riscv/traps.c @@ -13,6 +13,13 @@ #include <asm/processor.h> #include <asm/traps.h> +void trap_init(void) +{ + 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[] = {