On Fri, 29 May 2015 21:27:19 +0200
Paolo Bonzini <pbonz...@redhat.com> wrote:
>
>
> On 29/05/2015 20:04, Eduardo Habkost wrote:
> > static int apic_no;
> > - static bool mmio_registered;
> > + CPUState *cpu = CPU(s->cpu);
> > + MemoryRegion *root;
> >
> > if (apic_no >= MAX_APICS) {
> > error_setg(errp, "%s initialization failed.",
> > @@ -307,11 +308,12 @@ static void apic_common_realize(DeviceState *dev,
> > Error **errp)
> >
> > info = APIC_COMMON_GET_CLASS(s);
> > info->realize(dev, errp);
> > - if (!mmio_registered) {
> > - ICCBus *b = ICC_BUS(qdev_get_parent_bus(dev));
> > - memory_region_add_subregion(b->apic_address_space, 0,
> > &s->io_memory);
> > - mmio_registered = true;
> > - }
> > +
> > + root = address_space_root_memory_region(cpu->as);
>
> I think just using cpu->as->root is okay.
>
> > + memory_region_add_subregion_overlap(root,
> > + s->apicbase &
> > MSR_IA32_APICBASE_BASE,
> > + &s->io_memory,
> > + 0x1000);
>
> I think this patch is incorrect, because you do not install a separate
> address space for each CPU. Also, the CPU address space is only used
> with TCG so it should be guarded by "if (tcg_enabled())".
Don't we need it be mapped on for KVM for MSI to work
when using kvm-apic?
kvm_apic_io_ops->write = kvm_apic_mem_write->kvm_irqchip_send_msi()
>
> Paolo
>
> > /* Note: We need at least 1M to map the VAPIC option ROM */
> > if (!vapic && s->vapic_control & VAPIC_ENABLE_MASK &&
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index b61c84f..a16650f 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1295,6 +1295,11 @@ void *address_space_map(AddressSpace *as, hwaddr
> > addr,
> > void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
> > int is_write, hwaddr access_len);
> >
> > +/* address_space_root_memory_region: get root memory region
> > + *
> > + * @as: #AddressSpace to be accessed
> > + */
> > +MemoryRegion *address_space_root_memory_region(AddressSpace *as);
> >
> > #endif
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 3305e09..f83e526 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2740,6 +2740,8 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error
> > **errp)
> > /* TODO: convert to link<> */
> > apic = APIC_COMMON(cpu->apic_state);
> > apic->cpu = cpu;
> > + cpu_set_apic_base(cpu->apic_state,
> > + APIC_DEFAULT_ADDRESS | MSR_IA32_APICBASE_ENABLE);
> > }
> >
>
