* Re: [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account
2024-10-20 14:29 [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account Aurelien Jarno
@ 2024-10-21 6:07 ` Anand Moon
2024-10-21 15:26 ` Jing Luo
2024-10-22 12:42 ` Ulf Hansson
2 siblings, 0 replies; 5+ messages in thread
From: Anand Moon @ 2024-10-21 6:07 UTC (permalink / raw)
To: Aurelien Jarno
Cc: William Qiu, open list:RISC-V MISC SOC SUPPORT, Jaehoon Chung,
Ulf Hansson, Sam Protsenko,
open list:SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER, open list,
Ron Economos, Jing Luo, stable
Hi Aurelien,
On Sun, 20 Oct 2024 at 20:01, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> The Synopsys DesignWare mmc controller on the JH7110 SoC
> (dw_mmc-starfive.c driver) is using a 32-bit IDMAC address bus width,
> and thus requires the use of SWIOTLB.
>
> The commit 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages
> bigger than 4K") increased the max_seq_size, even for 4K pages, causing
> "swiotlb buffer is full" to happen because swiotlb can only handle a
> memory size up to 256kB only.
>
> Fix the issue, by making sure the dw_mmc driver doesn't use segments
> bigger than what SWIOTLB can handle.
>
> Reported-by: Ron Economos <re@w6rz.net>
> Reported-by: Jing Luo <jing@jing.rocks>
> Fixes: 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages bigger than 4K")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
Please add my
Tested-by: Anand Moon <linux.amoon@gmail.com>
Thanks for fixing the warning below.
[ 511.837216][ T148] dwmmc_starfive 16020000.mmc: swiotlb buffer is
full (sz: 290816 bytes), total 65536 (slots), used 246 (slots)
[ 511.837423][ C0] dwmmc_starfive 16020000.mmc: swiotlb buffer is
full (sz: 278528 bytes), total 65536 (slots), used 222 (slots)
[ 511.916951][ C0] dwmmc_starfive 16020000.mmc: swiotlb buffer is
full (sz: 290816 bytes), total 65536 (slots), used 24 (slots)
[ 516.803916][ T575] dwmmc_starfive 16020000.mmc: swiotlb buffer is
full (sz: 507904 bytes), total 65536 (slots), used 122 (slots)
[ 516.805450][ C0] dwmmc_starfive 16020000.mmc: swiotlb buffer is
full (sz: 507904 bytes), total 65536 (slots), used 364 (slots)
Thanks
-Anand
> drivers/mmc/host/dw_mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 41e451235f637..dc0d6201f7b73 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2958,7 +2958,8 @@ static int dw_mci_init_slot(struct dw_mci *host)
> mmc->max_segs = host->ring_size;
> mmc->max_blk_size = 65535;
> mmc->max_req_size = DW_MCI_DESC_DATA_LENGTH * host->ring_size;
> - mmc->max_seg_size = mmc->max_req_size;
> + mmc->max_seg_size =
> + min_t(size_t, mmc->max_req_size, dma_max_mapping_size(host->dev));
> mmc->max_blk_count = mmc->max_req_size / 512;
> } else if (host->use_dma == TRANS_MODE_EDMAC) {
> mmc->max_segs = 64;
> --
> 2.45.2
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account
2024-10-20 14:29 [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account Aurelien Jarno
2024-10-21 6:07 ` Anand Moon
@ 2024-10-21 15:26 ` Jing Luo
2024-10-22 12:42 ` Ulf Hansson
2 siblings, 0 replies; 5+ messages in thread
From: Jing Luo @ 2024-10-21 15:26 UTC (permalink / raw)
To: Aurelien Jarno
Cc: William Qiu, linux-riscv, Jaehoon Chung, Ulf Hansson,
Sam Protsenko, linux-mmc, linux-kernel, Ron Economos, stable
On 2024-10-20 23:29, Aurelien Jarno wrote:
> The Synopsys DesignWare mmc controller on the JH7110 SoC
> (dw_mmc-starfive.c driver) is using a 32-bit IDMAC address bus width,
> and thus requires the use of SWIOTLB.
>
> The commit 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages
> bigger than 4K") increased the max_seq_size, even for 4K pages, causing
> "swiotlb buffer is full" to happen because swiotlb can only handle a
> memory size up to 256kB only.
>
> Fix the issue, by making sure the dw_mmc driver doesn't use segments
> bigger than what SWIOTLB can handle.
>
> Reported-by: Ron Economos <re@w6rz.net>
> Reported-by: Jing Luo <jing@jing.rocks>
> Fixes: 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages
> bigger than 4K")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Feel free to add:
Tested-by: Jing Luo <jing@jing.rocks>
This patch not only fixes the kernel log spam by dwmmc_starfive
reporting "swiotlb buffer is full", but also seems to have fixed a
serious bug that causes data corruption on emmc (as I reported to Debian
[1]), which at least can be observed on both Visionfive 2 and Star64
boards. To add a cherry on the top, with this patch applied, I see
massive performance improvement (sequential rw speed) on emmc: with a
quick-and-dirty test using `dd bs=1M`, on Visionfive 2, it goes from
28MB/s to 42MB/s (+50%); on Star64, it goes from 13MB/s to 46MB/s
(+253%).
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1085425
Thanks & cheers,
Jing Luo
> ---
> drivers/mmc/host/dw_mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 41e451235f637..dc0d6201f7b73 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2958,7 +2958,8 @@ static int dw_mci_init_slot(struct dw_mci *host)
> mmc->max_segs = host->ring_size;
> mmc->max_blk_size = 65535;
> mmc->max_req_size = DW_MCI_DESC_DATA_LENGTH * host->ring_size;
> - mmc->max_seg_size = mmc->max_req_size;
> + mmc->max_seg_size =
> + min_t(size_t, mmc->max_req_size,
> dma_max_mapping_size(host->dev));
> mmc->max_blk_count = mmc->max_req_size / 512;
> } else if (host->use_dma == TRANS_MODE_EDMAC) {
> mmc->max_segs = 64;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account
2024-10-20 14:29 [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account Aurelien Jarno
2024-10-21 6:07 ` Anand Moon
2024-10-21 15:26 ` Jing Luo
@ 2024-10-22 12:42 ` Ulf Hansson
2024-10-27 12:16 ` Emil Renner Berthing
2 siblings, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2024-10-22 12:42 UTC (permalink / raw)
To: Aurelien Jarno
Cc: William Qiu, open list:RISC-V MISC SOC SUPPORT, Jaehoon Chung,
Sam Protsenko, open list:SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER,
open list, Ron Economos, Jing Luo, stable, Adam Green, Shawn Lin,
sydarn, Arnd Bergmann
+ Adam, Arnd, Shawn-Lin, sydarn
On Sun, 20 Oct 2024 at 16:30, Aurelien Jarno <aurelien@aurel32.net> wrote:
>
> The Synopsys DesignWare mmc controller on the JH7110 SoC
> (dw_mmc-starfive.c driver) is using a 32-bit IDMAC address bus width,
> and thus requires the use of SWIOTLB.
>
> The commit 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages
> bigger than 4K") increased the max_seq_size, even for 4K pages, causing
> "swiotlb buffer is full" to happen because swiotlb can only handle a
> memory size up to 256kB only.
>
> Fix the issue, by making sure the dw_mmc driver doesn't use segments
> bigger than what SWIOTLB can handle.
>
> Reported-by: Ron Economos <re@w6rz.net>
> Reported-by: Jing Luo <jing@jing.rocks>
> Fixes: 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages bigger than 4K")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Thanks for working on this! Looks like we have managed to mess things
up. Besides the issue that you have been working on to fix, apparently
there seems to be another one too [1].
Unfortunately, $subject patch doesn't seem to fix the problem in [1],
as has been reported by Adam.
I have looped in some more people to this thread, hopefully we agree
on how this should be fixed properly. Otherwise, I tend to say that we
should simply revert the offending commit and start over.
Kind regards
Uffe
[1]
https://lore.kernel.org/all/CAC8uq=Ppnmv98mpa1CrWLawWoPnu5abtU69v-=G-P7ysATQ2Pw@mail.gmail.com/
> ---
> drivers/mmc/host/dw_mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 41e451235f637..dc0d6201f7b73 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2958,7 +2958,8 @@ static int dw_mci_init_slot(struct dw_mci *host)
> mmc->max_segs = host->ring_size;
> mmc->max_blk_size = 65535;
> mmc->max_req_size = DW_MCI_DESC_DATA_LENGTH * host->ring_size;
> - mmc->max_seg_size = mmc->max_req_size;
> + mmc->max_seg_size =
> + min_t(size_t, mmc->max_req_size, dma_max_mapping_size(host->dev));
> mmc->max_blk_count = mmc->max_req_size / 512;
> } else if (host->use_dma == TRANS_MODE_EDMAC) {
> mmc->max_segs = 64;
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: dw_mmc: take SWIOTLB memory size limitation into account
2024-10-22 12:42 ` Ulf Hansson
@ 2024-10-27 12:16 ` Emil Renner Berthing
0 siblings, 0 replies; 5+ messages in thread
From: Emil Renner Berthing @ 2024-10-27 12:16 UTC (permalink / raw)
To: Ulf Hansson, Aurelien Jarno
Cc: William Qiu, open list:RISC-V MISC SOC SUPPORT, Jaehoon Chung,
Sam Protsenko, open list:SYNOPSYS DESIGNWARE MMC/SD/SDIO DRIVER,
open list, Ron Economos, Jing Luo, stable, Adam Green, Shawn Lin,
sydarn, Arnd Bergmann
Ulf Hansson wrote:
> + Adam, Arnd, Shawn-Lin, sydarn
>
>
> On Sun, 20 Oct 2024 at 16:30, Aurelien Jarno <aurelien@aurel32.net> wrote:
> >
> > The Synopsys DesignWare mmc controller on the JH7110 SoC
> > (dw_mmc-starfive.c driver) is using a 32-bit IDMAC address bus width,
> > and thus requires the use of SWIOTLB.
> >
> > The commit 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages
> > bigger than 4K") increased the max_seq_size, even for 4K pages, causing
> > "swiotlb buffer is full" to happen because swiotlb can only handle a
> > memory size up to 256kB only.
> >
> > Fix the issue, by making sure the dw_mmc driver doesn't use segments
> > bigger than what SWIOTLB can handle.
> >
> > Reported-by: Ron Economos <re@w6rz.net>
> > Reported-by: Jing Luo <jing@jing.rocks>
> > Fixes: 8396c793ffdf ("mmc: dw_mmc: Fix IDMAC operation with pages bigger than 4K")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
>
> Thanks for working on this!
+1
> Looks like we have managed to mess things
> up. Besides the issue that you have been working on to fix, apparently
> there seems to be another one too [1].
>
> Unfortunately, $subject patch doesn't seem to fix the problem in [1],
> as has been reported by Adam.
>
> I have looped in some more people to this thread, hopefully we agree
> on how this should be fixed properly. Otherwise, I tend to say that we
> should simply revert the offending commit and start over.
Yes, unfortunately this patch also doesn't fix MMC when running 6.12-rc4 on the
StarFive VisionFive V1 (JH7100 SoC).
/Emil
^ permalink raw reply [flat|nested] 5+ messages in thread