Message ID | bbcfb79b8b1fd7a4041097690a07484971e0fd84.1367849167.git.jan.kiszka@siemens.com |
---|---|
State | New |
Headers | show |
On 2013-05-06 16:55, Andreas Färber wrote: > 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 finally >> allow us to drop the ioport tables. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> 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; >> >> pci_conf = s->dev.config; >> - pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10; >> + pci_conf[0x5f] = (isa_is_ioport_assigned(NULL, 0x378) ? 0x80 : 0) | 0x10; >> pci_conf[0x63] = 0x60; >> - pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | >> - (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); >> + pci_conf[0x67] = (isa_is_ioport_assigned(NULL, 0x3f8) ? 0x08 : 0) | >> + (isa_is_ioport_assigned(NULL, 0x2f8) ? 0x90 : 0); >> >> } >> > > Is there really no way to access the ISABus from this device? Would be > nice to get rid of global ISA variables and not introduce more > dependencies. :) There is likely a way, just didn't find a direct one. So I prefer to limit the scope and leave such cleanups for later. Keep in mind that the existing API had an implicit NULL device, so this is no regression! Jan
On 2013-05-06 16:59, Paolo Bonzini wrote: > Il 06/05/2013 16:55, Andreas Färber 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 finally >>> allow us to drop the ioport tables. >>> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> --- >>> 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; >>> >>> pci_conf = s->dev.config; >>> - pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10; >>> + pci_conf[0x5f] = (isa_is_ioport_assigned(NULL, 0x378) ? 0x80 : 0) | 0x10; >>> pci_conf[0x63] = 0x60; >>> - pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | >>> - (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); >>> + pci_conf[0x67] = (isa_is_ioport_assigned(NULL, 0x3f8) ? 0x08 : 0) | >>> + (isa_is_ioport_assigned(NULL, 0x2f8) ? 0x90 : 0); >>> >>> } >>> >> >> Is there really no way to access the ISABus from this device? Would be >> 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 = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL); Err, in what way is this better? It also assumes that there is only one. Jan
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ärber 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 finally >>>> allow us to drop the ioport tables. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>>> --- >>>> 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; >>>> >>>> pci_conf = s->dev.config; >>>> - pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10; >>>> + pci_conf[0x5f] = (isa_is_ioport_assigned(NULL, 0x378) ? 0x80 : 0) | 0x10; >>>> pci_conf[0x63] = 0x60; >>>> - pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | >>>> - (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); >>>> + pci_conf[0x67] = (isa_is_ioport_assigned(NULL, 0x3f8) ? 0x08 : 0) | >>>> + (isa_is_ioport_assigned(NULL, 0x2f8) ? 0x90 : 0); >>>> >>>> } >>>> >>> >>> Is there really no way to access the ISABus from this device? Would be >>> 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 = (ISABus *) object_resolve_path_type("", TYPE_ISA_BUS, NULL); > > 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
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; pci_conf = s->dev.config; - pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10; + pci_conf[0x5f] = (isa_is_ioport_assigned(NULL, 0x378) ? 0x80 : 0) | 0x10; pci_conf[0x63] = 0x60; - pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | - (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); + pci_conf[0x67] = (isa_is_ioport_assigned(NULL, 0x3f8) ? 0x08 : 0) | + (isa_is_ioport_assigned(NULL, 0x2f8) ? 0x90 : 0); } diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 7860b17..598dd86 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -279,4 +279,15 @@ MemoryRegion *isa_address_space_io(ISADevice *dev) return isabus->address_space_io; } +bool isa_is_ioport_assigned(ISABus *bus, pio_addr_t start) +{ + if (!bus) { + bus = isabus; + } + if (!bus) { + hw_error("No isa bus present."); + } + return memory_region_find(bus->address_space_io, start, 1).mr != NULL; +} + type_init(isabus_register_types) diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 667e882..641227a 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -480,19 +480,19 @@ static void ich9_lpc_machine_ready(Notifier *n, void *opaque) uint8_t *pci_conf; pci_conf = s->d.config; - if (isa_is_ioport_assigned(0x3f8)) { + if (isa_is_ioport_assigned(s->isa_bus, 0x3f8)) { /* com1 */ pci_conf[0x82] |= 0x01; } - if (isa_is_ioport_assigned(0x2f8)) { + if (isa_is_ioport_assigned(s->isa_bus, 0x2f8)) { /* com2 */ pci_conf[0x82] |= 0x02; } - if (isa_is_ioport_assigned(0x378)) { + if (isa_is_ioport_assigned(s->isa_bus, 0x378)) { /* lpt */ pci_conf[0x82] |= 0x04; } - if (isa_is_ioport_assigned(0x3f0)) { + if (isa_is_ioport_assigned(s->isa_bus, 0x3f0)) { /* floppy */ pci_conf[0x82] |= 0x08; } diff --git a/include/exec/ioport.h b/include/exec/ioport.h index 4953892..eb99ffe 100644 --- a/include/exec/ioport.h +++ b/include/exec/ioport.h @@ -40,7 +40,6 @@ typedef void (IOPortDestructor)(void *opaque); void ioport_register(IORange *iorange); void isa_unassign_ioport(pio_addr_t start, int length); -bool isa_is_ioport_assigned(pio_addr_t start); void cpu_outb(pio_addr_t addr, uint8_t val); void cpu_outw(pio_addr_t addr, uint16_t val); diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h index 82da37c..27fb51e 100644 --- a/include/hw/isa/isa.h +++ b/include/hw/isa/isa.h @@ -80,6 +80,8 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, const MemoryRegionPortio *portio, void *opaque, const char *name); +bool isa_is_ioport_assigned(ISABus *bus, pio_addr_t start); + static inline ISABus *isa_bus_from_device(ISADevice *d) { return ISA_BUS(qdev_get_parent_bus(DEVICE(d))); diff --git a/ioport.c b/ioport.c index d5b7fbd..56470c5 100644 --- a/ioport.c +++ b/ioport.c @@ -273,13 +273,6 @@ void isa_unassign_ioport(pio_addr_t start, int length) } } -bool isa_is_ioport_assigned(pio_addr_t start) -{ - return (ioport_read_table[0][start] || ioport_write_table[0][start] || - ioport_read_table[1][start] || ioport_write_table[1][start] || - ioport_read_table[2][start] || ioport_write_table[2][start]); -} - /***********************************************************/ void cpu_outb(pio_addr_t addr, uint8_t val)
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 finally allow us to drop the ioport tables. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- 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(-)