| Message ID | 20240227201857.25814-1-cmax@mailbox.org |
|---|---|
| State | Superseded |
| Delegated to: | Jaehoon Chung |
| Headers | show |
| Series | mmc: arm_pl180: Limit data transfer to U16_MAX | expand |
Quick reminder: Can anyone help to review this? Thanks! Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Tue, 27 Feb 2024 at 22:02, <cmax@mailbox.org> wrote: > From: max <cmax@mailbox.org> > > Currently fetching files bigger that cause a data transfer greater than > U16_MAX fails. > > The reason is that the specification defines the datalength register > as a 16 bit wide register, but in u-boot it is used as if it is an > 32 bit register. Therefore values greater than U16_MAX cause an > infinite loop inside u-boot. U-boot expects to get more data from > interface/hardware then it will ever get and therefore inifintely waits > for more data that will never come. > > Signed-off-by: max <cmax@mailbox.org> > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Jaehoon Chung <jh80.chung@samsung.com> > --- > drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c > index 5cf5502ed5..af2f9a5a84 100644 > --- a/drivers/mmc/arm_pl180_mmci.c > +++ b/drivers/mmc/arm_pl180_mmci.c > @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev, > u32 blksz = 0; > u32 data_ctrl = 0; > u32 data_len = (u32) (data->blocks * data->blocksize); > + assert(data_len < U16_MAX); // should be ensured by > arm_pl180_get_b_max > > if (!host->version2) { > blksz = (ffs(data->blocksize) - 1); > @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev) > return 0; > } > > +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t > blkcnt) > +{ > + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); > + struct mmc *mmc = upriv->mmc; > + > + return U16_MAX / mmc->read_bl_len; > +} > + > #ifndef CONFIG_DM_MMC > /* MMC uses open drain drivers in the enumeration phase */ > static int mmc_host_reset(struct mmc *dev) > @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = { > .send_cmd = host_request, > .set_ios = host_set_ios, > .init = mmc_host_reset, > + .get_b_max = arm_pl180_get_b_max, > }; > > /* > @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = { > .send_cmd = dm_host_request, > .set_ios = dm_host_set_ios, > .get_cd = dm_mmc_getcd, > + .get_b_max = arm_pl180_get_b_max, > }; > > static int arm_pl180_mmc_of_to_plat(struct udevice *dev) > -- > 2.43.0 > >
+ @Simon Best Regards, *Lean Sheng Tan* 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany Email: sheng.tan@9elements.com Phone: *+49 234 68 94 188 <+492346894188>* Mobile: *+49 176 76 113842 <+4917676113842>* Registered office: Bochum Commercial register: Amtsgericht Bochum, HRB 17519 Management: Sebastian German, Eray Bazaar Data protection information according to Art. 13 GDPR <https://9elements.com/privacy> On Mon 4. Mar 2024 at 16:39, Lean Sheng Tan <sheng.tan@9elements.com> wrote: > Quick reminder: > Can anyone help to review this? > Thanks! > > Best Regards, > *Lean Sheng Tan* > > > > 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany > Email: sheng.tan@9elements.com > Phone: *+49 234 68 94 188 <+492346894188>* > Mobile: *+49 176 76 113842 <+4917676113842>* > > Registered office: Bochum > Commercial register: Amtsgericht Bochum, HRB 17519 > Management: Sebastian German, Eray Bazaar > > Data protection information according to Art. 13 GDPR > <https://9elements.com/privacy> > > > On Tue, 27 Feb 2024 at 22:02, <cmax@mailbox.org> wrote: > >> From: max <cmax@mailbox.org> >> >> Currently fetching files bigger that cause a data transfer greater than >> U16_MAX fails. >> >> The reason is that the specification defines the datalength register >> as a 16 bit wide register, but in u-boot it is used as if it is an >> 32 bit register. Therefore values greater than U16_MAX cause an >> infinite loop inside u-boot. U-boot expects to get more data from >> interface/hardware then it will ever get and therefore inifintely waits >> for more data that will never come. >> >> Signed-off-by: max <cmax@mailbox.org> >> Cc: Peng Fan <peng.fan@nxp.com> >> Cc: Jaehoon Chung <jh80.chung@samsung.com> >> --- >> drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c >> index 5cf5502ed5..af2f9a5a84 100644 >> --- a/drivers/mmc/arm_pl180_mmci.c >> +++ b/drivers/mmc/arm_pl180_mmci.c >> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev, >> u32 blksz = 0; >> u32 data_ctrl = 0; >> u32 data_len = (u32) (data->blocks * data->blocksize); >> + assert(data_len < U16_MAX); // should be ensured by >> arm_pl180_get_b_max >> >> if (!host->version2) { >> blksz = (ffs(data->blocksize) - 1); >> @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev) >> return 0; >> } >> >> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t >> blkcnt) >> +{ >> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); >> + struct mmc *mmc = upriv->mmc; >> + >> + return U16_MAX / mmc->read_bl_len; >> +} >> + >> #ifndef CONFIG_DM_MMC >> /* MMC uses open drain drivers in the enumeration phase */ >> static int mmc_host_reset(struct mmc *dev) >> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = { >> .send_cmd = host_request, >> .set_ios = host_set_ios, >> .init = mmc_host_reset, >> + .get_b_max = arm_pl180_get_b_max, >> }; >> >> /* >> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = >> { >> .send_cmd = dm_host_request, >> .set_ios = dm_host_set_ios, >> .get_cd = dm_mmc_getcd, >> + .get_b_max = arm_pl180_get_b_max, >> }; >> >> static int arm_pl180_mmc_of_to_plat(struct udevice *dev) >> -- >> 2.43.0 >> >>
Hi, On 2/28/24 05:18, cmax@mailbox.org wrote: > From: max <cmax@mailbox.org> > > Currently fetching files bigger that cause a data transfer greater than > U16_MAX fails. > > The reason is that the specification defines the datalength register > as a 16 bit wide register, but in u-boot it is used as if it is an > 32 bit register. Therefore values greater than U16_MAX cause an > infinite loop inside u-boot. U-boot expects to get more data from > interface/hardware then it will ever get and therefore inifintely waits > for more data that will never come. > > Signed-off-by: max <cmax@mailbox.org> Could you add your full name as Signed-off's tag? > Cc: Peng Fan <peng.fan@nxp.com> > Cc: Jaehoon Chung <jh80.chung@samsung.com> > --- > drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c > index 5cf5502ed5..af2f9a5a84 100644 > --- a/drivers/mmc/arm_pl180_mmci.c > +++ b/drivers/mmc/arm_pl180_mmci.c > @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev, > u32 blksz = 0; > u32 data_ctrl = 0; > u32 data_len = (u32) (data->blocks * data->blocksize); > + assert(data_len < U16_MAX); // should be ensured by arm_pl180_get_b_max Add the comment at above with "/* ... */" /* Should be ensured by arm_pl180_get_b_max */ assert(data_len < U16_MAX); Best Regards, Jaehoon Chung > > if (!host->version2) { > blksz = (ffs(data->blocksize) - 1); > @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev) > return 0; > } > > +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt) > +{ > + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); > + struct mmc *mmc = upriv->mmc; > + > + return U16_MAX / mmc->read_bl_len; > +} > + > #ifndef CONFIG_DM_MMC > /* MMC uses open drain drivers in the enumeration phase */ > static int mmc_host_reset(struct mmc *dev) > @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = { > .send_cmd = host_request, > .set_ios = host_set_ios, > .init = mmc_host_reset, > + .get_b_max = arm_pl180_get_b_max, > }; > > /* > @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = { > .send_cmd = dm_host_request, > .set_ios = dm_host_set_ios, > .get_cd = dm_mmc_getcd, > + .get_b_max = arm_pl180_get_b_max, > }; > > static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c index 5cf5502ed5..af2f9a5a84 100644 --- a/drivers/mmc/arm_pl180_mmci.c +++ b/drivers/mmc/arm_pl180_mmci.c @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev, u32 blksz = 0; u32 data_ctrl = 0; u32 data_len = (u32) (data->blocks * data->blocksize); + assert(data_len < U16_MAX); // should be ensured by arm_pl180_get_b_max if (!host->version2) { blksz = (ffs(data->blocksize) - 1); @@ -358,6 +359,14 @@ static int host_set_ios(struct mmc *dev) return 0; } +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt) +{ + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct mmc *mmc = upriv->mmc; + + return U16_MAX / mmc->read_bl_len; +} + #ifndef CONFIG_DM_MMC /* MMC uses open drain drivers in the enumeration phase */ static int mmc_host_reset(struct mmc *dev) @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = { .send_cmd = host_request, .set_ios = host_set_ios, .init = mmc_host_reset, + .get_b_max = arm_pl180_get_b_max, }; /* @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = { .send_cmd = dm_host_request, .set_ios = dm_host_set_ios, .get_cd = dm_mmc_getcd, + .get_b_max = arm_pl180_get_b_max, }; static int arm_pl180_mmc_of_to_plat(struct udevice *dev)