On Mon, 18 Dec 2023 at 07:26, Markus Armbruster <arm...@redhat.com> wrote: > > Peter Maydell <peter.mayd...@linaro.org> writes: > > > On Thu, 14 Dec 2023 at 17:14, Philippe Mathieu-Daudé <phi...@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 <arm...@redhat.com> > >> Signed-off-by: Philippe Mathieu-Daudé <phi...@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-phi...@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().
But we don't in most of these cases really want to know "is this a CPU with feature foo?". What we're asking is "does this QOM property exist so it won't blow up if I set/get it?". > 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. I agree, and the Arm CPU is a bit of an outlier in what it's doing, for reasons that are largely I think historical. I'd be happy to review patches that change these to class properties where applicable, but I suspect that might be tricky... thanks -- PMM
