Re: [kvm-unit-tests PATCH v2 4/5] configure: Add --qemu-cpu option

[Andrew Jones <andrew.jones@linux.dev>]

On Mon, Mar 24, 2025 at 10:41:03AM +0000, Alexandru Elisei wrote:
> 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.

I wouldn't mind it only being in $ARCH/run, but this series adds the same
logic to $ARCH/run and to ./configure. I think an additional, potentially
unused, variable in config.mak is better than code duplication.

Thanks,
drew