Re: [Qemu-arm] [PATCH v3 00/11] hw: arm: exynos: Bring up secondary CPU, QOM-ify Soc, other improvements

[Krzysztof Kozlowski]

On Thu, Jun 01, 2017 at 05:31:41PM +0100, Peter Maydell wrote:
> On 31 May 2017 at 09:58, Krzysztof Kozlowski <address@hidden> wrote:
> > On Tue, May 30, 2017 at 2:04 PM, Peter Maydell <address@hidden> wrote:
> >> So is this a regression compared to our current model? I was
> >> under the impression from the previous thread that it was,
> >> which is why I didn't apply that patchset.
> >
> > It depends how wide understanding of regression you have. :)
> > 1. Before this patch, second CPU could not boot. System was usable but
> > worked on only one CPU.
> > 2. Before this patch, kernel's IRQ work is broken. None of IRQ work
> > are executed which is mostly visible during poweroff - system hangs on
> > syncing IRQ works.
> > 3. With this patch, second CPU boots and IRQ works properly (so
> > poweroff is possible).
> > 4. However with this patch, Linux kernel with cpuidle enabled (which
> > is also included in many kernel defconfigs), the system is very
> > unresponsive.
> >
> > So overall... a lot of things were broken already. This patches fixes
> > them... but breaks different thing.
> 
> It sounds like it breaks a key thing, ie "boot a single cpu kernel
> built from a defconfig", though. That's a regression I'd rather
> not have.

I am not sure if I understand you correctly... the system previously
booted into one CPU mode because second CPU just could not boot. Now by
default, second CPU succeeds and this means that *default* settings are
indeed more broken than before.

On the other hand, after removing a line from hw/arm/exynos4210.c:
        qdev_prop_set_uint32(dev, "num-cpu", EXYNOS4210_NCPUS);
and running qemu with "-accel tcg -smp cpus=1,maxcpus=1,cores=1"
everything is okay. Booting of one CPU is unaffected.

> If there's a subset of these patches which don't break
> that I'm happy to take those. Otherwise we need to investigate
> and fix whatever the problem is that causes that unresponsiveness
> before we can apply them.

Yes, you can take everything except first patch
("hw/intc/exynos4210_gic: Fix GIC memory mappings for secondary CPU")
and it should be fine. Rest of patches are just independent.

Best regards,
Krzysztof