On 7/2/2025 3:56 PM, Zhao Liu wrote:
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0d35e95430fe..bf290262cbfe 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -9044,6 +9044,12 @@ static void x86_cpu_post_initfn(Object *obj)
X86_CONFIDENTIAL_GUEST(current_machine->cgs), (CPU(obj)));
}
#endif
+
+ /*
+ * Re-apply the "feature[=foo]" from '-cpu' option since they might
+ * be overwritten by above
+ */
+ qdev_prop_set_globals(DEVICE(obj));
}
This patch LGTM.
This solution will call qdev_prop_set_globals() twice. My concern is
that this masks the problem: previous x86 CPU assumptions about the
order of global property initialization break down...
Per the commit message of Paolo's commit:
"This is incorrect because the leaf class cannot observe property
values applied by the superclasses; for example, a compat property
will be set on a device *after* the class's post_init callback has
run."
After checking the history of why .post_instance_init() was introduced
in the first place: 8231c2dd2203 ("qom: Introduce instance_post_init
hook"). It turns out to be used for qdev_prop_set_globals(), to ensure
global properties being applied after all sub-device specific
instance_init() were called. And I think the order from child to parent
was defined purposely.
And the reverse of Paolo's patch breaks the usage of "-global" than it
won't take effect if the sub-device changes the property in its
post_instance_init() callback.
Back to Paolo's example of "a compat property will be set on a device
*after* the class's post_init callback has run". I think the behavior of
compat property is applied after the class's post_init callback is also
what we want. If reversing the order, then compat prop can be
overwritten by subclass's post_init callback, and doesn't it fail the
purpose of compat prop?
So I think we might revert this patch, and document clearly the reverse
order of .post_instance_init() callback.
X86 CPUs have the issue (e.g., "vendor" doesn't work) now because
they - as leaf class, don't care about the property values of
superclass - the DeviceState. If a property is just for initialization,
like "vendor", it should be placed in the instance_init() instead of
instance_post_init().
In addition, if other places handle it similarly, the device's
post_init seems pointless. :-(
Thanks,
Zhao
