[RFC PATCH 2/2] riscv: Introduce support for hardware break/watchpoints

[Himanshu Chauhan]

Hi Qingfang,

Thanks for pointing it out! That explains the difference in behaviour.
I will fix it.

Regards
Himanshu

On Mon, May 12, 2025 at 9:01 AM Qingfang Deng <dqfext at gmail.com> wrote:
>
> Hi, Himanshu and Charlie,
>
> > +static int arch_smp_setup_sbi_shmem(unsigned int cpu)
> > +{
> > +     struct sbi_dbtr_shmem_entry *dbtr_shmem;
> > +     unsigned long shmem_pa;
> > +     struct sbiret ret;
> > +     int rc;
> > +
> > +     dbtr_shmem = per_cpu_ptr(sbi_dbtr_shmem, cpu);
> > +     if (!dbtr_shmem) {
> > +             pr_err("Invalid per-cpu shared memory for debug triggers\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     shmem_pa = __pa(dbtr_shmem);
> > +
> > +     ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> > +                     (!MEM_LO(shmem_pa) ? 0xFFFFFFFFUL : MEM_LO(shmem_pa)),
> > +                     (!MEM_HI(shmem_pa) ? 0xFFFFFFFFUL : MEM_HI(shmem_pa)),
> > +                      0, 0, 0, 0);
> > +
> > +     if (ret.error) {
> > +             switch(ret.error) {
> > +             case SBI_ERR_DENIED:
> > +                     pr_warn("%s: Access denied for shared memory at %lx\n",
> > +                             __func__, shmem_pa);
> > +                     rc = -EPERM;
> > +                     break;
> > +
> > +             case SBI_ERR_INVALID_PARAM:
> > +             case SBI_ERR_INVALID_ADDRESS:
> > +                     pr_warn("%s: Invalid address parameter (%lu)\n",
> > +                             __func__, ret.error);
> > +                     rc = -EINVAL;
> > +                     break;
> > +
> > +             case SBI_ERR_ALREADY_AVAILABLE:
> > +                     pr_warn("%s: Shared memory is already set\n",
> > +                             __func__);
> > +                     rc = -EADDRINUSE;
> > +                     break;
> > +
> > +             case SBI_ERR_FAILURE:
> > +                     pr_err("%s: Internal sdtrig state error\n",
> > +                            __func__);
> > +                     rc = -ENXIO;
> > +                     break;
> > +
> > +             default:
> > +                     pr_warn("%s: Unknown error %lu\n", __func__, ret.error);
> > +                     rc = -ENXIO;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     pr_warn("CPU %d: HW Breakpoint shared memory registered.\n", cpu);
>
> This is printed unconditionally, even if there is an error above.
>
> > +
> > +     return rc;
>
> If ret.error is 0, rc is uninitialized here, which may explain the error
> that Charlie came across.
>
> Regards,
>
> -- Qingfang