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