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

[Aurelien Jarno]

On Sun, Mar 08, 2009 at 03:00:17AM +0900, address@hidden 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 <address@hidden>
> ---
> 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
address@hidden                 http://www.aurel32.net