[Qemu-devel] Re: Commit 9c9bb6c89d4 breaks code execution from flash

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

[-- Attachment #1: Type: text/plain, Size: 2721 bytes --]

Michael Walle wrote:
> [sorry didn't see the CC to the mailinglist]
> 
> Am Friday 23 April 2010 09:23:49 schrieb Jan Kiszka:
>> Michael Walle wrote:
>>> Hi Jan,
>>>
>>> your commit "Optimize consecutive CFI02 writes by remapping memory
>>> lazily" breaks the code execution from flash.
>>>
>>> If you write to the flash, the flash will switch into I/O mode. Now if
>>> code is executed from this flash, a cpu_abort will be raised ("Trying to
>>> execute code outside RAM or ROM").
>> Hmm, guess I didn't test execute-in-place back then. Do you happen to
>> have a test case for this scenario? I'll look into this.
> Only for my qemu-lm32 port.. But reading the flash id, while executing this 
> code from flash should trigger the bug.
> 

OK, that was a hard nut. After various dead ends, I think I found an
possible solution. Can you give this a try?

diff --git a/exec-all.h b/exec-all.h
index 1016de2..b070da9 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -329,6 +329,10 @@ static inline tb_page_addr_t
get_page_addr_code(CPUState *env1, target_ulong add
     if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code !=
                  (addr & TARGET_PAGE_MASK))) {
         ldub_code(addr);
+        if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code &
+                     TLB_INVALID_MASK)) {
+            ldub_code(addr);
+        }
     }
     pd = env1->tlb_table[mmu_idx][page_index].addr_code &
~TARGET_PAGE_MASK;
     if (pd > IO_MEM_ROM && !(pd & IO_MEM_ROMD)) {
diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index f3d3f41..201e410 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -40,7 +40,7 @@
 #include "qemu-timer.h"
 #include "block.h"

-//#define PFLASH_DEBUG
+#define PFLASH_DEBUG
 #ifdef PFLASH_DEBUG
 #define DPRINTF(fmt, ...)                          \
 do {                                               \
@@ -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);
@@ -185,7 +185,7 @@ static uint32_t pflash_read (pflash_t *pfl,
target_phys_addr_t offset,
         default:
             goto flash_read;
         }
-        DPRINTF("%s: ID " TARGET_FMT_pld " %x\n", __func__, boff, ret);
+        DPRINTF("%s: ID " TARGET_FMT_plx " %x\n", __func__, boff, ret);
         break;
     case 0xA0:
     case 0x10:


Still requires proper patch split up, and I need to think about possible
side effects.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]