[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/11] target-i386: cpu: replace custom apic-id
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH 05/11] target-i386: cpu: replace custom apic-id setter/getter with static property |
Date: |
Tue, 28 Jun 2016 08:43:59 +0200 |
On Mon, 27 Jun 2016 14:55:24 -0300
Eduardo Habkost <address@hidden> wrote:
> On Thu, Jun 23, 2016 at 04:54:23PM +0200, Igor Mammedov wrote:
> > custom apic-id setter/getter doesn't do any property specific
> > checks anymorer, so clean it up and use more compact static
> > property DEFINE_PROP_UINT32 instead.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> > target-i386/cpu.c | 45 ++++++---------------------------------------
> > 1 file changed, 6 insertions(+), 39 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 9511474..9294b3d 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1824,37 +1824,6 @@ static void x86_cpuid_set_tsc_freq(Object *obj,
> > Visitor *v, const char *name,
> > cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000;
> > }
> >
> > -static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, const char
> > *name,
> > - void *opaque, Error **errp)
> > -{
> > - X86CPU *cpu = X86_CPU(obj);
> > - int64_t value = cpu->apic_id;
> > -
> > - visit_type_int(v, name, &value, errp);
> > -}
> > -
> > -static void x86_cpuid_set_apic_id(Object *obj, Visitor *v, const char
> > *name,
> > - void *opaque, Error **errp)
> > -{
> > - X86CPU *cpu = X86_CPU(obj);
> > - DeviceState *dev = DEVICE(obj);
> > - Error *error = NULL;
> > - int64_t value;
> > -
> > - if (dev->realized) {
> > - error_setg(errp, "Attempt to set property '%s' on '%s' after "
> > - "it was realized", name, object_get_typename(obj));
> > - return;
> > - }
> > -
> > - visit_type_int(v, name, &value, &error);
> > - if (error) {
> > - error_propagate(errp, error);
> > - return;
> > - }
> > - cpu->apic_id = value;
> > -}
> > -
> > /* Generic getter for "feature-words" and "filtered-features" properties */
> > static void x86_cpu_get_feature_words(Object *obj, Visitor *v,
> > const char *name, void *opaque,
> > @@ -3127,9 +3096,6 @@ static void x86_cpu_initfn(Object *obj)
> > object_property_add(obj, "tsc-frequency", "int",
> > x86_cpuid_get_tsc_freq,
> > x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> > - object_property_add(obj, "apic-id", "int",
> > - x86_cpuid_get_apic_id,
> > - x86_cpuid_set_apic_id, NULL, NULL, NULL);
> > object_property_add(obj, "feature-words", "X86CPUFeatureWordInfo",
> > x86_cpu_get_feature_words,
> > NULL, NULL, (void *)env->features, NULL);
> > @@ -3139,11 +3105,6 @@ static void x86_cpu_initfn(Object *obj)
> >
> > cpu->hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
> >
> > -#ifndef CONFIG_USER_ONLY
> > - /* Any code creating new X86CPU objects have to set apic-id explicitly
> > */
> > - cpu->apic_id = UNASSIGNED_APIC_ID;
> > -#endif
> > -
> > for (w = 0; w < FEATURE_WORDS; w++) {
> > int bitnr;
> >
> > @@ -3200,6 +3161,12 @@ static bool x86_cpu_has_work(CPUState *cs)
> > }
> >
> > static Property x86_cpu_properties[] = {
> > +#ifdef CONFIG_USER_ONLY
> > + /* apic_id = 0 by default for *-user, see commit 9886e834 */
> > + DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, 0),
> > +#else
> > + DEFINE_PROP_UINT32("apic-id", X86CPU, apic_id, UNASSIGNED_APIC_ID),
> > +#endif
>
> Suggestion for a follow-up patch: setting the default to
> UNASSIGNED_APIC_ID unconditionally, and just adding this to
> x86_cpu_realizefn().
>
> #ifdef CONFIG_USER_ONLY
> if (cpu->apic_id == UNASSIGNED_APIC_ID) {
> cpu->apic_id = 0;
> }
> #endif
Putting default along with property definition seemed cleaner to me,
considering that we want do similar thing for socket/core/thread-ids
it still seems a better way.
BTW:
there is a v2 on list already, in case you missed it.
[PATCH v2 00/18] pc: add CPU hot-add/hot-remove with device_add/device_del
- Re: [Qemu-devel] [PATCH 03/11] pc: extract CPU lookup into a separate function, (continued)
[Qemu-devel] [PATCH 05/11] target-i386: cpu: replace custom apic-id setter/getter with static property, Igor Mammedov, 2016/06/23
[Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Igor Mammedov, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Eduardo Habkost, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Igor Mammedov, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Eduardo Habkost, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Igor Mammedov, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Eduardo Habkost, 2016/06/23
- Re: [Qemu-devel] [PATCH 06/11] target-i386: add socket/core/thread properties to X86CPU, Igor Mammedov, 2016/06/24
[Qemu-devel] [PATCH 08/11] pc: implement query-hotpluggable-cpus callback, Igor Mammedov, 2016/06/23
[Qemu-devel] [PATCH 09/11] pc: register created initial and hotpluged CPUs in one place pc_cpu_plug(), Igor Mammedov, 2016/06/23