Re: [PATCH v10 4/5] xen/riscv: enable GENERIC_BUG_FRAME

[oleksii.kurochko@xxxxxxxxx]

On Mon, 2024-07-22 at 17:32 +0200, Jan Beulich wrote:
> On 22.07.2024 16:09, Oleksii wrote:
> > On Mon, 2024-07-22 at 13:02 +0200, Jan Beulich wrote:
> > > On 12.07.2024 18:18, Oleksii Kurochko wrote:
> > > > ---
> > > > Changes in V10:
> > > >  - put 'select GENERIC_BUG_FRAME' in "Config RISCV".
> > > >  - rework do_trap() to not fetch an instruction in case when
> > > > the
> > > > cause of trap
> > > >    is BUG_insn.
> > > 
> > > It's BUG_insn here, but then ...
> > > 
> > > > @@ -103,7 +104,29 @@ static void do_unexpected_trap(const
> > > > struct
> > > > cpu_user_regs *regs)
> > > >  
> > > >  void do_trap(struct cpu_user_regs *cpu_regs)
> > > >  {
> > > > -    do_unexpected_trap(cpu_regs);
> > > > +    register_t pc = cpu_regs->sepc;
> > > > +    unsigned long cause = csr_read(CSR_SCAUSE);
> > > > +
> > > > +    switch ( cause )
> > > > +    {
> > > > +    case CAUSE_BREAKPOINT:
> > > 
> > > ... BREAKPOINT here? Generally I'd deem something named
> > > "breakpoint"
> > > as
> > > debugging related (and hence continuable). I'd have expected
> > > CAUSE_ILLEGAL_INSTRUCTION here, but likely I'm missing something.
> > Agree, that is is confusing, but BUG_insn is defined as ebreak
> > instruction ( Linux kernel uses also ebreak ) and it generates
> > CAUSE_BREAKPOINT.
> 
> I'm curious: How do you / does a debugger tell a breakpoint set on
> such an EBREAK insn (e.g. as a result of a use of WARN_ON()) from
> the original, unmodified insn? If there's a breakpoint, you want
> to forward to the debugger. Whereas if there's no breakpoint, you
> want to process the WARN_ON() normally.
I don't know details of debug spec but AFAIU ebreak triggers a debug
trap, which starts in debug mode and is then filtered by every mode as
it goes towards user-mode. So first GDB will handle this debug trap and
will check if this ebreak was set by him or not.

~ Oleksii