Re: [PATCH v6 1/5] riscv: Introduce support for hardware break/watchpoints

[Qingfang Deng]

Hi,

On 2026/8/3 21:49, Himanshu Chauhan wrote:
> +#define MEM_HI(_m)	((unsigned long)upper_32_bits(_m))
> +#define MEM_LO(_m)	((unsigned long)lower_32_bits(_m))
> +
> +static int arch_smp_setup_sbi_shmem(unsigned int cpu)
> +{
> +	union sbi_dbtr_shmem_entry *dbtr_shmem;
> +	phys_addr_t shmem_pa;
> +	struct sbiret ret;
> +
> +	dbtr_shmem = per_cpu_ptr(sbi_dbtr_shmem, cpu);
> +	if (!dbtr_shmem) {

per_cpu_ptr() never returns NULL, so this check can be removed.

> +		pr_err("Invalid per-cpu shared memory for debug triggers\n");
> +		return -ENODEV;
> +	}
> +
> +	shmem_pa = per_cpu_ptr_to_phys(dbtr_shmem);
> +
> +	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
> +			MEM_LO(shmem_pa), MEM_HI(shmem_pa), 0, 0, 0, 0);

The ABI takes lower and upper XLEN bits instead of fixed 32 bits, so 
this will break on an XLEN == 64 system and a physical address >= 4GiB.

#if __riscv_xlen == 64
#define MEM_HI(_m)    0UL
#define MEM_LO(_m)    (unsigned long)(_m)
#elif __riscv_xlen == 32
#define MEM_HI(_m)    (unsigned long)upper_32_bits(_m)
#define MEM_LO(_m)    (unsigned long)lower_32_bits(_m)
#else
#error "Unknown __riscv_xlen"
#endif

> +
> +	if (ret.error) {
> +		pr_warn("%s: failed to setup shared memory. error: %ld\n",
> +			__func__, ret.error);
> +		return sbi_err_map_linux_errno(ret.error);
> +	}
> +
> +	pr_info("CPU %d: HW Breakpoint shared memory registered.\n", cpu);
> +
> +	return 0;
> +}
Kind regards,
Qingfang