[PATCH RFC v1 1/1] lib: sbi: add support for debug triggers

Sergey Matyukevich geomatsi at gmail.com
Tue Nov 1 03:55:44 PDT 2022


Hi Jessica and all,

> > RISC-V Debug specification includes Sdtrig ISA extension.
> > This extension describes Trigger Module. Triggers can cause
> > a breakpoint exception, entry into Debug Mode, or a trace
> > action without having to execute a special instruction. For
> > native debugging triggers can be used to implement hardware
> > breakpoints and watchpoints.
> > 
> > Software support for RISC-V hardware triggers consists of the
> > following major components:
> > - U-mode: gdb support for setting hw breakpoints/watchpoints
> > - S/VS-mode: hardware breakpoints framework in Linux kernel
> > - M-mode: SBI firmware code to handle triggers
> > 
> > SBI Debug Trigger extension proposal (draft v4) has been posted
> > by Anup Patel to lists.riscv.org tech-debug mailing list:
> > 
> > https://lists.riscv.org/g/tech-debug/topic/92375492
> > 
> > This patch provides initial implementation of SBI Debug
> > Trigger Extension in OpenSBI library based on the
> > suggested extension proposal.
> > 
> > Initial implementation has the following limitations:
> > - only mcontrol6 trigger type is supported
> > - no support for chained triggers
> > - trigger update supports only address change

...

> > +union riscv_dbtr_tdata1 {
> > +	unsigned long value;
> > +	struct {
> > +#if __riscv_xlen == 64
> > +		unsigned long data:59;
> > +#elif __riscv_xlen == 32
> > +		unsigned long data:27;
> > +#else
> > +#error "Unexpected __riscv_xlen"
> > +#endif
> > +		unsigned long dmode:1;
> > +		unsigned long type:4;
> > +	};
> > +};
> 
> Bitfield order depends on endianness (type is in the MSBs for
> little-endian but LSBs for big-endian).

Do we need to support non-standard BE cases as well ? If so, then
accessing fields in shared memory messages between OpenSBI and kernel
will have to be wrapped by cpu_to_le/le_to_cpu macros on both sides.

> > +/** SBI shared mem messages layout */
> > +struct sbi_dbtr_data_msg {
> > +	unsigned long tstate;
> > +	unsigned long tdata1;
> > +	unsigned long tdata2;
> > +	unsigned long tdata3;
> > +} __packed;
> 
> Why? This will give awful codegen.

Both packed structures define messages for data exchange via shared
memory between OpenSBI and kernel. We have to make sure that these
structures have the same layout on both sides.
 
Regards,
Sergey



More information about the opensbi mailing list