On Wed, Jul 7, 2010 at 7:02 PM, Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 07/07/2010 12:53 PM, Blue Swirl wrote:
>>
>> Add I/O port registration functions which separate registration
>> from the mapping stage.
>>
>> Move IOIO and MMIO BAR mapping to pci.c.
>>
>> TODO: fix dirty logging, coalesced MMIO and base address comparisons
>> (eepro100 etc). Bridge filtering may be broken. Broke virtio-pci and MSIX.
>>
>> Signed-off-by: Blue Swirl<blauwir...@gmail.com>
>> ---
>> i386 boots but resets. PPC and Sparc64 can't even start.
>>
>> Patch also available at
>> git://repo.or.cz/qemu/blueswirl.git
>>
>> It may be worthwhile to break this into some kind of smaller steps.
>>
>> hw/ac97.c | 60 +++++++++++---------
>> hw/cirrus_vga.c | 40 +++-----------
>> hw/e1000.c | 37 +-----------
>> hw/eepro100.c | 77 ++++++++++----------------
>> hw/es1370.c | 32 +++++------
>> hw/ide/cmd646.c | 149
>> +++++++++++++++++++++++++++++++-------------------
>> hw/ide/piix.c | 74 ++++++++++++++++---------
>> hw/ide/via.c | 67 ++++++++++++++--------
>> hw/isa.h | 1 +
>> hw/isa_mmio.c | 17 +++++-
>> hw/lsi53c895a.c | 60 ++++++--------------
>> hw/macio.c | 107 +++++++++++-------------------------
>> hw/ne2000.c | 66 +++++++++++++++-------
>> hw/openpic.c | 36 ++----------
>> hw/pci.c | 158
>> ++++++++++++++++++++++++++++++++--------------------
>> hw/pci.h | 18 +++++-
>> hw/pcnet.c | 62 ++++++++++-----------
>> hw/ppc_mac.h | 5 +-
>> hw/ppc_newworld.c | 2 +-
>> hw/ppc_oldworld.c | 4 +-
>> hw/rtl8139.c | 42 +++++---------
>> hw/sun4u.c | 29 +++------
>> hw/usb-ohci.c | 10 +---
>> hw/usb-uhci.c | 31 +++++-----
>> hw/vga-pci.c | 22 +------
>> hw/virtio-pci.c | 39 ++++++-------
>> hw/vmware_vga.c | 107 ++++++++++++++++++------------------
>> hw/wdt_i6300esb.c | 38 +++++--------
>> ioport.c | 119 ++++++++++++++++++++++++++++++++++++----
>> ioport.h | 6 ++
>> 30 files changed, 778 insertions(+), 737 deletions(-)
>>
>> diff --git a/hw/ac97.c b/hw/ac97.c
>> index 4319bc8..28d0c19 100644
>> --- a/hw/ac97.c
>> +++ b/hw/ac97.c
>> @@ -1234,31 +1234,29 @@ static const VMStateDescription vmstate_ac97 = {
>> }
>> };
>>
>> -static void ac97_map (PCIDevice *pci_dev, int region_num,
>> - pcibus_t addr, pcibus_t size, int type)
>> -{
>> - AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, pci_dev);
>> - PCIDevice *d =&s->dev;
>> -
>> - if (!region_num) {
>> - s->base[0] = addr;
>> - register_ioport_read (addr, 256 * 1, 1, nam_readb, d);
>> - register_ioport_read (addr, 256 * 2, 2, nam_readw, d);
>> - register_ioport_read (addr, 256 * 4, 4, nam_readl, d);
>> - register_ioport_write (addr, 256 * 1, 1, nam_writeb, d);
>> - register_ioport_write (addr, 256 * 2, 2, nam_writew, d);
>> - register_ioport_write (addr, 256 * 4, 4, nam_writel, d);
>> - }
>> - else {
>> - s->base[1] = addr;
>> - register_ioport_read (addr, 64 * 1, 1, nabm_readb, d);
>> - register_ioport_read (addr, 64 * 2, 2, nabm_readw, d);
>> - register_ioport_read (addr, 64 * 4, 4, nabm_readl, d);
>> - register_ioport_write (addr, 64 * 1, 1, nabm_writeb, d);
>> - register_ioport_write (addr, 64 * 2, 2, nabm_writew, d);
>> - register_ioport_write (addr, 64 * 4, 4, nabm_writel, d);
>> - }
>> -}
>> +static IOPortWriteFunc * const nam_writes[] = {
>> + nam_writeb,
>> + nam_writew,
>> + nam_writel,
>> +};
>> +
>> +static IOPortReadFunc * const nam_reads[] = {
>> + nam_readb,
>> + nam_readw,
>> + nam_readl,
>> +};
>> +
>> +static IOPortWriteFunc * const nabm_writes[] = {
>> + nabm_writeb,
>> + nabm_writew,
>> + nabm_writel,
>> +};
>> +
>> +static IOPortReadFunc * const nabm_reads[] = {
>> + nabm_readb,
>> + nabm_readw,
>> + nabm_readl,
>> +};
>>
>> static void ac97_on_reset (void *opaque)
>> {
>> @@ -1280,6 +1278,7 @@ static int ac97_initfn (PCIDevice *dev)
>> {
>> AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
>> uint8_t *c = s->dev.config;
>> + int io_index;
>>
>> pci_config_set_vendor_id (c, PCI_VENDOR_ID_INTEL); /* ro */
>> pci_config_set_device_id (c, PCI_DEVICE_ID_INTEL_82801AA_5); /* ro */
>> @@ -1321,9 +1320,14 @@ static int ac97_initfn (PCIDevice *dev)
>> /* TODO: RST# value should be 0. */
>> c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */
>>
>> - pci_register_bar (&s->dev, 0, 256 * 4, PCI_BASE_ADDRESS_SPACE_IO,
>> - ac97_map);
>> - pci_register_bar (&s->dev, 1, 64 * 4, PCI_BASE_ADDRESS_SPACE_IO,
>> ac97_map);
>> + pci_register_bar(&s->dev, 0, 256 * 4, PCI_BASE_ADDRESS_SPACE_IO);
>> + io_index = cpu_register_io(nam_reads, nam_writes, 256 * 4, s);
>> + pci_bar_map(&s->dev, 0, 0, 0, 256 * 4, io_index);
>> +
>> + pci_register_bar(&s->dev, 1, 64 * 4, PCI_BASE_ADDRESS_SPACE_IO);
>> + io_index = cpu_register_io(nabm_reads, nabm_writes, 64 * 4, s);
>> + pci_bar_map(&s->dev, 1, 0, 0, 64 * 4, io_index);
>>
>
> Any reason to keep this a three step process? I see no reason not to unify
> the io and mem function pointers into a single type.
I was also considering this, but the function signatures don't match
now. With a lot more effort (or smaller steps) it could be doable.
> These callbacks should
> be working off of pci bus addresses and should not be tied directly to CPU
> memory/pio callbacks.
The CPU callbacks may be useful outside of PCI code, like ISA.
