linux-kernel - Re: [PATCH V6 2/3] scsi: ufs-qcom: Add support to dump MCQ registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <15df00e9-d5d9-4d16-b334-e1b8cb96f654@acm.org>
Date: Wed, 9 Apr 2025 14:08:27 -0700
From: Bart Van Assche <bvanassche@....org>
To: Manish Pandey <quic_mapa@...cinc.com>,
 Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>
Cc: linux-arm-msm@...r.kernel.org, linux-scsi@...r.kernel.org,
 linux-kernel@...r.kernel.org, quic_nitirawa@...cinc.com,
 quic_bhaskarv@...cinc.com, quic_rampraka@...cinc.com, quic_cang@...cinc.com,
 quic_nguyenb@...cinc.com
Subject: Re: [PATCH V6 2/3] scsi: ufs-qcom: Add support to dump MCQ registers

On 4/7/25 7:21 AM, Manish Pandey wrote:
> +int ufs_qcom_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
> +		     const char *prefix, enum ufshcd_res id)
> +{
> +	u32 *regs __free(kfree) = NULL;
> +	size_t pos;
> +
> +	if (offset % 4 != 0 || len % 4 != 0)
> +		return -EINVAL;
> +
> +	regs = kzalloc(len, GFP_ATOMIC);
> +	if (!regs)
> +		return -ENOMEM;
> +
> +	for (pos = 0; pos < len; pos += 4)
> +		regs[pos / 4] = readl(hba->res[id].base + offset + pos);
> +
> +	print_hex_dump(KERN_ERR, prefix,
> +			len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE,
> +				16, 4, regs, len, false);

The indentation of the print_hex_dump() arguments is not compliant with
the Linux kernel coding style.

> +	return 0;
> +}
> +
> +static void ufs_qcom_dump_mcq_hci_regs(struct ufs_hba *hba)
> +{
> +	/* voluntarily yield the CPU to prevent CPU hog during data dumps */
> +	/* RES_MCQ_1 */
> +	ufs_qcom_dump_regs(hba, 0x0, 256 * 4, "MCQ HCI 1da0000-1da03f0", RES_MCQ);
> +	cond_resched();
> +
> +	/* RES_MCQ_2 */
> +	ufs_qcom_dump_regs(hba, 0x400, 256 * 4, "MCQ HCI 1da0400-1da07f0", RES_MCQ);
> +	cond_resched();
> +
> +	/*RES_MCQ_VS */
> +	ufs_qcom_dump_regs(hba, 0x0, 5 * 4, "MCQ VS 1da4000-1da4010", RES_MCQ_VS);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_1 */
> +	ufs_qcom_dump_regs(hba, 0x0, 256 * 4, "MCQ SQD 1da5000-1da53f0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_2 */
> +	ufs_qcom_dump_regs(hba, 0x400, 256 * 4, "MCQ SQD 1da5400-1da57f0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_3 */
> +	ufs_qcom_dump_regs(hba, 0x800, 256 * 4, "MCQ SQD 1da5800-1da5bf0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_4 */
> +	ufs_qcom_dump_regs(hba, 0xc00, 256 * 4, "MCQ SQD 1da5c00-1da5ff0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_5 */
> +	ufs_qcom_dump_regs(hba, 0x1000, 256 * 4, "MCQ SQD 1da6000-1da63f0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_6 */
> +	ufs_qcom_dump_regs(hba, 0x1400, 256 * 4, "MCQ SQD 1da6400-1da67f0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_7 */
> +	ufs_qcom_dump_regs(hba, 0x1800, 256 * 4, "MCQ SQD 1da6800-1da6bf0", RES_MCQ_SQD);
> +	cond_resched();
> +
> +	/* RES_MCQ_SQD_8 */
> +	ufs_qcom_dump_regs(hba, 0x1c00, 256 * 4, "MCQ SQD 1da6c00-1da6ff0", RES_MCQ_SQD);
> +	cond_resched();
> +}

There is a lot of repetition in the ufs_qcom_dump_mcq_hci_regs()
function. Has it been considered to move the cond_resched() call into
ufs_qcom_dump_regs() such that it occurs only once in this patch?

For the ufs_qcom_dump_mcq_hci_regs() function, a table-based approach
may be appropriate since what that function does is to call
ufs_qcom_dump_regs() repeatedly but each time with different arguments.

Thanks,

Bart.