Patchwork [qom-next,3/5] pc: move apic_mapped initialization into common apic init code

login
register
mail settings
Submitter Igor Mammedov
Date May 22, 2012, 10:35 a.m.
Message ID <1337682954-20618-4-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/160622/
State New
Headers show

Comments

Igor Mammedov - May 22, 2012, 10:35 a.m.
Move from apic_init in pc.c  the code that belongs to apic_init_common
and create/init apic in pc_new_cpu instead of separate func.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/apic_common.c |   16 ++++++++++++++++
 hw/msi.h         |    2 ++
 hw/pc.c          |   47 ++++++++---------------------------------------
 3 files changed, 26 insertions(+), 39 deletions(-)
Igor Mammedov - May 22, 2012, 12:42 p.m.
On 05/22/2012 12:48 PM, Jan Kiszka wrote:
> On 2012-05-22 07:35, Igor Mammedov wrote:
>> Move from apic_init in pc.c  the code that belongs to apic_init_common
>> and create/init apic in pc_new_cpu instead of separate func.
>>
>> Signed-off-by: Igor Mammedov<imammedo@redhat.com>
>> ---
>>   hw/apic_common.c |   16 ++++++++++++++++
>>   hw/msi.h         |    2 ++
>>   hw/pc.c          |   47 ++++++++---------------------------------------
>>   3 files changed, 26 insertions(+), 39 deletions(-)
>>
>> diff --git a/hw/apic_common.c b/hw/apic_common.c
>> index 23d51e8..703931b 100644
>> --- a/hw/apic_common.c
>> +++ b/hw/apic_common.c
>> @@ -21,6 +21,7 @@
>>   #include "apic_internal.h"
>>   #include "trace.h"
>>   #include "kvm.h"
>> +#include "msi.h"
>>
>>   static int apic_irq_delivered;
>>   bool apic_report_tpr_access;
>> @@ -284,6 +285,7 @@ static int apic_init_common(SysBusDevice *dev)
>>       APICCommonClass *info;
>>       static DeviceState *vapic;
>>       static int apic_no;
>> +    static int apic_mapped;
>>
>>       if (apic_no>= MAX_APICS) {
>>           return -1;
>> @@ -295,6 +297,20 @@ static int apic_init_common(SysBusDevice *dev)
>>
>>       sysbus_init_mmio(dev,&s->io_memory);
>>
>> +    /* 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(&s->busdev.qdev), 0, MSI_ADDR_BASE);
>> +        apic_mapped = 1;
>> +    }
>> +
>> +    /* KVM does not support MSI yet. */
>> +    if (!kvm_irqchip_in_kernel()) {
>> +        msi_supported = true;
>> +    }
>> +
>>       if (!vapic&&  s->vapic_control&  VAPIC_ENABLE_MASK) {
>>           vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>>       }
>> diff --git a/hw/msi.h b/hw/msi.h
>> index 3040bb0..abd52b6 100644
>> --- a/hw/msi.h
>> +++ b/hw/msi.h
>> @@ -40,4 +40,6 @@ static inline bool msi_present(const PCIDevice *dev)
>>       return dev->cap_present&  QEMU_PCI_CAP_MSI;
>>   }
>>
>> +#define MSI_ADDR_BASE 0xfee00000
>> +
>>   #endif /* QEMU_MSI_H */
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 00d738d..0eb0b73 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -879,44 +879,6 @@ DeviceState *cpu_get_current_apic(void)
>>       }
>>   }
>>
>> -static DeviceState *apic_init(void *env, uint8_t apic_id)
>> -{
>> -    DeviceState *dev;
>> -    static int apic_mapped;
>> -
>> -    if (kvm_irqchip_in_kernel()) {
>> -        dev = qdev_create(NULL, "kvm-apic");
>> -    } else if (xen_enabled()) {
>> -        dev = qdev_create(NULL, "xen-apic");
>> -    } else {
>> -        dev = qdev_create(NULL, "apic");
>> -    }
>> -
>> -    qdev_prop_set_uint8(dev, "id", apic_id);
>> -    qdev_prop_set_ptr(dev, "cpu_env", env);
>> -    qdev_init_nofail(dev);
>> -
>> -    /* 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(dev), 0, MSI_ADDR_BASE);
>
> While at it, you should drop MSI_ADDR_BASE definition from pc.c.

Consider it done.

>
>> -        apic_mapped = 1;
>> -    }
>> -
>> -    /* KVM does not support MSI yet. */
>> -    if (!kvm_irqchip_in_kernel()) {
>> -        msi_supported = true;
>> -    }
>> -
>> -    if (xen_msi_support()) {
>> -        msi_supported = true;
>> -    }
>> -
>> -    return dev;
>> -}
>> -
>
> You are loosing some xen bits here. But this will collide with latest
> kvm pull request
> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
> You may want to base on uq/master.
>

This patchset is based on Andreas' qom-next tree. Probably I should wait
till above mentioned kvm pull is pulled in and it aprears in qom-next.

...
Andreas Färber - May 22, 2012, 2:24 p.m.
Am 22.05.2012 14:42, schrieb Igor Mammedov:
> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>> -        apic_mapped = 1;
>>> -    }
>>> -
>>> -    /* KVM does not support MSI yet. */
>>> -    if (!kvm_irqchip_in_kernel()) {
>>> -        msi_supported = true;
>>> -    }
>>> -
>>> -    if (xen_msi_support()) {
>>> -        msi_supported = true;
>>> -    }
>>> -
>>> -    return dev;
>>> -}
>>> -
>>
>> You are loosing some xen bits here. But this will collide with latest
>> kvm pull request
>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>> You may want to base on uq/master.
>>
> 
> This patchset is based on Andreas' qom-next tree. Probably I should wait
> till above mentioned kvm pull is pulled in and it aprears in qom-next.

Jan, we currently have a chaos of concurrent, colliding QOM series.
qom-next was intended to resolve this but so far it's a rebasing patch
queue on top of master and not a repository with stable hashes so I
can't do PULLs myself but I could cherry-pick related patches if needed.

Igor, if you put the code movement init -> initfn into its own patch
I'll apply it to qom-next right away. Haven't looked at the series
in-depth yet.

We're not quite there yet with qom-next due to series and counterseries
and lack of input on realize/QBus. My current merge plan is as follows:

* Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
last night, prerequisite for part 4.

* Apply the last two remaining ARM cpu_reset followup cleanups - waiting
for one ack by PMM.

* Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
bisectability, hope to post today. This will show areas of conflicts wrt
apic and x86 and is quite invasive (qom-cpu branch on GitHub).

* Mix and match patches from Paolo's and Anthony's series for realizefn.
Hope to post a short-term compromise soon, leaving properties aside for
now. Apply it so that we finally have a realizefn.

* Post QOM CPUState series part 5 (CPUState conditionally as device).
WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
will enable integration with machine reset. Doesn't depend on part 4 so far.

* Apply PMM's ARM copro series - waiting for acks, still need to
carefully review the final CPUID movements.
* Post realizefn implementation on top - probably to be merged after
PMM's holiday, i.e. to master not to qom-next.

* Align part 4 with Igor's series, possibly rebase on part 5. See how
close to 1.2 we get and how the review of all open series goes.

Whatever progress we make on qom-next, the idea is to have qom-next
merged into master *first*, since it's getting really large. Don't know
what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
it but otherwise I expect series to get rebased onto master w/qom-next
before sending a PULL. That's why I asked target maintainers to queue
the patches from my part 3 in their queues, to avoid merge conflicts
once the 1.2 window opens.

For the current QOM series I'm fine rebasing myself so far.

Regards,
Andreas
Jan Kiszka - May 22, 2012, 2:35 p.m.
On 2012-05-22 11:24, Andreas Färber wrote:
> Am 22.05.2012 14:42, schrieb Igor Mammedov:
>> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>>> -        apic_mapped = 1;
>>>> -    }
>>>> -
>>>> -    /* KVM does not support MSI yet. */
>>>> -    if (!kvm_irqchip_in_kernel()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    if (xen_msi_support()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    return dev;
>>>> -}
>>>> -
>>>
>>> You are loosing some xen bits here. But this will collide with latest
>>> kvm pull request
>>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>>> You may want to base on uq/master.
>>>
>>
>> This patchset is based on Andreas' qom-next tree. Probably I should wait
>> till above mentioned kvm pull is pulled in and it aprears in qom-next.
> 
> Jan, we currently have a chaos of concurrent, colliding QOM series.
> qom-next was intended to resolve this but so far it's a rebasing patch
> queue on top of master and not a repository with stable hashes so I
> can't do PULLs myself but I could cherry-pick related patches if needed.
> 
> Igor, if you put the code movement init -> initfn into its own patch
> I'll apply it to qom-next right away. Haven't looked at the series
> in-depth yet.
> 
> We're not quite there yet with qom-next due to series and counterseries
> and lack of input on realize/QBus. My current merge plan is as follows:
> 
> * Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
> last night, prerequisite for part 4.
> 
> * Apply the last two remaining ARM cpu_reset followup cleanups - waiting
> for one ack by PMM.
> 
> * Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
> bisectability, hope to post today. This will show areas of conflicts wrt
> apic and x86 and is quite invasive (qom-cpu branch on GitHub).
> 
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.
> 
> * Post QOM CPUState series part 5 (CPUState conditionally as device).
> WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
> will enable integration with machine reset. Doesn't depend on part 4 so far.
> 
> * Apply PMM's ARM copro series - waiting for acks, still need to
> carefully review the final CPUID movements.
> * Post realizefn implementation on top - probably to be merged after
> PMM's holiday, i.e. to master not to qom-next.
> 
> * Align part 4 with Igor's series, possibly rebase on part 5. See how
> close to 1.2 we get and how the review of all open series goes.
> 
> Whatever progress we make on qom-next, the idea is to have qom-next
> merged into master *first*, since it's getting really large. Don't know
> what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
> it but otherwise I expect series to get rebased onto master w/qom-next
> before sending a PULL. That's why I asked target maintainers to queue
> the patches from my part 3 in their queues, to avoid merge conflicts
> once the 1.2 window opens.

In this case, conflicts may only be caused between
http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91186 (pc: Enable
MSI support at APIC level) and this particular patch (that requires some
more work anyway).

Maybe you can test-merge the KVM pull in your current tree that is
supposed to go in first and ask Avi/Marcelo to the provide uq/master
baseline on top of yours in case of conflicts. I would provide an update
of my patches on top of that afterward.

Jan
Paolo Bonzini - May 22, 2012, 3:47 p.m.
Il 22/05/2012 16:24, Andreas Färber ha scritto:
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.

It's still not clear to me what's missing to get the QBus series in,
except perhaps rebasing it... once that is in, I can post again the
realize patches.

Paolo
Igor Mammedov - May 23, 2012, 9:17 a.m.
On 05/22/2012 04:24 PM, Andreas Färber wrote:
> Am 22.05.2012 14:42, schrieb Igor Mammedov:
>> On 05/22/2012 12:48 PM, Jan Kiszka wrote:
>>> On 2012-05-22 07:35, Igor Mammedov wrote:
>>>> -        apic_mapped = 1;
>>>> -    }
>>>> -
>>>> -    /* KVM does not support MSI yet. */
>>>> -    if (!kvm_irqchip_in_kernel()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    if (xen_msi_support()) {
>>>> -        msi_supported = true;
>>>> -    }
>>>> -
>>>> -    return dev;
>>>> -}
>>>> -
>>>
>>> You are loosing some xen bits here. But this will collide with latest
>>> kvm pull request
>>> (http://thread.gmane.org/gmane.comp.emulators.kvm.devel/91171) anyway.
>>> You may want to base on uq/master.
>>>
>>
>> This patchset is based on Andreas' qom-next tree. Probably I should wait
>> till above mentioned kvm pull is pulled in and it aprears in qom-next.
>
> Jan, we currently have a chaos of concurrent, colliding QOM series.
> qom-next was intended to resolve this but so far it's a rebasing patch
> queue on top of master and not a repository with stable hashes so I
> can't do PULLs myself but I could cherry-pick related patches if needed.
>
> Igor, if you put the code movement init ->  initfn into its own patch
> I'll apply it to qom-next right away. Haven't looked at the series
> in-depth yet.
I'll split this patch in apic and cpu parts and repost with fixes for issues
Jan and Peter spotted.

>
> We're not quite there yet with qom-next due to series and counterseries
> and lack of input on realize/QBus. My current merge plan is as follows:
>
> * Apply QOM CPUState series part 3 (cpu_state_reset) - aggressively done
> last night, prerequisite for part 4.
>
> * Apply the last two remaining ARM cpu_reset followup cleanups - waiting
> for one ack by PMM.
>
> * Post QOM CPUState series part 4 (CPU_COMMON) - still fiddling with
> bisectability, hope to post today. This will show areas of conflicts wrt
> apic and x86 and is quite invasive (qom-cpu branch on GitHub).
>
> * Mix and match patches from Paolo's and Anthony's series for realizefn.
> Hope to post a short-term compromise soon, leaving properties aside for
> now. Apply it so that we finally have a realizefn.
>
> * Post QOM CPUState series part 5 (CPUState conditionally as device).
> WIP (qom-cpu-dev branch on GitHub), needed for hotplug IIUC and this
> will enable integration with machine reset. Doesn't depend on part 4 so far.
>
> * Apply PMM's ARM copro series - waiting for acks, still need to
> carefully review the final CPUID movements.
> * Post realizefn implementation on top - probably to be merged after
> PMM's holiday, i.e. to master not to qom-next.
>
> * Align part 4 with Igor's series, possibly rebase on part 5. See how
> close to 1.2 we get and how the review of all open series goes.
>
> Whatever progress we make on qom-next, the idea is to have qom-next
> merged into master *first*, since it's getting really large. Don't know
> what KVM PULL Jan is referring to - if it's for 1.1 then I'll rebase on
> it but otherwise I expect series to get rebased onto master w/qom-next
> before sending a PULL. That's why I asked target maintainers to queue
> the patches from my part 3 in their queues, to avoid merge conflicts
> once the 1.2 window opens.
>
> For the current QOM series I'm fine rebasing myself so far.
>
> Regards,
> Andreas
>

Patch

diff --git a/hw/apic_common.c b/hw/apic_common.c
index 23d51e8..703931b 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -21,6 +21,7 @@ 
 #include "apic_internal.h"
 #include "trace.h"
 #include "kvm.h"
+#include "msi.h"
 
 static int apic_irq_delivered;
 bool apic_report_tpr_access;
@@ -284,6 +285,7 @@  static int apic_init_common(SysBusDevice *dev)
     APICCommonClass *info;
     static DeviceState *vapic;
     static int apic_no;
+    static int apic_mapped;
 
     if (apic_no >= MAX_APICS) {
         return -1;
@@ -295,6 +297,20 @@  static int apic_init_common(SysBusDevice *dev)
 
     sysbus_init_mmio(dev, &s->io_memory);
 
+    /* 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(&s->busdev.qdev), 0, MSI_ADDR_BASE);
+        apic_mapped = 1;
+    }
+
+    /* KVM does not support MSI yet. */
+    if (!kvm_irqchip_in_kernel()) {
+        msi_supported = true;
+    }
+
     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
         vapic = sysbus_create_simple("kvmvapic", -1, NULL);
     }
diff --git a/hw/msi.h b/hw/msi.h
index 3040bb0..abd52b6 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -40,4 +40,6 @@  static inline bool msi_present(const PCIDevice *dev)
     return dev->cap_present & QEMU_PCI_CAP_MSI;
 }
 
+#define MSI_ADDR_BASE 0xfee00000
+
 #endif /* QEMU_MSI_H */
diff --git a/hw/pc.c b/hw/pc.c
index 00d738d..0eb0b73 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -879,44 +879,6 @@  DeviceState *cpu_get_current_apic(void)
     }
 }
 
-static DeviceState *apic_init(void *env, uint8_t apic_id)
-{
-    DeviceState *dev;
-    static int apic_mapped;
-
-    if (kvm_irqchip_in_kernel()) {
-        dev = qdev_create(NULL, "kvm-apic");
-    } else if (xen_enabled()) {
-        dev = qdev_create(NULL, "xen-apic");
-    } else {
-        dev = qdev_create(NULL, "apic");
-    }
-
-    qdev_prop_set_uint8(dev, "id", apic_id);
-    qdev_prop_set_ptr(dev, "cpu_env", env);
-    qdev_init_nofail(dev);
-
-    /* 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(dev), 0, MSI_ADDR_BASE);
-        apic_mapped = 1;
-    }
-
-    /* KVM does not support MSI yet. */
-    if (!kvm_irqchip_in_kernel()) {
-        msi_supported = true;
-    }
-
-    if (xen_msi_support()) {
-        msi_supported = true;
-    }
-
-    return dev;
-}
-
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
 {
     CPUX86State *s = opaque;
@@ -943,7 +905,14 @@  static X86CPU *pc_new_cpu(const char *cpu_model)
     }
     env = &cpu->env;
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
-        env->apic_state = apic_init(env, env->cpuid_apic_id);
+        if (kvm_irqchip_in_kernel()) {
+            env->apic_state = qdev_create(NULL, "kvm-apic");
+        } else {
+            env->apic_state = qdev_create(NULL, "apic");
+        }
+        qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
+        qdev_prop_set_ptr(env->apic_state, "cpu_env", env);
+        qdev_init_nofail(env->apic_state);
     }
     qemu_register_reset(pc_cpu_reset, cpu);
     pc_cpu_reset(cpu);