On Fri, 6 Jan 2023 at 19:33, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 1/6/23 11:16, Peter Maydell wrote: > > On Tue, 3 Jan 2023 at 18:24, Richard Henderson > > <richard.hender...@linaro.org> wrote: > >> > >> Replace ARMCPU.mp_affinity with CPUARMState.cp15.mpidr_el1, > >> setting the additional bits as required. In particular, > >> always set the U bit when there is only one cpu in the system. > >> Remove the mp_is_up bit which attempted to do the same thing. > >> > >> Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > >> --- > >> target/arm/cpu.h | 7 ++-- > >> target/arm/cpu.c | 80 +++++++++++++++++++++++++++++++++++++------- > >> target/arm/cpu_tcg.c | 1 - > >> target/arm/helper.c | 25 ++------------ > >> target/arm/hvf/hvf.c | 2 +- > >> target/arm/kvm64.c | 4 +-- > >> 6 files changed, 75 insertions(+), 44 deletions(-) > > > > Based purely on the diffstat it's not super-obvious why this > > is an improvement. What's the rationale ? > > It gets rid of cpu->mp_is_up, set only by cortex-r5, and then generalizes. > > > Side note, we don't currently handle the MT bit, where some > > CPUs end up putting the cpu number in the Aff1 field rather > > than Aff0. We ideally ought to handle that too. > > Would that be set by the board as well? I don't think we model cpu > packages/clusters/whatchacallums at that level currently.
It's a fixed property of the CPU implementation, same as mp_is_up. We currently mismodel Cortex-A55, Cortex-A76 and Neoverse-N1, which should all be setting the MT bit in MPIDR and having the cpu number in Aff1. See this thread: https://lore.kernel.org/qemu-devel/CAFEAcA9P2-v94p8H8+ktnf-Yf-rucbGySXE6AGPdwvDxXfP=z...@mail.gmail.com/ + if (arm_feature(env, ARM_FEATURE_V7MP)) { + cpu->mpidr_el1 |= (1u << 31); /* M */ + if (cpu->core_count == 1) { + cpu->mpidr_el1 |= 1 << 30; /* U */ + } + } This is wrong, incidentally -- a single Cortex A9, A53, etc does not set the U bit. (It's "a cluster with 1 core in it", not "a uniprocessor system".) The reason mp_is_up is set only by the R5 model is because "we implement the MP extensions but consider ourselves to be not part of a cluster" is a behaviour of only this CPU. I forget why I didn't implement it as an ARM_FEATURE_FOO bit. thanks -- PMM
