Am 05.03.2014 15:14, schrieb Romain Dolbeau: > Try to implement proper QOM
Sorry for not getting back to you earlier but you seem to have found out. Some small nits below. > > Signed-off-by: Romain Dolbeau <rom...@dolbeau.org> > --- > hw/net/e1000.c | 394 > +++++++++++++++++++++++++++++++++++++++++++++++---- > hw/net/e1000_regs.h | 149 ++++++++++++++++--- > 2 files changed, 492 insertions(+), 51 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index 8387443..9737ecf 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -1,8 +1,10 @@ > /* > * QEMU e1000 emulation > * > - * Software developer's manual: > - * http://download.intel.com/design/network/manuals/8254x_GBe_SDM.pdf > + * Software developer's manual (PCI, PCI-X): > + * <http://download.intel.com/design/network/manuals/8254x_GBe_SDM.pdf> > + * Software developer's manual (PCIe): > + * > <http://www.intel.com/content/dam/www/public/us/en/documents/manuals/pcie-gbe-controllers-open-source-manual.pdf> > * > * Nir Peleg, Tutis Systems Ltd. for Qumranet Inc. > * Copyright (c) 2008 Qumranet > @@ -10,6 +12,8 @@ > * Copyright (c) 2007 Dan Aloni > * Copyright (c) 2004 Antony T Curtis > * > + * Additional modifications (c) 2014 Romain Dolbeau > + * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > * License as published by the Free Software Foundation; either > @@ -58,6 +62,7 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL); > > #define IOPORT_SIZE 0x40 > #define PNPMMIO_SIZE 0x20000 > +#define FLASH_RSIZE 0x80 > #define MIN_BUF_SIZE 60 /* Min. octets in an ethernet frame sans FCS */ > > /* this is the size past which hardware will drop packets when setting LPE=0 > */ > @@ -69,10 +74,12 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL); > > /* > * HW models: > - * E1000_DEV_ID_82540EM works with Windows and Linux > - * E1000_DEV_ID_82573L OK with windoze and Linux 2.6.22, > - * appears to perform better than 82540EM, but breaks with Linux 2.6.18 > + * E1000_DEV_ID_82540EM works with Windows and Linux, and is the default > * E1000_DEV_ID_82544GC_COPPER appears to work; not well tested > + * E1000_DEV_ID_82545EM_COPPER appears to work with OSX 10.9[.1]; not well > tested > + * E1000_DEV_ID_ICH9_IGP_AMT appears to work with Linux kernel 3.12; not > well tested > + * It seems those 3 are mostly identical anyway, so picking one > + * over the others is a matter of guest support. > * Others never tested > */ > enum { E1000_DEVID = E1000_DEV_ID_82540EM }; > @@ -81,10 +88,24 @@ enum { E1000_DEVID = E1000_DEV_ID_82540EM }; > * May need to specify additional MAC-to-PHY entries -- > * Intel's Windows driver refuses to initialize unless they match > */ > -enum { > - PHY_ID2_INIT = E1000_DEVID == E1000_DEV_ID_82573L ? 0xcc2 : > - E1000_DEVID == E1000_DEV_ID_82544GC_COPPER ? 0xc30 : > - /* default to E1000_DEV_ID_82540EM */ 0xc20 > +/* PHY_ID1: > + * Most 8254x uses 0x141, but 82541xx and 82547GI/EI uses 0x2a8, > + * and so do the 631xESB/632xESB, 82571EB/82572EI. > + * The 82573E/82573V/82573L and 82563EB/82564EB also uses 0x141. > + * <http://download.intel.com/design/network/manuals/8254x_GBe_SDM.pdf> page > 250 > + * > <http://www.intel.com/content/dam/www/public/us/en/documents/manuals/pcie-gbe-controllers-open-source-manual.pdf> > page 305 > + */ > +const uint16_t PHY_ID1_INIT[][2] = { > + { E1000_DEV_ID_80003ES2LAN_COPPER_DPT, 0x2a8 }, > + { E1000_DEV_ID_ICH9_IGP_AMT, 0x2a8 }, > + { -1, 0x141 } /* default for 82540EM and many others */ > +}; > +const uint16_t PHY_ID2_INIT[][2] = { > + { E1000_DEV_ID_82573L, 0xcc2 }, > + { E1000_DEV_ID_82544GC_COPPER, 0xc30 }, > + { E1000_DEV_ID_ICH9_IGP_AMT, 0x390 }, > + { -1, 0xc20 } /* default for 82540EM and many others; this one > + is a lot more device-specific than phy_id1 */ > }; > > typedef struct E1000State_st { > @@ -95,12 +116,15 @@ typedef struct E1000State_st { > NICState *nic; > NICConf conf; > MemoryRegion mmio; > + MemoryRegion flash; > MemoryRegion io; > > uint32_t mac_reg[0x8000]; > uint16_t phy_reg[0x20]; > uint16_t eeprom_data[64]; > + uint16_t flash_reg[FLASH_RSIZE]; > > + uint32_t rxbuf_edesc; > uint32_t rxbuf_size; > uint32_t rxbuf_min_shift; > struct e1000_tx { > @@ -151,7 +175,7 @@ typedef struct E1000State_st { > uint32_t compat_flags; > } E1000State; > > -#define TYPE_E1000 "e1000" > +#define TYPE_E1000 "e1000_abstract" Convention is dashes, maybe "common-e1000"? or "base-e1000"? > > #define E1000(obj) \ > OBJECT_CHECK(E1000State, (obj), TYPE_E1000) > @@ -170,6 +194,7 @@ enum { > defreg(RA), defreg(MTA), defreg(CRCERRS),defreg(VFTA), > defreg(VET), defreg(RDTR), defreg(RADV), defreg(TADV), > defreg(ITR), > + defreg(EXTCNF_CTRL), defreg(FWSM), defreg(KABGTXD), defreg(FACTPS), > }; > > static void > @@ -229,13 +254,19 @@ static const char phy_regcap[0x20] = { > [PHY_CTRL] = PHY_RW, [PHY_1000T_CTRL] = PHY_RW, > [PHY_LP_ABILITY] = PHY_R, [PHY_1000T_STATUS] = PHY_R, > [PHY_AUTONEG_ADV] = PHY_RW, [M88E1000_RX_ERR_CNTR] = PHY_R, > - [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R > + [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R, > + [ICH_IGP_PHY_REGADD_ALT_MDIO] = PHY_RW /* ICH IGP ? */, > + [PHY_AUTONEG_EXP] = PHY_RW, /* ICH IGP ? */ > + [PHY_EXT_STATUS] = PHY_RW, /* ICH IGP ? */ > + [M88E1000_INT_ENABLE] = PHY_RW, /* ICH IGP ? */ > + [M88E1000_INT_STATUS] = PHY_RW, /* ICH IGP ? */ > }; > > static const uint16_t phy_reg_init[] = { > [PHY_CTRL] = 0x1140, > [PHY_STATUS] = 0x794d, /* link initially up with not completed autoneg */ > - [PHY_ID1] = 0x141, [PHY_ID2] = > PHY_ID2_INIT, > + /* both phy_id will be replaced */ > + [PHY_ID1] = 0x141, [PHY_ID2] = 0xc20, > [PHY_1000T_CTRL] = 0x0e00, > [M88E1000_PHY_SPEC_CTRL] = 0x360, > [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, [PHY_AUTONEG_ADV] = 0xde1, > [PHY_LP_ABILITY] = 0x1e0, [PHY_1000T_STATUS] = > 0x3c00, > @@ -269,10 +300,11 @@ static void > set_interrupt_cause(E1000State *s, int index, uint32_t val) > { > PCIDevice *d = PCI_DEVICE(s); > + PCIDeviceClass *pdc = PCI_DEVICE_GET_CLASS(d); > uint32_t pending_ints; > uint32_t mit_delay; > > - if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) { > + if (val && (pdc->device_id >= E1000_DEV_ID_82547EI_MOBILE)) { > /* Only for 8257x */ > val |= E1000_ICR_INT_ASSERTED; > } > @@ -375,8 +407,11 @@ rxbufsize(uint32_t v) > static void e1000_reset(void *opaque) > { > E1000State *d = opaque; > + PCIDevice *dev = PCI_DEVICE(d); > + PCIDeviceClass *pdc = PCI_DEVICE_GET_CLASS(dev); You could just do PCI_DEVICE_GET_CLASS(d) to spare the dev variable. In particular "dev" for PCIDevice is not so nice in case we ever need DeviceState here. > uint8_t *macaddr = d->conf.macaddr.a; > int i; > + uint16_t phy_id1 = -1, phy_id2 = -1; > > timer_del(d->autoneg_timer); > timer_del(d->mit_timer); > @@ -385,6 +420,24 @@ static void e1000_reset(void *opaque) > d->mit_ide = 0; > memset(d->phy_reg, 0, sizeof d->phy_reg); > memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init); > + for (i = 0 ; PHY_ID1_INIT[i][0] != (uint16_t)-1 ; i++) { Extra spaces before semicolons. > + if (PHY_ID1_INIT[i][0] == pdc->device_id) { > + phy_id1 = PHY_ID1_INIT[i][1]; > + } > + } > + if (phy_id1 == (uint16_t)-1) { > + phy_id1 = PHY_ID1_INIT[i][1]; > + } > + for (i = 0 ; PHY_ID2_INIT[i][0] != (uint16_t)-1 ; i++) { Dito. > + if (PHY_ID2_INIT[i][0] == pdc->device_id) { > + phy_id2 = PHY_ID2_INIT[i][1]; > + } > + } > + if (phy_id2 == (uint16_t)-1) { > + phy_id2 = PHY_ID2_INIT[i][1]; > + } > + d->phy_reg[PHY_ID1] = phy_id1; > + d->phy_reg[PHY_ID2] = phy_id2; > memset(d->mac_reg, 0, sizeof d->mac_reg); > memmove(d->mac_reg, mac_reg_init, sizeof mac_reg_init); > d->rxbuf_min_shift = 1; > @@ -402,6 +455,13 @@ static void e1000_reset(void *opaque) > d->mac_reg[RA + 1] |= (i < 2) ? macaddr[i + 4] << (8 * i) : 0; > } > qemu_format_nic_info_str(qemu_get_queue(d->nic), macaddr); > + > + /* reset flash (clear locks) */ > + memset(d->flash_reg, 0, FLASH_RSIZE*sizeof(uint16_t)); Spaces around operators please. > + union ich8_hws_flash_status hsfsts; > + hsfsts.regval = 0; > + hsfsts.hsf_status.fldesvalid = 1; > + d->flash_reg[ICH_FLASH_HSFSTS] = hsfsts.regval; > } > > static void > @@ -409,6 +469,9 @@ set_ctrl(E1000State *s, int index, uint32_t val) > { > /* RST is self clearing */ > s->mac_reg[CTRL] = val & ~E1000_CTRL_RST; > + if (val & E1000_CTRL_RST) { > + e1000_reset(s); > + } > } > > static void > @@ -1017,7 +1080,17 @@ e1000_receive_iov(NetClientState *nc, const struct > iovec *iov, int iovcnt) > } else { // as per intel docs; skip descriptors with null buf addr > DBGOUT(RX, "Null RX descriptor!!\n"); > } > - pci_dma_write(d, base, &desc, sizeof(desc)); > + if (!s->rxbuf_edesc) { > + pci_dma_write(d, base, &desc, sizeof(desc)); > + } else { /* extended rx descriptor */ > + union e1000_rx_desc_extended edesc; > + edesc.wb.lower.mrq = 0; > + edesc.wb.lower.hi_dword.rss = 0; > + edesc.wb.upper.status_error = /* desc.errors << 24 | */ > desc.status; Why is this commented out? > + edesc.wb.upper.length = desc.length; > + edesc.wb.upper.vlan = desc.special; > + pci_dma_write(d, base, &edesc, sizeof(edesc)); > + } > > if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN]) > s->mac_reg[RDH] = 0; > @@ -1173,6 +1246,7 @@ static uint32_t (*macreg_readops[])(E1000State *, int) > = { > getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL), > getreg(TDLEN), getreg(RDLEN), getreg(RDTR), getreg(RADV), > getreg(TADV), getreg(ITR), > + getreg(EXTCNF_CTRL), getreg(FWSM), getreg(KABGTXD), getreg(FACTPS), > > [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, [GPRC] = mac_read_clr4, > [GPTC] = mac_read_clr4, [TPR] = mac_read_clr4, [TPT] = mac_read_clr4, > @@ -1189,6 +1263,7 @@ static void (*macreg_writeops[])(E1000State *, int, > uint32_t) = { > putreg(PBA), putreg(EERD), putreg(SWSM), putreg(WUFC), > putreg(TDBAL), putreg(TDBAH), putreg(TXDCTL), putreg(RDBAH), > putreg(RDBAL), putreg(LEDCTL), putreg(VET), > + putreg(EXTCNF_CTRL), putreg(FWSM), putreg(KABGTXD), putreg(FACTPS), > [TDLEN] = set_dlen, [RDLEN] = set_dlen, [TCTL] = set_tctl, > [TDT] = set_tctl, [MDIC] = set_mdic, [ICS] = set_ics, > [TDH] = set_16bit, [RDH] = set_16bit, [RDT] = set_rdt, > @@ -1234,6 +1309,73 @@ e1000_mmio_read(void *opaque, hwaddr addr, unsigned > size) > return 0; > } > > + > + > + White lines intentional? > +static void > +e1000_flash_write(void *opaque, hwaddr addr, uint64_t val, > + unsigned size) > +{ > + E1000State *s = opaque; > + unsigned int index = addr % 2048; > + > + if (index < FLASH_RSIZE) { > + s->flash_reg[index] = val & 0xFFFF; > + switch (index) { > + case ICH_FLASH_HSFSTS: { > + break; > + } Braces not strictly necessary here (no variables). > + case ICH_FLASH_HSFCTL: { > + union ich8_hws_flash_ctrl ctrl; > + ctrl.regval = s->flash_reg[ICH_FLASH_HSFCTL]; > + if (ctrl.hsf_ctrl.flcgo) { > + /* says we're done, clear go, > + copy data to proper register */ > + union ich8_hws_flash_status hsfsts; > + int fldbcount; > + uint16_t offset; > + uint16_t res; > + hsfsts.regval = s->flash_reg[ICH_FLASH_HSFSTS]; > + hsfsts.hsf_status.flcdone = 1; > + hsfsts.hsf_status.flcerr = 0; > + s->flash_reg[ICH_FLASH_HSFSTS] = hsfsts.regval; > + fldbcount = ctrl.hsf_ctrl.fldbcount; > + ctrl.hsf_ctrl.flcgo = 0; > + s->flash_reg[ICH_FLASH_HSFCTL] = ctrl.regval; > + offset = s->flash_reg[ICH_FLASH_FADDR] >> 1; > + res = s->eeprom_data[offset]; > + if (fldbcount == 0) { > + if (s->flash_reg[ICH_FLASH_FADDR] % 2) { > + res = res >> 8; > + } else { > + res = res & 0x00FF; > + } > + } > + s->flash_reg[ICH_FLASH_FDATA0] = res; > + } > + } > + default: > + break; > + } > + } else { > + DBGOUT(UNKNOWN, "Flash unknown write > addr=0x%08x,val=0x%08"PRIx64"\n", > + index<<2, val); Spaces > + } > +} > + > +static uint64_t > +e1000_flash_read(void *opaque, hwaddr addr, unsigned size) > +{ > + E1000State *s = opaque; > + unsigned int index = addr % 2048; > + > + if (index < FLASH_RSIZE) { > + return s->flash_reg[index]; > + } > + DBGOUT(UNKNOWN, "Flash unknown read addr=0x%08x\n", index<<2); Spaces > + return 0; > +} > + > static const MemoryRegionOps e1000_mmio_ops = { > .read = e1000_mmio_read, > .write = e1000_mmio_write, > @@ -1244,6 +1386,16 @@ static const MemoryRegionOps e1000_mmio_ops = { > }, > }; > > +static const MemoryRegionOps e1000_flash_ops = { > + .read = e1000_flash_read, > + .write = e1000_flash_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .impl = { > + .min_access_size = 4, > + .max_access_size = 4, > + }, > +}; > + > static uint64_t e1000_io_read(void *opaque, hwaddr addr, > unsigned size) > { > @@ -1425,6 +1577,8 @@ static const VMStateDescription vmstate_e1000 = { > VMSTATE_UINT32(mac_reg[TXDCTL], E1000State), > VMSTATE_UINT32(mac_reg[WUFC], E1000State), > VMSTATE_UINT32(mac_reg[VET], E1000State), > + VMSTATE_UINT32(mac_reg[EXTCNF_CTRL], E1000State), > + VMSTATE_UINT32(mac_reg[FWSM], E1000State), > VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, RA, 32), > VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128), > VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128), You can't just add fields here, you need to use VMSTATE_UINT32_V() and increment the version number. Or use a subsection conditional on whatever makes these fields relevant. Keep in mind that -device e1000 from v1.7 would have the fields initialized to whatever init/reset set them to. > @@ -1440,15 +1594,106 @@ static const VMStateDescription vmstate_e1000 = { > } > }; > > +/* > + * The content of EEPROM is documented in the documentation > + * PCI/X: "Table 5-2. Ethernet Controller Address Map" page 98 (except > 82544GC/EI and 82541ER) > + * PCI/X: "Table 5-3. 82544GC/EI and 82541ER EEPROM Address Map" page 102 > + * PCIe: "Table 5-2. Ethernet Controller EEPROM Map" page 134 > + */ > static const uint16_t e1000_eeprom_template[64] = { > - 0x0000, 0x0000, 0x0000, 0x0000, 0xffff, 0x0000, 0x0000, 0x0000, > - 0x3000, 0x1000, 0x6403, E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, 0x3040, > - 0x0008, 0x2000, 0x7e14, 0x0048, 0x1000, 0x00d8, 0x0000, 0x2700, > - 0x6cc9, 0x3150, 0x0722, 0x040b, 0x0984, 0x0000, 0xc000, 0x0706, > - 0x1008, 0x0000, 0x0f04, 0x7fff, 0x4d01, 0xffff, 0xffff, 0xffff, > - 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > - 0x0100, 0x4000, 0x121c, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > - 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0x0000, > + /* 00h - 02h: Ethernet address, will be overridden */ > + 0x0000, 0x0000, 0x0000, > + /* 03h: compatibility, this seems a bit device-specific > + and probably should be overridden */ > + 0x0000, > + /* 04h: compatibility (PCIe) or SerDes config (most PCI/X) or LED */ > + 0xffff, > + /* 05h - 07h: EEprom version & OEM (PCIe other than 82573), > + compatibility (most PCI/X, 82573) */ > + 0x0000, 0x0000, 0x0000, > + /* 08h - 09h: PBA (irrelevant) */ > + 0x3000, 0x1000, > + /* 0ah: init control 1 */ > + 0x6403, > + /* 0bh - 0eh: PCI ID, will be overridden */ > + E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, > + /* 0fh: init control 2 */ > + 0x3040, > + /* 10h - 12h: seem quite device-specific with several variants */ > + 0x0008, 0x2000, 0x7e14, > + /* 13h: management */ > + 0x0048, > + /* 14h: init control 3 (2nd LAN?), not 82573 */ > + 0x1000, > + /* 15h - 16h: IPv4 (PCI/X) or reserved (PCIe), not 82573 */ > + 0x00d8, 0x0000, > + /* 17h - 1Eh: Another batch of variants; IPv6 LAN in PCI/X > + * but is FW Config Start Address (17h, most PCIe) followed > + * by PCI init configuration and stuff > + */ > + 0x2700, 0x6cc9, 0x3150, 0x0722, 0x040b, 0x0984, 0x0000, 0xc000, > + /* 1fh: mostly reserved (PCI/X), LED conf (PCIe) */ > + 0x0706, > + /* 20h - 21h: software defined pin controls, 21h: mostly control */ > + 0x1008, 0x0000, > + /* 22h - 23h: LAN power, management control (not 82573) */ > + 0x0f04, 0x7fff, > + /* 24h: init control 3 (again?) */ > + 0x4d01, > + /* 25h-2eh: either IP4/6 (PCI/X) or mostly reserved (PCIe) */ > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + /* 2fh: LEDCTL Default (PCI/X) or Vital Product Data (VPD) Pointer > (PCIe) */ > + 0xffff, > + /* 30h-3eh: mostly PXE/boot stuff */ > + 0x0100, 0x4000, 0x121c, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + /* 3fh: checksum to be computed */ > + 0x0000 > +}; > +static const uint16_t e1000_ich8_flash_template[64] = { > + /* 00h - 02h: Ethernet address, will be overridden */ > + 0x0000, 0x0000, 0x0000, > + /* 03h - 04h: reserved */ > + 0x0800, 0xFFFF, > + /* 05h - 07h: image version information, reserved*/ > + 0x0000, 0xFFFF, 0xFFFF, > + /* 08h - 09h: PBA (irrelevant) */ > + 0x3000, 0x1000, > + /* 0ah: init control 1 */ > + 0x10c7, > + /* 0bh - 0eh: PCI ID, will be overridden */ > + E1000_DEVID, 0x8086, E1000_DEVID, 0x8086, > + /* 0fh: device revision id (ich9), reserved (ich8) */ > + 0x0000, /* fixme */ > + /* 10h - 12h: LAN power consumption, reserved */ > + 0x0D01, 0x0000, 0x0000, > + /* 13h: Shared Initialization Control Word */ > + 0x9607, > + /* 14h - 16h: extended configuration word 1-3 */ > + 0x7020, 0x3800, 0x0000, > + /* 17h - 18h: LEDCTL 1, LEDCTL 0 2 */ > + 0x8d07, 0x0684, > + /* 19h - 1ah: future initialization words */ > + (0x0181 | 0x0040), 0x0800, > + /* 1bh - 1dh: reserved */ > + 0x0000, 0x294C, 0x294C, > + /* 1eh - 1fh: device id (some devices) */ > + 0x10BE, 0x10BF, > + /* 20h - 21h: reserved */ > + 0x294C, 0x294C, > + /* 22h - 23h: device id (some devices) */ > + 0x10bd, 0x294C, > + /* 24h-2fh: reserved */ > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, > + /* 30h-3eh: mostly PXE/boot stuff & reserved */ > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, > + /* 3fh: checksum to be computed */ > + 0x0000 > }; > > /* PCI interface */ > @@ -1468,6 +1713,10 @@ e1000_mmio_setup(E1000State *d) > for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++) > memory_region_add_coalescing(&d->mmio, excluded_regs[i] + 4, > excluded_regs[i+1] - excluded_regs[i] - > 4); > + > + memory_region_init_io(&d->flash, OBJECT(d), &e1000_flash_ops, d, > + "e1000-flash", FLASH_RSIZE); > + > memory_region_init_io(&d->io, OBJECT(d), &e1000_io_ops, d, "e1000-io", > IOPORT_SIZE); > } > > @@ -1506,6 +1755,7 @@ static NetClientInfo net_e1000_info = { > static int pci_e1000_init(PCIDevice *pci_dev) > { > DeviceState *dev = DEVICE(pci_dev); > + PCIDeviceClass *pdc = PCI_DEVICE_GET_CLASS(pci_dev); > E1000State *d = E1000(pci_dev); > uint8_t *pci_conf; > uint16_t checksum = 0; > @@ -1523,7 +1773,9 @@ static int pci_e1000_init(PCIDevice *pci_dev) > > pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio); > > - pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &d->io); > + pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->flash); > + > + pci_register_bar(pci_dev, 2, PCI_BASE_ADDRESS_SPACE_IO, &d->io); > > memmove(d->eeprom_data, e1000_eeprom_template, > sizeof e1000_eeprom_template); I wonder if this change is migration-compatible? > @@ -1531,11 +1783,38 @@ static int pci_e1000_init(PCIDevice *pci_dev) > macaddr = d->conf.macaddr.a; > for (i = 0; i < 3; i++) > d->eeprom_data[i] = (macaddr[2*i+1]<<8) | macaddr[2*i]; > + /* update eeprom with the proper device_id */ > + d->eeprom_data[11] = pdc->device_id; > + d->eeprom_data[13] = pdc->device_id; > for (i = 0; i < EEPROM_CHECKSUM_REG; i++) > checksum += d->eeprom_data[i]; > checksum = (uint16_t) EEPROM_SUM - checksum; > d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; > - > + d->rxbuf_edesc = 0; > + if (pdc->device_id == E1000_DEV_ID_ICH9_IGP_AMT) { > + /* fixme: other devices */ Is this a FIXME that you intend to fix before it's applied? Otherwise suggest to use "FIXME:" for highlighting in editors. > + for (i = 0 ; i < EEPROM_CHECKSUM_REG; i++) { Extra space > + d->eeprom_data[i] = e1000_ich8_flash_template[i]; > + } > + for (i = 0; i < 3; i++) { > + d->eeprom_data[i] = (macaddr[2*i+1]<<8) | macaddr[2*i]; Spaces around << at least please, not sure about file's surrounding style - Stefan's call. > + } > + d->eeprom_data[11] = pdc->device_id; > + d->eeprom_data[13] = pdc->device_id; > + checksum = 0; > + for (i = 0; i < EEPROM_CHECKSUM_REG; i++) { > + checksum += d->eeprom_data[i]; > + } > + checksum = (uint16_t) EEPROM_SUM - checksum; > + d->eeprom_data[EEPROM_CHECKSUM_REG] = checksum; > + /* init flash registers */ > + memset(d->flash_reg, 0, FLASH_RSIZE*sizeof(uint16_t)); Spaces > + union ich8_hws_flash_status hsfsts; Move to beginning of if block? > + hsfsts.regval = 0; > + hsfsts.hsf_status.fldesvalid = 1; > + d->flash_reg[ICH_FLASH_HSFSTS] = hsfsts.regval; > + d->rxbuf_edesc = 1; > + } > d->nic = qemu_new_nic(&net_e1000_info, &d->conf, > object_get_typename(OBJECT(d)), dev->id, d); > > @@ -1551,7 +1830,8 @@ static int pci_e1000_init(PCIDevice *pci_dev) > > static void qdev_e1000_reset(DeviceState *dev) > { > - E1000State *d = E1000(dev); > + PCIDevice *pci_dev = PCI_DEVICE(dev); > + E1000State *d = E1000(pci_dev); Just E1000(dev), they no longer depend on each other. > e1000_reset(d); > } > > @@ -1564,17 +1844,26 @@ static Property e1000_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > +typedef struct E1000Info E1000Info; > +struct E1000Info { > + const char *name; > + uint16_t vendor_id; > + uint16_t device_id; > + uint8_t revision; > +}; > + > static void e1000_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + E1000Info *info = data; const > > k->init = pci_e1000_init; > k->exit = pci_e1000_uninit; > k->romfile = "efi-e1000.rom"; > - k->vendor_id = PCI_VENDOR_ID_INTEL; > - k->device_id = E1000_DEVID; > - k->revision = 0x03; > + k->vendor_id = info->vendor_id; > + k->device_id = info->device_id; > + k->revision = info->revision; > k->class_id = PCI_CLASS_NETWORK_ETHERNET; > set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); > dc->desc = "Intel Gigabit Ethernet"; > @@ -1583,16 +1872,59 @@ static void e1000_class_init(ObjectClass *klass, void > *data) > dc->props = e1000_properties; > } > > -static const TypeInfo e1000_info = { > +static E1000Info e1000_info_array[] = { static const > + { > + .name = "e1000", > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = E1000_DEVID, > + .revision = 0x03, > + }, > + { > + .name = "82540EM", > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = E1000_DEV_ID_82540EM, > + .revision = 0x03, > + }, > + { > + .name = "82544GC", > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = E1000_DEV_ID_82544GC_COPPER, > + .revision = 0x03, > + }, > + { > + .name = "82545EM", > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = E1000_DEV_ID_82545EM_COPPER, > + .revision = 0x03, > + }, > + { > + .name = "82566DM", > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = E1000_DEV_ID_ICH9_IGP_AMT, > + .revision = 0x00, > + } > +}; > + > +static const TypeInfo e1000_info_abstract = { > .name = TYPE_E1000, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(E1000State), > - .class_init = e1000_class_init, > + .abstract = true, > }; > > static void e1000_register_types(void) > { > - type_register_static(&e1000_info); > + int i; > + type_register_static(&e1000_info_abstract); > + for (i = 0; i < ARRAY_SIZE(e1000_info_array); i++) { > + TypeInfo e1000_info = { > + .name = e1000_info_array[i].name, > + .parent = TYPE_E1000, > + .class_data = e1000_info_array + i, (void *)&e1000_info_array[i] would match above usage. > + .class_init = e1000_class_init, > + }; > + type_register(&e1000_info); > + } > } > > type_init(e1000_register_types) [snip] Otherwise looks good now! Thanks, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
