On Fri, May 09, 2025 at 12:04:19PM +0200, Thomas Huth wrote:
> Date: Fri, 9 May 2025 12:04:19 +0200
> From: Thomas Huth <th...@redhat.com>
> Subject: How to mark internal properties (was: Re: [PATCH v4 12/27]
> target/i386/cpu: Remove CPUX86State::enable_cpuid_0xb field)
>
> On 09/05/2025 09.32, Zhao Liu wrote:
> > On Fri, May 09, 2025 at 02:49:27PM +0800, Xiaoyao Li wrote:
> > > Date: Fri, 9 May 2025 14:49:27 +0800
> > > From: Xiaoyao Li <xiaoyao...@intel.com>
> > > Subject: Re: [PATCH v4 12/27] target/i386/cpu: Remove
> > > CPUX86State::enable_cpuid_0xb field
> > >
> > > On 5/8/2025 9:35 PM, Philippe Mathieu-Daudé wrote:
> > > > The CPUX86State::enable_cpuid_0xb boolean was only disabled
> > > > for the pc-q35-2.6 and pc-i440fx-2.6 machines, which got
> > > > removed. Being now always %true, we can remove it and simplify
> > > > cpu_x86_cpuid().
> > > >
> > > > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> > > > ---
> > > > target/i386/cpu.h | 3 ---
> > > > target/i386/cpu.c | 6 ------
> > > > 2 files changed, 9 deletions(-)
> > > >
> > > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> > > > index 0db70a70439..06817a31cf9 100644
> > > > --- a/target/i386/cpu.h
> > > > +++ b/target/i386/cpu.h
> > > > @@ -2241,9 +2241,6 @@ struct ArchCPU {
> > > > */
> > > > bool legacy_multi_node;
> > > > - /* Compatibility bits for old machine types: */
> > > > - bool enable_cpuid_0xb;
> > > > -
> > > > /* Enable auto level-increase for all CPUID leaves */
> > > > bool full_cpuid_auto_level;
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > index 49179f35812..6fe37f71b1e 100644
> > > > --- a/target/i386/cpu.c
> > > > +++ b/target/i386/cpu.c
> > > > @@ -6982,11 +6982,6 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> > > > index, uint32_t count,
> > > > break;
> > > > case 0xB:
> > > > /* Extended Topology Enumeration Leaf */
> > > > - if (!cpu->enable_cpuid_0xb) {
> > > > - *eax = *ebx = *ecx = *edx = 0;
> > > > - break;
> > > > - }
> > > > -
> > > > *ecx = count & 0xff;
> > > > *edx = cpu->apic_id;
> > > > @@ -8828,7 +8823,6 @@ static const Property x86_cpu_properties[] = {
> > > > DEFINE_PROP_UINT64("ucode-rev", X86CPU, ucode_rev, 0),
> > > > DEFINE_PROP_BOOL("full-cpuid-auto-level", X86CPU,
> > > > full_cpuid_auto_level, true),
> > > > DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
> > > > - DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
> > >
> > > It's deprecating the "cpuid-0xb" property.
> > >
> > > I think we need go with the standard process to deprecate it.
> >
> > Thanks! I got your point.
> >
> > Though this property is introduced for compatibility, as its comment
> > said "Compatibility bits for old machine types", it is also useful for
> > somer users.
>
> Thanks for your clarifications, Zhao! But I think this shows again the
> problem that we have hit a couple of times in the past already: Properties
> are currently used for both, config knobs for the users and internal
> switches for configuration of the machine. We lack a proper way to say "this
> property is usable for the user" and "this property is meant for internal
> configuration only".
Hi Thomas, thank you.
AFAIK, there are two ways to configure whether an object/device is
allowed to be created by user or not:
* TYPE_USER_CREATABLE
* DeviceClass: user_creatable
So, it looks like it would be tricky to change the infrastructure around
object_property_add because it's not easy to be compatible with both of the
above user creation ways.
> I wonder whether we could maybe come up with a naming scheme to better
> distinguish the two sets, e.g. by using a prefix similar to the "x-" prefix
> for experimental properties? We could e.g. say that all properties starting
> with a "q-" are meant for QEMU-internal configuration only or something
> similar (and maybe even hide those from the default help output when running
> "-device xyz,help" ?)? Anybody any opinions or better ideas on this?
Therefore, I think the “q-” prefix might be a good way, simple and effective.
Let's see if any other maintainers have a better idea.
Regards,
Zhao
