Re: [Qemu-arm] [Qemu-devel] [PATCH for-2.13 v2 5/5] arm/boot: split load_dtb() from arm_load_kernel()

[Peter Maydell]

On 27 April 2018 at 14:47, Andrew Jones <address@hidden> wrote:
> On Wed, Apr 18, 2018 at 04:28:05PM +0200, Igor Mammedov wrote:
>> @@ -1173,15 +1141,6 @@ static void arm_load_kernel_notify(Notifier 
>> *notifier, void *data)
>>      for (cs = CPU(cpu); cs; cs = CPU_NEXT(cs)) {
>>          ARM_CPU(cs)->env.boot_info = info;
>>      }
>
> I wonder why we need to start at cpu here, but first_cpu below. If
> they could both be first_cpu, then we could merge the loop statements
> into one loop. Reading enough code to build confidence that it could
> be first_cpu is too much to ask for a Friday afternoon though...

It should be starting at first_cpu -- starting with 'cpu' is
a bug. However as with the bug fixed in 75ed2c02484101d5, it
isn't currently causing any incorrect behaviour, because every
board we have is passing first_cpu as the boot cpu, either directly
or indirectly.

There is a theoretical use case for only feeding the boot_info
to a subset of CPUs, which is where you have a setup like
the xilinx zynqmp which has 4x A-class cores which run Linux
and 4x R-class cores which run something else; in that setup
you might want to say "the boot_info stuff we have here is
just for the A-class cluster, and the R-class cores should
look after themselves". However, (a) we don't really properly
support heterogenous setups like that -- zynqmp works by
accident rather than design -- and (b) if we do want to
support them we need a sensible API for indicating which
CPUs should or should not be involved in -kernel boot as
primary or secondaries.

So we should fix this loop to start at first_cpu, and worry
about the heterogenous setup usecase if and when it becomes
reality rather than theory.

thanks
-- PMM