linux-kernel - Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <74bedcff32df0c93c14b004814017f3344247528.camel@mediatek.com>
Date:   Thu, 23 Feb 2023 14:13:38 +0000
From:   Powen Kao (高伯文) <Powen.Kao@...iatek.com>
To:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "quic_ziqichen@...cinc.com" <quic_ziqichen@...cinc.com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "avri.altman@....com" <avri.altman@....com>,
        "bvanassche@....org" <bvanassche@....org>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>
CC:     Peter Wang (王信友) 
        <peter.wang@...iatek.com>,
        Eddie Huang (黃智傑) 
        <eddie.huang@...iatek.com>,
        Jiajie Hao (郝加节) 
        <jiajie.hao@...iatek.com>,
        CC Chou (周志杰) <cc.chou@...iatek.com>,
        Alice Chao (趙珮均) 
        <Alice.Chao@...iatek.com>,
        wsd_upstream <wsd_upstream@...iatek.com>,
        Chun-Hung Wu (巫駿宏) 
        <Chun-hung.Wu@...iatek.com>,
        Chaotian Jing (井朝天) 
        <Chaotian.Jing@...iatek.com>,
        Naomi Chu (朱詠田) <Naomi.Chu@...iatek.com>,
        Stanley Chu (朱原陞) 
        <stanley.chu@...iatek.com>,
        Mason Zhang (章辉) <Mason.Zhang@...iatek.com>
Subject: Re: [PATCH v2 4/7] scsi: ufs: core: Add hwq print for debug

Hi Ziqi,

Thanks for ur comments.

This piece of code successfully dump relevent registers on our
platform. As you know, mcq error handling flow is not ready yet so the
insertion point might not seems to be reasonable. 

Maybe drop this patch for now, I will send it later with error handling
patches.


On Thu, 2023-02-23 at 18:14 +0800, Ziqi Chen wrote:
> Hi Po-Wen,
> 
> On 2/22/2023 11:04 AM, Po-Wen Kao wrote:
> > +void ufshcd_mcq_print_hwqs(struct ufs_hba *hba, unsigned long
> > bitmap)
> > +{
> > +	int id, i;
> > +	char prefix[15];
> > +
> > +	if (!is_mcq_enabled(hba))
> > +		return;
> > +
> > +	for_each_set_bit(id, &bitmap, hba->nr_hw_queues) {
> > +		snprintf(prefix, sizeof(prefix), "q%d SQCFG: ", id);
> > +		ufshcd_hex_dump(prefix,
> > +			hba->mcq_base + MCQ_QCFG_SIZE * id,
> > MCQ_QCFG_SQ_SIZE);
> 
> Is your purpose dump per hardware queue registers here?  If yes, why 
> don't use ufsmcq_readl() to save to a buffer and then use
> ufshcd_hex_dump()
> 
> to dump ? Are you sure ufshcd_hex_dump() can dump register directly?
> 
> > +
> > +		snprintf(prefix, sizeof(prefix), "q%d CQCFG: ", id);
> > +		ufshcd_hex_dump(prefix,
> > +			hba->mcq_base + MCQ_QCFG_SIZE * id +
> > MCQ_QCFG_SQ_SIZE, MCQ_QCFG_CQ_SIZE);
> 
> Same to above comment.
> > +
> > +		for (i = 0; i < OPR_MAX ; i++) {
> > +			snprintf(prefix, sizeof(prefix), "q%d OPR%d: ",
> > id, i);
> > +			ufshcd_hex_dump(prefix, mcq_opr_base(hba, i,
> > id), mcq_opr_size[i]);
> 
> Same.
> > +		}
> > +	}
> > +}
> > +
> > 
> >   
> > @@ -574,7 +569,16 @@ void ufshcd_print_trs(struct ufs_hba *hba,
> > unsigned long bitmap, bool pr_prdt)
> >   		if (pr_prdt)
> >   			ufshcd_hex_dump("UPIU PRDT: ", lrbp-
> > >ucd_prdt_ptr,
> >   				ufshcd_sg_entry_size(hba) *
> > prdt_length);
> > +
> > +		if (is_mcq_enabled(hba)) {
> > +			cmd = lrbp->cmd;
> > +			if (!cmd)
> > +				return;
> > +			hwq = ufshcd_mcq_req_to_hwq(hba,
> > scsi_cmd_to_rq(cmd));
> > +			ufshcd_mcq_print_hwqs(hba, 1 << hwq->id);
> 
> Calling registers dump function in ufshcd_print_trs() is not
> reasonable, 
> eg.. for each aborted request, it would print out all hwq registers, 
> it's not make sense.
> 
> I think we should move it out of ufshcd_print_trs().
> 
> > +		}
> >   	}
> > +
> >   }
> 
> 
> Best Regards,
> 
> Ziqi
>