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

[Alexey Kardashevskiy]

On 06/07/16 11:35, David Gibson wrote:
> On Tue, Jul 05, 2016 at 04:42:37PM +0200, Laurent Vivier 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.
>>
>> Signed-off-by: Laurent Vivier 
> 
> I'm not comfortable applying this.  We actually used to do this ages
> ago, but changed to always running through SLOF, and there were
> reasons for doing so.
> 
> I don't remember exactly what they were, but I think it boiled down to
> slight differences in state between booting from SLOF and booting
> without SLOF leading to confusing errors from the guest kernel.
> 

PCI resource allocation is still done by SLOF (however having them not set
will trigger allocation in the guest but this is rather unexpected
workaround than a feature); "client-architecture-support" won't work
without SLOF either (i.e. compatibile PowerISA 2.0x CPUs).



-- 
Alexey



signature.asc
Description: OpenPGP digital signature

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

[David Gibson]

On Tue, Jul 05, 2016 at 04:42:37PM +0200, Laurent Vivier 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.
> 
> Signed-off-by: Laurent Vivier 

I'm not comfortable applying this.  We actually used to do this ages
ago, but changed to always running through SLOF, and there were
reasons for doing so.

I don't remember exactly what they were, but I think it boiled down to
slight differences in state between booting from SLOF and booting
without SLOF leading to confusing errors from the guest kernel.

> ---
>  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 */

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature