[Qemu-devel] sh4: r2d --append option support

[Qemu-devel] sh4: r2d --append option support

[takasi-y]

Add linux kernel command line ("--append" option) support.
Fix kernel loading address to appropriate position when --append used.
Using --kernel but --append case is left untouched for backward compatibility.

Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
---
I found this has not been posted as a patch to apply.
I hope this be merged into the branch(if any:) for next stable release.
/yoshii
---
 hw/r2d.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/r2d.c b/hw/r2d.c
index e6b2bf4..7dcb723 100644
--- a/hw/r2d.c
+++ b/hw/r2d.c
@@ -233,20 +233,25 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
         pci_nic_init(pci, &nd_table[i], -1, "ne2k_pci");
 
     /* Todo: register on board registers */
-    {
+    if (kernel_filename) {
       int kernel_size;
       /* initialization which should be done by firmware */
       stl_phys(SH7750_BCR1, 1<<3); /* cs3 SDRAM */
       stw_phys(SH7750_BCR2, 3<<(3*2)); /* cs3 32bit */
 
-      kernel_size = load_image(kernel_filename, phys_ram_base);
+      if (kernel_cmdline) {
+          kernel_size = load_image(kernel_filename, phys_ram_base + 0x80000);
+          env->pc = (SDRAM_BASE + 0x80000) | 0xa0000000;
+          pstrcpy(phys_ram_base + 0x10100, 256, kernel_cmdline);
+      } else {
+          kernel_size = load_image(kernel_filename, phys_ram_base);
+          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
+      }
 
       if (kernel_size < 0) {
         fprintf(stderr, "qemu: could not load kernel '%s'\n", kernel_filename);
         exit(1);
       }
-
-      env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
     }
 }
 
-- 
1.5.6.3

Re: [Qemu-devel] sh4: r2d --append option support

[Shin-ichiro KAWASAKI]

takasi-y@ops.dti.ne.jp wrote:
> Add linux kernel command line ("--append" option) support.
> Fix kernel loading address to appropriate position when --append used.
> Using --kernel but --append case is left untouched for backward compatibility.

This patch finely adds --append feature to my environment.  Thank you!

Tested-by: Shin-ichiro KAWASAKI <kawasaki@juno.dti.ne.jp>

> Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
> ---
> I found this has not been posted as a patch to apply.
> I hope this be merged into the branch(if any:) for next stable release.
> /yoshii
> ---
>  hw/r2d.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
(snip)


Regards,
Shin-ichiro KAWASAKI

Re: [Qemu-devel] sh4: r2d --append option support

[Aurelien Jarno]

On Wed, Feb 11, 2009 at 10:15:52PM +0900, takasi-y@ops.dti.ne.jp wrote:
> Add linux kernel command line ("--append" option) support.
> Fix kernel loading address to appropriate position when --append used.
> Using --kernel but --append case is left untouched for backward compatibility.
> 
> Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
> ---
> I found this has not been posted as a patch to apply.
> I hope this be merged into the branch(if any:) for next stable release.
> /yoshii
> ---
>  hw/r2d.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/r2d.c b/hw/r2d.c
> index e6b2bf4..7dcb723 100644
> --- a/hw/r2d.c
> +++ b/hw/r2d.c
> @@ -233,20 +233,25 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
>          pci_nic_init(pci, &nd_table[i], -1, "ne2k_pci");
>  
>      /* Todo: register on board registers */
> -    {
> +    if (kernel_filename) {
>        int kernel_size;
>        /* initialization which should be done by firmware */
>        stl_phys(SH7750_BCR1, 1<<3); /* cs3 SDRAM */
>        stw_phys(SH7750_BCR2, 3<<(3*2)); /* cs3 32bit */
>  
> -      kernel_size = load_image(kernel_filename, phys_ram_base);
> +      if (kernel_cmdline) {
> +          kernel_size = load_image(kernel_filename, phys_ram_base + 0x80000);
> +          env->pc = (SDRAM_BASE + 0x80000) | 0xa0000000;
> +          pstrcpy(phys_ram_base + 0x10100, 256, kernel_cmdline);

You should use pstrcpy_targphys() instead and remove phys_ram_base.
Otherwise looks good.

> +      } else {
> +          kernel_size = load_image(kernel_filename, phys_ram_base);
> +          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
> +      }
>  
>        if (kernel_size < 0) {
>          fprintf(stderr, "qemu: could not load kernel '%s'\n", kernel_filename);
>          exit(1);
>        }
> -
> -      env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
>      }
>  }
>  
> -- 
> 1.5.6.3
> 
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH] fread_targphys(): Do not cut off the tail.

[takasi-y]

loader.c:fread_targphys() read file by 4096 byte chunks and store them to
memory. But did not store the last chunk if its size was not 4096.

Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
---

On Tue, 3 Mar 2009 07:20:29 +0100
Aurelien Jarno <aurelien@aurel32.net> wrote:
> > +          kernel_size = load_image(kernel_filename, phys_ram_base + 0x80000);
> > +          env->pc = (SDRAM_BASE + 0x80000) | 0xa0000000;
> > +          pstrcpy(phys_ram_base + 0x10100, 256, kernel_cmdline);
> 
> You should use pstrcpy_targphys() instead and remove phys_ram_base.

I think load_image also should be load_image_targphys.
But the program did not work. The result was the issue fixed by this patch.
No error/eof handling is here. Even without it, this function behave like
 fread() about terminate condition.

/yoshii

---
 loader.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/loader.c b/loader.c
index 71b9ba7..55625cf 100644
--- a/loader.c
+++ b/loader.c
@@ -90,11 +90,12 @@ int fread_targphys(target_phys_addr_t dst_addr, size_t nbytes, FILE *f)
     while (nbytes) {
 	want = nbytes > sizeof(buf) ? sizeof(buf) : nbytes;
 	did = fread(buf, 1, want, f);
-	if (did != want) break;
 
 	cpu_physical_memory_write_rom(dst_addr, buf, did);
 	dst_addr += did;
 	nbytes -= did;
+	if (did != want)
+	    break;
     }
     return dst_addr - dst_begin;
 }
-- 
1.5.6.3

Re: [Qemu-devel] [PATCH] fread_targphys(): Do not cut off the tail.

[Blue Swirl]

On 3/7/09, takasi-y@ops.dti.ne.jp <takasi-y@ops.dti.ne.jp> wrote:
> loader.c:fread_targphys() read file by 4096 byte chunks and store them to
>  memory. But did not store the last chunk if its size was not 4096.
>
>  Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>

Thanks, applied.

Re: [Qemu-devel] sh4: r2d --append option support

[takasi-y]

Add linux kernel command line ("--append" option) support.
Fix kernel loading address to appropriate position when --append used.
Using --kernel but --append case is left untouched for backward compatibility.

This also change the host<->SH address mapping for r2d to
 host addr == phys_ram_base + SH addr.

Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
---
Hi,

> You should use pstrcpy_targphys() instead and remove phys_ram_base.
> Otherwise looks good.
This reminds me of host/target address mapping of r2d, which is currently
 host addr == phys_ram_base + SH addr - 0x0c000000 (== top of SDRAM)
This patch change it to
 host addr == phys_ram_base + SH addr

That one itself is not a problem because QEMU's core memory system allows the
 constant offset between host and target, but functions in loader.c.

Of course I could write as pstrcpy_targphys(0x10100, 256, kernel_cmdline);
But, I think this is confusing because arg for *_targphys() that typed
 target_phy_addr_t is not equal to target's physical address.

And other one....
Last time, loading offset was 0x80000, but was my mistake. Fixed to 0x800000.

/yoshii

---
 hw/r2d.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/r2d.c b/hw/r2d.c
index 713fc53..0f70d16 100644
--- a/hw/r2d.c
+++ b/hw/r2d.c
@@ -37,6 +37,9 @@
 
 #define SM501_VRAM_SIZE 0x800000
 
+/* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
+#define LINUX_LOAD_OFFSET 0x800000
+
 #define PA_IRLMSK	0x00
 #define PA_POWOFF	0x30
 #define PA_VERREG	0x32
@@ -212,6 +215,7 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
     }
 
     /* Allocate memory space */
+    qemu_ram_alloc(SDRAM_BASE); /* to adjust the offset */
     sdram_addr = qemu_ram_alloc(SDRAM_SIZE);
     cpu_register_physical_memory(SDRAM_BASE, SDRAM_SIZE, sdram_addr);
     /* Register peripherals */
@@ -233,20 +237,27 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
         pci_nic_init(pci, &nd_table[i], (i==0)? 2<<3: -1, "rtl8139");
 
     /* Todo: register on board registers */
-    {
+    if (kernel_filename) {
       int kernel_size;
       /* initialization which should be done by firmware */
       stl_phys(SH7750_BCR1, 1<<3); /* cs3 SDRAM */
       stw_phys(SH7750_BCR2, 3<<(3*2)); /* cs3 32bit */
 
-      kernel_size = load_image(kernel_filename, phys_ram_base);
+      if (kernel_cmdline) {
+          kernel_size = load_image_targphys(kernel_filename,
+				   SDRAM_BASE + LINUX_LOAD_OFFSET,
+				   SDRAM_SIZE - LINUX_LOAD_OFFSET);
+          env->pc = (SDRAM_BASE + LINUX_LOAD_OFFSET) | 0xa0000000;
+          pstrcpy_targphys(SDRAM_BASE + 0x10100, 256, kernel_cmdline);
+      } else {
+          kernel_size = load_image(kernel_filename, SDRAM_BASE);
+          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
+      }
 
       if (kernel_size < 0) {
         fprintf(stderr, "qemu: could not load kernel '%s'\n", kernel_filename);
         exit(1);
       }
-
-      env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
     }
 }
 
@@ -254,5 +265,5 @@ QEMUMachine r2d_machine = {
     .name = "r2d",
     .desc = "r2d-plus board",
     .init = r2d_init,
-    .ram_require = (SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
+    .ram_require = (SDRAM_BASE + SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
 };
-- 
1.5.6.3

Re: [Qemu-devel] sh4: r2d --append option support

[Aurelien Jarno]

On Sun, Mar 08, 2009 at 03:00:17AM +0900, takasi-y@ops.dti.ne.jp wrote:
> Add linux kernel command line ("--append" option) support.
> Fix kernel loading address to appropriate position when --append used.
> Using --kernel but --append case is left untouched for backward compatibility.
> 
> This also change the host<->SH address mapping for r2d to
>  host addr == phys_ram_base + SH addr.
> 
> Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
> ---
> Hi,
> 
> > You should use pstrcpy_targphys() instead and remove phys_ram_base.
> > Otherwise looks good.
> This reminds me of host/target address mapping of r2d, which is currently
>  host addr == phys_ram_base + SH addr - 0x0c000000 (== top of SDRAM)
> This patch change it to
>  host addr == phys_ram_base + SH addr
> 
> That one itself is not a problem because QEMU's core memory system allows the
>  constant offset between host and target, but functions in loader.c.
> 
> Of course I could write as pstrcpy_targphys(0x10100, 256, kernel_cmdline);
> But, I think this is confusing because arg for *_targphys() that typed
>  target_phy_addr_t is not equal to target's physical address.
> 
> And other one....
> Last time, loading offset was 0x80000, but was my mistake. Fixed to 0x800000.
> 
> /yoshii
> 
> ---
>  hw/r2d.c |   21 ++++++++++++++++-----
>  1 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/r2d.c b/hw/r2d.c
> index 713fc53..0f70d16 100644
> --- a/hw/r2d.c
> +++ b/hw/r2d.c
> @@ -37,6 +37,9 @@
>  
>  #define SM501_VRAM_SIZE 0x800000
>  
> +/* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
> +#define LINUX_LOAD_OFFSET 0x800000
> +
>  #define PA_IRLMSK	0x00
>  #define PA_POWOFF	0x30
>  #define PA_VERREG	0x32
> @@ -212,6 +215,7 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
>      }
>  
>      /* Allocate memory space */
> +    qemu_ram_alloc(SDRAM_BASE); /* to adjust the offset */

This should not be needed, as long as you access to the target address
space using dedicated functions. For which reason do you added it?

>      sdram_addr = qemu_ram_alloc(SDRAM_SIZE);
>      cpu_register_physical_memory(SDRAM_BASE, SDRAM_SIZE, sdram_addr);
>      /* Register peripherals */
> @@ -233,20 +237,27 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
>          pci_nic_init(pci, &nd_table[i], (i==0)? 2<<3: -1, "rtl8139");
>  
>      /* Todo: register on board registers */
> -    {
> +    if (kernel_filename) {
>        int kernel_size;
>        /* initialization which should be done by firmware */
>        stl_phys(SH7750_BCR1, 1<<3); /* cs3 SDRAM */
>        stw_phys(SH7750_BCR2, 3<<(3*2)); /* cs3 32bit */
>  
> -      kernel_size = load_image(kernel_filename, phys_ram_base);
> +      if (kernel_cmdline) {
> +          kernel_size = load_image_targphys(kernel_filename,
> +				   SDRAM_BASE + LINUX_LOAD_OFFSET,
> +				   SDRAM_SIZE - LINUX_LOAD_OFFSET);
> +          env->pc = (SDRAM_BASE + LINUX_LOAD_OFFSET) | 0xa0000000;
> +          pstrcpy_targphys(SDRAM_BASE + 0x10100, 256, kernel_cmdline);
> +      } else {
> +          kernel_size = load_image(kernel_filename, SDRAM_BASE);

It think it should be:

             kernel_size = load_image_targphys(kernel_filename, SDRAM_BASE, SDRAM_SIZE);

> +          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
> +      }
>  
>        if (kernel_size < 0) {
>          fprintf(stderr, "qemu: could not load kernel '%s'\n", kernel_filename);
>          exit(1);
>        }
> -
> -      env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
>      }
>  }
>  
> @@ -254,5 +265,5 @@ QEMUMachine r2d_machine = {
>      .name = "r2d",
>      .desc = "r2d-plus board",
>      .init = r2d_init,
> -    .ram_require = (SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
> +    .ram_require = (SDRAM_BASE + SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,

I think this is needed because of the "qemu_ram_alloc(SDRAM_BASE);"
line. They should probably be removed alltogether.

>  };
> -- 
> 1.5.6.3
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] sh4: r2d --append option support

[Aurelien Jarno]

On Sat, Mar 28, 2009 at 11:51:00PM +0100, Aurelien Jarno wrote:
> On Sun, Mar 08, 2009 at 03:00:17AM +0900, takasi-y@ops.dti.ne.jp wrote:
> > Add linux kernel command line ("--append" option) support.
> > Fix kernel loading address to appropriate position when --append used.
> > Using --kernel but --append case is left untouched for backward compatibility.
> > 
> > This also change the host<->SH address mapping for r2d to
> >  host addr == phys_ram_base + SH addr.
> > 
> > Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
> > ---
> > Hi,
> > 
> > > You should use pstrcpy_targphys() instead and remove phys_ram_base.
> > > Otherwise looks good.
> > This reminds me of host/target address mapping of r2d, which is currently
> >  host addr == phys_ram_base + SH addr - 0x0c000000 (== top of SDRAM)
> > This patch change it to
> >  host addr == phys_ram_base + SH addr
> > 
> > That one itself is not a problem because QEMU's core memory system allows the
> >  constant offset between host and target, but functions in loader.c.
> > 
> > Of course I could write as pstrcpy_targphys(0x10100, 256, kernel_cmdline);
> > But, I think this is confusing because arg for *_targphys() that typed
> >  target_phy_addr_t is not equal to target's physical address.
> > 
> > And other one....
> > Last time, loading offset was 0x80000, but was my mistake. Fixed to 0x800000.
> > 
> > /yoshii
> > 
> > ---
> >  hw/r2d.c |   21 ++++++++++++++++-----
> >  1 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/r2d.c b/hw/r2d.c
> > index 713fc53..0f70d16 100644
> > --- a/hw/r2d.c
> > +++ b/hw/r2d.c
> > @@ -37,6 +37,9 @@
> >  
> >  #define SM501_VRAM_SIZE 0x800000
> >  
> > +/* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
> > +#define LINUX_LOAD_OFFSET 0x800000
> > +
> >  #define PA_IRLMSK	0x00
> >  #define PA_POWOFF	0x30
> >  #define PA_VERREG	0x32
> > @@ -212,6 +215,7 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
> >      }
> >  
> >      /* Allocate memory space */
> > +    qemu_ram_alloc(SDRAM_BASE); /* to adjust the offset */
> 
> This should not be needed, as long as you access to the target address
> space using dedicated functions. For which reason do you added it?
> 
> >      sdram_addr = qemu_ram_alloc(SDRAM_SIZE);
> >      cpu_register_physical_memory(SDRAM_BASE, SDRAM_SIZE, sdram_addr);
> >      /* Register peripherals */
> > @@ -233,20 +237,27 @@ static void r2d_init(ram_addr_t ram_size, int vga_ram_size,
> >          pci_nic_init(pci, &nd_table[i], (i==0)? 2<<3: -1, "rtl8139");
> >  
> >      /* Todo: register on board registers */
> > -    {
> > +    if (kernel_filename) {
> >        int kernel_size;
> >        /* initialization which should be done by firmware */
> >        stl_phys(SH7750_BCR1, 1<<3); /* cs3 SDRAM */
> >        stw_phys(SH7750_BCR2, 3<<(3*2)); /* cs3 32bit */
> >  
> > -      kernel_size = load_image(kernel_filename, phys_ram_base);
> > +      if (kernel_cmdline) {
> > +          kernel_size = load_image_targphys(kernel_filename,
> > +				   SDRAM_BASE + LINUX_LOAD_OFFSET,
> > +				   SDRAM_SIZE - LINUX_LOAD_OFFSET);
> > +          env->pc = (SDRAM_BASE + LINUX_LOAD_OFFSET) | 0xa0000000;
> > +          pstrcpy_targphys(SDRAM_BASE + 0x10100, 256, kernel_cmdline);
> > +      } else {
> > +          kernel_size = load_image(kernel_filename, SDRAM_BASE);
> 
> It think it should be:
> 
>              kernel_size = load_image_targphys(kernel_filename, SDRAM_BASE, SDRAM_SIZE);
> 
> > +          env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
> > +      }
> >  
> >        if (kernel_size < 0) {
> >          fprintf(stderr, "qemu: could not load kernel '%s'\n", kernel_filename);
> >          exit(1);
> >        }
> > -
> > -      env->pc = SDRAM_BASE | 0xa0000000; /* Start from P2 area */
> >      }
> >  }
> >  
> > @@ -254,5 +265,5 @@ QEMUMachine r2d_machine = {
> >      .name = "r2d",
> >      .desc = "r2d-plus board",
> >      .init = r2d_init,
> > -    .ram_require = (SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
> > +    .ram_require = (SDRAM_BASE + SDRAM_SIZE + SM501_VRAM_SIZE) | RAMSIZE_FIXED,
> 
> I think this is needed because of the "qemu_ram_alloc(SDRAM_BASE);"
> line. They should probably be removed alltogether.
> 
> >  };

I have actually committed your patch by mistake, so I have made the
suggested changes to the SVN. I am still able to boot and SH4 kernel on
the R2D board with them.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] sh4: r2d --append option support

[takasi-y]

Thank you for your fix.

me> +    qemu_ram_alloc(SDRAM_BASE); /* to adjust the offset */
> This should not be needed, as long as you access to the target address
> space using dedicated functions. For which reason do you added it?

Ah, sorry, I had missunderstood the target -> host physical address conversion
(didn't aware of the phys_offset in PhysPageDesc).
# The reason why I didn't hesitate to change the base is because r2d and most
# of all SH boards has memory(ROM or so) at 0. This may come later, anyway.
/yoshii