From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDJ8e-0006iT-JZ for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:32:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDJ8Z-0003Hm-0r for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:32:28 -0500 Received: from ssl.serverraum.org ([88.198.40.39]:46237) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDJ8Y-0003HJ-IP for qemu-devel@nongnu.org; Wed, 06 Mar 2013 13:32:22 -0500 From: Michael Walle Date: Wed, 6 Mar 2013 19:31:55 +0100 References: <1351586664-20525-1-git-send-email-peter.maydell@linaro.org> <1351586664-20525-29-git-send-email-peter.maydell@linaro.org> In-Reply-To: <1351586664-20525-29-git-send-email-peter.maydell@linaro.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201303061931.55967.michael@walle.cc> Subject: Re: [Qemu-devel] [PATCH 28/28] hw/sd.c: add SD card save/load support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Peter Maydell , Paul Brook , Anthony Liguori Hi all, Sorry for digging out such an old thread :) but this patch introduced a memory corruption, see below. Am Dienstag 30 Oktober 2012, 09:44:24 schrieb Peter Maydell: > From: Igor Mitsyanko > > This patch updates SD card model to support save/load of card's state. > > Signed-off-by: Igor Mitsyanko > Reviewed-by: Peter Maydell > Signed-off-by: Peter Maydell > --- > hw/sd.c | 89 > +++++++++++++++++++++++++++++++++++++++++++++------------------ 1 file > changed, 64 insertions(+), 25 deletions(-) > > diff --git a/hw/sd.c b/hw/sd.c > index b2f211c..3c34d43 100644 > --- a/hw/sd.c > +++ b/hw/sd.c > @@ -55,24 +55,28 @@ typedef enum { > sd_illegal = -2, > } sd_rsp_type_t; > > +enum SDCardModes { > + sd_inactive, > + sd_card_identification_mode, > + sd_data_transfer_mode, > +}; > + > +enum SDCardStates { > + sd_inactive_state = -1, > + sd_idle_state = 0, > + sd_ready_state, > + sd_identification_state, > + sd_standby_state, > + sd_transfer_state, > + sd_sendingdata_state, > + sd_receivingdata_state, > + sd_programming_state, > + sd_disconnect_state, > +}; > + > struct SDState { > - enum { > - sd_inactive, > - sd_card_identification_mode, > - sd_data_transfer_mode, > - } mode; > - enum { > - sd_inactive_state = -1, > - sd_idle_state = 0, > - sd_ready_state, > - sd_identification_state, > - sd_standby_state, > - sd_transfer_state, > - sd_sendingdata_state, > - sd_receivingdata_state, > - sd_programming_state, > - sd_disconnect_state, > - } state; > + uint32_t mode; /* current card mode, one of SDCardModes */ > + int32_t state; /* current card state, one of SDCardStates */ > uint32_t ocr; > uint8_t scr[8]; > uint8_t cid[16]; > @@ -83,21 +87,22 @@ struct SDState { > uint32_t vhs; > bool wp_switch; > unsigned long *wp_groups; > + int32_t wpgrps_size; > uint64_t size; > - int blk_len; > + uint32_t blk_len; > uint32_t erase_start; > uint32_t erase_end; > uint8_t pwd[16]; > - int pwd_len; > - int function_group[6]; > + uint32_t pwd_len; > + uint8_t function_group[6]; > > bool spi; > - int current_cmd; > + uint8_t current_cmd; > /* True if we will handle the next command as an ACMD. Note that this > does * *not* track the APP_CMD status bit! > */ > bool expecting_acmd; > - int blk_written; > + uint32_t blk_written; > uint64_t data_start; > uint32_t data_offset; > uint8_t data[512]; > @@ -421,8 +426,9 @@ static void sd_reset(SDState *sd, BlockDriverState > *bdrv) if (sd->wp_groups) > g_free(sd->wp_groups); > sd->wp_switch = bdrv ? bdrv_is_read_only(bdrv) : false; > - sd->wp_groups = bitmap_new(sect); > - memset(sd->function_group, 0, sizeof(int) * 6); > + sd->wpgrps_size = sect; > + sd->wp_groups = bitmap_new(sd->wpgrps_size); > + memset(sd->function_group, 0, sizeof(sd->function_group)); > sd->erase_start = 0; > sd->erase_end = 0; > sd->size = size; > @@ -446,6 +452,38 @@ static const BlockDevOps sd_block_ops = { > .change_media_cb = sd_cardchange, > }; > > +static const VMStateDescription sd_vmstate = { > + .name = "sd-card", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(mode, SDState), > + VMSTATE_INT32(state, SDState), > + VMSTATE_UINT8_ARRAY(cid, SDState, 16), > + VMSTATE_UINT8_ARRAY(csd, SDState, 16), > + VMSTATE_UINT16(rca, SDState), > + VMSTATE_UINT32(card_status, SDState), > + VMSTATE_PARTIAL_BUFFER(sd_status, SDState, 1), > + VMSTATE_UINT32(vhs, SDState), > + VMSTATE_BITMAP(wp_groups, SDState, 0, wpgrps_size), > + VMSTATE_UINT32(blk_len, SDState), > + VMSTATE_UINT32(erase_start, SDState), > + VMSTATE_UINT32(erase_end, SDState), > + VMSTATE_UINT8_ARRAY(pwd, SDState, 16), > + VMSTATE_UINT32(pwd_len, SDState), > + VMSTATE_UINT8_ARRAY(function_group, SDState, 6), > + VMSTATE_UINT8(current_cmd, SDState), > + VMSTATE_BOOL(expecting_acmd, SDState), > + VMSTATE_UINT32(blk_written, SDState), > + VMSTATE_UINT64(data_start, SDState), > + VMSTATE_UINT32(data_offset, SDState), > + VMSTATE_UINT8_ARRAY(data, SDState, 512), > + VMSTATE_BUFFER_UNSAFE(buf, SDState, 1, 512), buf is dynamically allocated in the sd_init(), see also the SDState: struct SDState { [...] uint8_t *buf; bool enable; }; > + VMSTATE_BOOL(enable, SDState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > /* We do not model the chip select pin, so allow the board to select > whether card should be in SSI or MMC/SD mode. It is also up to the > board to ensure that ssi transfers only occur when the chip select > @@ -463,6 +501,7 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi) > bdrv_attach_dev_nofail(sd->bdrv, sd); > bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd); > } > + vmstate_register(NULL, -1, &sd_vmstate, sd); > return sd; > } > > @@ -576,7 +615,7 @@ static void sd_lock_command(SDState *sd) > sd->card_status |= LOCK_UNLOCK_FAILED; > return; > } > - bitmap_zero(sd->wp_groups, sd_addr_to_wpnum(sd->size) + 1); > + bitmap_zero(sd->wp_groups, sd->wpgrps_size); > sd->csd[14] &= ~0x10; > sd->card_status &= ~CARD_IS_LOCKED; > sd->pwd_len = 0; -- Michael