mmc-uclass: spl: upriv->mmc override by host descriptor address
PATCH 1/1 mmc-uclass: spl: upriv->mmc override by host descriptor address
This specific issue observed with SPL_DM_MMC in falcon mode on rk3288 which used dw_mmc.c driver.
Bug details:
-----------
based on the falcon configuration, SPL trying to read the kernel from specified sectors, while mmc sending multi-block command(CMD18) the
host descriptor address here next_addr(from dw_mmc.c, on below Bug log at blk_cnt = 938) is trying to override upriv(which further corrupting upriv->mmc) since after mmc pointing to wrong address which is causing next commands(CMD12)
is unable to get the host pointer since it's reading wrong address which eventually block the booting at mmc stage.
Bug log:
-------
For blk_cnt = 946
Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fcdwmci_set_idma_desc: addr = 0x274dfc0, next_addr = 0x7e080
After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fc For blk_cnt = 938
Before: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0xff7160fcdwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0
After: mmc_get_mmc_dev: dev = 0xff716088, upriv = 0x7e088, mmc = 0x274efc0
Based on the above information, this patch is trying to allocate mmc_uclass_priv using .priv_auto_alloc_size so if it is zero, the respective uclass driver is responsible for allocating any data required. So in this scenario memory is not override between upriv->mmc and host description.
mmc_uclass_priv with ..priv_auto_alloc_size:
--------------------------------------------
For blk_cnt = 938
Before: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100dwmci_set_idma_desc: addr = 0x274efc0, next_addr = 0x7e0c0
After: mmc_get_mmc_dev: dev = 0xff71608c, upriv = 0x0, mmc = 0xff716100
---
Note:
- debug code:
https://paste.ubuntu.com/25879159/
- Bug log:
https://paste.ubuntu.com/25879138/
- Fix log:
https://paste.ubuntu.com/25879139/
drivers/mmc/mmc-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/mmc/mmc-uclass.c +4 -0
@@ -299,5 +299,9 @@ UCLASS_DRIVER(mmc) = { .id = UCLASS_MMC, .name = "mmc", .flags = DM_UC_FLAG_SEQ_ALIAS, +#ifdef CONFIG_SPL_BUILD + .priv_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#else .per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#endif };
Signature · 1 line
2.7.4
PATCH 1/1 mmc-uclass: spl: upriv->mmc override by host descriptor address
Hi Jagan,
Quoted 64 lines · Jagan Teki
This specific issue observed with
SPL_DM_MMCin falcon mode on rk3288 which useddw_mmc.cdriver.Bug details:
-----------
based on the falcon configuration, SPL trying to read the kernel from specified sectors, while mmc sending multi-block command(CMD18) the
host descriptor address herenext_addr(from dw_mmc.c, on below Bug log at blk_cnt = 938)is trying to overrideupriv(which further corrupting upriv->mmc)since after mmc pointing to wrong address which is causing next commands(CMD12)
is unable to get the host pointer since it's reading wrong address which eventually block the booting at mmc stage.Bug log:
-------
Forblk_cnt= 946
Before:mmc_get_mmc_dev: dev =0xff716088, upriv =0x7e088, mmc =0xff7160fcdwmci_set_idma_desc: addr =0x274dfc0,next_addr=0x7e080
After:mmc_get_mmc_dev: dev =0xff716088, upriv =0x7e088, mmc =0xff7160fcForblk_cnt= 938
Before:mmc_get_mmc_dev: dev =0xff716088, upriv =0x7e088, mmc =0xff7160fcdwmci_set_idma_desc: addr =0x274efc0,next_addr=0x7e0c0
After:mmc_get_mmc_dev: dev =0xff716088, upriv =0x7e088, mmc =0x274efc0Based on the above information, this patch is trying to allocate
mmc_uclass_privusing .priv_auto_alloc_size so if it is zero, the respective uclass driver is responsible for allocating any data required. So in this scenario memory is not override betweenupriv->mmcand host description.
mmc_uclass_privwith ..priv_auto_alloc_size:
--------------------------------------------
Forblk_cnt= 938
Before:mmc_get_mmc_dev: dev =0xff71608c, upriv =0x0, mmc =0xff716100dwmci_set_idma_desc: addr =0x274efc0,next_addr=0x7e0c0
After:mmc_get_mmc_dev: dev =0xff71608c, upriv =0x0, mmc =0xff716100Signed-off-by Jagan Teki <[email protected]>---
Note:
- debug code:
https://paste.ubuntu.com/25879159/
- Bug log:
https://paste.ubuntu.com/25879138/
- Fix log:
https://paste.ubuntu.com/25879139/drivers/mmc/mmc-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/mmc/mmc-uclass.c +4
-1@@ -299,5 +299,9 @@ UCLASS_DRIVER(mmc) = { .id = UCLASS_MMC, .name = "mmc", .flags = DM_UC_FLAG_SEQ_ALIAS, +#ifdef CONFIG_SPL_BUILD + .priv_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#else .per_device_auto_alloc_size = sizeof(struct mmc_uclass_priv), +#endif }; --
Quoted 2 lines
2.7.4
This is so strange, I don't really understand it.
But priv_auto_alloc_size should be used with dev_get_uclass_priv() which is what the code is using.
priv_auto_alloc_size is used for the uclass' priv-> pointer. We don't need the uclass to store anything as far as I can tell.
So I am not sure why your fix works, but it seems wrong to me.
But as I say, it is strange, and perhaps there is something else wrong.
Regards,
Simon