From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:35667) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QVmes-0006wM-BM for qemu-devel@nongnu.org; Sun, 12 Jun 2011 11:33:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QVmer-0006QH-0e for qemu-devel@nongnu.org; Sun, 12 Jun 2011 11:33:02 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]:37380) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QVmeq-0006QB-HK for qemu-devel@nongnu.org; Sun, 12 Jun 2011 11:33:00 -0400 Message-Id: From: =?ISO-8859-1?Q?Andreas_F=E4rber?= In-Reply-To: <20110612134824.GK491@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed; delsp=yes Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Apple Message framework v936) Date: Sun, 12 Jun 2011 17:32:57 +0200 References: <4DEF2F25.5070104@redhat.com> <1307559319-16183-1-git-send-email-andreas.faerber@web.de> <1307559319-16183-2-git-send-email-andreas.faerber@web.de> <1307559319-16183-3-git-send-email-andreas.faerber@web.de> <1307559319-16183-4-git-send-email-andreas.faerber@web.de> <1307559319-16183-5-git-send-email-andreas.faerber@web.de> <8A9CB932-9F44-4B19-BFB2-8B1267F990BC@web.de> <20110612134824.GK491@redhat.com> Sender: andreas.faerber@web.de Subject: Re: [Qemu-devel] [RFC v4 04/12] isa: Allow to un-assign I/O ports List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gleb Natapov Cc: Gerd Hoffmann , =?ISO-8859-1?Q?Herv=E9_Poussineau?= , Markus Armbruster , qemu-devel Developers Am 12.06.2011 um 15:48 schrieb Gleb Natapov: > On Sun, Jun 12, 2011 at 01:59:51PM +0200, Andreas F=E4rber wrote: >> Am 09.06.2011 um 17:03 schrieb Markus Armbruster: >> >>> Andreas F=E4rber writes: >>> >>>> Signed-off-by: Andreas F=E4rber >>>> --- >>>> hw/isa-bus.c | 14 ++++++++++++++ >>>> hw/isa.h | 1 + >>>> 2 files changed, 15 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/hw/isa-bus.c b/hw/isa-bus.c >>>> index d258932..1f64673 100644 >>>> --- a/hw/isa-bus.c >>>> +++ b/hw/isa-bus.c >>>> @@ -105,6 +105,20 @@ void isa_init_ioport(ISADevice *dev, >>>> uint16_t ioport) >>>> isa_init_ioport_range(dev, ioport, 1); >>>> } >>>> >>>> +void isa_discard_ioport_range(ISADevice *dev, uint16_t start, >>>> uint16_t length) >>>> +{ >>>> + int i, j; >>>> + for (i =3D 0; i < dev->nioports; i++) { >>>> + if (dev->ioports[i] =3D=3D start) { >>>> + for (j =3D 0; j < dev->nioports - i; j++) { >>>> + dev->ioports[i + j] =3D dev->ioports[i + length + =20= >>>> j]; >>>> + } >>>> + dev->nioports -=3D length; >>>> + break; >>>> + } >>>> + } >>>> +} >>>> + >>> >>> "discard"? It's the opposite of isa_init_ioport_range(), name =20 >>> should >>> make that obvious. "uninit"? >> >> "uninit" felt wrong. >> >>> Note: this only affects the I/O-port bookkeeping within ISADevice, =20= >>> not >>> the actual I/O port handler registration. Any use must be =20 >>> accompanied >>> by a matching handler de-registration. Just like any use of >>> isa_init_ioport_range() must be accompanied by matching >>> register_ioport_FOO()s. You can thank Gleb for this mess. >> >> Right, I didn't spot any wrong usage though. >> >> So what about cleaning up the mess and doing >> isa_[un]assign_ioport_range(), wrapping the ioport.c functions? >> The overhead of calling it up to six times for the different widths >> and read/write would seem negligible as a startup cost. And for >> pc87312 we don't seriously have to care about performance imo. >> > Ideally every ioport registration should be moved to use IORange. I > tried to move all isa devices to it, but it is complicated for two > reasons. First not every device is qdevified and second some devises > can be instantiated as isa device and non-isa device and they do =20 > ioport > registration in a common code. With your approach you will have to > duplicate ioport registration code for isa and non-isa case for such > devices and code duplication is not good. I'm not trying to duplicate anything! What I've been seeing is that =20 many of the ISA devices I've touched in this series first register the =20= I/O ports and then associate them with the ISADevice, in ISA-only =20 code. So the numbers *are* duplicated and I'm proposing to consolidate =20= this into one call. The existing "init" would stay, so that code with disseparate ISA and =20= common parts (like IDE) remains unaffected. Andreas=