Re: [Qemu-arm] [PATCH v4] Add arm SBSA reference machine

[Hongbo Zhang]

On Fri, 16 Nov 2018 at 17:58, Peter Maydell <address@hidden> wrote:
>
> On 16 November 2018 at 08:23, Hongbo Zhang <address@hidden> wrote:
> > Well, for the SMP booting, when GICv2 used, there is no problem, max
> > CPU number 8 can be booted, including all the three cases: kernel
> > only, UEFI+kernel and ATF+UEFI+kernel.
> >
> > But when GICv3 used, these two cases still work: kernel only, and
> > UEFI+kernel, but ATF+UEFI+kernel fails booting more than 4 cores with
> > GICv3.
> > The original ATF didn't support GICv3, so I added the support:
> > http://git.linaro.org/people/hongbo.zhang/atf-sbsa.git/log/?h=sbsa_gicv3
> >
> > Root cause of failing to boot more than 4 cores with ATF+UEFI+kernel
> > with my GICv3 enabled is due to this:
> > In QEMU, we have this defination
> > #define ARM_DEFAULT_CPUS_PER_CLUSTER 8
> > But in ATF, the defination is
> > #define PLATFORM_MAX_CPUS_PER_CLUSTER 4
> > So when we pass smp=6 for example, QEMU generates MPIDR showing all
> > the 6 cores are at cluster 0, but when ATF parses such MPIDR, the
> > function plat_core_pos_by_mpidr() in plat/qemu/topology.c returns
> > error since there should be no more cores than 4.
> >
> > I think we should change the definition in QEMU to 4, instead of
> > changing the ATF's, because I checked Cortext a57/a72/a73/a75 spec, it
> > says there are 4 cores max at one cluster.
>
> Architecturally 8 per cluster is entirely legal. This is
> an ATF limitation and there's an argument for fixing it there.
> However, since for this board we're trying to match what real
> hardware with these CPUs would look like, it does make sense
> to use the same number of CPUs per cluster as the hardware.
> If you want your board to instantiate 4 per cluster this is possible,
> and you don't need to change ARM_DEFAULT_CPUS_PER_CLUSTER.
> This value is only used if the board code does not set the
> cpu->mp_affinity value itself (it is the default, as the
> name suggests). So you can just make your board code set
> the mp_affinity up however you like, after the CPU object
> is created but before it is realized.
>
Get it, thanks.

> thanks
> -- PMM