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

[Qingfang Deng <dqfext@gmail.com>]

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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv