linux-kernel - RE: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE

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] [day] [month] [year] [list]

Message-ID: <DB6PR05MB32239F8202BF9FA1840C1942A1AF0@DB6PR05MB3223.eurprd05.prod.outlook.com>
Date:   Tue, 4 Dec 2018 22:19:14 +0000
From:   Liming Sun <lsun@...lanox.com>
To:     Alex Lemberg <Alex.Lemberg@....com>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Jaehoon Chung <jh80.chung@...sung.com>,
        David Woods <dwoods@...lanox.com>,
        Shawn Lin <shawn.lin@...k-chips.com>
CC:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device
 tree

Thanks for the comment. Yes, I actually have the same question.
The change here is to fix a condition based on the existing code. Probably a better fix is to get rid of the 'connection'...

-----Original Message-----
From: Alex Lemberg <Alex.Lemberg@....com> 
Sent: Tuesday, December 4, 2018 11:27 AM
To: Liming Sun <lsun@...lanox.com>; Ulf Hansson <ulf.hansson@...aro.org>; Jaehoon Chung <jh80.chung@...sung.com>; David Woods <dwoods@...lanox.com>; Shawn Lin <shawn.lin@...k-chips.com>
Cc: linux-mmc@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] mmc: core: control EXT_CSD_CACHE_CTRL from device tree


Hi,


On 11/20/18, 10:07 PM, "linux-mmc-owner@...r.kernel.org on behalf of Liming Sun" <linux-mmc-owner@...r.kernel.org on behalf of lsun@...lanox.com> wrote:

    Currently the EXT_CSD_CACHE_CTRL register is controlled by API
    mmc_card_broken_hpi(), which only works for the quirks settings.
    This commit enhances it to use card->ext_csd.hpi instead. This
    flag works for both the quirks and the broken-hpi configuration
    in the device tree.
    
    Signed-off-by: Liming Sun <lsun@...lanox.com>
    ---
     drivers/mmc/core/mmc.c | 3 +--
     1 file changed, 1 insertion(+), 2 deletions(-)
    
    diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
    index bc1bd2c..2f2b65c 100644
    --- a/drivers/mmc/core/mmc.c
    +++ b/drivers/mmc/core/mmc.c
    @@ -1791,8 +1791,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
     	 * If cache size is higher than 0, this indicates
     	 * the existence of cache and it can be turned on.
     	 */
    -	if (!mmc_card_broken_hpi(card) &&
    -	    card->ext_csd.cache_size > 0) {
    +	if (card->ext_csd.hpi && card->ext_csd.cache_size > 0) {


Any reason for making a connection between HPI and Cache in current driver implementation?
BTW, I missed it before, but the previous commit making this connection - is also doesn't make any sense.
Although by the spec FLUSH_CACHE is interruptible command, I don't think we should prevent devices with no HPI support, using the Cache.
Please correct me if I wrong, but the current driver implementation allows to send HPI for BKOPS and Sanitize (on timeout error) commands only - so why we should limit the FLUSH_CACHE command?


     		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
     				EXT_CSD_CACHE_CTRL, 1,
     				card->ext_csd.generic_cmd6_time);
    -- 
    1.8.3.1