Re: [Qemu-devel] [RFC] [PATCH] r2d add pflash support

[takasi-y]

Hi,
Sorry for slow response (as is usual....)

> +#define FLASH_BASE 0xa0000000
I think this should be physical address but P2.
Like #define FLASH_BASE 0x00000000

> +     } else {
> +            env->pc = FLASH_BASE;
> +     }
I think "-pflash" should not set PC, but simply provede initial content
 of FLASH.
On the other hand, "-kernel" sets PC because it is a pseudo firmware task.

> -          env->pc = (SDRAM_BASE + 0x80000) | 0xa0000000;
> +          env->pc = (SDRAM_BASE + 0x80000) | FLASH_BASE;
...
> -          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
> +          env->pc = SDRAM_BASE | FLASH_BASE; /* Start from P2 area */
These are not needed.
These 0xa0000000 mean P2, nothing related with FLASH.

> --- a/target-sh4/helper.c
> +++ b/target-sh4/helper.c
> @@ -436,8 +436,8 @@ int get_physical_address(CPUState * env, target_ulong * 
> physical,
>           else
>               return MMU_IADDR_ERROR;
>       }
> -     if (address >= 0x80000000 && address < 0xc0000000) {
> -         /* Mask upper 3 bits for P1 and P2 areas */
> +     if (address >= 0x80000000 && address < 0xa0000000) {
> +         /* Mask upper 3 bits for P1 area */
>           *physical = address & 0x1fffffff;
>       } else {
>           *physical = address;
You needed this because you have defined FLASH address as P2.
Chaning it to physical as shown above makes this not be needed.

Perhaps, you need increase "ram_require" for FLASH.
@@ -175,5 +203,5 @@ QEMUMachine r2d_machine = {
     .name = "r2d",
     .desc = "r2d-plus board",
     .init = r2d_init,
-    .ram_require = (SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
+    .ram_require = (FLASH_SIZE + SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,

Cheers,
/yoshii