linux-kernel - Re: [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr

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]

Date:   Fri, 6 Aug 2021 09:37:08 -0700
From:   Bart Van Assche <bvanassche@....org>
To:     Kiwoong Kim <kwmad.kim@...sung.com>, linux-scsi@...r.kernel.org,
        linux-kernel@...r.kernel.org, alim.akhtar@...sung.com,
        avri.altman@....com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com, beanhuo@...ron.com,
        cang@...eaurora.org, adrian.hunter@...el.com, sc.suh@...sung.com,
        hy50.seo@...sung.com, sh425.lee@...sung.com,
        bhoon95.kim@...sung.com
Subject: Re: [RFC PATCH v1 2/2] scsi: ufs: ufs-exynos: implement exynos isr

On 8/5/21 11:34 PM, Kiwoong Kim wrote:
> Based on some events in the real world

Which events? Please clarify.

> I implement
> this to block the host's working in some abnormal
> conditions using an vendor specific interrupt for
> cases that some contexts of a pending request in the
> host isn't the same with those of its corresponding UPIUs
> if they should have been the same exactly.

The entire patch description sounds very vague to me. Please make the 
description more clear.

> +enum exynos_ufs_vs_interrupt {
> +	/*
> +	 * This occurs when information of a pending request isn't
> +	 * the same with incoming UPIU for the request. For example,
> +	 * if UFS driver rings with task tag #1, subsequential UPIUs
> +	 * for this must have one as the value of task tag. But if
> +	 * it's corrutped until the host receives it or incoming UPIUs
> +	 * has an unexpected value for task tag, this raises.
> +	 */
> +	RX_UPIU_HIT_ERROR	= 1 << 19,
> +};

The above description needs to be improved. If a request is submitted 
with task tag one, only one UPIU can have that task tag instead of all 
subsequent UPIUs.

>   	hci_writel(ufs, UFS_SW_RST_MASK, HCI_SW_RST);
> -
>   	do {
>   		if (!(hci_readl(ufs, HCI_SW_RST) & UFS_SW_RST_MASK))
> -			goto out;
> +			return 0;
>   	} while (time_before(jiffies, timeout));

Since the above loop is a busy-waiting loop, please insert an msleep() 
or cpu_relax() call.

> +	 * some unexpected events could happen, such as tranferring
                                                         ^^^^^^^^^^^
Please fix the spelling of this word.

Thanks,

Bart.