Re: [Qemu-devel] [PATCH v3 6/7] bcm2836: add bcm2836 soc device

[Andrew Baumann]

Hi Peter,

Thanks again for the reviews.

> From: Peter Crosthwaite [mailto:address@hidden
> Sent: Monday, 11 January 2016 19:57
> On Thu, Dec 31, 2015 at 04:31:33PM -0800, Andrew Baumann wrote:
> > +    /* TODO: probably shouldn't be using smp_cpus here */
> 
> I agree. I have started ignoring smp_cpus completely for new ARM SoCs,
> as if you change the number of CPUs for a SoC, it is not that SoC
> anymore. The virt platform is suitable for CPU scalability, whereas
> with ARM SoCs, cpu # variation is invalid.
> 
> > +    assert(smp_cpus <= BCM2836_NCPUS);
> > +    for (n = 0; n < smp_cpus; n++) {
> 
> So can we just use BCM2836_NCPUS here as the loop bound? Any
> conditionals out there check the existance of CPUs can be removed or
> promoted to assert() as a BCM2836 must always have 4 CPUs.

I'd love to do that, but there's at least one good reason to respect the -smp 
parameter and not start all the CPUs: with full-system emulation, qemu is 
noticeably faster emulating a single-core target than multi-core. E.g., Linux 
boots fine with -smp 1 (it fails to start the other CPUs but proceeds with just 
one), and many users will be better off running it this way, so I definitely 
don't want to break that.

I tried always initing all 4 CPUs in bcm2836_init, and only starting a 
configurable number (based on a property) in realize. However, arm 
cpu_exec_init already adds the cpu to the global list of all CPUs, and if you 
try to start the system in this state it quickly segfaults on uninitialized 
state in the CPU, so it seems we shouldn't even init CPUs that won't later be 
started. However, I can't refer to properties in the _init method, which is why 
I'm stuck using the smp_cpus global.

If you have a better suggestion, I'm all ears :) Would it make sense to defer 
calling init() until the realize method, when we can access a property?

Cheers,
Andrew