Re: [PATCH kernel] RFC: prom_init: Fetch flatten device tree from the system firmware

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3019 bytes --]

On Mon, Oct 16, 2017 at 06:07:06PM +1100, Alexey Kardashevskiy wrote:
> On 16/10/17 17:46, David Gibson wrote:
> > On Mon, Oct 16, 2017 at 05:22:55PM +1100, Alexey Kardashevskiy wrote:
> >> On 16/10/17 17:11, David Gibson wrote:
> >>> On Mon, Oct 16, 2017 at 04:49:17PM +1100, Alexey Kardashevskiy wrote:
> >>>> At the moment, on 256CPU + 256 PCI devices guest, it takes the guest
> >>>> about 8.5sec to read the entire device tree. Some explanation can be
> >>>> found here: https://patchwork.ozlabs.org/patch/826124/ but mostly it is
> >>>> because the kernel traverses the tree twice and it calls "getprop" for
> >>>> each properly which is really SLOF as it searches from the linked list
> >>>> beginning every time.
> >>>>
> >>>> Since SLOF has just learned to build FDT and this takes less than 0.5sec
> >>>> for such a big guest, this makes use of the proposed client interface
> >>>> method - "fdt-fetch".
> >>>>
> >>>> If "fdt-fetch" is not available, the old method is used.
> >>>>
> >>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >>>
> >>> I like the concept, few details though..
> >>>
> >>>> ---
> >>>>  arch/powerpc/kernel/prom_init.c | 26 ++++++++++++++++++++++++++
> >>>>  1 file changed, 26 insertions(+)
> >>>>
> >>>> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> >>>> index 02190e90c7ae..daa50a153737 100644
> >>>> --- a/arch/powerpc/kernel/prom_init.c
> >>>> +++ b/arch/powerpc/kernel/prom_init.c
> >>>> @@ -2498,6 +2498,31 @@ static void __init flatten_device_tree(void)
> >>>>  		prom_panic("Can't allocate initial device-tree chunk\n");
> >>>>  	mem_end = mem_start + room;
> >>>>  
> >>>> +	if (!call_prom_ret("fdt-fetch", 2, 1, NULL, mem_start,
> >>>> +			   room - sizeof(mem_reserve_map))) {
> >>>> +		u32 size;
> >>>> +
> >>>> +		hdr = (void *) mem_start;
> >>>> +
> >>>> +		/* Fixup the boot cpuid */
> >>>> +		hdr->boot_cpuid_phys = cpu_to_be32(prom.cpu);
> >>>
> >>> If SLOF is generating a tree it really should get this header field
> >>> right as well.
> >>
> >>
> >> Ah, I did not realize it is just a phandle from /chosen/cpu. Will
> >> fix.
> > 
> > It's not a phandle.  It's just the "address" (i.e. reg value) of the
> > boot cpu.
> 
> 
> Well, it is "reg" of a CPU with phandle==/chosen/cpu so my fdt code needs
> to look there to pick the right "reg" rather than just plain 0.

Ah, right, I see what you mean.

> I'll fix
> this but in general can it possibly be not a zero in QEMU/SLOF?

Erm.. probably not, but I'm not totally certain what could happen if
you tried creating all your cpu cores explicitly with -device instead
of just using -smp.

I think it's safer to look it up in SLOF, so that it won't break if we
change how cpu addresses are assigned in qemu.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]