On 7/2/2025 8:12 PM, Paolo Bonzini wrote:
Il mer 2 lug 2025, 07:42 Xiaoyao Li <xiaoyao...@intel.com> ha scritto:
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?
I wrote the patch because of a latent issue with TDX. The issue was roughly
that a compat property was overwriting the TDX-specific modifications to
CPUID. I think this case shows that you *do* want the subclass to overwrite
the compat property—of course the subclass code must be aware that compat
properties exist and limit changes appropriately.
IIRC that's on rhel QEMU which ports the TDX code before it's merged
upstream. Now TDX is upstreamed, it works with upstream compat property
and I think future new compat property won't affect TDX or anything,
since it's compat property and it's to guarantee the existing behavior
when introducing new behavior?
In general I don't see how the reverse order makes sense: the subclass
knows what the superclass does, so it can do the right thing if it runs
last; but the superclass cannot know what all of its subclasses do in
post_init, so it makes less sense to run it last.
I agree in general the parent to child order makes more sense,
especially when we treat .instance_init() as the phase 1 init and
.post_instance_init() as the phase 2 init.
But the original purpose of introducing .post_instance_init() was to
ensure qdev_prop_set_globals() is called at last for Device. Reverse the
order breaks this purpose.
It seems not good option like the reversed order from child to parent
that can achieve it by just putting the last step in top parent's.
I'm looking forward to seeing a better solution.
Paolo
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
