Hi Drew, On Mon, Mar 24, 2025 at 09:19:27AM +0100, Andrew Jones wrote: > On Sun, Mar 23, 2025 at 11:16:19AM +0000, Alexandru Elisei wrote: > ... > > > > +if [ -z "$qemu_cpu" ]; then > > > > + if ( [ "$ACCEL" = "kvm" ] || [ "$ACCEL" = "hvf" ] ) && > > > > + ( [ "$HOST" = "aarch64" ] || [ "$HOST" = "arm" ] ); then > > > > + qemu_cpu="host" > > > > if [ "$ARCH" = "arm" ] && [ "$HOST" = "aarch64" ]; then > > > > - processor+=",aarch64=off" > > > > + qemu_cpu+=",aarch64=off" > > > > fi > > > > + elif [ "$ARCH" = "arm64" ]; then > > > > + qemu_cpu="cortex-a57" > > > > + else > > > > + qemu_cpu="cortex-a15" > > > > > > configure could set this in config.mak as DEFAULT_PROCESSOR, avoiding the > > > need to duplicate it here. > > > > That was my first instinct too, having the default value in config.mak seemed > > like the correct solution. > > > > But the problem with this is that the default -cpu type depends on -accel (set > > via unittests.cfg or as an environment variable), host and test architecture > > combination. All of these variables are known only at runtime. > > > > Let's say we have DEFAULT_QEMU_CPU=cortex-a57 in config.mak. If we keep the > > above heuristic, arm/run will override it with host,aarch64=off. IMO, having it > > in config.mak, but arm/run using it only under certain conditions is worse than > > not having it at all. arm/run choosing the default value **all the time** is at > > least consistent. > > I think having 'DEFAULT' in the name implies that it will only be used if > there's nothing better, and we don't require everything in config.mak to > be used (there's even some s390x-specific stuff in there for all > architectures...) I'm still leaning towards having the default value and the heuristics for when to pick it in one place ($ARCH/run) as being more convenient, but I can certainly see your point of view. So yeah, up to you :) > > > > > We could modify the help text for --qemu-cpu to say something like "If left > > unset, the $ARCH/run script will choose a best value based on the host system > > and test configuration." > > This is helpful, so we should add it regardless. Thanks, Alex
