* [Qemu-devel] [PATCH 0/2] e1000e: Reimplement e1000 as a variant of e1000e
@ 2017-10-27 2:49 Ed Swierk
2017-10-27 2:49 ` [Qemu-devel] [PATCH 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Ed Swierk @ 2017-10-27 2:49 UTC (permalink / raw)
To: Ed Swierk, qemu-devel
Cc: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell,
Stefan Hajnoczi
>From a hardware functionality and register perspective, the Intel
8254x Gigabit Ethernet Controller[1] is roughly a subset of the Intel
82574[2], making it practical to implement e1000 device emulation as
an extension of e1000e.
That would be a step towards eliminating the existing e1000
implementation--a bunch of semi-redundant code with its own bugs (like
the faulty tx checksum offload I reported recently[3]).
[1] https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf
[2] https://www.intel.com/content/dam/doc/datasheet/82574l-gbe-controller-datasheet.pdf
[3] https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03008.html
This patch series adds a new e1000-ng device but leaves the existing
e1000 device alone. Linux 4.1 and Windows 10 e1000 guest drivers
recognize the e1000-ng device and successfully pass traffic on a
Linux host.
Preliminary performance measurements are encouraging: with e1000-ng,
single-threaded TCP iperf shows about a 2x speedup in tx and rx
throughput vs e1000, and CPU usage is slighly lower.
A ton of work is needed before e1000-ng will be ready to supplant
e1000: testing with every random guest OS, fixing bugs, figuring out
migration and other missing functionality. There's no way I can do
this myself.
I'd like to propose a more modest and achievable short-term goal: find
and fix any regressions that might affect users of e1000e and e1000,
so that the patches can be applied and folks can easily start trying
out e1000-ng with their favorite guest OS. That means accepting (for
now) known deficiencies in the new e1000-ng devices, which can be
addressed by myself and (hopefully) others in follow-up patches. Does
that sound reasonable?
Ed Swierk (2):
e1000e: Infrastructure for e1000 devices
e1000e: Add e1000 devices
hw/net/e1000e.c | 236 +++++++++++++++++++++++++++++++++++++-----------
hw/net/e1000e_core.c | 249 +++++++++++++++++++++++++++++++++++++--------------
hw/net/e1000e_core.h | 25 +++++-
3 files changed, 387 insertions(+), 123 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH 1/2] e1000e: Infrastructure for e1000-ng 2017-10-27 2:49 [Qemu-devel] [PATCH 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk @ 2017-10-27 2:49 ` Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 2/2] e1000e: Add e1000-ng devices Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2 siblings, 0 replies; 8+ messages in thread From: Ed Swierk @ 2017-10-27 2:49 UTC (permalink / raw) To: Ed Swierk, qemu-devel Cc: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi Generalize e1000e to support e1000 and other devices with a similar register spec: - plain PCI instead of PCIe, skipping setup of MSI-X, AER, etc. - model-specific PHY ID2, register values and access permissions - model-specific EEPROM template and read/write methods This is just infrastructure, no functional changes. Signed-off-by: Ed Swierk <eswierk@skyportsystems.com> --- hw/net/e1000e.c | 186 ++++++++++++++++++++++++++++++++++++--------------- hw/net/e1000e_core.c | 131 ++++++++++++++++++++---------------- hw/net/e1000e_core.h | 13 ++-- 3 files changed, 217 insertions(+), 113 deletions(-) diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index f1af279..100979c 100644 --- a/hw/net/e1000e.c +++ b/hw/net/e1000e.c @@ -49,8 +49,13 @@ #include "trace.h" #include "qapi/error.h" -#define TYPE_E1000E "e1000e" -#define E1000E(obj) OBJECT_CHECK(E1000EState, (obj), TYPE_E1000E) +#define TYPE_E1000E_BASE "e1000e-base" +#define E1000E(obj) \ + OBJECT_CHECK(E1000EState, (obj), TYPE_E1000E_BASE) +#define E1000E_DEVICE_CLASS(klass) \ + OBJECT_CLASS_CHECK(E1000EBaseClass, (klass), TYPE_E1000E_BASE) +#define E1000E_DEVICE_GET_CLASS(obj) \ + OBJECT_GET_CLASS(E1000EBaseClass, (obj), TYPE_E1000E_BASE) typedef struct E1000EState { PCIDevice parent_obj; @@ -76,6 +81,28 @@ typedef struct E1000EState { } E1000EState; +typedef struct E1000EInfo { + const char *name; + const char *desc; + + uint16_t device_id; + uint8_t revision; + uint16_t subsystem_vendor_id; + uint16_t subsystem_id; + int is_express; + const char *romfile; + + const uint16_t *eeprom_templ; + uint32_t eeprom_size; + + uint16_t phy_id2; +} E1000EInfo; + +typedef struct E1000EBaseClass { + PCIDeviceClass parent_class; + const E1000EInfo *info; +} E1000EBaseClass; + #define E1000E_MMIO_IDX 0 #define E1000E_FLASH_IDX 1 #define E1000E_IO_IDX 2 @@ -416,7 +443,9 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) static const uint16_t e1000e_pcie_offset = 0x0E0; static const uint16_t e1000e_aer_offset = 0x100; static const uint16_t e1000e_dsn_offset = 0x140; + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); E1000EState *s = E1000E(pci_dev); + E1000EBaseClass *edc = E1000E_DEVICE_GET_CLASS(s); uint8_t *macaddr; int ret; @@ -427,11 +456,16 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) pci_dev->config[PCI_CACHE_LINE_SIZE] = 0x10; pci_dev->config[PCI_INTERRUPT_PIN] = 1; - pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID, s->subsys_ven); - pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID, s->subsys); + if (s->subsys_ven != (uint16_t)-1) { + pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID, s->subsys_ven); + } + if (s->subsys != (uint16_t)-1) { + pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID, s->subsys); + } - s->subsys_ven_used = s->subsys_ven; - s->subsys_used = s->subsys; + s->subsys_ven_used = pci_get_word(pci_dev->config + + PCI_SUBSYSTEM_VENDOR_ID); + s->subsys_used = pci_get_word(pci_dev->config + PCI_SUBSYSTEM_ID); /* Define IO/MMIO regions */ memory_region_init_io(&s->mmio, OBJECT(s), &mmio_ops, s, @@ -453,65 +487,75 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) pci_register_bar(pci_dev, E1000E_IO_IDX, PCI_BASE_ADDRESS_SPACE_IO, &s->io); - memory_region_init(&s->msix, OBJECT(s), "e1000e-msix", - E1000E_MSIX_SIZE); - pci_register_bar(pci_dev, E1000E_MSIX_IDX, - PCI_BASE_ADDRESS_SPACE_MEMORY, &s->msix); + if (pc->is_express) { + memory_region_init(&s->msix, OBJECT(s), "e1000e-msix", + E1000E_MSIX_SIZE); + pci_register_bar(pci_dev, E1000E_MSIX_IDX, + PCI_BASE_ADDRESS_SPACE_MEMORY, &s->msix); + } /* Create networking backend */ qemu_macaddr_default_if_unset(&s->conf.macaddr); macaddr = s->conf.macaddr.a; - e1000e_init_msix(s); + if (pc->is_express) { + e1000e_init_msix(s); - if (pcie_endpoint_cap_v1_init(pci_dev, e1000e_pcie_offset) < 0) { - hw_error("Failed to initialize PCIe capability"); + if (pcie_endpoint_cap_v1_init(pci_dev, e1000e_pcie_offset) < 0) { + hw_error("Failed to initialize PCIe capability"); + } + + ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL); + if (ret) { + trace_e1000e_msi_init_fail(ret); + } + + if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset, + PCI_PM_CAP_DSI) < 0) { + hw_error("Failed to initialize PM capability"); + } + + if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset, + PCI_ERR_SIZEOF, NULL) < 0) { + hw_error("Failed to initialize AER capability"); + } + + pcie_dev_ser_num_init(pci_dev, e1000e_dsn_offset, + e1000e_gen_dsn(macaddr)); } - ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL); - if (ret) { - trace_e1000e_msi_init_fail(ret); - } - - if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset, - PCI_PM_CAP_DSI) < 0) { - hw_error("Failed to initialize PM capability"); - } - - if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset, - PCI_ERR_SIZEOF, NULL) < 0) { - hw_error("Failed to initialize AER capability"); - } - - pcie_dev_ser_num_init(pci_dev, e1000e_dsn_offset, - e1000e_gen_dsn(macaddr)); - e1000e_init_net_peer(s, pci_dev, macaddr); /* Initialize core */ e1000e_core_realize(s); e1000e_core_pci_realize(&s->core, - e1000e_eeprom_template, - sizeof(e1000e_eeprom_template), - macaddr); + edc->info->eeprom_templ, + edc->info->eeprom_size, + macaddr, + edc->info->phy_id2); } static void e1000e_pci_uninit(PCIDevice *pci_dev) { + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); E1000EState *s = E1000E(pci_dev); trace_e1000e_cb_pci_uninit(); e1000e_core_pci_uninit(&s->core); - pcie_aer_exit(pci_dev); - pcie_cap_exit(pci_dev); + if (pc->is_express) { + pcie_aer_exit(pci_dev); + pcie_cap_exit(pci_dev); + } qemu_del_nic(s->nic); - e1000e_cleanup_msix(s); - msi_uninit(pci_dev); + if (pc->is_express) { + e1000e_cleanup_msix(s); + msi_uninit(pci_dev); + } } static void e1000e_qdev_reset(DeviceState *dev) @@ -655,10 +699,9 @@ static Property e1000e_properties[] = { DEFINE_NIC_PROPERTIES(E1000EState, conf), DEFINE_PROP_SIGNED("disable_vnet_hdr", E1000EState, disable_vnet, false, e1000e_prop_disable_vnet, bool), - DEFINE_PROP_SIGNED("subsys_ven", E1000EState, subsys_ven, - PCI_VENDOR_ID_INTEL, + DEFINE_PROP_SIGNED("subsys_ven", E1000EState, subsys_ven, -1, e1000e_prop_subsys_ven, uint16_t), - DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, 0, + DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, -1, e1000e_prop_subsys, uint16_t), DEFINE_PROP_END_OF_LIST(), }; @@ -667,21 +710,27 @@ static void e1000e_class_init(ObjectClass *class, void *data) { DeviceClass *dc = DEVICE_CLASS(class); PCIDeviceClass *c = PCI_DEVICE_CLASS(class); + E1000EBaseClass *edc = E1000E_DEVICE_CLASS(class); + const E1000EInfo *info = data; c->realize = e1000e_pci_realize; c->exit = e1000e_pci_uninit; c->vendor_id = PCI_VENDOR_ID_INTEL; - c->device_id = E1000_DEV_ID_82574L; - c->revision = 0; - c->romfile = "efi-e1000e.rom"; + c->device_id = info->device_id; + c->revision = info->revision; c->class_id = PCI_CLASS_NETWORK_ETHERNET; - c->is_express = 1; + c->subsystem_vendor_id = info->subsystem_vendor_id; + c->subsystem_id = info->subsystem_id; + c->is_express = info->is_express; + c->romfile = info->romfile; - dc->desc = "Intel 82574L GbE Controller"; + dc->desc = info->desc; dc->reset = e1000e_qdev_reset; dc->vmsd = &e1000e_vmstate; dc->props = e1000e_properties; + edc->info = info; + e1000e_prop_disable_vnet = qdev_prop_uint8; e1000e_prop_disable_vnet.description = "Do not use virtio headers, " "perform SW offloads emulation " @@ -704,21 +753,52 @@ static void e1000e_instance_init(Object *obj) DEVICE(obj), NULL); } -static const TypeInfo e1000e_info = { - .name = TYPE_E1000E, +static const TypeInfo e1000e_base_info = { + .name = TYPE_E1000E_BASE, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(E1000EState), - .class_init = e1000e_class_init, .instance_init = e1000e_instance_init, - .interfaces = (InterfaceInfo[]) { - { INTERFACE_PCIE_DEVICE }, - { } + .class_size = sizeof(E1000EBaseClass), + .abstract = true, +}; + +static const E1000EInfo e1000e_devices[] = { + { + .name = "e1000e", + .desc = "Intel 82574L GbE Controller", + .device_id = E1000_DEV_ID_82574L, + .revision = 0, + .subsystem_vendor_id = PCI_VENDOR_ID_INTEL, + .subsystem_id = 0, + .is_express = 1, + .romfile = "efi-e1000e.rom", + .eeprom_templ = e1000e_eeprom_template, + .eeprom_size = sizeof(e1000e_eeprom_template), + .phy_id2 = E1000_PHY_ID2_82574x, }, }; static void e1000e_register_types(void) { - type_register_static(&e1000e_info); + int i; + + type_register_static(&e1000e_base_info); + for (i = 0; i < ARRAY_SIZE(e1000e_devices); i++) { + const E1000EInfo *info = &e1000e_devices[i]; + TypeInfo type_info = {}; + + type_info.name = info->name; + type_info.parent = TYPE_E1000E_BASE; + type_info.class_data = (void *)info; + type_info.class_init = e1000e_class_init; + type_info.interfaces = (InterfaceInfo[]) { + { info->is_express ? INTERFACE_PCIE_DEVICE + : INTERFACE_CONVENTIONAL_PCI_DEVICE }, + { } + }; + + type_register(&type_info); + } } type_init(e1000e_register_types) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 43a8d89..959c697 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -2213,7 +2213,7 @@ e1000e_get_reg_index_with_offset(const uint16_t *mac_reg_access, hwaddr addr) return index + (mac_reg_access[index] & 0xfffe); } -static const char e1000e_phy_regcap[E1000E_PHY_PAGES][0x20] = { +static const char e1000e_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { [0] = { [PHY_CTRL] = PHY_ANYPAGE | PHY_RW, [PHY_STATUS] = PHY_ANYPAGE | PHY_R, @@ -2266,14 +2266,14 @@ e1000e_phy_reg_check_cap(E1000ECore *core, uint32_t addr, char cap, uint8_t *page) { *page = - (e1000e_phy_regcap[0][addr] & PHY_ANYPAGE) ? 0 - : core->phy[0][PHY_PAGE]; + ((*core->phy_regcap)[0][addr] & PHY_ANYPAGE) ? 0 + : core->phy[0][PHY_PAGE]; if (*page >= E1000E_PHY_PAGES) { return false; } - return e1000e_phy_regcap[*page][addr] & cap; + return (*core->phy_regcap)[*page][addr] & cap; } static void @@ -2729,6 +2729,12 @@ e1000e_mac_setmacaddr(E1000ECore *core, int index, uint32_t val) trace_e1000e_mac_set_sw(MAC_ARG(macaddr)); } +static uint32_t +e1000e_get_eecd(E1000ECore *core, int index) +{ + return e1000e_mac_readreg(core, index); +} + static void e1000e_set_eecd(E1000ECore *core, int index, uint32_t val) { @@ -3028,6 +3034,7 @@ static uint32_t (*e1000e_macreg_readops[])(E1000ECore *, int) = { [TARC1] = e1000e_get_tarc, [SWSM] = e1000e_mac_swsm_read, [IMS] = e1000e_mac_ims_read, + [EECD] = e1000e_get_eecd, [CRCERRS ... MPC] = e1000e_mac_readreg, [IP6AT ... IP6AT + 3] = e1000e_mac_readreg, @@ -3305,56 +3312,6 @@ e1000e_vm_state_change(void *opaque, int running, RunState state) } } -void -e1000e_core_pci_realize(E1000ECore *core, - const uint16_t *eeprom_templ, - uint32_t eeprom_size, - const uint8_t *macaddr) -{ - int i; - - core->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, - e1000e_autoneg_timer, core); - e1000e_intrmgr_pci_realize(core); - - core->vmstate = - qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); - - for (i = 0; i < E1000E_NUM_QUEUES; i++) { - net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, - E1000E_MAX_TX_FRAGS, core->has_vnet); - } - - net_rx_pkt_init(&core->rx_pkt, core->has_vnet); - - e1000x_core_prepare_eeprom(core->eeprom, - eeprom_templ, - eeprom_size, - PCI_DEVICE_GET_CLASS(core->owner)->device_id, - macaddr); - e1000e_update_rx_offloads(core); -} - -void -e1000e_core_pci_uninit(E1000ECore *core) -{ - int i; - - timer_del(core->autoneg_timer); - timer_free(core->autoneg_timer); - - e1000e_intrmgr_pci_unint(core); - - qemu_del_vm_change_state_handler(core->vmstate); - - for (i = 0; i < E1000E_NUM_QUEUES; i++) { - net_tx_pkt_reset(core->tx[i].tx_pkt); - net_tx_pkt_uninit(core->tx[i].tx_pkt); - } - - net_rx_pkt_uninit(core->rx_pkt); -} - static const uint16_t e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { [0] = { @@ -3373,7 +3330,7 @@ e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { MII_SR_100X_FD_CAPS, [PHY_ID1] = 0x141, - [PHY_ID2] = E1000_PHY_ID2_82574x, + /* [PHY_ID2] set by e1000e_core_reset() */ [PHY_AUTONEG_ADV] = 0xde1, [PHY_LP_ABILITY] = 0x7e0, [PHY_AUTONEG_EXP] = BIT(2), @@ -3438,6 +3395,67 @@ static const uint32_t e1000e_mac_reg_init[] = { }; void +e1000e_core_pci_realize(E1000ECore *core, + const uint16_t *eeprom_templ, + uint32_t eeprom_size, + const uint8_t *macaddr, + uint16_t phy_id2) +{ + int i; + + core->phy_id2 = phy_id2; + switch (phy_id2) { + case E1000_PHY_ID2_82574x: + core->phy_regcap = &e1000e_phy_regcap; + core->phy_reg_init = &e1000e_phy_reg_init; + break; + default: + g_assert_not_reached(); + } + + core->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, + e1000e_autoneg_timer, core); + e1000e_intrmgr_pci_realize(core); + + core->vmstate = + qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); + + for (i = 0; i < E1000E_NUM_QUEUES; i++) { + net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, + E1000E_MAX_TX_FRAGS, core->has_vnet); + } + + net_rx_pkt_init(&core->rx_pkt, core->has_vnet); + + e1000x_core_prepare_eeprom(core->eeprom, + eeprom_templ, + eeprom_size, + PCI_DEVICE_GET_CLASS(core->owner)->device_id, + macaddr); + e1000e_update_rx_offloads(core); +} + +void +e1000e_core_pci_uninit(E1000ECore *core) +{ + int i; + + timer_del(core->autoneg_timer); + timer_free(core->autoneg_timer); + + e1000e_intrmgr_pci_unint(core); + + qemu_del_vm_change_state_handler(core->vmstate); + + for (i = 0; i < E1000E_NUM_QUEUES; i++) { + net_tx_pkt_reset(core->tx[i].tx_pkt); + net_tx_pkt_uninit(core->tx[i].tx_pkt); + } + + net_rx_pkt_uninit(core->rx_pkt); +} + +void e1000e_core_reset(E1000ECore *core) { int i; @@ -3447,7 +3465,8 @@ e1000e_core_reset(E1000ECore *core) e1000e_intrmgr_reset(core); memset(core->phy, 0, sizeof core->phy); - memmove(core->phy, e1000e_phy_reg_init, sizeof e1000e_phy_reg_init); + memmove(core->phy, *core->phy_reg_init, sizeof *core->phy_reg_init); + core->phy[0][PHY_ID2] = core->phy_id2; memset(core->mac, 0, sizeof core->mac); memmove(core->mac, e1000e_mac_reg_init, sizeof e1000e_mac_reg_init); diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index 1ff6978..9ac6f32 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -56,6 +56,10 @@ typedef struct E1000IntrDelayTimer_st { } E1000IntrDelayTimer; struct E1000Core { + uint16_t phy_id2; + const char (*phy_regcap)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + const uint16_t (*phy_reg_init)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + uint32_t mac[E1000E_MAC_SIZE]; uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; uint16_t eeprom[E1000E_EEPROM_SIZE]; @@ -116,10 +120,11 @@ uint64_t e1000e_core_read(E1000ECore *core, hwaddr addr, unsigned size); void -e1000e_core_pci_realize(E1000ECore *regs, - const uint16_t *eeprom_templ, - uint32_t eeprom_size, - const uint8_t *macaddr); +e1000e_core_pci_realize(E1000ECore *regs, + const uint16_t *eeprom_templ, + uint32_t eeprom_size, + const uint8_t *macaddr, + uint16_t phy_id2); void e1000e_core_reset(E1000ECore *core); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] e1000e: Add e1000-ng devices 2017-10-27 2:49 [Qemu-devel] [PATCH 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk @ 2017-10-27 2:49 ` Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2 siblings, 0 replies; 8+ messages in thread From: Ed Swierk @ 2017-10-27 2:49 UTC (permalink / raw) To: Ed Swierk, qemu-devel Cc: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi Implement e1000-compatible devices using the reworked e1000e code: - e1000-ng: Intel 82540EM - e1000-82544gc-ng: Intel 82544GC - e1000-82545em-ng: Intel 82545EM >From a guest's perspective, these should be drop-in replacements for the existing e1000 devices. Among other deficiencies, this version does not handle migration. But it should work well enough to start testing e1000-ng devices with a variety of guest OSes. Signed-off-by: Ed Swierk <eswierk@skyportsystems.com> --- hw/net/e1000e.c | 50 +++++++++++++++++++++ hw/net/e1000e_core.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++----- hw/net/e1000e_core.h | 12 ++++++ 3 files changed, 171 insertions(+), 11 deletions(-) diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index 100979c..5f75a41 100644 --- a/hw/net/e1000e.c +++ b/hw/net/e1000e.c @@ -281,6 +281,17 @@ static const uint16_t e1000e_eeprom_template[64] = { 0xffff, 0xffff, 0xffff, 0xffff, 0x0000, 0x0120, 0xffff, 0x0000, }; +static const uint16_t e1000_eeprom_template[64] = { + 0x0000, 0x0000, 0x0000, 0x0000, 0xffff, 0x0000, 0x0000, 0x0000, + 0x3000, 0x1000, 0x6403, 0 /*DevId*/, 0x8086, 0 /*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, +}; + static void e1000e_core_realize(E1000EState *s) { s->core.owner = &s->parent_obj; @@ -776,6 +787,45 @@ static const E1000EInfo e1000e_devices[] = { .eeprom_size = sizeof(e1000e_eeprom_template), .phy_id2 = E1000_PHY_ID2_82574x, }, + { + .name = "e1000-ng", + .desc = "Intel 82540EM Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82540EM, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT, + }, + { + .name = "e1000-82544gc-ng", + .desc = "Intel 82544GC Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82544GC_COPPER, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_82544x, + }, + { + .name = "e1000-82545em-ng", + .desc = "Intel 82545EM Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82545EM_COPPER, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT, + }, }; static void e1000e_register_types(void) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 959c697..02a60a1 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -2261,6 +2261,24 @@ static const char e1000e_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { } }; +static const char e1000_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { + [0] = { + [PHY_CTRL] = PHY_RW, + [PHY_STATUS] = PHY_R, + [PHY_ID1] = PHY_R, + [PHY_ID2] = PHY_R, + [PHY_AUTONEG_ADV] = PHY_RW, + [PHY_LP_ABILITY] = PHY_R, + [PHY_AUTONEG_EXP] = PHY_R, + [PHY_1000T_CTRL] = PHY_RW, + [PHY_1000T_STATUS] = PHY_R, + [M88E1000_PHY_SPEC_CTRL] = PHY_RW, + [M88E1000_PHY_SPEC_STATUS] = PHY_R, + [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW, + [M88E1000_RX_ERR_CNTR] = PHY_R, + } +}; + static bool e1000e_phy_reg_check_cap(E1000ECore *core, uint32_t addr, char cap, uint8_t *page) @@ -2616,6 +2634,10 @@ e1000e_mac_icr_read(E1000ECore *core, int index) e1000e_clear_ims_bits(core, core->mac[IAM]); } + if (core->clear_icr_on_read) { + core->mac[ICR] = 0; + } + trace_e1000e_irq_icr_read_exit(core->mac[ICR]); e1000e_update_interrupt_state(core); return ret; @@ -2732,50 +2754,83 @@ e1000e_mac_setmacaddr(E1000ECore *core, int index, uint32_t val) static uint32_t e1000e_get_eecd(E1000ECore *core, int index) { - return e1000e_mac_readreg(core, index); + uint32_t ret = E1000_EECD_PRES | E1000_EECD_GNT | core->eecd_state.old_eecd; + + if (!core->eecd_state.reading || + ((core->eeprom[(core->eecd_state.bitnum_out >> 4) & 0x3f] >> + ((core->eecd_state.bitnum_out & 0xf) ^ 0xf))) & 1) { + ret |= E1000_EECD_DO; + } + return ret; } static void e1000e_set_eecd(E1000ECore *core, int index, uint32_t val) { - static const uint32_t ro_bits = E1000_EECD_PRES | - E1000_EECD_AUTO_RD | - E1000_EECD_SIZE_EX_MASK; + uint32_t oldval = core->eecd_state.old_eecd; - core->mac[EECD] = (core->mac[EECD] & ro_bits) | (val & ~ro_bits); + core->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS | + E1000_EECD_DI | E1000_EECD_FWE_MASK | + E1000_EECD_REQ); + if (!(E1000_EECD_CS & val)) { /* CS inactive; nothing to do */ + return; + } + if (E1000_EECD_CS & (val ^ oldval)) { /* CS rise edge; reset state */ + core->eecd_state.val_in = 0; + core->eecd_state.bitnum_in = 0; + core->eecd_state.bitnum_out = 0; + core->eecd_state.reading = 0; + } + if (!(E1000_EECD_SK & (val ^ oldval))) { /* no clock edge */ + return; + } + if (!(E1000_EECD_SK & val)) { /* falling edge */ + core->eecd_state.bitnum_out++; + return; + } + core->eecd_state.val_in <<= 1; + if (val & E1000_EECD_DI) { + core->eecd_state.val_in |= 1; + } + if (++core->eecd_state.bitnum_in == 9 && !core->eecd_state.reading) { + core->eecd_state.bitnum_out = ((core->eecd_state.val_in & 0x3f) + << 4) - 1; + core->eecd_state.reading = (((core->eecd_state.val_in >> 6) & 7) == + EEPROM_READ_OPCODE_MICROWIRE); + } } static void e1000e_set_eerd(E1000ECore *core, int index, uint32_t val) { - uint32_t addr = (val >> E1000_EERW_ADDR_SHIFT) & E1000_EERW_ADDR_MASK; + uint32_t addr = (val >> core->eerw_addr_shift) & core->eerw_addr_mask; uint32_t flags = 0; uint32_t data = 0; if ((addr < E1000E_EEPROM_SIZE) && (val & E1000_EERW_START)) { data = core->eeprom[addr]; - flags = E1000_EERW_DONE; + flags = core->eerw_done; } core->mac[EERD] = flags | - (addr << E1000_EERW_ADDR_SHIFT) | + (addr << core->eerw_addr_shift) | (data << E1000_EERW_DATA_SHIFT); } static void e1000e_set_eewr(E1000ECore *core, int index, uint32_t val) { - uint32_t addr = (val >> E1000_EERW_ADDR_SHIFT) & E1000_EERW_ADDR_MASK; + uint32_t addr = (val >> core->eerw_addr_shift) & core->eerw_addr_mask; uint32_t data = (val >> E1000_EERW_DATA_SHIFT) & E1000_EERW_DATA_MASK; uint32_t flags = 0; if ((addr < E1000E_EEPROM_SIZE) && (val & E1000_EERW_START)) { core->eeprom[addr] = data; - flags = E1000_EERW_DONE; + flags = core->eerw_done; } core->mac[EERD] = flags | - (addr << E1000_EERW_ADDR_SHIFT) | + (addr << core->eerw_addr_shift) | (data << E1000_EERW_DATA_SHIFT); } @@ -3352,6 +3407,36 @@ e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { } }; +static const uint16_t +e1000_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { + [0] = { + [PHY_CTRL] = MII_CR_SPEED_SELECT_MSB | + MII_CR_FULL_DUPLEX | + MII_CR_AUTO_NEG_EN, + + [PHY_STATUS] = MII_SR_EXTENDED_CAPS | + MII_SR_LINK_STATUS | + MII_SR_AUTONEG_CAPS | + MII_SR_PREAMBLE_SUPPRESS | + MII_SR_EXTENDED_STATUS | + MII_SR_10T_HD_CAPS | + MII_SR_10T_FD_CAPS | + MII_SR_100X_HD_CAPS | + MII_SR_100X_FD_CAPS, + + [PHY_ID1] = 0x141, + /* [PHY_ID2] set by e1000e_core_reset() */ + [PHY_AUTONEG_ADV] = 0xde1, + [PHY_LP_ABILITY] = 0x1e0, + [PHY_1000T_CTRL] = 0x0e00, + [PHY_1000T_STATUS] = 0x3c00, + + [M88E1000_PHY_SPEC_CTRL] = 0x360, + [M88E1000_PHY_SPEC_STATUS] = 0xac00, + [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, + } +}; + static const uint32_t e1000e_mac_reg_init[] = { [PBA] = 0x00140014, [LEDCTL] = BIT(1) | BIT(8) | BIT(9) | BIT(15) | BIT(17) | BIT(18), @@ -3408,6 +3493,19 @@ e1000e_core_pci_realize(E1000ECore *core, case E1000_PHY_ID2_82574x: core->phy_regcap = &e1000e_phy_regcap; core->phy_reg_init = &e1000e_phy_reg_init; + core->clear_icr_on_read = false; + core->eerw_done = BIT(1); + core->eerw_addr_shift = 2; + core->eerw_addr_mask = ((1L << 14) - 1); + break; + case E1000_PHY_ID2_8254xx_DEFAULT: + case E1000_PHY_ID2_82544x: + core->phy_regcap = &e1000_phy_regcap; + core->phy_reg_init = &e1000_phy_reg_init; + core->clear_icr_on_read = true; + core->eerw_done = BIT(4); + core->eerw_addr_shift = 8; + core->eerw_addr_mask = ((1L << 8) - 1); break; default: g_assert_not_reached(); diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index 9ac6f32..f667631 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -59,6 +59,7 @@ struct E1000Core { uint16_t phy_id2; const char (*phy_regcap)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; const uint16_t (*phy_reg_init)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + bool clear_icr_on_read; uint32_t mac[E1000E_MAC_SIZE]; uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; @@ -108,6 +109,17 @@ struct E1000Core { uint8_t permanent_mac[ETH_ALEN]; + int eerw_done; + int eerw_addr_shift; + int eerw_addr_mask; + struct { + uint32_t val_in; /* shifted in from guest driver */ + uint16_t bitnum_in; + uint16_t bitnum_out; + uint16_t reading; + uint32_t old_eecd; + } eecd_state; + NICState *owner_nic; PCIDevice *owner; void (*owner_start_recv)(PCIDevice *d); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e 2017-10-27 2:49 [Qemu-devel] [PATCH 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 2/2] e1000e: Add e1000-ng devices Ed Swierk @ 2017-11-09 4:39 ` Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk ` (2 more replies) 2 siblings, 3 replies; 8+ messages in thread From: Ed Swierk @ 2017-11-09 4:39 UTC (permalink / raw) To: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi Cc: qemu-devel, Ed Swierk >From a hardware functionality and register perspective, the Intel 8254x Gigabit Ethernet Controller[1] is roughly a subset of the Intel 82574[2], making it practical to implement e1000 device emulation as an extension of e1000e. That would be a step towards eliminating the existing e1000 implementation--a bunch of semi-redundant code with its own bugs (like the faulty tx checksum offload I reported recently[3]). [1] https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf [2] https://www.intel.com/content/dam/doc/datasheet/82574l-gbe-controller-datasheet.pdf [3] https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03008.html This patch series adds a new e1000-ng device but leaves the existing e1000 device alone. Linux 4.1 and Windows 10 e1000 guest drivers recognize the e1000-ng device and successfully pass traffic on a Linux host. Preliminary performance measurements are encouraging: with e1000-ng, single-threaded TCP iperf shows about a 2x speedup in tx and rx throughput vs e1000, and CPU usage is slighly lower. A ton of work is needed before e1000-ng will be ready to supplant e1000: testing with every random guest OS, fixing bugs, figuring out migration and other missing functionality. There's no way I can do this myself. I'd like to propose a more modest and achievable short-term goal: find and fix any regressions that might affect users of e1000e and e1000, so that the patches can be applied and folks can easily start trying out e1000-ng with their favorite guest OS. That means accepting (for now) known deficiencies in the new e1000-ng devices, which can be addressed by myself and (hopefully) others in follow-up patches. Does that sound reasonable? v2: - added new eecd_state fields to e1000e_vmstate and e1000_vmstate - got savevm/loadvm working with e1000-ng - leave MSI enabled for e1000-ng (though guest drivers seem to ignore it) Ed Swierk (2): e1000e: Infrastructure for e1000-ng e1000e: Add e1000-ng devices hw/net/e1000e.c | 277 +++++++++++++++++++++++++++++++++++++++++++-------- hw/net/e1000e_core.c | 249 +++++++++++++++++++++++++++++++++------------ hw/net/e1000e_core.h | 25 ++++- 3 files changed, 437 insertions(+), 114 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] e1000e: Infrastructure for e1000-ng 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk @ 2017-11-09 4:39 ` Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 2/2] e1000e: Add e1000-ng devices Ed Swierk 2017-11-09 13:53 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Daniel P. Berrange 2 siblings, 0 replies; 8+ messages in thread From: Ed Swierk @ 2017-11-09 4:39 UTC (permalink / raw) To: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi Cc: qemu-devel, Ed Swierk Generalize e1000e to support e1000 and other devices with a similar register spec: - plain PCI instead of PCIe, skipping setup of MSI-X, AER, etc. - model-specific PHY ID2, register values and access permissions - model-specific EEPROM template and read/write methods This is just infrastructure, no functional changes. Signed-off-by: Ed Swierk <eswierk@skyportsystems.com> --- hw/net/e1000e.c | 166 ++++++++++++++++++++++++++++++++++++++------------- hw/net/e1000e_core.c | 131 +++++++++++++++++++++++----------------- hw/net/e1000e_core.h | 13 ++-- 3 files changed, 209 insertions(+), 101 deletions(-) diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index f1af279..7685ab0 100644 --- a/hw/net/e1000e.c +++ b/hw/net/e1000e.c @@ -49,8 +49,13 @@ #include "trace.h" #include "qapi/error.h" -#define TYPE_E1000E "e1000e" -#define E1000E(obj) OBJECT_CHECK(E1000EState, (obj), TYPE_E1000E) +#define TYPE_E1000E_BASE "e1000e-base" +#define E1000E(obj) \ + OBJECT_CHECK(E1000EState, (obj), TYPE_E1000E_BASE) +#define E1000E_DEVICE_CLASS(klass) \ + OBJECT_CLASS_CHECK(E1000EBaseClass, (klass), TYPE_E1000E_BASE) +#define E1000E_DEVICE_GET_CLASS(obj) \ + OBJECT_GET_CLASS(E1000EBaseClass, (obj), TYPE_E1000E_BASE) typedef struct E1000EState { PCIDevice parent_obj; @@ -76,6 +81,28 @@ typedef struct E1000EState { } E1000EState; +typedef struct E1000EInfo { + const char *name; + const char *desc; + + uint16_t device_id; + uint8_t revision; + uint16_t subsystem_vendor_id; + uint16_t subsystem_id; + int is_express; + const char *romfile; + + const uint16_t *eeprom_templ; + uint32_t eeprom_size; + + uint16_t phy_id2; +} E1000EInfo; + +typedef struct E1000EBaseClass { + PCIDeviceClass parent_class; + const E1000EInfo *info; +} E1000EBaseClass; + #define E1000E_MMIO_IDX 0 #define E1000E_FLASH_IDX 1 #define E1000E_IO_IDX 2 @@ -416,7 +443,9 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) static const uint16_t e1000e_pcie_offset = 0x0E0; static const uint16_t e1000e_aer_offset = 0x100; static const uint16_t e1000e_dsn_offset = 0x140; + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); E1000EState *s = E1000E(pci_dev); + E1000EBaseClass *edc = E1000E_DEVICE_GET_CLASS(s); uint8_t *macaddr; int ret; @@ -427,6 +456,13 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) pci_dev->config[PCI_CACHE_LINE_SIZE] = 0x10; pci_dev->config[PCI_INTERRUPT_PIN] = 1; + if (s->subsys_ven == (uint16_t)-1) { + s->subsys_ven = pci_get_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID); + } + if (s->subsys == (uint16_t)-1) { + s->subsys = pci_get_word(pci_dev->config + PCI_SUBSYSTEM_ID); + } + pci_set_word(pci_dev->config + PCI_SUBSYSTEM_VENDOR_ID, s->subsys_ven); pci_set_word(pci_dev->config + PCI_SUBSYSTEM_ID, s->subsys); @@ -453,19 +489,23 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) pci_register_bar(pci_dev, E1000E_IO_IDX, PCI_BASE_ADDRESS_SPACE_IO, &s->io); - memory_region_init(&s->msix, OBJECT(s), "e1000e-msix", - E1000E_MSIX_SIZE); - pci_register_bar(pci_dev, E1000E_MSIX_IDX, - PCI_BASE_ADDRESS_SPACE_MEMORY, &s->msix); + if (pc->is_express) { + memory_region_init(&s->msix, OBJECT(s), "e1000e-msix", + E1000E_MSIX_SIZE); + pci_register_bar(pci_dev, E1000E_MSIX_IDX, + PCI_BASE_ADDRESS_SPACE_MEMORY, &s->msix); + } /* Create networking backend */ qemu_macaddr_default_if_unset(&s->conf.macaddr); macaddr = s->conf.macaddr.a; - e1000e_init_msix(s); + if (pc->is_express) { + e1000e_init_msix(s); - if (pcie_endpoint_cap_v1_init(pci_dev, e1000e_pcie_offset) < 0) { - hw_error("Failed to initialize PCIe capability"); + if (pcie_endpoint_cap_v1_init(pci_dev, e1000e_pcie_offset) < 0) { + hw_error("Failed to initialize PCIe capability"); + } } ret = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL); @@ -473,18 +513,20 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) trace_e1000e_msi_init_fail(ret); } - if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset, - PCI_PM_CAP_DSI) < 0) { - hw_error("Failed to initialize PM capability"); - } + if (pc->is_express) { + if (e1000e_add_pm_capability(pci_dev, e1000e_pmrb_offset, + PCI_PM_CAP_DSI) < 0) { + hw_error("Failed to initialize PM capability"); + } - if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset, - PCI_ERR_SIZEOF, NULL) < 0) { - hw_error("Failed to initialize AER capability"); - } + if (pcie_aer_init(pci_dev, PCI_ERR_VER, e1000e_aer_offset, + PCI_ERR_SIZEOF, NULL) < 0) { + hw_error("Failed to initialize AER capability"); + } - pcie_dev_ser_num_init(pci_dev, e1000e_dsn_offset, - e1000e_gen_dsn(macaddr)); + pcie_dev_ser_num_init(pci_dev, e1000e_dsn_offset, + e1000e_gen_dsn(macaddr)); + } e1000e_init_net_peer(s, pci_dev, macaddr); @@ -492,25 +534,31 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp) e1000e_core_realize(s); e1000e_core_pci_realize(&s->core, - e1000e_eeprom_template, - sizeof(e1000e_eeprom_template), - macaddr); + edc->info->eeprom_templ, + edc->info->eeprom_size, + macaddr, + edc->info->phy_id2); } static void e1000e_pci_uninit(PCIDevice *pci_dev) { + PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev); E1000EState *s = E1000E(pci_dev); trace_e1000e_cb_pci_uninit(); e1000e_core_pci_uninit(&s->core); - pcie_aer_exit(pci_dev); - pcie_cap_exit(pci_dev); + if (pc->is_express) { + pcie_aer_exit(pci_dev); + pcie_cap_exit(pci_dev); + } qemu_del_nic(s->nic); - e1000e_cleanup_msix(s); + if (pc->is_express) { + e1000e_cleanup_msix(s); + } msi_uninit(pci_dev); } @@ -544,7 +592,7 @@ static int e1000e_post_load(void *opaque, int version_id) (s->subsys_ven != s->subsys_ven_used)) { fprintf(stderr, "ERROR: Cannot migrate while device properties " - "(subsys/subsys_ven) differ"); + "(subsys/subsys_ven) differ\n"); return -1; } @@ -655,10 +703,9 @@ static Property e1000e_properties[] = { DEFINE_NIC_PROPERTIES(E1000EState, conf), DEFINE_PROP_SIGNED("disable_vnet_hdr", E1000EState, disable_vnet, false, e1000e_prop_disable_vnet, bool), - DEFINE_PROP_SIGNED("subsys_ven", E1000EState, subsys_ven, - PCI_VENDOR_ID_INTEL, + DEFINE_PROP_SIGNED("subsys_ven", E1000EState, subsys_ven, -1, e1000e_prop_subsys_ven, uint16_t), - DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, 0, + DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, -1, e1000e_prop_subsys, uint16_t), DEFINE_PROP_END_OF_LIST(), }; @@ -667,21 +714,27 @@ static void e1000e_class_init(ObjectClass *class, void *data) { DeviceClass *dc = DEVICE_CLASS(class); PCIDeviceClass *c = PCI_DEVICE_CLASS(class); + E1000EBaseClass *edc = E1000E_DEVICE_CLASS(class); + const E1000EInfo *info = data; c->realize = e1000e_pci_realize; c->exit = e1000e_pci_uninit; c->vendor_id = PCI_VENDOR_ID_INTEL; - c->device_id = E1000_DEV_ID_82574L; - c->revision = 0; - c->romfile = "efi-e1000e.rom"; + c->device_id = info->device_id; + c->revision = info->revision; c->class_id = PCI_CLASS_NETWORK_ETHERNET; - c->is_express = 1; + c->subsystem_vendor_id = info->subsystem_vendor_id; + c->subsystem_id = info->subsystem_id; + c->is_express = info->is_express; + c->romfile = info->romfile; - dc->desc = "Intel 82574L GbE Controller"; + dc->desc = info->desc; dc->reset = e1000e_qdev_reset; dc->vmsd = &e1000e_vmstate; dc->props = e1000e_properties; + edc->info = info; + e1000e_prop_disable_vnet = qdev_prop_uint8; e1000e_prop_disable_vnet.description = "Do not use virtio headers, " "perform SW offloads emulation " @@ -704,21 +757,52 @@ static void e1000e_instance_init(Object *obj) DEVICE(obj), NULL); } -static const TypeInfo e1000e_info = { - .name = TYPE_E1000E, +static const TypeInfo e1000e_base_info = { + .name = TYPE_E1000E_BASE, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(E1000EState), - .class_init = e1000e_class_init, .instance_init = e1000e_instance_init, - .interfaces = (InterfaceInfo[]) { - { INTERFACE_PCIE_DEVICE }, - { } + .class_size = sizeof(E1000EBaseClass), + .abstract = true, +}; + +static const E1000EInfo e1000e_devices[] = { + { + .name = "e1000e", + .desc = "Intel 82574L GbE Controller", + .device_id = E1000_DEV_ID_82574L, + .revision = 0, + .subsystem_vendor_id = PCI_VENDOR_ID_INTEL, + .subsystem_id = 0, + .is_express = 1, + .romfile = "efi-e1000e.rom", + .eeprom_templ = e1000e_eeprom_template, + .eeprom_size = sizeof(e1000e_eeprom_template), + .phy_id2 = E1000_PHY_ID2_82574x, }, }; static void e1000e_register_types(void) { - type_register_static(&e1000e_info); + int i; + + type_register_static(&e1000e_base_info); + for (i = 0; i < ARRAY_SIZE(e1000e_devices); i++) { + const E1000EInfo *info = &e1000e_devices[i]; + TypeInfo type_info = {}; + + type_info.name = info->name; + type_info.parent = TYPE_E1000E_BASE; + type_info.class_data = (void *)info; + type_info.class_init = e1000e_class_init; + type_info.interfaces = (InterfaceInfo[]) { + { info->is_express ? INTERFACE_PCIE_DEVICE + : INTERFACE_CONVENTIONAL_PCI_DEVICE }, + { } + }; + + type_register(&type_info); + } } type_init(e1000e_register_types) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 43a8d89..959c697 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -2213,7 +2213,7 @@ e1000e_get_reg_index_with_offset(const uint16_t *mac_reg_access, hwaddr addr) return index + (mac_reg_access[index] & 0xfffe); } -static const char e1000e_phy_regcap[E1000E_PHY_PAGES][0x20] = { +static const char e1000e_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { [0] = { [PHY_CTRL] = PHY_ANYPAGE | PHY_RW, [PHY_STATUS] = PHY_ANYPAGE | PHY_R, @@ -2266,14 +2266,14 @@ e1000e_phy_reg_check_cap(E1000ECore *core, uint32_t addr, char cap, uint8_t *page) { *page = - (e1000e_phy_regcap[0][addr] & PHY_ANYPAGE) ? 0 - : core->phy[0][PHY_PAGE]; + ((*core->phy_regcap)[0][addr] & PHY_ANYPAGE) ? 0 + : core->phy[0][PHY_PAGE]; if (*page >= E1000E_PHY_PAGES) { return false; } - return e1000e_phy_regcap[*page][addr] & cap; + return (*core->phy_regcap)[*page][addr] & cap; } static void @@ -2729,6 +2729,12 @@ e1000e_mac_setmacaddr(E1000ECore *core, int index, uint32_t val) trace_e1000e_mac_set_sw(MAC_ARG(macaddr)); } +static uint32_t +e1000e_get_eecd(E1000ECore *core, int index) +{ + return e1000e_mac_readreg(core, index); +} + static void e1000e_set_eecd(E1000ECore *core, int index, uint32_t val) { @@ -3028,6 +3034,7 @@ static uint32_t (*e1000e_macreg_readops[])(E1000ECore *, int) = { [TARC1] = e1000e_get_tarc, [SWSM] = e1000e_mac_swsm_read, [IMS] = e1000e_mac_ims_read, + [EECD] = e1000e_get_eecd, [CRCERRS ... MPC] = e1000e_mac_readreg, [IP6AT ... IP6AT + 3] = e1000e_mac_readreg, @@ -3305,56 +3312,6 @@ e1000e_vm_state_change(void *opaque, int running, RunState state) } } -void -e1000e_core_pci_realize(E1000ECore *core, - const uint16_t *eeprom_templ, - uint32_t eeprom_size, - const uint8_t *macaddr) -{ - int i; - - core->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, - e1000e_autoneg_timer, core); - e1000e_intrmgr_pci_realize(core); - - core->vmstate = - qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); - - for (i = 0; i < E1000E_NUM_QUEUES; i++) { - net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, - E1000E_MAX_TX_FRAGS, core->has_vnet); - } - - net_rx_pkt_init(&core->rx_pkt, core->has_vnet); - - e1000x_core_prepare_eeprom(core->eeprom, - eeprom_templ, - eeprom_size, - PCI_DEVICE_GET_CLASS(core->owner)->device_id, - macaddr); - e1000e_update_rx_offloads(core); -} - -void -e1000e_core_pci_uninit(E1000ECore *core) -{ - int i; - - timer_del(core->autoneg_timer); - timer_free(core->autoneg_timer); - - e1000e_intrmgr_pci_unint(core); - - qemu_del_vm_change_state_handler(core->vmstate); - - for (i = 0; i < E1000E_NUM_QUEUES; i++) { - net_tx_pkt_reset(core->tx[i].tx_pkt); - net_tx_pkt_uninit(core->tx[i].tx_pkt); - } - - net_rx_pkt_uninit(core->rx_pkt); -} - static const uint16_t e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { [0] = { @@ -3373,7 +3330,7 @@ e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { MII_SR_100X_FD_CAPS, [PHY_ID1] = 0x141, - [PHY_ID2] = E1000_PHY_ID2_82574x, + /* [PHY_ID2] set by e1000e_core_reset() */ [PHY_AUTONEG_ADV] = 0xde1, [PHY_LP_ABILITY] = 0x7e0, [PHY_AUTONEG_EXP] = BIT(2), @@ -3438,6 +3395,67 @@ static const uint32_t e1000e_mac_reg_init[] = { }; void +e1000e_core_pci_realize(E1000ECore *core, + const uint16_t *eeprom_templ, + uint32_t eeprom_size, + const uint8_t *macaddr, + uint16_t phy_id2) +{ + int i; + + core->phy_id2 = phy_id2; + switch (phy_id2) { + case E1000_PHY_ID2_82574x: + core->phy_regcap = &e1000e_phy_regcap; + core->phy_reg_init = &e1000e_phy_reg_init; + break; + default: + g_assert_not_reached(); + } + + core->autoneg_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, + e1000e_autoneg_timer, core); + e1000e_intrmgr_pci_realize(core); + + core->vmstate = + qemu_add_vm_change_state_handler(e1000e_vm_state_change, core); + + for (i = 0; i < E1000E_NUM_QUEUES; i++) { + net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, + E1000E_MAX_TX_FRAGS, core->has_vnet); + } + + net_rx_pkt_init(&core->rx_pkt, core->has_vnet); + + e1000x_core_prepare_eeprom(core->eeprom, + eeprom_templ, + eeprom_size, + PCI_DEVICE_GET_CLASS(core->owner)->device_id, + macaddr); + e1000e_update_rx_offloads(core); +} + +void +e1000e_core_pci_uninit(E1000ECore *core) +{ + int i; + + timer_del(core->autoneg_timer); + timer_free(core->autoneg_timer); + + e1000e_intrmgr_pci_unint(core); + + qemu_del_vm_change_state_handler(core->vmstate); + + for (i = 0; i < E1000E_NUM_QUEUES; i++) { + net_tx_pkt_reset(core->tx[i].tx_pkt); + net_tx_pkt_uninit(core->tx[i].tx_pkt); + } + + net_rx_pkt_uninit(core->rx_pkt); +} + +void e1000e_core_reset(E1000ECore *core) { int i; @@ -3447,7 +3465,8 @@ e1000e_core_reset(E1000ECore *core) e1000e_intrmgr_reset(core); memset(core->phy, 0, sizeof core->phy); - memmove(core->phy, e1000e_phy_reg_init, sizeof e1000e_phy_reg_init); + memmove(core->phy, *core->phy_reg_init, sizeof *core->phy_reg_init); + core->phy[0][PHY_ID2] = core->phy_id2; memset(core->mac, 0, sizeof core->mac); memmove(core->mac, e1000e_mac_reg_init, sizeof e1000e_mac_reg_init); diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index 1ff6978..9ac6f32 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -56,6 +56,10 @@ typedef struct E1000IntrDelayTimer_st { } E1000IntrDelayTimer; struct E1000Core { + uint16_t phy_id2; + const char (*phy_regcap)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + const uint16_t (*phy_reg_init)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + uint32_t mac[E1000E_MAC_SIZE]; uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; uint16_t eeprom[E1000E_EEPROM_SIZE]; @@ -116,10 +120,11 @@ uint64_t e1000e_core_read(E1000ECore *core, hwaddr addr, unsigned size); void -e1000e_core_pci_realize(E1000ECore *regs, - const uint16_t *eeprom_templ, - uint32_t eeprom_size, - const uint8_t *macaddr); +e1000e_core_pci_realize(E1000ECore *regs, + const uint16_t *eeprom_templ, + uint32_t eeprom_size, + const uint8_t *macaddr, + uint16_t phy_id2); void e1000e_core_reset(E1000ECore *core); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] e1000e: Add e1000-ng devices 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk @ 2017-11-09 4:39 ` Ed Swierk 2017-11-09 13:53 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Daniel P. Berrange 2 siblings, 0 replies; 8+ messages in thread From: Ed Swierk @ 2017-11-09 4:39 UTC (permalink / raw) To: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi Cc: qemu-devel, Ed Swierk Implement e1000-compatible devices using the reworked e1000e code: - e1000-ng: Intel 82540EM - e1000-82544gc-ng: Intel 82544GC - e1000-82545em-ng: Intel 82545EM >From a guest's perspective, these should be drop-in replacements for the existing e1000 devices. This version has undergone minimal testing, but should work well enough to start experimenting with e1000-ng devices with a variety of guest OSes. Signed-off-by: Ed Swierk <eswierk@skyportsystems.com> --- hw/net/e1000e.c | 111 +++++++++++++++++++++++++++++++++++++++++++++-- hw/net/e1000e_core.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++----- hw/net/e1000e_core.h | 12 ++++++ 3 files changed, 229 insertions(+), 14 deletions(-) diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c index 7685ab0..9ae5105 100644 --- a/hw/net/e1000e.c +++ b/hw/net/e1000e.c @@ -281,6 +281,17 @@ static const uint16_t e1000e_eeprom_template[64] = { 0xffff, 0xffff, 0xffff, 0xffff, 0x0000, 0x0120, 0xffff, 0x0000, }; +static const uint16_t e1000_eeprom_template[64] = { + 0x0000, 0x0000, 0x0000, 0x0000, 0xffff, 0x0000, 0x0000, 0x0000, + 0x3000, 0x1000, 0x6403, 0 /*DevId*/, 0x8086, 0 /*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, +}; + static void e1000e_core_realize(E1000EState *s) { s->core.owner = &s->parent_obj; @@ -644,8 +655,8 @@ static const VMStateDescription e1000e_vmstate_intr_timer = { static const VMStateDescription e1000e_vmstate = { .name = "e1000e", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .pre_save = e1000e_pre_save, .post_load = e1000e_post_load, .fields = (VMStateField[]) { @@ -691,6 +702,61 @@ static const VMStateDescription e1000e_vmstate = { VMSTATE_STRUCT_ARRAY(core.tx, E1000EState, E1000E_NUM_QUEUES, 0, e1000e_vmstate_tx, struct e1000e_tx), + + VMSTATE_UINT32(core.eecd_state.val_in, E1000EState), + VMSTATE_UINT16(core.eecd_state.bitnum_in, E1000EState), + VMSTATE_UINT16(core.eecd_state.bitnum_out, E1000EState), + VMSTATE_UINT16(core.eecd_state.reading, E1000EState), + VMSTATE_UINT32(core.eecd_state.old_eecd, E1000EState), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription e1000_vmstate = { + .name = "e1000-ng", + .version_id = 1, + .minimum_version_id = 1, + .pre_save = e1000e_pre_save, + .post_load = e1000e_post_load, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(parent_obj, E1000EState), + + VMSTATE_UINT32(ioaddr, E1000EState), + VMSTATE_UINT32(core.rxbuf_min_shift, E1000EState), + VMSTATE_UINT8(core.rx_desc_len, E1000EState), + VMSTATE_UINT32_ARRAY(core.rxbuf_sizes, E1000EState, + E1000_PSRCTL_BUFFS_PER_DESC), + VMSTATE_UINT32(core.rx_desc_buf_size, E1000EState), + VMSTATE_UINT16_ARRAY(core.eeprom, E1000EState, E1000E_EEPROM_SIZE), + VMSTATE_UINT16_2DARRAY(core.phy, E1000EState, + E1000E_PHY_PAGES, E1000E_PHY_PAGE_SIZE), + VMSTATE_UINT32_ARRAY(core.mac, E1000EState, E1000E_MAC_SIZE), + VMSTATE_UINT8_ARRAY(core.permanent_mac, E1000EState, ETH_ALEN), + + VMSTATE_UINT32(core.delayed_causes, E1000EState), + + VMSTATE_UINT16(subsys, E1000EState), + VMSTATE_UINT16(subsys_ven, E1000EState), + + VMSTATE_E1000E_INTR_DELAY_TIMER(core.rdtr, E1000EState), + VMSTATE_E1000E_INTR_DELAY_TIMER(core.radv, E1000EState), + VMSTATE_E1000E_INTR_DELAY_TIMER(core.raid, E1000EState), + VMSTATE_E1000E_INTR_DELAY_TIMER(core.tadv, E1000EState), + VMSTATE_E1000E_INTR_DELAY_TIMER(core.tidv, E1000EState), + + VMSTATE_E1000E_INTR_DELAY_TIMER(core.itr, E1000EState), + VMSTATE_BOOL(core.itr_intr_pending, E1000EState), + + VMSTATE_UINT16(core.vet, E1000EState), + + VMSTATE_STRUCT_ARRAY(core.tx, E1000EState, E1000E_NUM_QUEUES, 0, + e1000e_vmstate_tx, struct e1000e_tx), + + VMSTATE_UINT32(core.eecd_state.val_in, E1000EState), + VMSTATE_UINT16(core.eecd_state.bitnum_in, E1000EState), + VMSTATE_UINT16(core.eecd_state.bitnum_out, E1000EState), + VMSTATE_UINT16(core.eecd_state.reading, E1000EState), + VMSTATE_UINT32(core.eecd_state.old_eecd, E1000EState), VMSTATE_END_OF_LIST() } }; @@ -730,7 +796,7 @@ static void e1000e_class_init(ObjectClass *class, void *data) dc->desc = info->desc; dc->reset = e1000e_qdev_reset; - dc->vmsd = &e1000e_vmstate; + dc->vmsd = info->is_express ? &e1000e_vmstate : &e1000_vmstate; dc->props = e1000e_properties; edc->info = info; @@ -780,6 +846,45 @@ static const E1000EInfo e1000e_devices[] = { .eeprom_size = sizeof(e1000e_eeprom_template), .phy_id2 = E1000_PHY_ID2_82574x, }, + { + .name = "e1000-ng", + .desc = "Intel 82540EM Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82540EM, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT, + }, + { + .name = "e1000-82544gc-ng", + .desc = "Intel 82544GC Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82544GC_COPPER, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_82544x, + }, + { + .name = "e1000-82545em-ng", + .desc = "Intel 82545EM Gigabit Ethernet Controller", + .device_id = E1000_DEV_ID_82545EM_COPPER, + .revision = 3, + .subsystem_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET, + .subsystem_id = PCI_SUBDEVICE_ID_QEMU, + .is_express = 0, + .romfile = "efi-e1000.rom", + .eeprom_templ = e1000_eeprom_template, + .eeprom_size = sizeof(e1000_eeprom_template), + .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT, + }, }; static void e1000e_register_types(void) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 959c697..02a60a1 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -2261,6 +2261,24 @@ static const char e1000e_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { } }; +static const char e1000_phy_regcap[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { + [0] = { + [PHY_CTRL] = PHY_RW, + [PHY_STATUS] = PHY_R, + [PHY_ID1] = PHY_R, + [PHY_ID2] = PHY_R, + [PHY_AUTONEG_ADV] = PHY_RW, + [PHY_LP_ABILITY] = PHY_R, + [PHY_AUTONEG_EXP] = PHY_R, + [PHY_1000T_CTRL] = PHY_RW, + [PHY_1000T_STATUS] = PHY_R, + [M88E1000_PHY_SPEC_CTRL] = PHY_RW, + [M88E1000_PHY_SPEC_STATUS] = PHY_R, + [M88E1000_EXT_PHY_SPEC_CTRL] = PHY_RW, + [M88E1000_RX_ERR_CNTR] = PHY_R, + } +}; + static bool e1000e_phy_reg_check_cap(E1000ECore *core, uint32_t addr, char cap, uint8_t *page) @@ -2616,6 +2634,10 @@ e1000e_mac_icr_read(E1000ECore *core, int index) e1000e_clear_ims_bits(core, core->mac[IAM]); } + if (core->clear_icr_on_read) { + core->mac[ICR] = 0; + } + trace_e1000e_irq_icr_read_exit(core->mac[ICR]); e1000e_update_interrupt_state(core); return ret; @@ -2732,50 +2754,83 @@ e1000e_mac_setmacaddr(E1000ECore *core, int index, uint32_t val) static uint32_t e1000e_get_eecd(E1000ECore *core, int index) { - return e1000e_mac_readreg(core, index); + uint32_t ret = E1000_EECD_PRES | E1000_EECD_GNT | core->eecd_state.old_eecd; + + if (!core->eecd_state.reading || + ((core->eeprom[(core->eecd_state.bitnum_out >> 4) & 0x3f] >> + ((core->eecd_state.bitnum_out & 0xf) ^ 0xf))) & 1) { + ret |= E1000_EECD_DO; + } + return ret; } static void e1000e_set_eecd(E1000ECore *core, int index, uint32_t val) { - static const uint32_t ro_bits = E1000_EECD_PRES | - E1000_EECD_AUTO_RD | - E1000_EECD_SIZE_EX_MASK; + uint32_t oldval = core->eecd_state.old_eecd; - core->mac[EECD] = (core->mac[EECD] & ro_bits) | (val & ~ro_bits); + core->eecd_state.old_eecd = val & (E1000_EECD_SK | E1000_EECD_CS | + E1000_EECD_DI | E1000_EECD_FWE_MASK | + E1000_EECD_REQ); + if (!(E1000_EECD_CS & val)) { /* CS inactive; nothing to do */ + return; + } + if (E1000_EECD_CS & (val ^ oldval)) { /* CS rise edge; reset state */ + core->eecd_state.val_in = 0; + core->eecd_state.bitnum_in = 0; + core->eecd_state.bitnum_out = 0; + core->eecd_state.reading = 0; + } + if (!(E1000_EECD_SK & (val ^ oldval))) { /* no clock edge */ + return; + } + if (!(E1000_EECD_SK & val)) { /* falling edge */ + core->eecd_state.bitnum_out++; + return; + } + core->eecd_state.val_in <<= 1; + if (val & E1000_EECD_DI) { + core->eecd_state.val_in |= 1; + } + if (++core->eecd_state.bitnum_in == 9 && !core->eecd_state.reading) { + core->eecd_state.bitnum_out = ((core->eecd_state.val_in & 0x3f) + << 4) - 1; + core->eecd_state.reading = (((core->eecd_state.val_in >> 6) & 7) == + EEPROM_READ_OPCODE_MICROWIRE); + } } static void e1000e_set_eerd(E1000ECore *core, int index, uint32_t val) { - uint32_t addr = (val >> E1000_EERW_ADDR_SHIFT) & E1000_EERW_ADDR_MASK; + uint32_t addr = (val >> core->eerw_addr_shift) & core->eerw_addr_mask; uint32_t flags = 0; uint32_t data = 0; if ((addr < E1000E_EEPROM_SIZE) && (val & E1000_EERW_START)) { data = core->eeprom[addr]; - flags = E1000_EERW_DONE; + flags = core->eerw_done; } core->mac[EERD] = flags | - (addr << E1000_EERW_ADDR_SHIFT) | + (addr << core->eerw_addr_shift) | (data << E1000_EERW_DATA_SHIFT); } static void e1000e_set_eewr(E1000ECore *core, int index, uint32_t val) { - uint32_t addr = (val >> E1000_EERW_ADDR_SHIFT) & E1000_EERW_ADDR_MASK; + uint32_t addr = (val >> core->eerw_addr_shift) & core->eerw_addr_mask; uint32_t data = (val >> E1000_EERW_DATA_SHIFT) & E1000_EERW_DATA_MASK; uint32_t flags = 0; if ((addr < E1000E_EEPROM_SIZE) && (val & E1000_EERW_START)) { core->eeprom[addr] = data; - flags = E1000_EERW_DONE; + flags = core->eerw_done; } core->mac[EERD] = flags | - (addr << E1000_EERW_ADDR_SHIFT) | + (addr << core->eerw_addr_shift) | (data << E1000_EERW_DATA_SHIFT); } @@ -3352,6 +3407,36 @@ e1000e_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { } }; +static const uint16_t +e1000_phy_reg_init[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE] = { + [0] = { + [PHY_CTRL] = MII_CR_SPEED_SELECT_MSB | + MII_CR_FULL_DUPLEX | + MII_CR_AUTO_NEG_EN, + + [PHY_STATUS] = MII_SR_EXTENDED_CAPS | + MII_SR_LINK_STATUS | + MII_SR_AUTONEG_CAPS | + MII_SR_PREAMBLE_SUPPRESS | + MII_SR_EXTENDED_STATUS | + MII_SR_10T_HD_CAPS | + MII_SR_10T_FD_CAPS | + MII_SR_100X_HD_CAPS | + MII_SR_100X_FD_CAPS, + + [PHY_ID1] = 0x141, + /* [PHY_ID2] set by e1000e_core_reset() */ + [PHY_AUTONEG_ADV] = 0xde1, + [PHY_LP_ABILITY] = 0x1e0, + [PHY_1000T_CTRL] = 0x0e00, + [PHY_1000T_STATUS] = 0x3c00, + + [M88E1000_PHY_SPEC_CTRL] = 0x360, + [M88E1000_PHY_SPEC_STATUS] = 0xac00, + [M88E1000_EXT_PHY_SPEC_CTRL] = 0x0d60, + } +}; + static const uint32_t e1000e_mac_reg_init[] = { [PBA] = 0x00140014, [LEDCTL] = BIT(1) | BIT(8) | BIT(9) | BIT(15) | BIT(17) | BIT(18), @@ -3408,6 +3493,19 @@ e1000e_core_pci_realize(E1000ECore *core, case E1000_PHY_ID2_82574x: core->phy_regcap = &e1000e_phy_regcap; core->phy_reg_init = &e1000e_phy_reg_init; + core->clear_icr_on_read = false; + core->eerw_done = BIT(1); + core->eerw_addr_shift = 2; + core->eerw_addr_mask = ((1L << 14) - 1); + break; + case E1000_PHY_ID2_8254xx_DEFAULT: + case E1000_PHY_ID2_82544x: + core->phy_regcap = &e1000_phy_regcap; + core->phy_reg_init = &e1000_phy_reg_init; + core->clear_icr_on_read = true; + core->eerw_done = BIT(4); + core->eerw_addr_shift = 8; + core->eerw_addr_mask = ((1L << 8) - 1); break; default: g_assert_not_reached(); diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index 9ac6f32..0b318a4 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -59,6 +59,10 @@ struct E1000Core { uint16_t phy_id2; const char (*phy_regcap)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; const uint16_t (*phy_reg_init)[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; + bool clear_icr_on_read; + int eerw_done; + int eerw_addr_shift; + int eerw_addr_mask; uint32_t mac[E1000E_MAC_SIZE]; uint16_t phy[E1000E_PHY_PAGES][E1000E_PHY_PAGE_SIZE]; @@ -108,6 +112,14 @@ struct E1000Core { uint8_t permanent_mac[ETH_ALEN]; + struct { + uint32_t val_in; /* shifted in from guest driver */ + uint16_t bitnum_in; + uint16_t bitnum_out; + uint16_t reading; + uint32_t old_eecd; + } eecd_state; + NICState *owner_nic; PCIDevice *owner; void (*owner_start_recv)(PCIDevice *d); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 2/2] e1000e: Add e1000-ng devices Ed Swierk @ 2017-11-09 13:53 ` Daniel P. Berrange 2017-11-14 23:50 ` Ed Swierk 2 siblings, 1 reply; 8+ messages in thread From: Daniel P. Berrange @ 2017-11-09 13:53 UTC (permalink / raw) To: Ed Swierk Cc: Jason Wang, Dmitry Fleytman, Leonid Bloch, Peter Maydell, Stefan Hajnoczi, qemu-devel FWIW, it is generally recommended that new versions of patch series be posted as a new top level thread, not in-reply-to the previous version. That way the new posting gets attention near top of a dev's email inbox, as opposed to hidden away as a reply to a weeks old msg at the back of the inbox. On Wed, Nov 08, 2017 at 08:39:15PM -0800, Ed Swierk via Qemu-devel wrote: > From a hardware functionality and register perspective, the Intel > 8254x Gigabit Ethernet Controller[1] is roughly a subset of the Intel > 82574[2], making it practical to implement e1000 device emulation as > an extension of e1000e. > > That would be a step towards eliminating the existing e1000 > implementation--a bunch of semi-redundant code with its own bugs (like > the faulty tx checksum offload I reported recently[3]). > > [1] https://www.intel.com/content/dam/doc/manual/pci-pci-x-family-gbe-controllers-software-dev-manual.pdf > [2] https://www.intel.com/content/dam/doc/datasheet/82574l-gbe-controller-datasheet.pdf > [3] https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03008.html > > This patch series adds a new e1000-ng device but leaves the existing > e1000 device alone. Linux 4.1 and Windows 10 e1000 guest drivers > recognize the e1000-ng device and successfully pass traffic on a Linux > host. > > Preliminary performance measurements are encouraging: with e1000-ng, > single-threaded TCP iperf shows about a 2x speedup in tx and rx > throughput vs e1000, and CPU usage is slighly lower. That is certainly attractive. > A ton of work is needed before e1000-ng will be ready to supplant > e1000: testing with every random guest OS, fixing bugs, figuring out > migration and other missing functionality. There's no way I can do > this myself. > > I'd like to propose a more modest and achievable short-term goal: find > and fix any regressions that might affect users of e1000e and e1000, > so that the patches can be applied and folks can easily start trying > out e1000-ng with their favorite guest OS. That means accepting (for > now) known deficiencies in the new e1000-ng devices, which can be > addressed by myself and (hopefully) others in follow-up patches. Does > that sound reasonable? My fear is that this approach of building a new e1000-ng device in parallel with having the existing e1000 device is going to cause long term pain, possibly never getting to a state where the e1000-ng device can replace the e1000 device. Any time there needs to be a "big bang" to switch from one impl to another completely different impl always causes trouble IME. With need for migration wire format & state compatibility, this is even more difficult. From a code review POV it will be essentially impossible to have confidence that the new impl can be a viable drop-in replacement for the old impl. Is there really no way that you can change the approach to do a more incremental conversion of the existing code base, but still end up in the same place at the very end ? eg just copy all the e1000.c code into the e1000e.c file to start with. Then gradually merge functional areas over a longish series of patches to eliminate the duplication. This would make it far more practical to identify where any regressions come from, and will give reviewers more confidence that we're not breaking migration compat. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e 2017-11-09 13:53 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Daniel P. Berrange @ 2017-11-14 23:50 ` Ed Swierk 0 siblings, 0 replies; 8+ messages in thread From: Ed Swierk @ 2017-11-14 23:50 UTC (permalink / raw) To: Daniel P. Berrange Cc: Jason Wang, Dmitry Fleytman, Peter Maydell, Stefan Hajnoczi, qemu-devel On Thu, Nov 9, 2017 at 5:53 AM, Daniel P. Berrange <berrange@redhat.com> wrote: > My fear is that this approach of building a new e1000-ng device in > parallel with having the existing e1000 device is going to cause > long term pain, possibly never getting to a state where the e1000-ng > device can replace the e1000 device. Any time there needs to be a > "big bang" to switch from one impl to another completely different > impl always causes trouble IME. With need for migration wire format > & state compatibility, this is even more difficult. From a code review > POV it will be essentially impossible to have confidence that the new > impl can be a viable drop-in replacement for the old impl. > > Is there really no way that you can change the approach to do a more > incremental conversion of the existing code base, but still end up in > the same place at the very end ? > > eg just copy all the e1000.c code into the e1000e.c file to start with. > Then gradually merge functional areas over a longish series of patches > to eliminate the duplication. This would make it far more practical to > identify where any regressions come from, and will give reviewers more > confidence that we're not breaking migration compat. I agree an incremental conversion is the only realistic way to unearth potential regressions; testing won't be enough. In the past couple of days I've run into several cases where e1000 works but e1000-ng doesn't. Apparently e1000e guest drivers just happen not to tickle these bugs. So e1000-ng isn't ready for prime time anyway. I'll shelve this patch series for the moment. Meanwhile I'll post separate patches for the bugs I've encountered with e1000 UDP checksum offload. --Ed ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-11-14 23:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-27 2:49 [Qemu-devel] [PATCH 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk 2017-10-27 2:49 ` [Qemu-devel] [PATCH 2/2] e1000e: Add e1000-ng devices Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 1/2] e1000e: Infrastructure for e1000-ng Ed Swierk 2017-11-09 4:39 ` [Qemu-devel] [PATCH v2 2/2] e1000e: Add e1000-ng devices Ed Swierk 2017-11-09 13:53 ` [Qemu-devel] [PATCH v2 0/2] e1000e: Reimplement e1000 as a variant of e1000e Daniel P. Berrange 2017-11-14 23:50 ` Ed Swierk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.