Re: [Qemu-devel] [PATCH] hw/sh4: Add dtb support

[Aurelien Jarno]

On 2016-06-19 14:34, Yoshinori Sato wrote:
> New SH kernel use dtb.

Do you have a pointer to the corresponding kernel code? I can't find in
linus' tree.

> This patch add dtb support for R2D board emulation.
> 
> Signed-off-by: Yoshinori Sato <address@hidden>
> ---
>  hw/sh4/r2d.c | 52 +++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index f2547ed..203c117 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -41,6 +41,7 @@
>  #include "hw/usb.h"
>  #include "hw/block/flash.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/device_tree.h"
>  #include "exec/address-spaces.h"
>  
>  #define FLASH_BASE 0x00000000
> @@ -51,7 +52,7 @@
>  
>  #define SM501_VRAM_SIZE 0x800000
>  
> -#define BOOT_PARAMS_OFFSET 0x0001000
> +#define BOOT_PARAMS_OFFSET 0x0010000

Hmm, the current code already contains the value 0x0010000

>  /* CONFIG_BOOT_LINK_OFFSET of Linux kernel */
>  #define LINUX_LOAD_OFFSET  0x0800000
>  #define INITRD_LOAD_OFFSET 0x1800000
> @@ -198,6 +199,7 @@ static qemu_irq *r2d_fpga_init(MemoryRegion *sysmem,
>  typedef struct ResetData {
>      SuperHCPU *cpu;
>      uint32_t vector;
> +    uint32_t dtb;
>  } ResetData;
>  
>  static void main_cpu_reset(void *opaque)
> @@ -207,6 +209,8 @@ static void main_cpu_reset(void *opaque)
>  
>      cpu_reset(CPU(s->cpu));
>      env->pc = s->vector;
> +    /* r4_bank1 is DTB address */
> +    env->gregs[4 + 16] = s->dtb;
>  }
>  
>  static struct QEMU_PACKED
> @@ -225,10 +229,12 @@ static struct QEMU_PACKED
>  
>  static void r2d_init(MachineState *machine)
>  {
> +    QemuOpts *machine_opts;
>      const char *cpu_model = machine->cpu_model;
> -    const char *kernel_filename = machine->kernel_filename;
> -    const char *kernel_cmdline = machine->kernel_cmdline;
> -    const char *initrd_filename = machine->initrd_filename;
> +    const char *kernel_filename;
> +    const char *kernel_cmdline;
> +    const char *initrd_filename;
> +    const char *dtb_filename;
>      SuperHCPU *cpu;
>      CPUSH4State *env;
>      ResetData *reset_info;
> @@ -258,6 +264,12 @@ static void r2d_init(MachineState *machine)
>      reset_info->vector = env->pc;
>      qemu_register_reset(main_cpu_reset, reset_info);
>  
> +    machine_opts = qemu_get_machine_opts();
> +    kernel_filename = qemu_opt_get(machine_opts, "kernel");
> +    kernel_cmdline = qemu_opt_get(machine_opts, "append");
> +    initrd_filename = qemu_opt_get(machine_opts, "initrd");
> +    dtb_filename = qemu_opt_get(machine_opts, "dtb");
> +
>      /* Allocate memory space */
>      memory_region_init_ram(sdram, NULL, "r2d.sdram", SDRAM_SIZE, 
> &error_fatal);
>      vmstate_register_ram_global(sdram);
> @@ -347,11 +359,33 @@ static void r2d_init(MachineState *machine)
>          boot_params.initrd_size = tswap32(initrd_size);
>      }
>  
> -    if (kernel_cmdline) {
> -        /* I see no evidence that this .kernel_cmdline buffer requires
> -           NUL-termination, so using strncpy should be ok. */
> -        strncpy(boot_params.kernel_cmdline, kernel_cmdline,
> -                sizeof(boot_params.kernel_cmdline));
> +    if (!dtb_filename) {
> +        if (kernel_cmdline) {
> +            /* I see no evidence that this .kernel_cmdline buffer requires
> +               NUL-termination, so using strncpy should be ok. */
> +            strncpy(boot_params.kernel_cmdline, kernel_cmdline,
> +                    sizeof(boot_params.kernel_cmdline));
> +        }
> +    } else {
> +        void *fdt;
> +        int r = 0;
> +        uint32_t addr;
> +        int fdt_size;
> +
> +        fdt = load_device_tree(dtb_filename, &fdt_size);
> +        if (!fdt) {
> +            return;
> +        }

If the user explicitly passed a dtb file, the error should not be
silently ignore if the file can't be loaded. It should print an error
message and bail out, just like it's done when the kernel image can't be
loaded.

> +        if (kernel_cmdline) {
> +            r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
> +                                        kernel_cmdline);
> +            if (r < 0) {
> +                fprintf(stderr, "couldn't set /chosen/bootargs\n");
> +            }
> +        }
> +        addr = (SDRAM_BASE + SDRAM_SIZE - fdt_size) & ~0xfff;
> +        cpu_physical_memory_write(addr, fdt, fdt_size);
> +        reset_info->dtb = addr;
>      }
>  
>      rom_add_blob_fixed("boot_params", &boot_params, sizeof(boot_params),

When a dtb file is passed, this memory area is initialized. Either the
command line should also be copied there, or this blob should not be
added in the dtb case.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net