| Message ID | 1311635951-11047-3-git-send-email-vpalatin@chromium.org |
|---|---|
| State | New |
| Headers | show |
Hi Vincent, On 26 July 2011 01:19, Vincent Palatin <vpalatin@chromium.org> wrote: > We need to check that we are not crossing the boundaries of the card for > the current access not for the next one which might not happen. > > Signed-off-by: Vincent Palatin <vpalatin@chromium.org> > --- > hw/sd.c | 22 ++++++++++++---------- > 1 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/hw/sd.c b/hw/sd.c > index f48d589..de477fe 100644 > --- a/hw/sd.c > +++ b/hw/sd.c > @@ -1451,11 +1451,6 @@ void sd_write_data(SDState *sd, uint8_t value) > sd->data[sd->data_offset ++] = value; > if (sd->data_offset >= sd->blk_len) { > /* TODO: Check CRC before committing */ > - sd->state = sd_programming_state; > - BLK_WRITE_BLOCK(sd->data_start, sd->data_offset); > - sd->blk_written ++; > - sd->data_start += sd->blk_len; > - sd->data_offset = 0; > if (sd->data_start + sd->blk_len > sd->size) { > sd->card_status |= ADDRESS_ERROR; > break; > @@ -1464,6 +1459,11 @@ void sd_write_data(SDState *sd, uint8_t value) > sd->card_status |= WP_VIOLATION; > break; > } > + sd->state = sd_programming_state; > + BLK_WRITE_BLOCK(sd->data_start, sd->data_offset); > + sd->blk_written ++; > + sd->data_start += sd->blk_len; > + sd->data_offset = 0; > sd->csd[14] |= 0x40; > > /* Bzzzzzzztt .... Operation complete. */ > @@ -1606,17 +1606,19 @@ 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; > + ret = 0; > + break; > + } > 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; I have to look more closely at the spec but it looks like ADDRESS_ERROR might not even be the right error code for when we read/write outside of the memory, because the spec says it's for incorrect alignment. Also the first chunk of this patch is unnecessary now because I committed a patch from Peter Maydell that did the same thing. I pushed your patch 01 in the series, thanks. Cheers
diff --git a/hw/sd.c b/hw/sd.c index f48d589..de477fe 100644 --- a/hw/sd.c +++ b/hw/sd.c @@ -1451,11 +1451,6 @@ void sd_write_data(SDState *sd, uint8_t value) sd->data[sd->data_offset ++] = value; if (sd->data_offset >= sd->blk_len) { /* TODO: Check CRC before committing */ - sd->state = sd_programming_state; - BLK_WRITE_BLOCK(sd->data_start, sd->data_offset); - sd->blk_written ++; - sd->data_start += sd->blk_len; - sd->data_offset = 0; if (sd->data_start + sd->blk_len > sd->size) { sd->card_status |= ADDRESS_ERROR; break; @@ -1464,6 +1459,11 @@ void sd_write_data(SDState *sd, uint8_t value) sd->card_status |= WP_VIOLATION; break; } + sd->state = sd_programming_state; + BLK_WRITE_BLOCK(sd->data_start, sd->data_offset); + sd->blk_written ++; + sd->data_start += sd->blk_len; + sd->data_offset = 0; sd->csd[14] |= 0x40; /* Bzzzzzzztt .... Operation complete. */ @@ -1606,17 +1606,19 @@ 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; + ret = 0; + break; + } 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;
We need to check that we are not crossing the boundaries of the card for the current access not for the next one which might not happen. Signed-off-by: Vincent Palatin <vpalatin@chromium.org> --- hw/sd.c | 22 ++++++++++++---------- 1 files changed, 12 insertions(+), 10 deletions(-)