From 3b2a067b98b45f7a7dafe21c34a3ae744c697f0f Mon Sep 17 00:00:00 2001 From: Shawn Lin Date: Fri, 2 Sep 2016 12:14:37 +0800 Subject: [PATCH] mmc: dw_mmc: avoid race condition of cpu and IDMAC We could see an obvious race condition by test that the former write operation by IDMAC aiming to clear OWN bit reach right after the later configuration of the same desc, which makes the IDMAC be in SUSPEND state as the OWN bit was cleared by the asynchronous write operation of IDMAC. The bug can be very easy reproduced on RK3288 or similar when we reduce the running rate of system buses and keep the CPU running faster. So as two separate masters, IDMAC and cpu write the same descriptor stored on the same address, and this should be protected by adding check of OWN bit before preparing new descriptors. Signed-off-by: Shawn Lin Signed-off-by: Jaehoon Chung Signed-off-by: Ulf Hansson --- drivers/mmc/host/dw_mmc.c | 328 ++++++++++++++++++++++---------------- 1 file changed, 189 insertions(+), 139 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index ff9b0fcf8300..38099bafcd4c 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -467,145 +467,6 @@ static void dw_mci_dmac_complete_dma(void *arg) } } -static inline void dw_mci_prepare_desc64(struct dw_mci *host, - struct mmc_data *data, - unsigned int sg_len) -{ - unsigned int desc_len; - struct idmac_desc_64addr *desc_first, *desc_last, *desc; - int i; - - desc_first = desc_last = desc = host->sg_cpu; - - for (i = 0; i < sg_len; i++) { - unsigned int length = sg_dma_len(&data->sg[i]); - - u64 mem_addr = sg_dma_address(&data->sg[i]); - - for ( ; length ; desc++) { - desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ? - length : DW_MCI_DESC_DATA_LENGTH; - - length -= desc_len; - - /* - * Set the OWN bit and disable interrupts - * for this descriptor - */ - desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC | - IDMAC_DES0_CH; - - /* Buffer length */ - IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len); - - /* Physical address to DMA to/from */ - desc->des4 = mem_addr & 0xffffffff; - desc->des5 = mem_addr >> 32; - - /* Update physical address for the next desc */ - mem_addr += desc_len; - - /* Save pointer to the last descriptor */ - desc_last = desc; - } - } - - /* Set first descriptor */ - desc_first->des0 |= IDMAC_DES0_FD; - - /* Set last descriptor */ - desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC); - desc_last->des0 |= IDMAC_DES0_LD; -} - - -static inline void dw_mci_prepare_desc32(struct dw_mci *host, - struct mmc_data *data, - unsigned int sg_len) -{ - unsigned int desc_len; - struct idmac_desc *desc_first, *desc_last, *desc; - int i; - - desc_first = desc_last = desc = host->sg_cpu; - - for (i = 0; i < sg_len; i++) { - unsigned int length = sg_dma_len(&data->sg[i]); - - u32 mem_addr = sg_dma_address(&data->sg[i]); - - for ( ; length ; desc++) { - desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ? - length : DW_MCI_DESC_DATA_LENGTH; - - length -= desc_len; - - /* - * Set the OWN bit and disable interrupts - * for this descriptor - */ - desc->des0 = cpu_to_le32(IDMAC_DES0_OWN | - IDMAC_DES0_DIC | - IDMAC_DES0_CH); - - /* Buffer length */ - IDMAC_SET_BUFFER1_SIZE(desc, desc_len); - - /* Physical address to DMA to/from */ - desc->des2 = cpu_to_le32(mem_addr); - - /* Update physical address for the next desc */ - mem_addr += desc_len; - - /* Save pointer to the last descriptor */ - desc_last = desc; - } - } - - /* Set first descriptor */ - desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD); - - /* Set last descriptor */ - desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH | - IDMAC_DES0_DIC)); - desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD); -} - -static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len) -{ - u32 temp; - - if (host->dma_64bit_address == 1) - dw_mci_prepare_desc64(host, host->data, sg_len); - else - dw_mci_prepare_desc32(host, host->data, sg_len); - - /* drain writebuffer */ - wmb(); - - /* Make sure to reset DMA in case we did PIO before this */ - dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET); - dw_mci_idmac_reset(host); - - /* Select IDMAC interface */ - temp = mci_readl(host, CTRL); - temp |= SDMMC_CTRL_USE_IDMAC; - mci_writel(host, CTRL, temp); - - /* drain writebuffer */ - wmb(); - - /* Enable the IDMAC */ - temp = mci_readl(host, BMOD); - temp |= SDMMC_IDMAC_ENABLE | SDMMC_IDMAC_FB; - mci_writel(host, BMOD, temp); - - /* Start it running */ - mci_writel(host, PLDMND, 1); - - return 0; -} - static int dw_mci_idmac_init(struct dw_mci *host) { int i; @@ -680,6 +541,195 @@ static int dw_mci_idmac_init(struct dw_mci *host) return 0; } +static inline int dw_mci_prepare_desc64(struct dw_mci *host, + struct mmc_data *data, + unsigned int sg_len) +{ + unsigned int desc_len; + struct idmac_desc_64addr *desc_first, *desc_last, *desc; + unsigned long timeout; + int i; + + desc_first = desc_last = desc = host->sg_cpu; + + for (i = 0; i < sg_len; i++) { + unsigned int length = sg_dma_len(&data->sg[i]); + + u64 mem_addr = sg_dma_address(&data->sg[i]); + + for ( ; length ; desc++) { + desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ? + length : DW_MCI_DESC_DATA_LENGTH; + + length -= desc_len; + + /* + * Wait for the former clear OWN bit operation + * of IDMAC to make sure that this descriptor + * isn't still owned by IDMAC as IDMAC's write + * ops and CPU's read ops are asynchronous. + */ + timeout = jiffies + msecs_to_jiffies(100); + while (readl(&desc->des0) & IDMAC_DES0_OWN) { + if (time_after(jiffies, timeout)) + goto err_own_bit; + udelay(10); + } + + /* + * Set the OWN bit and disable interrupts + * for this descriptor + */ + desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC | + IDMAC_DES0_CH; + + /* Buffer length */ + IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, desc_len); + + /* Physical address to DMA to/from */ + desc->des4 = mem_addr & 0xffffffff; + desc->des5 = mem_addr >> 32; + + /* Update physical address for the next desc */ + mem_addr += desc_len; + + /* Save pointer to the last descriptor */ + desc_last = desc; + } + } + + /* Set first descriptor */ + desc_first->des0 |= IDMAC_DES0_FD; + + /* Set last descriptor */ + desc_last->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC); + desc_last->des0 |= IDMAC_DES0_LD; + + return 0; +err_own_bit: + /* restore the descriptor chain as it's polluted */ + dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n"); + memset(host->sg_cpu, 0, PAGE_SIZE); + dw_mci_idmac_init(host); + return -EINVAL; +} + + +static inline int dw_mci_prepare_desc32(struct dw_mci *host, + struct mmc_data *data, + unsigned int sg_len) +{ + unsigned int desc_len; + struct idmac_desc *desc_first, *desc_last, *desc; + unsigned long timeout; + int i; + + desc_first = desc_last = desc = host->sg_cpu; + + for (i = 0; i < sg_len; i++) { + unsigned int length = sg_dma_len(&data->sg[i]); + + u32 mem_addr = sg_dma_address(&data->sg[i]); + + for ( ; length ; desc++) { + desc_len = (length <= DW_MCI_DESC_DATA_LENGTH) ? + length : DW_MCI_DESC_DATA_LENGTH; + + length -= desc_len; + + /* + * Wait for the former clear OWN bit operation + * of IDMAC to make sure that this descriptor + * isn't still owned by IDMAC as IDMAC's write + * ops and CPU's read ops are asynchronous. + */ + timeout = jiffies + msecs_to_jiffies(100); + while (readl(&desc->des0) & + cpu_to_le32(IDMAC_DES0_OWN)) { + if (time_after(jiffies, timeout)) + goto err_own_bit; + udelay(10); + } + + /* + * Set the OWN bit and disable interrupts + * for this descriptor + */ + desc->des0 = cpu_to_le32(IDMAC_DES0_OWN | + IDMAC_DES0_DIC | + IDMAC_DES0_CH); + + /* Buffer length */ + IDMAC_SET_BUFFER1_SIZE(desc, desc_len); + + /* Physical address to DMA to/from */ + desc->des2 = cpu_to_le32(mem_addr); + + /* Update physical address for the next desc */ + mem_addr += desc_len; + + /* Save pointer to the last descriptor */ + desc_last = desc; + } + } + + /* Set first descriptor */ + desc_first->des0 |= cpu_to_le32(IDMAC_DES0_FD); + + /* Set last descriptor */ + desc_last->des0 &= cpu_to_le32(~(IDMAC_DES0_CH | + IDMAC_DES0_DIC)); + desc_last->des0 |= cpu_to_le32(IDMAC_DES0_LD); + + return 0; +err_own_bit: + /* restore the descriptor chain as it's polluted */ + dev_dbg(host->dev, "desciptor is still owned by IDMAC.\n"); + memset(host->sg_cpu, 0, PAGE_SIZE); + dw_mci_idmac_init(host); + return -EINVAL; +} + +static int dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len) +{ + u32 temp; + int ret; + + if (host->dma_64bit_address == 1) + ret = dw_mci_prepare_desc64(host, host->data, sg_len); + else + ret = dw_mci_prepare_desc32(host, host->data, sg_len); + + if (ret) + goto out; + + /* drain writebuffer */ + wmb(); + + /* Make sure to reset DMA in case we did PIO before this */ + dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET); + dw_mci_idmac_reset(host); + + /* Select IDMAC interface */ + temp = mci_readl(host, CTRL); + temp |= SDMMC_CTRL_USE_IDMAC; + mci_writel(host, CTRL, temp); + + /* drain writebuffer */ + wmb(); + + /* Enable the IDMAC */ + temp = mci_readl(host, BMOD); + temp |= SDMMC_IDMAC_ENABLE | SDMMC_IDMAC_FB; + mci_writel(host, BMOD, temp); + + /* Start it running */ + mci_writel(host, PLDMND, 1); + +out: + return ret; +} + static const struct dw_mci_dma_ops dw_mci_idmac_ops = { .init = dw_mci_idmac_init, .start = dw_mci_idmac_start_dma,