Re: [Qemu-devel] [PATCH qom-next 3/5] pc: move apic_mapped initialization into common apic init code

[Jan Kiszka <jan.kiszka@siemens.com>]

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.

> -        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.

>  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);

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux