[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-i386: initialize APIC at CPU level
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] target-i386: initialize APIC at CPU level |
Date: |
Fri, 5 Oct 2012 17:32:54 +0200 |
On Thu, 04 Oct 2012 18:15:48 +0200
Andreas Färber <address@hidden> wrote:
> > + env->apic_state = qdev_create(NULL, apic_type);
> > +
> > + if (env->apic_state == NULL) {
> > + error_set(errp, QERR_DEVICE_INIT_FAILED, apic_type);
> > + return;
> > + }
> > +
> > + object_property_add_child(OBJECT(cpu), "apic",
> > + OBJECT(env->apic_state), NULL);
> > + qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
> > + /* TODO: convert to link<> */
> > + qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
>
> Do we still need env->apic_state and a property on the APIC at all? From
> the X86CPU side we can access the "apic" property to retrieve the
> pointer,
It would introduce too much overhead. For test I've just substituted all uses
of env->apic_state with a func that could serve as get_child(name)
+DeviceState *get_apic(CPUX86State *env) {
+ X86CPU *cpu = x86_env_get_cpu(env);
+ Object *o = object_resolve_path_component(OBJECT(cpu), (gchar *)"apic");
+ DeviceState *dev = OBJECT_CHECK(DeviceState, o, "device");
+ return dev;
+}
and run qemu under valgrind --tool=callgrind, booting openbsd for 5 min.
hot path get_apic() call count get_apic() incl cost.
TCG: pic_irq_request() 7M 44 (8th from top 10)
KVM: kvm_arch_post_run() 2M 38 (6th from top 10)
NO PATCH: pic_irq_request() 18M 0.8
pic_irq_request() calls get_apic() 3x, kvm_arch_post_run() - 2x times
I suppose calling get_apic() only once on hot path could reduce amount of
calls but property searching and dynamic cast won't disappear any way.
Looks like it's better to leave apic_state as it is.
> and from the APIC we should be able to navigate to its parent,
> right?
while adding object_get_parent(), I've noticed in object.h
"
* There is no way for a child to determine what its parent is. It is not
* a bidirectional relationship. This is by design.
"
Would be it acceptable to allow child access to parent?
...
>
> Andreas
>
Thanks,
Igor