qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org, Stephen Checkoway <stephen.checkoway@oberlin.edu>
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>, "Thomas Huth" <thuth@redhat.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-block@nongnu.org, "Alex Bennée" <alex.bennee@linaro.org>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>
Subject: [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison
Date: Mon,  6 May 2019 00:15:43 +0200	[thread overview]
Message-ID: <20190505221544.31568-13-philmd@redhat.com> (raw)
In-Reply-To: <20190505221544.31568-1-philmd@redhat.com>

From: Stephen Checkoway <stephen.checkoway@oberlin.edu>

Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.

Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20190426162624.55977-4-stephen.checkoway@oberlin.edu>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi02.c   |  8 +++++++-
 tests/pflash-cfi02-test.c | 12 ++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 59daade2ee6..4c17dbf99f4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -297,11 +297,13 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
 
     DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
             offset, value, width);
-    boff = offset & (pfl->sector_len - 1);
+    boff = offset;
     if (pfl->width == 2)
         boff = boff >> 1;
     else if (pfl->width == 4)
         boff = boff >> 2;
+    /* Only the least-significant 11 bits are used in most cases. */
+    boff &= 0x7FF;
     switch (pfl->wcycle) {
     case 0:
         /* Set the device in I/O access mode if required */
@@ -520,6 +522,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* Only 11 bits are used in the comparison. */
+    pfl->unlock_addr0 &= 0x7FF;
+    pfl->unlock_addr1 &= 0x7FF;
+
     chip_len = pfl->sector_len * pfl->nb_blocs;
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 3c37465499a..1e5d9124b71 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -22,8 +22,8 @@
 
 #define FLASH_WIDTH 2
 #define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x5555)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
 
 #define CFI_CMD 0x98
 #define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(const void *opaque)
     g_assert_cmpint(flash_read(6), ==, 0xCDEF);
     g_assert_cmpint(flash_read(8), ==, 0xFFFF);
 
+    /* Test ignored high order bits of address. */
+    flash_write(FLASH_WIDTH * 0x5555, UNLOCK0_CMD);
+    flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD);
+    flash_write(FLASH_WIDTH * 0x5555, AUTOSELECT_CMD);
+    g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF);
+    g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+    reset();
+
     qtest_quit(global_qtest);
 }
 
-- 
2.20.1

WARNING: multiple messages have this Message-ID (diff)
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org, Stephen Checkoway <stephen.checkoway@oberlin.edu>
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>, "Thomas Huth" <thuth@redhat.com>,
	qemu-block@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Max Reitz" <mreitz@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>
Subject: [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison
Date: Mon,  6 May 2019 00:15:43 +0200	[thread overview]
Message-ID: <20190505221544.31568-13-philmd@redhat.com> (raw)
Message-ID: <20190505221543.0rGF5THjhozFYjhkW-BhRm_CHoyi6QI3aeQCjzJqxrw@z> (raw)
In-Reply-To: <20190505221544.31568-1-philmd@redhat.com>

From: Stephen Checkoway <stephen.checkoway@oberlin.edu>

Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.

Signed-off-by: Stephen Checkoway <stephen.checkoway@oberlin.edu>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20190426162624.55977-4-stephen.checkoway@oberlin.edu>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/block/pflash_cfi02.c   |  8 +++++++-
 tests/pflash-cfi02-test.c | 12 ++++++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 59daade2ee6..4c17dbf99f4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -297,11 +297,13 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
 
     DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
             offset, value, width);
-    boff = offset & (pfl->sector_len - 1);
+    boff = offset;
     if (pfl->width == 2)
         boff = boff >> 1;
     else if (pfl->width == 4)
         boff = boff >> 2;
+    /* Only the least-significant 11 bits are used in most cases. */
+    boff &= 0x7FF;
     switch (pfl->wcycle) {
     case 0:
         /* Set the device in I/O access mode if required */
@@ -520,6 +522,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* Only 11 bits are used in the comparison. */
+    pfl->unlock_addr0 &= 0x7FF;
+    pfl->unlock_addr1 &= 0x7FF;
+
     chip_len = pfl->sector_len * pfl->nb_blocs;
 
     memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 3c37465499a..1e5d9124b71 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -22,8 +22,8 @@
 
 #define FLASH_WIDTH 2
 #define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x5555)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
 
 #define CFI_CMD 0x98
 #define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(const void *opaque)
     g_assert_cmpint(flash_read(6), ==, 0xCDEF);
     g_assert_cmpint(flash_read(8), ==, 0xFFFF);
 
+    /* Test ignored high order bits of address. */
+    flash_write(FLASH_WIDTH * 0x5555, UNLOCK0_CMD);
+    flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD);
+    flash_write(FLASH_WIDTH * 0x5555, AUTOSELECT_CMD);
+    g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0000), ==, 0x00BF);
+    g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+    reset();
+
     qtest_quit(global_qtest);
 }
 
-- 
2.20.1



  parent reply	other threads:[~2019-05-05 22:16 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-05 22:15 [Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write() Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read() Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-05-05 22:15 ` Philippe Mathieu-Daudé [this message]
2019-05-05 22:15   ` [Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison Philippe Mathieu-Daudé
2019-05-05 22:15 ` [Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table Philippe Mathieu-Daudé
2019-05-05 22:15   ` Philippe Mathieu-Daudé
2019-06-26 20:33 ` [Qemu-devel] [Qemu-block] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes John Snow
2019-06-26 21:06   ` Philippe Mathieu-Daudé
2019-06-26 21:09     ` John Snow

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190505221544.31568-13-philmd@redhat.com \
    --to=philmd@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stephen.checkoway@oberlin.edu \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).