[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3] e1000: add the ability to select among sever
|
From: |
Andreas Färber |
|
Subject: |
Re: [Qemu-devel] [PATCH v3] e1000: add the ability to select among several specific types of e1000[e]; 82566DM emulation ; some pointers to documentations and details. |
|
Date: |
Wed, 05 Mar 2014 17:55:08 +0100 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
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 <address@hidden>
> ---
> 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