From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QrBiC-00072T-Hw for qemu-devel@nongnu.org; Wed, 10 Aug 2011 12:33:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QrBi7-0000Lv-WF for qemu-devel@nongnu.org; Wed, 10 Aug 2011 12:32:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18626) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QrBi7-0000Lq-MG for qemu-devel@nongnu.org; Wed, 10 Aug 2011 12:32:51 -0400 Message-ID: <4E42B2AF.1060201@redhat.com> Date: Wed, 10 Aug 2011 19:32:47 +0300 From: Avi Kivity MIME-Version: 1.0 References: <1312823229-12822-1-git-send-email-avi@redhat.com> <1312823229-12822-22-git-send-email-avi@redhat.com> <4E42B0A5.1010005@twiddle.net> In-Reply-To: <4E42B0A5.1010005@twiddle.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 21/24] isa: add isa_address_space() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org On 08/10/2011 07:24 PM, Richard Henderson wrote: > > @@ -202,4 +203,9 @@ static char *isabus_get_fw_dev_path(DeviceState *dev) > > return strdup(path); > > } > > > > +MemoryRegion *isa_address_space(ISADevice *dev) > > +{ > > + return get_system_memory(); > > +} > > + > > This does not help get rid of isa_mem_base, as far as I can see. > All this is going to do is the equivalent of isa_mem_base == 0. It abstracts away the problem. We can now have multiple ISA buses, and each can have its own base, we just need to change the implementation. > Did you have a plan beyond this? I'll probably add two MemoryRegion *s to isabus_bridge_init(). The plan is to eliminate all get_system_memory() calls. > > The simplest replacement, as far as I can see, is to replace one > global variable with another: > > MemoryRegion *isa_address_space; > > Define that this variable *must* be set by any platform that > wants to use ISA. It can be set to the address_space_mem > parameter (i.e. the copy of get_system_memory() that the > platforms receive as a parameter). > > For Alpha, I can set this to the sub-region that I pass off to > the PCI subsystem. > > For those other non-x86 platforms that currently set isa_mem_base > to a non-zero value, they either re-use an otherwise existing > memory region or create a new sub-region properly placed. > > Of course, as far as I can see, this variable is only used by > the VGA devices. Surely we can arrange to pass down some address > space during setup of the VGA? > You mean pass VGA's memory regions to ISA, instead of vice versa? Yes, that's the right thing to do, and is how PCI and sysbus do it. Because of the huge amount of devices that need to be converted, I'm giving myself some leeway in the conversion process and introducing some shortcuts. I don't see a away to do a perfect conversion in a reasonable time frame, and in any case I'm learning a lot during the conversion (except that I call it "collecting requirements"). -- error compiling committee.c: too many arguments to function