Re: [Qemu-ppc] [Resend] Setting boot_info and order of loading kernel images

[Alexander Graf]

On 07.06.2012, at 01:13, Elvis Dowson wrote:

> Hi,
>     I just have a few questions regarding setting the boot_info structure.
> 
> For the following code snippet, which is hw/virtex5_ml507.c modified, 
> 
> Question 01: Immediately after I call load_uimage(), is it correct to set 
> boot_info.bootstrap_pc = uentry; 
> 
>       /* Boot a kernel uImage binary. */
>       kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, NULL);
>       boot_info.bootstrap_pc = uentry;

Yup, that part looks good.

> Question 02: Should I also compute the value of 'high', and if so, should it 
> look like this

High is used later in the code to find out where to put the device tree blob, 
so yes, absolutely.

> 
>       /* Boot a kernel uImage binary. */
>       kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, NULL);
>       boot_info.bootstrap_pc = uentry;
>       high = (loadaddr + kernel_size + 3) & ~3;
> or 
>       high = boot_info.bootstrap_pc + kernel_size + 8192;

Why 3? Why 8192?

Can't you just align it to something natural, like PAGE_SIZE? Ah, it's copied 
over from the raw case? Fine with me then ...

> 
> The full function is listed below:
> 
> static void virtex_init(ram_addr_t ram_size,
>                       const char *boot_device,
>                       const char *kernel_filename,
>                       const char *kernel_cmdline,
>                       const char *initrd_filename, const char *cpu_model)
> {
>   DeviceState *dev;
>   CPUState *env;
>   target_phys_addr_t loadaddr = UIMAGE_LOAD_BASE;
>   DriveInfo *dinfo;
>   ram_addr_t phys_ram;
>   ram_addr_t phys_flash;
>   qemu_irq irq[32], *cpu_irq;
>   clk_setup_t clk_setup[7];
>   int kernel_size;
>   int i;
> 
>   /* init CPUs */
>   if (cpu_model == NULL) {
>       cpu_model = "440-Xilinx";
>   }
> 
>   memset(clk_setup, 0, sizeof(clk_setup));
>   env = ppc440_init_xilinx(&ram_size, 1, cpu_model, &clk_setup[0],
>                            &clk_setup[1], 400000000);
>   qemu_register_reset(main_cpu_reset, env);
> 
>   phys_ram = qemu_ram_alloc(NULL, "ram", ram_size);
>   cpu_register_physical_memory(loadaddr, ram_size, phys_ram | IO_MEM_RAM);
> 
>   phys_flash = qemu_ram_alloc(NULL, "virtex.flash", FLASH_SIZE);
>   dinfo = drive_get(IF_PFLASH, 0, 0);
>   pflash_cfi01_register(0xfc000000, phys_flash,
>                         dinfo ? dinfo->bdrv : NULL, (64 * 1024),
>                         FLASH_SIZE >> 16,
>                         1, 0x89, 0x18, 0x0000, 0x0, 1);
> 
>   cpu_irq = (qemu_irq *) &env->irq_inputs[PPC40x_INPUT_INT];
> 
>   /* Xilinx xps_intc interrrup controller */
>   dev = xilinx_intc_create(XPS_INTC_BASEADDR, cpu_irq[0], 0);
>   for (i = 0; i < 32; i++) {
>       irq[i] = qdev_get_gpio_in(dev, i);
>   }
> 
>   serial_mm_init(0x83e01003ULL, 2, irq[9], 115200, serial_hds[0], 1, 0);
> 
>   /* Create Xilinx xps_timer x 2 @ 62 Mhz.  */
>   xilinx_timer_create(XPS_TIMER_BASEADDR, irq[XPS_TIMER_IRQ], 2, 62 * 
> 1000000);
> 
>   /* Xilinx xps_ethernetlite media access controller */
>   xilinx_ethlite_create(&nd_table[0], XPS_ETHERNETLITE_BASEADDR, 
> irq[XPS_ETHERNETLITE_IRQ], 0, 0);
> 
>   /* Load kernel. */
>   if (kernel_filename) {
>       target_phys_addr_t uentry;
>       uint64_t entry, low, high;
>       target_phys_addr_t boot_offset;
> 
>       /* Boot a kernel uImage binary. */
>       kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, NULL);
>       boot_info.bootstrap_pc = uentry;
> 
>       /* If we failed loading uImage, try again as a kernel elf binary.  */
>       if (kernel_size < 0) {
>           kernel_size = load_elf(kernel_filename, NULL, NULL,
>                                  &entry, &low, &high, 1, ELF_MACHINE, 0);
>           boot_info.bootstrap_pc = entry & 0x00ffffff;
>       }
> 
>       if (kernel_size < 0) {
>           boot_offset = 0x1200000;
>           /* If we failed loading ELF's try a raw image.  */
>           kernel_size = load_image_targphys(kernel_filename,
>                                             boot_offset,
>                                             ram_size);
>           boot_info.bootstrap_pc = boot_offset;
>           high = boot_info.bootstrap_pc + kernel_size + 8192;
>       }
> 
>       boot_info.ima_size = kernel_size;
> 
>       /* Provide a device-tree.  */
>       boot_info.fdt = high + (8192 * 2);
>       boot_info.fdt &= ~8191;
>       xilinx_load_device_tree(boot_info.fdt, ram_size, 0, 0, kernel_cmdline);
>   }
>   env->load_info = &boot_info;
> }
> 
> Question 03: Does the above call sequence for loading the kernel images look 
> correct? I just get the impression that I've got some hidden errors in the 
> program logic flow and might be inadvertently incorrectly setting boot_info, 
> while attempting to load the various kernel images.

Looks good to me, but an actual patch would be helpful to see what changed vs 
the previous flow.

> Question 04: Is the order uImage, elf binary and raw image okay, or should 
> the order be elf binary, uImage and then finally try the raw image?

Either way works :)


Alex