Re: [RFC PATCH] hw/arm: Prefer arm_feature() over object_property_find()

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 14 Dec 2023 at 17:14, Philippe Mathieu-Daudé <philmd@linaro.org> 
> wrote:
>>
>> QOM properties are added on the ARM vCPU object when a
>> feature is present. Rather than checking the property
>> is present, check the feature.
>>
>> Suggested-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> RFC: If there is no objection on this patch, I can split
>>      as a per-feature series if necessary.
>>
>> Based-on: <20231123143813.42632-1-philmd@linaro.org>
>>   "hw: Simplify accesses to CPUState::'start-powered-off' property"
>
> I'm not a super-fan of board-level code looking inside
> the QOM object with direct use of arm_feature() when
> it doesn't have to. What's wrong with asking whether
> the property exists before trying to set it?

I'm not a fan of using QOM instead of the native C interface.

The native C interface is CPUArmState method arm_feature().

Attempts to use it on anything but a CPUArmState * will be caught by the
compiler.  object_property_find() will happily take any Object.

Likewise, typos in its second argument will be caught by the compiler.
object_property_find() will happily return NULL then.


I also don't like adding QOM properties to instances.
arm_cpu_post_init() seems to do that.  I feel it's best to stick to
class properties whenever practical.

More so when management applications are expected to use them, because
introspection is much easier to use correctly when existence of
properties doesn't depend on run-time state.

Kevin's "[RFC PATCH 00/12] QOM/QAPI integration part 1" explores
QAPIfication of object configuration, loosely based on
<https://wiki.qemu.org/Features/QOM-QAPI_integration>.  A QOM class's
instance configuration interface becomes compile-time static.