From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1M0jlZ-0001X0-B1 for qemu-devel@nongnu.org; Sun, 03 May 2009 18:02:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1M0jlU-0001V2-2V for qemu-devel@nongnu.org; Sun, 03 May 2009 18:02:32 -0400 Received: from [199.232.76.173] (port=43930 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1M0jlT-0001Uv-U3 for qemu-devel@nongnu.org; Sun, 03 May 2009 18:02:27 -0400 Received: from mx2.redhat.com ([66.187.237.31]:39711) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1M0jlT-00053d-9V for qemu-devel@nongnu.org; Sun, 03 May 2009 18:02:27 -0400 Date: Mon, 4 May 2009 01:01:22 +0300 From: "Michael S. Tsirkin" Message-ID: <20090503220122.GA25047@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] [PATCH] qemu: cleanup default_write_config List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org, Anthony Liguori , Blue Swirl , mtosatti@redhat.com, armbru@redhat.com Change pci default write config to use symbolic constants and a table-driven design: add a mask table with writable bits set and readonly bits unset. This makes it much easier to support multi-byte registers. As a result, writing a single byte in BAR registers now works as it should. Writing to upper limit registers in the bridge also works as it should. Code is also shorter. Signed-off-by: Michael S. Tsirkin --- This patch applies to upstream qemu. I ported and tested this with qemu-kvm. hw/pci.c | 146 ++++++++++++++------------------------------------------------ hw/pci.h | 16 +++++++ 2 files changed, 49 insertions(+), 113 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 8278894..1e00f76 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -235,6 +235,17 @@ int pci_assign_devaddr(const char *addr, int *domp, int *busp, unsigned *slotp) return pci_parse_devaddr(devaddr, domp, busp, slotp); } +static void pci_init_mask(PCIDevice *dev) +{ + int i; + dev->mask[PCI_CACHE_LINE_SIZE] = 0xff; + dev->mask[PCI_INTERRUPT_LINE] = 0xff; + dev->mask[PCI_COMMAND] = PCI_COMMAND_IO | PCI_COMMAND_MEMORY + | PCI_COMMAND_MASTER; + for (i = PCI_CONFIG_HEADER_SIZE; i < PCI_CONFIG_SPACE_SIZE; ++i) + dev->mask[i] = 0xff; +} + /* -1 for devfn means auto assign */ PCIDevice *pci_register_device(PCIBus *bus, const char *name, int instance_size, int devfn, @@ -260,6 +271,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name, pstrcpy(pci_dev->name, sizeof(pci_dev->name), name); memset(pci_dev->irq_state, 0, sizeof(pci_dev->irq_state)); pci_set_default_subsystem_id(pci_dev); + pci_init_mask(pci_dev); if (!config_read) config_read = pci_default_read_config; @@ -321,6 +333,7 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, { PCIIORegion *r; uint32_t addr; + uint32_t mask; if ((unsigned int)region_num >= PCI_NUM_REGIONS) return; @@ -336,12 +349,17 @@ void pci_register_io_region(PCIDevice *pci_dev, int region_num, r->size = size; r->type = type; r->map_func = map_func; + + mask = ~(size - 1); if (region_num == PCI_ROM_SLOT) { addr = 0x30; + /* ROM enable bit is writeable */ + mask |= 1; } else { addr = 0x10 + region_num * 4; } *(uint32_t *)(pci_dev->config + addr) = cpu_to_le32(type); + *(uint32_t *)(pci_dev->mask + addr) = cpu_to_le32(mask); } static void pci_update_mappings(PCIDevice *d) @@ -451,117 +469,27 @@ uint32_t pci_default_read_config(PCIDevice *d, } void pci_default_write_config(PCIDevice *d, - uint32_t address, uint32_t val, int len) + uint32_t addr, uint32_t val, int len) { - int can_write, i; - uint32_t end, addr; - - if (len == 4 && ((address >= 0x10 && address < 0x10 + 4 * 6) || - (address >= 0x30 && address < 0x34))) { - PCIIORegion *r; - int reg; - - if ( address >= 0x30 ) { - reg = PCI_ROM_SLOT; - }else{ - reg = (address - 0x10) >> 2; - } - r = &d->io_regions[reg]; - if (r->size == 0) - goto default_config; - /* compute the stored value */ - if (reg == PCI_ROM_SLOT) { - /* keep ROM enable bit */ - val &= (~(r->size - 1)) | 1; - } else { - val &= ~(r->size - 1); - val |= r->type; - } - *(uint32_t *)(d->config + address) = cpu_to_le32(val); - pci_update_mappings(d); - return; - } - default_config: + int i, cmd_update = 0; /* not efficient, but simple */ - addr = address; - for(i = 0; i < len; i++) { - /* default read/write accesses */ - switch(d->config[0x0e]) { - case 0x00: - case 0x80: - switch(addr) { - case 0x00: - case 0x01: - case 0x02: - case 0x03: - case 0x06: - case 0x07: - case 0x08: - case 0x09: - case 0x0a: - case 0x0b: - case 0x0e: - case 0x10 ... 0x27: /* base */ - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x30 ... 0x33: /* rom */ - case 0x3d: - can_write = 0; - break; - default: - can_write = 1; - break; - } - break; - default: - case 0x01: - switch(addr) { - case 0x00: - case 0x01: - case 0x02: - case 0x03: - case 0x06: - case 0x07: - case 0x08: - case 0x09: - case 0x0a: - case 0x0b: - case 0x0e: - case 0x2c ... 0x2f: /* read-only subsystem ID & vendor ID */ - case 0x38 ... 0x3b: /* rom */ - case 0x3d: - can_write = 0; - break; - default: - can_write = 1; - break; - } - break; - } - if (can_write) { - /* Mask out writes to reserved bits in registers */ - switch (addr) { - case 0x05: - val &= ~PCI_COMMAND_RESERVED_MASK_HI; - break; - case 0x06: - val &= ~PCI_STATUS_RESERVED_MASK_LO; - break; - case 0x07: - val &= ~PCI_STATUS_RESERVED_MASK_HI; - break; - } - d->config[addr] = val; - } + for(i = 0; i < len; val >>= 8, ++i) { + uint8_t mask = d->mask[addr]; + uint8_t orgval = d->config[addr]; + uint8_t newval = (orgval & ~mask) | (val & mask); + d->config[addr] = newval; if (++addr > 0xff) - break; - val >>= 8; + break; + if (orgval == newval) + continue; + if (addr == PCI_COMMAND || (addr >= PCI_BASE_ADDRESS_0 && + addr < PCI_CAPABILITY_LIST)) + cmd_update = 1; } - end = address + len; - if (end > PCI_COMMAND && address < (PCI_COMMAND + 2)) { + if (cmd_update) /* if the command register is modified, we must modify the mappings */ pci_update_mappings(d); - } } void pci_data_write(void *opaque, uint32_t addr, uint32_t val, int len) @@ -833,16 +761,8 @@ static void pci_bridge_write_config(PCIDevice *d, { PCIBridge *s = (PCIBridge *)d; - if (address == 0x19 || (address == 0x18 && len > 1)) { - if (address == 0x19) - s->bus->bus_num = val & 0xff; - else - s->bus->bus_num = (val >> 8) & 0xff; -#if defined(DEBUG_PCI) - printf ("pci-bridge: %s: Assigned bus %d\n", d->name, s->bus->bus_num); -#endif - } pci_default_write_config(d, address, val, len); + s->bus->bus_num = d->config[PCI_SECONDARY_BUS]; } PCIBus *pci_find_bus(int bus_num) diff --git a/hw/pci.h b/hw/pci.h index ff858a1..c71f515 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -98,16 +98,24 @@ typedef struct PCIIORegion { #define PCI_COMMAND 0x04 /* 16 bits */ #define PCI_COMMAND_IO 0x1 /* Enable response in I/O space */ #define PCI_COMMAND_MEMORY 0x2 /* Enable response in Memory space */ +#define PCI_COMMAND_MASTER 0x4 /* Enable bus master */ #define PCI_STATUS 0x06 /* 16 bits */ #define PCI_REVISION_ID 0x08 /* 8 bits */ #define PCI_CLASS_DEVICE 0x0a /* Device class */ +#define PCI_CACHE_LINE_SIZE 0x0c /* 8 bits */ +#define PCI_LATENCY_TIMER 0x0d /* 8 bits */ #define PCI_HEADER_TYPE 0x0e /* 8 bits */ #define PCI_HEADER_TYPE_NORMAL 0 #define PCI_HEADER_TYPE_BRIDGE 1 #define PCI_HEADER_TYPE_CARDBUS 2 #define PCI_HEADER_TYPE_MULTI_FUNCTION 0x80 +#define PCI_BASE_ADDRESS_0 0x10 /* 32 bits */ +#define PCI_PRIMARY_BUS 0x18 /* Primary bus number */ +#define PCI_SECONDARY_BUS 0x19 /* Secondary bus number */ +#define PCI_SEC_STATUS 0x1e /* Secondary status register, only bit 14 used */ #define PCI_SUBSYSTEM_VENDOR_ID 0x2c /* 16 bits */ #define PCI_SUBSYSTEM_ID 0x2e /* 16 bits */ +#define PCI_CAPABILITY_LIST 0x34 /* Offset of first capability list entry */ #define PCI_INTERRUPT_LINE 0x3c /* 8 bits */ #define PCI_INTERRUPT_PIN 0x3d /* 8 bits */ #define PCI_MIN_GNT 0x3e /* 8 bits */ @@ -137,10 +145,18 @@ typedef struct PCIIORegion { #define PCI_COMMAND_RESERVED_MASK_HI (PCI_COMMAND_RESERVED >> 8) +/* Size of the standard PCI config header */ +#define PCI_CONFIG_HEADER_SIZE (PCI_INTERRUPT_PIN + 1) +/* Size of the standard PCI config space */ +#define PCI_CONFIG_SPACE_SIZE 0x100 + struct PCIDevice { /* PCI config space */ uint8_t config[256]; + /* Used to implement R/W bytes */ + uint8_t mask[256]; + /* the following fields are read only */ PCIBus *bus; int devfn; -- 1.6.3.rc3.dirty