From mboxrd@z Thu Jan  1 00:00:00 1970
From: Glauber Costa <glommer@redhat.com>
Subject: Re: [Qemu-devel] [PATCHv3 07/13] qemu: minimal MSI/MSI-X
	implementation for PC
Date: Tue, 9 Jun 2009 14:33:33 -0300
Message-ID: <20090609173333.GB19375@poweredge.glommer>
References: <cover.1244192535.git.mst@redhat.com> <20090605102347.GH26770@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Paul Brook <paul@codesourcery.com>, Avi Kivity <avi@redhat.com>,
	qemu-devel@nongnu.org, Carsten Otte <cotte@de.ibm.com>,
	kvm@vger.kernel.org, Rusty Russell <rusty@rustcorp.com.au>,
	virtualization@lists.linux-foundation.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Blue Swirl <blauwirbel@gmail.com>,
	Anthony Liguori <anthony@codemonkey.ws>
To: "Michael S. Tsirkin" <mst@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:39666 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751478AbZFIR3t (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 9 Jun 2009 13:29:49 -0400
Content-Disposition: inline
In-Reply-To: <20090605102347.GH26770@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

>      env = cpu_single_env;
>      if (!env)
> @@ -727,7 +762,6 @@ static void apic_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
>      printf("APIC write: %08x = %08x\n", (uint32_t)addr, val);
>  #endif
>  
> -    index = (addr >> 4) & 0xff;
>      switch(index) {
>      case 0x02:
>          s->id = (val >> 24);
> @@ -911,6 +945,7 @@ int apic_init(CPUState *env)
>      s->cpu_env = env;
>  
>      apic_reset(s);
> +    msix_supported = 1;
>  
>      /* XXX: mapping more APICs at the same memory location */
>      if (apic_io_memory == 0) {
> @@ -918,7 +953,8 @@ int apic_init(CPUState *env)
>             on the global memory bus. */
>          apic_io_memory = cpu_register_io_memory(0, apic_mem_read,
>                                                  apic_mem_write, NULL);
> -        cpu_register_physical_memory(s->apicbase & ~0xfff, 0x1000,
> +        /* XXX: what if the base changes? */
> +        cpu_register_physical_memory(MSI_ADDR_BASE, MSI_ADDR_SIZE,
>                                       apic_io_memory);
+1 

I think you have a point here. Your patch is in no way worse than what we had,
but we're currently not handling correctly the case of base address changing.
Guess it is not common in normal apic usage for OSes...