[Qemu-devel] [PATCH] pflash: Restore & fix lazy ROMD switching

[Jan Kiszka <jan.kiszka@web.de>]

On 2011-04-10 10:38, Jan Kiszka wrote:
> On 2011-04-03 22:16, Jordan Justen wrote:
>> When checking pfl->rom_mode for when to lazily reenter ROMD mode,
>> the value was check was the opposite of what it should have been.
>> This prevent the part from returning to ROMD mode after a write
>> was made to the CFI rom region.
>>
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> ---
>>  hw/pflash_cfi02.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
>> index 30c8aa4..370c5ee 100644
>> --- a/hw/pflash_cfi02.c
>> +++ b/hw/pflash_cfi02.c
>> @@ -112,7 +112,7 @@ static uint32_t pflash_read (pflash_t *pfl, target_phys_addr_t offset,
>>  
>>      DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset);
>>      ret = -1;
>> -    if (pfl->rom_mode) {
>> +    if (!pfl->rom_mode) {
>>          /* Lazy reset of to ROMD mode */
>>          if (pfl->wcycle == 0)
>>              pflash_register_memory(pfl, 1);
> 
> Indeed, that block looks weird to its author today as well. But
> inverting the logic completely defeats the purpose of lazy mode
> switching (will likely file a patch to remove the block). We should
> instead switch back using the timer.

That was wishful thinking. We were actually lacking a switch-back on
read, something that the old twisted logic was papering over. Patch
below should resolve that more gracefully, at least it does so here.

Jan

------8<------

Commit 5145b3d1cc revealed a bug in the lazy ROMD switch-back logic, but
resolved it by breaking that feature. This approach addresses the issue
by switching back to ROMD after a certain amount of read accesses
without further unlock sequences.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 hw/pflash_cfi02.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index 370c5ee..14bbc34 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -50,6 +50,8 @@ do {                                               \
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
+#define PFLASH_LAZY_ROMD_THRESHOLD 42
+
 struct pflash_t {
     BlockDriverState *bs;
     target_phys_addr_t base;
@@ -70,6 +72,7 @@ struct pflash_t {
     ram_addr_t off;
     int fl_mem;
     int rom_mode;
+    int read_counter; /* used for lazy switch-back to rom mode */
     void *storage;
 };
 
@@ -112,10 +115,10 @@ static uint32_t pflash_read (pflash_t *pfl, target_phys_addr_t offset,
 
     DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset);
     ret = -1;
-    if (!pfl->rom_mode) {
-        /* Lazy reset of to ROMD mode */
-        if (pfl->wcycle == 0)
-            pflash_register_memory(pfl, 1);
+    /* Lazy reset to ROMD mode after a certain amount of read accesses */
+    if (!pfl->rom_mode && pfl->wcycle == 0 &&
+        ++pfl->read_counter > PFLASH_LAZY_ROMD_THRESHOLD) {
+        pflash_register_memory(pfl, 1);
     }
     offset &= pfl->chip_len - 1;
     boff = offset & 0xFF;
@@ -254,6 +257,7 @@ static void pflash_write (pflash_t *pfl, target_phys_addr_t offset,
         /* Set the device in I/O access mode if required */
         if (pfl->rom_mode)
             pflash_register_memory(pfl, 0);
+        pfl->read_counter = 0;
         /* We're in read mode */
     check_unlock0:
         if (boff == 0x55 && cmd == 0x98) {
-- 
1.7.1