On 05/30/2012 04:38 PM, Jan Kiszka wrote:
On 2012-05-30 16:27, Igor Mammedov wrote:
+
+#ifndef CONFIG_USER_ONLY
+#define MSI_ADDR_BASE 0xfee00000
+
+static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
+{
+ static int apic_mapped;
+ CPUX86State *env =&cpu->env;
+
+ if (env->apic_state == NULL) {
+ return;
+ }
+
+ if (qdev_init(env->apic_state)) {
+ error_set(errp, QERR_DEVICE_INIT_FAILED,
+ object_get_typename(OBJECT(env->apic_state)));
+ return;
+ }
+
+ /* XXX: mapping more APICs at the same memory location */
+ if (apic_mapped == 0) {
+ /* NOTE: the APIC is directly connected to the CPU - it is not
+ on the global memory bus. */
+ /* XXX: what if the base changes? */
+ sysbus_mmio_map(sysbus_from_qdev(env->apic_state), 0, MSI_ADDR_BASE);
+ apic_mapped = 1;
}
}
+#endif
void x86_cpu_realize(Object *obj, Error **errp)
{
X86CPU *cpu = X86_CPU(obj);
+ x86_cpu_apic_init(cpu, errp);
+ if (error_is_set(errp)) {
+ return;
+ }
+
I guess you are lacking #ifndef CONFIG_USER_ONLY here already (for the
sake of bisectability). Or, likely better, let x86_cpu_apic_init do
nothing for usermode emulation.
initially x86_cpu_apic_init() was doing nothing for usermode, hence lack of
#ifndef CONFIG_USER_ONLY, but then for patch 12/12 #ifndef CONFIG_USER_ONLY
required here any way so I've removed usermode stub x86_cpu_apic_init() and
squashed change in wrong patch :(.
And since I need #ifndef in initfn anyway then probably there is no point
in having x86_cpu_apic_init(), I'll move its body in initfn then.
I think a function is cleaner, and we have some other examples for this
already in this context. Its body could easily be #ifdef'ed out (the
other reason for #ifdef in the initfn are temporary, no?).
Yes, is temporary.
However if Peter could be persuaded not to object for putting mapping of APIC
base
into apic's initfn then x86_cpu_apic_init() would be temporary as well
(qdev_init part
goes away with realize property and apic base mapping could be moved into it's
own property setter in apic object and default mapping set in its initfn).
Andreas seems liked putting it there.
Then overall code would look cleaner and with less ifdefs.