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

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> 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.

Ahh still part of the larger series:

  Subject: [PATCH  v16 99/99] gitlab: defend the new stripped down arm64 configs
  Date: Fri,  4 Jun 2021 16:53:12 +0100
  Message-Id: <20210604155312.15902-100-alex.bennee@linaro.org>


>
>>
>> -- PMM


-- 
Alex Bennée