From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42681) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHrOC-0000vz-8R for qemu-devel@nongnu.org; Tue, 28 Jun 2016 07:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHrO6-0007BT-5d for qemu-devel@nongnu.org; Tue, 28 Jun 2016 07:41:11 -0400 Received: from mx4-phx2.redhat.com ([209.132.183.25]:46110) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHrO5-0007BO-TZ for qemu-devel@nongnu.org; Tue, 28 Jun 2016 07:41:06 -0400 Date: Tue, 28 Jun 2016 07:41:01 -0400 (EDT) From: Paolo Bonzini Message-ID: <1655778647.2615556.1467114061506.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1467103170-5784-1-git-send-email-pbonzini@redhat.com> <1467103170-5784-4-git-send-email-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/3] m25p80: change cur_addr to 32 bit integer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?C=C3=A9dric?= Le Goater Cc: qemu-devel@nongnu.org > On 06/28/2016 10:39 AM, Paolo Bonzini wrote: > > The maximum amount of storage that can be addressed by the m25p80 comma= nd > > set is 4 GiB. However, cur_addr is currently a 64-bit integer. To avo= id > > further problems related to sign extension of signed 32-bit integer > > expressions, change cur_addr to a 32 bit integer. Preserve migration > > format by adding a dummy 4-byte field in place of the (big-endian) > > high four bytes in the formerly 64-bit cur_addr field. >=20 > I do not think that migration ever worked before. did it ? Who knows. :) But it is pretty easy to not break it further... Paolo > > Signed-off-by: Paolo Bonzini >=20 > Reviewed-by: C=C3=A9dric Le Goater >=20 > > --- > > hw/block/m25p80.c | 15 ++++++++------- > > 1 file changed, 8 insertions(+), 7 deletions(-) > >=20 > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c > > index 76a9bcf..7668b22 100644 > > --- a/hw/block/m25p80.c > > +++ b/hw/block/m25p80.c > > @@ -389,7 +389,7 @@ typedef struct Flash { > > uint32_t pos; > > uint8_t needed_bytes; > > uint8_t cmd_in_progress; > > - uint64_t cur_addr; > > + uint32_t cur_addr; > > uint32_t nonvolatile_cfg; > > /* Configuration register for Macronix */ > > uint32_t volatile_cfg; > > @@ -535,9 +535,9 @@ static inline void flash_sync_dirty(Flash *s, int64= _t > > newpage) > > } > > =20 > > static inline > > -void flash_write8(Flash *s, uint64_t addr, uint8_t data) > > +void flash_write8(Flash *s, uint32_t addr, uint8_t data) > > { > > - int64_t page =3D addr / s->pi->page_size; > > + uint32_t page =3D addr / s->pi->page_size; > > uint8_t prev =3D s->storage[s->cur_addr]; >=20 > This routine needs a cleanup. It takes an 'addr' parameter (it is called > with s->cur_addr) and uses s->cur_addr at the same time. >=20 > C. >=20 > > if (!s->write_enable) { > > @@ -545,7 +545,7 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t > > data) > > } > > =20 > > if ((prev ^ data) & data) { > > - DB_PRINT_L(1, "programming zero to one! addr=3D%" PRIx64 " %"= PRIx8 > > + DB_PRINT_L(1, "programming zero to one! addr=3D%" PRIx32 " %"= PRIx8 > > " -> %" PRIx8 "\n", addr, prev, data); > > } > > =20 > > @@ -1094,7 +1094,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, > > uint32_t tx) > > switch (s->state) { > > =20 > > case STATE_PAGE_PROGRAM: > > - DB_PRINT_L(1, "page program cur_addr=3D%#" PRIx64 " data=3D%" = PRIx8 > > "\n", > > + DB_PRINT_L(1, "page program cur_addr=3D%#" PRIx32 " data=3D%" = PRIx8 > > "\n", > > s->cur_addr, (uint8_t)tx); > > flash_write8(s, s->cur_addr, (uint8_t)tx); > > s->cur_addr =3D (s->cur_addr + 1) & (s->size - 1); > > @@ -1102,7 +1102,7 @@ static uint32_t m25p80_transfer8(SSISlave *ss, > > uint32_t tx) > > =20 > > case STATE_READ: > > r =3D s->storage[s->cur_addr]; > > - DB_PRINT_L(1, "READ 0x%" PRIx64 "=3D%" PRIx8 "\n", s->cur_addr= , > > + DB_PRINT_L(1, "READ 0x%" PRIx32 "=3D%" PRIx8 "\n", s->cur_addr= , > > (uint8_t)r); > > s->cur_addr =3D (s->cur_addr + 1) & (s->size - 1); > > break; > > @@ -1199,7 +1199,8 @@ static const VMStateDescription vmstate_m25p80 = =3D { > > VMSTATE_UINT32(pos, Flash), > > VMSTATE_UINT8(needed_bytes, Flash), > > VMSTATE_UINT8(cmd_in_progress, Flash), > > - VMSTATE_UINT64(cur_addr, Flash), > > + VMSTATE_UNUSED(4), > > + VMSTATE_UINT32(cur_addr, Flash), > > VMSTATE_BOOL(write_enable, Flash), > > VMSTATE_BOOL_V(reset_enable, Flash, 2), > > VMSTATE_UINT8_V(ear, Flash, 2), > >=20 >=20 >=20