Re: [PATCH 07/13] target/arm/cpu: always define kvm related registers

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

On 3/18/25 11:14, Philippe Mathieu-Daudé wrote:
> On 18/3/25 05:51, Pierrick Bouvier wrote:
>> This does not hurt, even if they are not used.
> 
> I'm not convinced by the rationale.
> 
>>
>> 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 23c2293f7d1..96f7801a239 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 */
> 
> Maybe we need an opaque ArchAccelCpuState structure...
> 

It's similar to the interesting question of how to expose some registers 
conditionnally.

We could put this in another struct, allocate if only if needed 
(kvm_enabled()), or just let it be present anytime like it is done with 
this patch.

I don't have a strong opinion, but having conditional presence here is 
just making things complicated without introducing any benefit. It does 
not prevent "unauthorized" access to it.

Now, if we start to have something more clean, implemented in another 
compilation units, only related to kvm, well, that could be useful. But 
we have to define the interface for that, add it to other architectures, 
and probably spend a few months in the middle where things are stuck here.

>>    
>>        /* Uniprocessor system with MP extensions */
>>        bool mp_is_up;
>