[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qom-next 11/12] target-i386: initialize APIC at
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH qom-next 11/12] target-i386: initialize APIC at CPU level |
Date: |
Wed, 30 May 2012 16:55:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2012-05-30 16:52, Igor Mammedov wrote:
> 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.
"Unfortunately", the mapping belongs to the CPU because it actually
performs it as we pointed out.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
- [Qemu-devel] [PATCH qom-next 09/12] target-i386: make cpu a child of /machine right after it's created, (continued)
[Qemu-devel] [PATCH qom-next 10/12] pc: Enable MSI support at APIC level, Igor Mammedov, 2012/05/29
[Qemu-devel] [PATCH qom-next v3 0/12] target-i386: re-factor CPU creation/initialization to QOM, Igor Mammedov, 2012/05/29