From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZN4I-0003my-NB for qemu-devel@nongnu.org; Mon, 06 May 2013 11:11:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UZN4F-0005F1-I4 for qemu-devel@nongnu.org; Mon, 06 May 2013 11:11:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56428) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UZN4F-0005Et-Ao for qemu-devel@nongnu.org; Mon, 06 May 2013 11:11:07 -0400 Message-ID: <5187C7F5.90807@redhat.com> Date: Mon, 06 May 2013 17:10:45 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <5187C445.6080703@suse.de> <5187C54C.1050407@redhat.com> <5187C5F5.1090500@siemens.com> In-Reply-To: <5187C5F5.1090500@siemens.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC][PATCH 08/15] isa: implement isa_is_ioport_assigned via memory_region_find List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Liu Ping Fan , =?ISO-8859-15?Q?Andreas_F=E4rber?= , qemu-devel Il 06/05/2013 17:02, Jan Kiszka ha scritto: > On 2013-05-06 16:59, Paolo Bonzini wrote: >> Il 06/05/2013 16:55, Andreas F=E4rber ha scritto: >>> Am 06.05.2013 16:26, schrieb Jan Kiszka: >>>> Move isa_is_ioport_assigned to the ISA core and implement it via a >>>> memory region lookup. As all IO ports are now directly or indirectly >>>> registered via the memory API, this becomes possible and will finall= y >>>> allow us to drop the ioport tables. >>>> >>>> Signed-off-by: Jan Kiszka >>>> --- >>>> hw/acpi/piix4.c | 6 +++--- >>>> hw/isa/isa-bus.c | 11 +++++++++++ >>>> hw/isa/lpc_ich9.c | 8 ++++---- >>>> include/exec/ioport.h | 1 - >>>> include/hw/isa/isa.h | 2 ++ >>>> ioport.c | 7 ------- >>>> 6 files changed, 20 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c >>>> index c4af1cc..5955217 100644 >>>> --- a/hw/acpi/piix4.c >>>> +++ b/hw/acpi/piix4.c >>>> @@ -386,10 +386,10 @@ static void piix4_pm_machine_ready(Notifier *n= , void *opaque) >>>> uint8_t *pci_conf; >>>> =20 >>>> pci_conf =3D s->dev.config; >>>> - pci_conf[0x5f] =3D (isa_is_ioport_assigned(0x378) ? 0x80 : 0) |= 0x10; >>>> + pci_conf[0x5f] =3D (isa_is_ioport_assigned(NULL, 0x378) ? 0x80 = : 0) | 0x10; >>>> pci_conf[0x63] =3D 0x60; >>>> - pci_conf[0x67] =3D (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | >>>> - (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); >>>> + pci_conf[0x67] =3D (isa_is_ioport_assigned(NULL, 0x3f8) ? 0x08 = : 0) | >>>> + (isa_is_ioport_assigned(NULL, 0x2f8) ? 0x90 : 0); >>>> =20 >>>> } >>>> =20 >>> >>> Is there really no way to access the ISABus from this device? Would b= e >>> nice to get rid of global ISA variables and not introduce more >>> dependencies. :) >> >> There's always a way to find the ISABus via QOM: >> >> ISABus *isa_bus =3D (ISABus *) object_resolve_path_type("", TYPE_I= SA_BUS, NULL); >=20 > Err, in what way is this better? It also assumes that there is only one. I didn't say it is better. :) Unfortunately, the PIIX4 has these register on the "wrong" function, ICH9 fixed it. You could make this take an AddressSpace instead of an ISABus, and use pci_address_space_io. The assumption then becomes that the ISA and PM devices have the same address spaces, which is somewhat hackish but reasonable. In fact, with the other new API I have added (address_space_valid), this would become address_space_valid(pci_address_space_io(dev), 0x2f8, 1) and similar. I need to check, but you could remove isa_is_ioport_assigned completely. Paolo