Re: [Qemu-ppc] [PATCH RFC] spapr: by-pass SLOF when -kernel is provided

[Laurent Vivier]

On 05/07/2016 20:44, Greg Kurz wrote:
> On Tue,  5 Jul 2016 16:42:37 +0200
> Laurent Vivier <address@hidden> wrote:
> 
>> As device-tree is now fully built by QEMU, we don't need SLOF
>> anymore if the kernel is provided on the command line.
>>
>> In this case, don't load SLOF and boot directly into the
>> kernel.
>>
>> This saves at least 5 seconds on the boot sequence.
>>
> 
> The concept looks great so I gave a try with a fedora24 guest.

I did the same test with a rhel7 vmlinuz adn intramfs and it works fine.

> 
> I copied the kernel and initramfs to the host and passed the kernel arguments
> taken from grub.
> 
> The kernel starts but the boot sequence stalls at:
> 
> [  OK  ] Reached target Basic System.
> [  126.238400] dracut-initqueue[290]: Warning: dracut-initqueue timeout - 
> starting timeout scripts

It happens when the initramfs didn't have the disk driver: do you use
the same disk controller with the "-kernel" than the one which was used
when the initramfs has been created?

> 
> I also noticed this error:
> 
> [    0.127303] WARNING: nvram corruption detected: 0-length partition
> [    0.127610] nvram: No room to create ibm,rtas-log partition, deleting any 
> obsolete OS partitions...
> [    0.128011] nvram: Failed to find or create ibm,rtas-log partition, err -28
> [    0.128300] nvram: No room to create lnx,oops-log partition, deleting any 
> obsolete OS partitions...
> [    0.128701] nvram: Failed to find or create lnx,oops-log partition, err -28
> Linux ppc6400] nvram: Failed to initialize oops partition!

I've also that: perhaps SLOF is initializing the NVRAM when it is empty,
and qemu doesn't?

I will check.

Thanks,
Laurent

> 
> Cc'ing Nikunj who has already worked on booting a guest without SLOF.
> 
> 
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>  hw/ppc/spapr.c | 37 ++++++++++++++++++++-----------------
>>  1 file changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 7f33a1b..bbdb21d 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1219,8 +1219,11 @@ static void ppc_spapr_reset(void)
>>      first_ppc_cpu->env.gpr[3] = spapr->fdt_addr;
>>      first_ppc_cpu->env.gpr[5] = 0;
>>      first_cpu->halted = 0;
>> -    first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
>> -
>> +    if (machine->kernel_filename) {
>> +        first_ppc_cpu->env.nip = KERNEL_LOAD_ADDR;
>> +    } else {
>> +        first_ppc_cpu->env.nip = SPAPR_ENTRY_POINT;
>> +    }
>>  }
>>  
>>  static void spapr_create_nvram(sPAPRMachineState *spapr)
>> @@ -2023,23 +2026,23 @@ static void ppc_spapr_init(MachineState *machine)
>>              initrd_base = 0;
>>              initrd_size = 0;
>>          }
>> +    } else {
>> +        if (bios_name == NULL) {
>> +            bios_name = FW_FILE_NAME;
>> +        }
>> +        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> +        if (!filename) {
>> +            error_report("Could not find LPAR firmware '%s'", bios_name);
>> +            exit(1);
>> +        }
>> +        fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
>> +        if (fw_size <= 0) {
>> +            error_report("Could not load LPAR firmware '%s'", filename);
>> +            exit(1);
>> +        }
>> +        g_free(filename);
>>      }
>>  
>> -    if (bios_name == NULL) {
>> -        bios_name = FW_FILE_NAME;
>> -    }
>> -    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> -    if (!filename) {
>> -        error_report("Could not find LPAR firmware '%s'", bios_name);
>> -        exit(1);
>> -    }
>> -    fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE);
>> -    if (fw_size <= 0) {
>> -        error_report("Could not load LPAR firmware '%s'", filename);
>> -        exit(1);
>> -    }
>> -    g_free(filename);
>> -
>>      /* FIXME: Should register things through the MachineState's qdev
>>       * interface, this is a legacy from the sPAPREnvironment structure
>>       * which predated MachineState but had a similar function */
>