qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] e1000: add the ability to select among several


From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH] e1000: add the ability to select among several specific types of e1000, plus some pointers to documentations and details.
Date: Tue, 25 Feb 2014 09:01:04 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0

Hi,

Am 24.02.2014 20:08, schrieb Romain Dolbeau:
> Hello,
> 
> This patch adds this ability to select a specific variant of
> the e1000 virtual ethernet from the command-line. Previously
> only a single device was available, selected at compile time.
> This helps for guests with limited hardware support, as with
> this patch not all guests are required to use the same device.
> 
> So for instance to use the emulated 82545EM in OSX[1], a user
> would add e.g. the options:
> 
> "-netdev user,id=mynet0 -device 82545EM,netdev=mynet0"
> 
> Note that the value 'e1000' is retained to mean "whatever
> is the compiled-in default", as previously.
> 
> There's also some comments to help future developers to
> identify relevant informations in the documentations.
> There's also a couple of fixed values for constants
> (didn't match the current driver).
> 
> Known cosmetic issues:
> 1) Some lines are long than 80 characters to avoid breaking
> down URLs and titles.
> 2) One of the array initializer generates the error:
> ####
> ERROR: space prohibited before open square bracket '['
> #107: FILE: hw/net/e1000.c:256:
> +    [PHY_ID1] = 0x141,                          [PHY_ID2] = 0xc20,
> ####
> Despite being formatted exactly like the surrounding lines
> (minus the tabs, another source of error in checkpatch.pl).
> 
> No maintainer CC:, because I couldn't find a maintainer for
> hw/net/e1000 or hw/net/*.

CC'ing Stefan as maintainer.

> 
> Cordially,
> 
> Romain Dolbeau

When you send a "PATCH" please send it in a form we can actually commit,
without letter-style text. That can go into a cover letter 0/1 or below
--- line.

> 
> [1] See <http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/>
>     for details
> 
> Signed-off-by: Romain Dolbeau <address@hidden>
> ---
>  hw/net/e1000.c      |  195 
> +++++++++++++++++++++++++++++++++++++++++++--------
>  hw/net/e1000_regs.h |   65 +++++++++++------
>  hw/pci/pci.c        |    6 ++
>  3 files changed, 215 insertions(+), 51 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 8387443..9073c1a 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
> @@ -69,10 +73,11 @@ 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]
> + *  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 +86,22 @@ 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 },
> +  { -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 },
> +  { -1, 0xc20 } /* default for 82540EM and many others ; this is one
> +                   is a lot more device-specific */
>  };
>  
>  typedef struct E1000State_st {
> @@ -153,8 +170,8 @@ typedef struct E1000State_st {
>  
>  #define TYPE_E1000 "e1000"
>  
> -#define E1000(obj) \
> -    OBJECT_CHECK(E1000State, (obj), TYPE_E1000)
> +#define E1000(obj)                              \
> +  DO_UPCAST(E1000State, parent_obj, obj)

No, don't go backwards in time please. Take a look at e.g. eepro100,
which already registers multiple data-driven subtypes for instance. The
key to making it work is to introduce an abstract base type matching
E1000() macro, with original "e1000" type becoming a subtype thereof.

>  
>  #define      defreg(x)       x = (E1000_##x>>2)
>  enum {
> @@ -235,7 +252,8 @@ static const char phy_regcap[0x20] = {
>  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 +287,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,6 +394,8 @@ 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);
>      uint8_t *macaddr = d->conf.macaddr.a;
>      int i;
>  
> @@ -385,6 +406,26 @@ 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);
> +    uint16_t phy_id1 = -1;
> +    uint16_t phy_id2 = -1;

Please declare variables at the top of the block alongside others.

> +    for (i = 0 ; PHY_ID1_INIT[i][0] != (uint16_t)-1 ; i++) {
> +        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++) {
> +        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;
> @@ -1440,15 +1481,62 @@ 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 seem a bit device-specific

"this seems"

> +            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,
> +    /* three next (word 10h, 11h, 12h) seem quite device-specific
> +            with several variants */
> +    0x0008, 0x2000, 0x7e14,
> +    /* management */
> +    0x0048,
> +    /* init control 3 (2nd LAN?), not 82573 */
> +    0x1000,
> +    /* IPv4 (PCI/X) or  reserved (PCIe), not 82573 */
> +    0x00d8, 0x0000,
> +    /* another batch of variants ; 17h - 1Eh is IPv6 LAN in PCI/X
> +     but is FW Config Start Address (17h, most PCIe) followed
> +    by PCI init configuration and stuff */

This comment looks off. Please use *-aligned multi-line comments.

While at it, please avoid the French punctuation here and elsewhere. ;)

> +    0x2700, 0x6cc9, 0x3150, 0x0722, 0x040b, 0x0984, 0x0000, 0xc000,
> +    /* 1fh : mostly reserved (PCI/X), LED conf (PCIe) */
> +    0x0706,
> +    /* 20h: software defined pin controls, 21h: mostly control */
> +    0x1008, 0x0000,
> +    /* LAN power, management control (not 82573) */
> +    0x0f04, 0x7fff,
> +    /* 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,
> +    /* checksum to be computed */
> +    0x0000
>  };
>  
>  /* PCI interface */
> @@ -1505,6 +1593,7 @@ static NetClientInfo net_e1000_info = {
>  
>  static int pci_e1000_init(PCIDevice *pci_dev)
>  {
> +    PCIDeviceClass *pdc = PCI_DEVICE_GET_CLASS(pci_dev);
>      DeviceState *dev = DEVICE(pci_dev);
>      E1000State *d = E1000(pci_dev);
>      uint8_t *pci_conf;

I'd prefer to see this either below or directly above E1000State since
the signature of the function is going to change to DeviceState *dev as
part of the QOM realize conversion.

> @@ -1531,6 +1620,9 @@ 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;
> @@ -1551,7 +1643,8 @@ static int pci_e1000_init(PCIDevice *pci_dev)
>  
>  static void qdev_e1000_reset(DeviceState *dev)
>  {
> -    E1000State *d = E1000(dev);
> +    PCIDevice *pci_dev = DO_UPCAST(PCIDevice, qdev, dev);

PCI_DEVICE(dev) please! DO_UPCAST() is deprecated for QOM types.

> +    E1000State *d = E1000(pci_dev);
>      e1000_reset(d);
>  }
>  
> @@ -1564,17 +1657,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;
>  
>      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 +1685,47 @@ static void e1000_class_init(ObjectClass *klass, void 
> *data)
>      dc->props = e1000_properties;
>  }
>  
> -static const TypeInfo e1000_info = {
> -    .name          = TYPE_E1000,
> -    .parent        = TYPE_PCI_DEVICE,
> -    .instance_size = sizeof(E1000State),
> -    .class_init    = e1000_class_init,
> +static E1000Info e1000_info_array[] = {

static const possibly?

> +    {
> +        .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,
> +    }

Each .name looks off by one?

>  };
>  
>  static void e1000_register_types(void)
>  {
> -    type_register_static(&e1000_info);
> +    TypeInfo e1000_info = {
> +      .name          = TYPE_E1000,

Since you're overriding this below, better drop it here to avoid confusion.

> +      .parent        = TYPE_PCI_DEVICE,
> +      .instance_size = sizeof(E1000State),

This you will only need for the base type, not for each derived type.

> +      .class_init    = e1000_class_init,
> +    };
> +    int i;
> +    for (i = 0; i < ARRAY_SIZE(e1000_info_array); i++) {
> +        e1000_info.name = e1000_info_array[i].name;
> +        e1000_info.class_data = e1000_info_array + i;
> +        type_register(&e1000_info);
> +    }
>  }
>  
>  type_init(e1000_register_types)
> diff --git a/hw/net/e1000_regs.h b/hw/net/e1000_regs.h
> index c9cb79e..93160c0 100644
> --- a/hw/net/e1000_regs.h
> +++ b/hw/net/e1000_regs.h
> @@ -34,46 +34,71 @@
>  
>  
>  /* PCI Device IDs */
> +/* Where is the documentation for those 3 ?
> +   (they are nonetheless in e1000.ko) */
>  #define E1000_DEV_ID_82542               0x1000
>  #define E1000_DEV_ID_82543GC_FIBER       0x1001
>  #define E1000_DEV_ID_82543GC_COPPER      0x1004
> -#define E1000_DEV_ID_82544EI_COPPER      0x1008
> -#define E1000_DEV_ID_82544EI_FIBER       0x1009
> -#define E1000_DEV_ID_82544GC_COPPER      0x100C
> -#define E1000_DEV_ID_82544GC_LOM         0x100D
> +/* 
> <http://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf>
> + * "PCI/PCI-X Family of Gigabit Ethernet Controllers Software Developer's 
> Manual"
> + * documents
> + * 82540EP/EM, 82541xx, 82544GC/EI, 82545GM/EM, 82546GB/EB, and 82547xx
> + * Those are handled by driver 'e1000'
> + */
>  #define E1000_DEV_ID_82540EM             0x100E
>  #define E1000_DEV_ID_82540EM_LOM         0x1015
>  #define E1000_DEV_ID_82540EP_LOM         0x1016
>  #define E1000_DEV_ID_82540EP             0x1017
>  #define E1000_DEV_ID_82540EP_LP          0x101E
> +
> +#define E1000_DEV_ID_82541EI             0x1013
> +#define E1000_DEV_ID_82541EI_MOBILE      0x1018
> +#define E1000_DEV_ID_82541ER_LOM         0x1014
> +#define E1000_DEV_ID_82541ER             0x1078
> +#define E1000_DEV_ID_82541GI             0x1076
> +#define E1000_DEV_ID_82541GI_MOBILE      0x1077
> +#define E1000_DEV_ID_82541GI_LF          0x107C
> +
> +#define E1000_DEV_ID_82544EI_COPPER      0x1008
> +#define E1000_DEV_ID_82544EI_FIBER       0x1009
> +#define E1000_DEV_ID_82544GC_COPPER      0x100C
> +#define E1000_DEV_ID_82544GC_LOM         0x100D
> +
>  #define E1000_DEV_ID_82545EM_COPPER      0x100F
>  #define E1000_DEV_ID_82545EM_FIBER       0x1011
>  #define E1000_DEV_ID_82545GM_COPPER      0x1026
>  #define E1000_DEV_ID_82545GM_FIBER       0x1027
>  #define E1000_DEV_ID_82545GM_SERDES      0x1028
> +
>  #define E1000_DEV_ID_82546EB_COPPER      0x1010
>  #define E1000_DEV_ID_82546EB_FIBER       0x1012
>  #define E1000_DEV_ID_82546EB_QUAD_COPPER 0x101D
> -#define E1000_DEV_ID_82541EI             0x1013
> -#define E1000_DEV_ID_82541EI_MOBILE      0x1018
> -#define E1000_DEV_ID_82541ER_LOM         0x1014
> -#define E1000_DEV_ID_82541ER             0x1078
> -#define E1000_DEV_ID_82547GI             0x1075
> -#define E1000_DEV_ID_82541GI             0x1076
> -#define E1000_DEV_ID_82541GI_MOBILE      0x1077
> -#define E1000_DEV_ID_82541GI_LF          0x107C
>  #define E1000_DEV_ID_82546GB_COPPER      0x1079
>  #define E1000_DEV_ID_82546GB_FIBER       0x107A
>  #define E1000_DEV_ID_82546GB_SERDES      0x107B
>  #define E1000_DEV_ID_82546GB_PCIE        0x108A
>  #define E1000_DEV_ID_82546GB_QUAD_COPPER 0x1099
> +#define E1000_DEV_ID_82546GB_QUAD_COPPER_KSP3 0x10B5
> +
> +#define E1000_DEV_ID_82547GI             0x1075
>  #define E1000_DEV_ID_82547EI             0x1019
>  #define E1000_DEV_ID_82547EI_MOBILE      0x101A
> +/* 
> <http://www.intel.com/content/dam/www/public/us/en/documents/manuals/pcie-gbe-controllers-open-source-manual.pdf>
> + * "PCIe* GbE Controllers Open Source Software Developer's Manual"
> + * documents:
> + * 631xESB/632xESB, 82563EB/82564EB, 82571EB/82572EI & 82573E/82573V/82573L
> + * Those are actually handled by driver 'e1000e', not 'e1000'
> + */
> +/* it seems the next four are alternative names for 631xESB/632xESB */
> +#define E1000_DEV_ID_80003ES2LAN_COPPER_DPT     0x1096
> +#define E1000_DEV_ID_80003ES2LAN_SERDES_DPT     0x1098
> +#define E1000_DEV_ID_80003ES2LAN_COPPER_SPT     0x10BA
> +#define E1000_DEV_ID_80003ES2LAN_SERDES_SPT     0x10BB
> +
>  #define E1000_DEV_ID_82571EB_COPPER      0x105E
>  #define E1000_DEV_ID_82571EB_FIBER       0x105F
>  #define E1000_DEV_ID_82571EB_SERDES      0x1060
>  #define E1000_DEV_ID_82571EB_QUAD_COPPER 0x10A4
> -#define E1000_DEV_ID_82571PT_QUAD_COPPER 0x10D5
>  #define E1000_DEV_ID_82571EB_QUAD_FIBER  0x10A5
>  #define E1000_DEV_ID_82571EB_QUAD_COPPER_LOWPROFILE  0x10BC
>  #define E1000_DEV_ID_82571EB_SERDES_DUAL 0x10D9
> @@ -82,15 +107,15 @@
>  #define E1000_DEV_ID_82572EI_FIBER       0x107E
>  #define E1000_DEV_ID_82572EI_SERDES      0x107F
>  #define E1000_DEV_ID_82572EI             0x10B9
> +/* is the next one from the same document ? */
> +#define E1000_DEV_ID_82571PT_QUAD_COPPER 0x10D5
> +
>  #define E1000_DEV_ID_82573E              0x108B
>  #define E1000_DEV_ID_82573E_IAMT         0x108C
>  #define E1000_DEV_ID_82573L              0x109A
> -#define E1000_DEV_ID_82546GB_QUAD_COPPER_KSP3 0x10B5
> -#define E1000_DEV_ID_80003ES2LAN_COPPER_DPT     0x1096
> -#define E1000_DEV_ID_80003ES2LAN_SERDES_DPT     0x1098
> -#define E1000_DEV_ID_80003ES2LAN_COPPER_SPT     0x10BA
> -#define E1000_DEV_ID_80003ES2LAN_SERDES_SPT     0x10BB
>  
> +/* Where is the documentation for those 7 ? */
> +/* also, plenty more ID in e1000e for integrated devices in ICH8/9... */
>  #define E1000_DEV_ID_ICH8_IGP_M_AMT      0x1049
>  #define E1000_DEV_ID_ICH8_IGP_AMT        0x104A
>  #define E1000_DEV_ID_ICH8_IGP_C          0x104B
> @@ -542,9 +567,9 @@
>  #define E1000_EEPROM_SWDPIN0   0x0001   /* SWDPIN 0 EEPROM Value */
>  #define E1000_EEPROM_LED_LOGIC 0x0020   /* Led Logic Word */
>  #define E1000_EEPROM_RW_REG_DATA   16   /* Offset to data in EEPROM 
> read/write registers */
> -#define E1000_EEPROM_RW_REG_DONE   0x10 /* Offset to READ/WRITE done bit */
> +#define E1000_EEPROM_RW_REG_DONE   0x2  /* Offset to READ/WRITE done bit */
>  #define E1000_EEPROM_RW_REG_START  1    /* First bit for telling part to 
> start operation */
> -#define E1000_EEPROM_RW_ADDR_SHIFT 8    /* Shift to the address bits */
> +#define E1000_EEPROM_RW_ADDR_SHIFT 2    /* Shift to the address bits */
>  #define E1000_EEPROM_POLL_WRITE    1    /* Flag for polling for write 
> complete */
>  #define E1000_EEPROM_POLL_READ     0    /* Flag for polling for read 
> complete */
>  /* Register Bit Masks */
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 4e0701d..07c8cdd 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1572,6 +1572,9 @@ static const char * const pci_nic_models[] = {
>      "i82559er",
>      "rtl8139",
>      "e1000",
> +    "82540EM",
> +    "82544GC",
> +    "82545EM",
>      "pcnet",
>      "virtio",
>      NULL
> @@ -1584,6 +1587,9 @@ static const char * const pci_nic_names[] = {
>      "i82559er",
>      "rtl8139",
>      "e1000",
> +    "82540EM",
> +    "82544GC",
> +    "82545EM",
>      "pcnet",
>      "virtio-net-pci",
>      NULL

I would hope that adding to these two legacy lists is not necessary for
new types. They should be created using -device, not -net nic,model=.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]