linux-kernel - Re: [PATCH v11 2/9] mmc: cavium: Add core MMC driver for Cavium SOCs

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, 3 Mar 2017 10:39:11 -0800
From:   David Daney <ddaney@...iumnetworks.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>,
        Jan Glauber <jglauber@...ium.com>
Cc:     "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Steven J . Hill" <Steven.Hill@...ium.com>,
        David Daney <david.daney@...ium.com>
Subject: Re: [PATCH v11 2/9] mmc: cavium: Add core MMC driver for Cavium SOCs

On 03/03/2017 03:47 AM, Ulf Hansson wrote:
> On 6 February 2017 at 14:39, Jan Glauber <jglauber@...ium.com> wrote:
[...]
>> +}
>> +
>> +static void prepare_ext_dma(struct mmc_host *mmc, struct mmc_request *mrq,
>> +                           union mio_emm_dma *emm_dma)
>> +{
>> +       struct cvm_mmc_slot *slot = mmc_priv(mmc);
>> +
>> +       /*
>> +        * Our MMC host hardware does not issue single commands,
>> +        * because that would require the driver and the MMC core
>> +        * to do work to determine the proper sequence of commands.
>
> I don't get this. The allowed sequence of the commands is determined
> by the SD/(e)MMC/SDIO spec and much of this knowledge is the
> responsibility of the mmc core.
>
>> +        * Instead, our hardware is superior to most other MMC bus
>
> No need to brag about your HW. Let's just describe how it works instead.
>
>> +        * hosts. The sequence of MMC commands required to execute
>> +        * a transfer are issued automatically by the bus hardware.
>
> What does this really mean? Is this about HW support for better
> dealing with data requests?

The entire comment should be removed.   It is laced with sarcasm that is 
very difficult for people not intimately familiar with the discussions 
to detect and decipher.

The real story is that the Cavium MMC hardware was designed based on a 
defective premise.  Hardwired in to the core of the MMC bus interface 
hardware is a multi-command synthesizer/sequencer that is hard coded to 
work with a limited subset of MMC-only command sequences.  This allows 
you to read data from the first few MB of an MMC device with only a 
handful of assembly language instructions contained in the on-die 
mask-ROM of the SoC.

When it comes to actually using the MMC/SD bus interface from within the 
Linux MMC framework, we have to apply a transform to the command 
sequences supplied by the Linux MMC core so that the command sequence 
synthesized by Cavium MMC hardware matches as closely as possible what 
was actually requested.  For many command sequences, the match between 
what is synthesized by the hardware and what was requested is not exact.

>
>> +        *
>> +        * - David Daney <ddaney@...ium.com>
>> +        */
>> +       emm_dma->val = 0;
>> +       emm_dma->s.bus_id = slot->bus_id;
>> +       emm_dma->s.dma_val = 1;
>> +       emm_dma->s.sector = (mrq->data->blksz == 512) ? 1 : 0;
>> +       emm_dma->s.rw = (mrq->data->flags & MMC_DATA_WRITE) ? 1 : 0;
>> +       emm_dma->s.block_cnt = mrq->data->blocks;
>> +       emm_dma->s.card_addr = mrq->cmd->arg;
>> +       if (mmc_card_mmc(mmc->card) || (mmc_card_sd(mmc->card) &&
>> +           (mmc->card->scr.cmds & SD_SCR_CMD23_SUPPORT)))
>> +               emm_dma->s.multi = 1;

Here is a prefect example.

If a multi-block transfer was requested, the hardware command 
synthesizer uses the "multi" bit to determine if it should generate a 
command sequence for a single multi-block transfer, or multiple 
single-block transfers.

Probably the MMC core would never request a multi-block transfer from a 
device that doesn't support it, but we check here just-in-case.

>> +
>> +       pr_debug("[%s] blocks: %u  multi: %d\n", (emm_dma->s.rw) ? "W" : "R",
>> +                mrq->data->blocks, emm_dma->s.multi);
>> +}
>> +