Re: [PATCH v2 20/30] target/arm/cpu: always define kvm related registers

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/2/25 06:36, Philippe Mathieu-Daudé wrote:
> On 25/3/25 02:24, Richard Henderson wrote:
>> On 3/24/25 14:11, Pierrick Bouvier wrote:
>>> On 3/23/25 12:37, Richard Henderson wrote:
>>>> On 3/20/25 15:29, Pierrick Bouvier wrote:
>>>>> This does not hurt, even if they are not used.
>>>>>
>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>> ---
>>>>>     target/arm/cpu.h | 2 --
>>>>>     1 file changed, 2 deletions(-)
>>>>>
>>>>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>>>>> index a8a1a8faf6b..ab7412772bc 100644
>>>>> --- a/target/arm/cpu.h
>>>>> +++ b/target/arm/cpu.h
>>>>> @@ -971,7 +971,6 @@ struct ArchCPU {
>>>>>          */
>>>>>         uint32_t kvm_target;
>>>>> -#ifdef CONFIG_KVM
>>>>>         /* KVM init features for this CPU */
>>>>>         uint32_t kvm_init_features[7];
>>>>> @@ -984,7 +983,6 @@ struct ArchCPU {
>>>>>         /* KVM steal time */
>>>>>         OnOffAuto kvm_steal_time;
>>>>> -#endif /* CONFIG_KVM */
>>>>>         /* Uniprocessor system with MP extensions */
>>>>>         bool mp_is_up;
>>>>
>>>> I'm not sure what this achieves?   CONFIG_KVM is a configure-time
>>>> selection.
>>>>
>>>
>>> CONFIG_KVM is a poisoned identifier.
>>> It's included via config-target.h, and not config-host.h.
>>
>> Whoops, yes.
> 
> If we go this way, can we consistently allow CONFIG_${HW_ACCEL}
> (read "remove poisoned defs in config-poison.h)?

It would be safe to do this for CONFIG_TCG, which is applied to all 
compilation units (through config_host). And we'll do it when we meet a 
case that really needs it, not before. As long as the code can be 
cleaned up from those ifdefs, it's better.

However, it's not safe for all other CONFIG_${HW_ACCEL}, which are 
applied selectively on some targets (basically, for the pair {host == 
target}, when host supports this acceleration).
For them, the right fix is to make sure we call "{accel}_enabled()", 
expose the associated code, and eventually deal with missing symbols at 
link.