Re: [PATCH v3 13/14] hw/arm: Prefer arm_feature(AARCH64) over object_property_find(aarch64)

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 11/1/24 10:47, Marc Zyngier wrote:
> On Thu, 11 Jan 2024 09:39:18 +0000,
> Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 10/1/24 20:53, Philippe Mathieu-Daudé wrote:
>>> The "aarch64" property is added to ARMCPU when the
>>> ARM_FEATURE_AARCH64 feature is available. Rather than
>>> checking whether the QOM property is present, directly
>>> check the feature.
>>>
>>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/arm/virt.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 49ed5309ff..a43e87874c 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -2140,7 +2140,7 @@ static void machvirt_init(MachineState *machine)
>>>            numa_cpu_pre_plug(&possible_cpus->cpus[cs->cpu_index], DEVICE(cpuobj),
>>>                              &error_fatal);
>>>    -        aarch64 &= object_property_get_bool(cpuobj, "aarch64",
>>> NULL);
>>> +        aarch64 &= arm_feature(cpu_env(cs), ARM_FEATURE_AARCH64);
>>
>> So after this patch there are no more use of the ARMCPU "aarch64"
>> property from code. Still it is exposed via the qom-tree. Thus it
>> can be set (see aarch64_cpu_set_aarch64). I could understand one
>> flip this feature to create a custom CPU (as a big-LITTLE setup
>> as Marc mentioned on IRC), but I don't understand what is the
>> expected behavior when this is flipped at runtime. Can that
>> happen in real hardware (how could the guest react to that...)?
> 
> I don't think it makes any sense to do that while a guest is running
> (and no HW I'm aware of would do this). However, it all depends what
> you consider "run time". You could imagine creating a skeletal VM with
> all features, and then apply a bunch of changes before the guest
> actually runs.

Thanks, this makes sense and confirms my guess.

> I don't know enough about the qom-tree and dynamic manipulation of
> these properties though, and I'm likely to be wrong about the expected
> usage model.

Kevin, Markus, this seems a good example of QOM "config" property that
is RW *before* Realize and should become RO *after* it.

QDev properties has PropertyInfo::realized_set_allowed set to false by
default, but here this property is added at the QOM (lower) layer, so
there is no such check IIUC.

Should "aarch64" become a static QDev property instead (registered via
device_class_set_props -> qdev_class_add_property)?

This just an analyzed example, unfortunately there are many more...

Thanks,

Phil.