Re: [PATCH v3 1/2] riscv: Introduce support for hardware break/watchpoints

[Himanshu Chauhan]

Hi,

On Mon, Feb 23, 2026 at 3:55 PM Conor Dooley <conor.dooley@xxxxxxxxxxxxx> wrote:
>
> On Mon, Feb 23, 2026 at 10:19:17AM +0530, Himanshu Chauhan wrote:
> > +static void init_sbi_dbtr(void)
> > +{
> > +     unsigned long tdata1;
> > +     struct sbiret ret;
> > +
> > +     if (sbi_probe_extension(SBI_EXT_DBTR) <= 0) {
> > +             pr_warn("%s: SBI_EXT_DBTR is not supported\n", __func__);
>
> This is going to run an all systems, right? A pr_warn() seems
> inappropriate, given that not supporting this extension isn't a problem.
> If you want to produce warnings, only do it for < 0 return values and
> maybe print the actual error code too?

It's correct that the absence of the extension is not a problem but an
informational message can be helpful.
So, I can use pr_info or pr_info_once for this with proper error code. Ok?

Regards
Himanshu

>
> > +             dbtr_total_num = 0;
> > +             goto done;
> > +     }
> > +
> > +     ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
> > +                     0, 0, 0, 0, 0, 0);
> > +     if (ret.error) {
> > +             pr_warn("%s: Failed to detect triggers\n", __func__);
> > +             dbtr_total_num = 0;
> > +             goto done;
> > +     }
> > +
> > +     tdata1 = 0;
> > +     tdata1 = RV_DBTR_SET_TDATA1_TYPE(tdata1, RV_DBTR_TRIG_MCONTROL6);
> > +
> > +     ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
> > +                     tdata1, 0, 0, 0, 0, 0);
> > +     if (ret.error) {
> > +             pr_warn("%s: failed to detect mcontrol6 triggers\n", __func__);
> > +     } else if (!ret.value) {
> > +             pr_warn("%s: type 6 triggers not available\n", __func__);
> > +     } else {
> > +             dbtr_total_num = ret.value;
> > +             dbtr_type = RV_DBTR_TRIG_MCONTROL6;
> > +             pr_warn("%s: mcontrol6 trigger available.\n", __func__);
> > +             goto done;
> > +     }
> > +
> > +     /* fallback to type 2 triggers if type 6 is not available */
> > +
> > +     tdata1 = 0;
> > +     tdata1 = RV_DBTR_SET_TDATA1_TYPE(tdata1, RV_DBTR_TRIG_MCONTROL);
> > +
> > +     ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
> > +                     tdata1, 0, 0, 0, 0, 0);
> > +     if (ret.error) {
> > +             pr_warn("%s: failed to detect mcontrol triggers\n", __func__);
> > +     } else if (!ret.value) {
> > +             pr_warn("%s: type 2 triggers not available\n", __func__);
> > +     } else {
> > +             dbtr_total_num = ret.value;
> > +             dbtr_type = RV_DBTR_TRIG_MCONTROL;
> > +             goto done;
> > +     }
> > +
> > +done:
> > +     dbtr_init = 1;
> > +}
>
> > +
> > +void hw_breakpoint_pmu_read(struct perf_event *bp)
> > +{
> > +     /* TODO */
> > +}
> > +
> > +void clear_ptrace_hw_breakpoint(struct task_struct *tsk)
> > +{
> > +     /* TODO */
> > +}
> > +
> > +void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
> > +{
> > +     /* TODO */
> > +}
>
> This isn't an RFC, why does it have TODOs?