On Tue, Sep 19, 2017 at 05:09:51PM -0700, Alistair Francis wrote:
> On Tue, Sep 19, 2017 at 1:23 AM, Michael Olbrich
> <m.olbr...@pengutronix.de> wrote:
> > On Mon, Sep 18, 2017 at 02:28:26PM -0700, Alistair Francis wrote:
> >> On Sat, Sep 16, 2017 at 3:35 AM, Michael Olbrich
> >> <m.olbr...@pengutronix.de> wrote:
> >> > hw/sd/sd.c | 12 ++++++------
> >> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> >> > index ba47bff4db80..35347a5bbcde 100644
> >> > --- a/hw/sd/sd.c
> >> > +++ b/hw/sd/sd.c
> >> > @@ -1797,8 +1797,13 @@ uint8_t sd_read_data(SDState *sd)
> >> > break;
> >> >
> >> > case 18: /* CMD18: READ_MULTIPLE_BLOCK */
> >> > - if (sd->data_offset == 0)
> >> > + if (sd->data_offset == 0) {
> >> > + if (sd->data_start + io_len > sd->size) {
> >> > + sd->card_status |= ADDRESS_ERROR;
> >> > + return 0x00;
> >> > + }
> >>
> >> Why move it inside the if (sd->data_offset == 0) and not just below
> >> the ret = sd->data[sd->data_offset ++] ?
> >>
> >> > BLK_READ_BLOCK(sd->data_start, io_len);
> >
> > Mostly because of the line above. This copies the full block from the
> > backend storage to sd->data, so we need to make sure that the data is
> > actually available to fill sd->data, not if it's ok to access a certain
> > byte within sd->data.
>
> Doesn't this mean that the check is only done for the first block
> then? When data_offset is 0.
No, data_offset is reset at the end of the block. That's not visible in the
patch. Here ist the relevant hunks with a bit more context:
case 18: /* CMD18: READ_MULTIPLE_BLOCK */
- if (sd->data_offset == 0)
+ if (sd->data_offset == 0) {
+ if (sd->data_start + io_len > sd->size) {
+ sd->card_status |= ADDRESS_ERROR;
+ return 0x00;
+ }
BLK_READ_BLOCK(sd->data_start, io_len);
+ }
ret = sd->data[sd->data_offset ++];
if (sd->data_offset >= io_len) {
sd->data_start += io_len;
sd->data_offset = 0;
[...]
-
- if (sd->data_start + io_len > sd->size) {
- sd->card_status |= ADDRESS_ERROR;
- break;
- }
}
break;
As you can see, the old check was inside the block that resets data_offset
to zero. This patch just delays exactly that check to the beginning of the
next access.
Regards,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
