Re: [PATCH v1 1/2] hw/tricore: fix inclusion of tricore_testboard

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Tue, 20 Jul 2021 at 10:47, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote:
>> >>
>> >> We inadvertently added a symbol clash causing the build not to include
>> >> the testboard needed for check-tcg.
>> >>
>> >> Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig")
>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >> ---
>> >>  configs/devices/tricore-softmmu/default.mak | 1 +
>> >>  hw/tricore/Kconfig                          | 3 +--
>> >>  hw/tricore/meson.build                      | 4 ++--
>> >>  3 files changed, 4 insertions(+), 4 deletions(-)
>> >
>> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> > as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be 
>> > better?)
>> >
>> > But I still don't understand and would like to know:
>> > (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as
>> > f4063f9c31 claims to be doing?
>>
>> It does (or should) thanks to meson:
>>
>>   'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y'
>
> Yeah, but it doesn't, as you can see if you look at the meson build
> log: we do pass CONFIG_TRICORE=y on the minikconf command line,
> but it doesn't appear in minikconf's output!
>
>> > (2) what are the CONFIG_$ARCH flags for? Apart from this, we
>> > don't seem to be using any of them, as demonstrated by the fact
>> > that nothing else broke :-)
>>
>> They need to be declared in Kconfig otherwise minikconf complains about
>> them not being defined when you pass it in. This is part of minikconf's
>> sanity checking code.
>
> No, I mean, if nothing anywhere in the build system is conditional
> on these flags, why do we have them at all ? We know we don't
> have anything that cares about them, because right now we have
> a bug where they're never set...

Well we have one place at the moment to ensure v7m gets included even if
you don't include the various M profile boards:

  default y if TCG && (ARM || AARCH64)

which is because translate.c still currently has a dependency on those
bits. Without that you'll get a linker failure with the following build:

  '../../configure' '--without-default-features' 
'--target-list=arm-softmmu,aarch64-softmmu' '--with-devices-aarch64=minimal'

I thought I'd added that to the build matrix but I can't find it now. 

>
> -- PMM


-- 
Alex Bennée