[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] sh4: r2d --append option support
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] sh4: r2d --append option support |
Date: |
Sun, 29 Mar 2009 00:20:10 +0100 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
On Sat, Mar 28, 2009 at 11:51:00PM +0100, Aurelien Jarno wrote:
> 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.
>
> > };
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
address@hidden http://www.aurel32.net