On 23 May 2012 17:39, Igor Mammedov <imamm...@redhat.com> wrote:
> @@ -295,6 +297,15 @@ 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;
> +    }
> +
>     if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK) {
>         vapic = sysbus_create_simple("kvmvapic", -1, NULL);
>     }

This looks wrong -- sysbus device init functions shouldn't
be mapping MMIO regions themselves, in general. They should
expose MMIO regions to be mapped by whichever device or board
model creates them. Which is what the code before this patch
was doing -- why do you want to move this code?

-- PMM

Reply via email to