All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register
@ 2021-07-21  4:15 Bin Meng
  2021-07-21  4:15 ` [PATCH v3 2/3] hw/net: e1000e: " Bin Meng
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Bin Meng @ 2021-07-21  4:15 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Christina Wang, Markus Carlstedt

From: Christina Wang <christina.wang@windriver.com>

The initial value of VLAN Ether Type (VET) register is 0x8100, as per
the manual and real hardware.

While Linux e1000 driver always writes VET register to 0x8100, it is
not always the case for everyone. Drivers relying on the reset value
of VET won't be able to transmit and receive VLAN frames in QEMU.

Reported-by: Markus Carlstedt <markus.carlstedt@windriver.com>
Signed-off-by: Christina Wang <christina.wang@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v3:
- add a "init-vet" property for versioned machines

 hw/core/machine.c | 27 +++++++++++++++++++++++++--
 hw/net/e1000.c    | 26 ++++++++++++++++++--------
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 775add0795..29982c1ef1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -41,6 +41,7 @@ GlobalProperty hw_compat_6_0[] = {
     { "gpex-pcihost", "allow-unmapped-accesses", "false" },
     { "i8042", "extended-state", "false"},
     { "nvme-ns", "eui64-default", "off"},
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
 
@@ -49,6 +50,7 @@ GlobalProperty hw_compat_5_2[] = {
     { "PIIX4_PM", "smm-compat", "on"},
     { "virtio-blk-device", "report-discard-granularity", "off" },
     { "virtio-net-pci", "vectors", "3"},
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
@@ -62,6 +64,7 @@ GlobalProperty hw_compat_5_1[] = {
     { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */
     { "pl011", "migrate-clk", "off" },
     { "virtio-pci", "x-ats-page-aligned", "off"},
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
 
@@ -73,6 +76,7 @@ GlobalProperty hw_compat_5_0[] = {
     { "vmport", "x-report-vmx-type", "off" },
     { "vmport", "x-cmds-v2", "off" },
     { "virtio-device", "x-disable-legacy-check", "true" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
 
@@ -89,11 +93,13 @@ GlobalProperty hw_compat_4_2[] = {
     { "qxl-vga", "revision", "4" },
     { "fw_cfg", "acpi-mr-restore", "false" },
     { "virtio-device", "use-disabled-flag", "false" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
 
 GlobalProperty hw_compat_4_1[] = {
     { "virtio-pci", "x-pcie-flr-init", "off" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
 
@@ -106,6 +112,7 @@ GlobalProperty hw_compat_4_0[] = {
     { "virtio-device", "use-started", "false" },
     { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
     { "pl031", "migrate-tick-offset", "false" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -123,10 +130,13 @@ GlobalProperty hw_compat_3_1[] = {
     { "virtio-blk-device", "write-zeroes", "false" },
     { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
     { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
-GlobalProperty hw_compat_3_0[] = {};
+GlobalProperty hw_compat_3_0[] = {
+    { "e1000", "init-vet", "off" },
+};
 const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
 
 GlobalProperty hw_compat_2_12[] = {
@@ -136,6 +146,7 @@ GlobalProperty hw_compat_2_12[] = {
     { "VGA", "global-vmstate", "true" },
     { "vmware-svga", "global-vmstate", "true" },
     { "qxl-vga", "global-vmstate", "true" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
 
@@ -144,12 +155,14 @@ GlobalProperty hw_compat_2_11[] = {
     { "virtio-blk-pci", "vectors", "2" },
     { "vhost-user-blk-pci", "vectors", "2" },
     { "e1000", "migrate_tso_props", "off" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
 
 GlobalProperty hw_compat_2_10[] = {
     { "virtio-mouse-device", "wheel-axis", "false" },
     { "virtio-tablet-device", "wheel-axis", "false" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
 
@@ -158,6 +171,7 @@ GlobalProperty hw_compat_2_9[] = {
     { "intel-iommu", "pt", "off" },
     { "virtio-net-device", "x-mtu-bypass-backend", "off" },
     { "pcie-root-port", "x-migrate-msix", "false" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
 
@@ -172,6 +186,7 @@ GlobalProperty hw_compat_2_8[] = {
     { "virtio-pci", "x-pcie-pm-init", "off" },
     { "cirrus-vga", "vgamem_mb", "8" },
     { "isa-cirrus-vga", "vgamem_mb", "8" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_8_len = G_N_ELEMENTS(hw_compat_2_8);
 
@@ -181,6 +196,7 @@ GlobalProperty hw_compat_2_7[] = {
     { "ioapic", "version", "0x11" },
     { "intel-iommu", "x-buggy-eim", "true" },
     { "virtio-pci", "x-ignore-backend-features", "on" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_7_len = G_N_ELEMENTS(hw_compat_2_7);
 
@@ -189,6 +205,7 @@ GlobalProperty hw_compat_2_6[] = {
     /* Optional because not all virtio-pci devices support legacy mode */
     { "virtio-pci", "disable-modern", "on",  .optional = true },
     { "virtio-pci", "disable-legacy", "off", .optional = true },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6);
 
@@ -198,6 +215,7 @@ GlobalProperty hw_compat_2_5[] = {
     { "pvscsi", "x-disable-pcie", "on" },
     { "vmxnet3", "x-old-msi-offsets", "on" },
     { "vmxnet3", "x-disable-pcie", "on" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
 
@@ -205,6 +223,7 @@ GlobalProperty hw_compat_2_4[] = {
     /* Optional because the 'scsi' property is Linux-only */
     { "virtio-blk-device", "scsi", "true", .optional = true },
     { "e1000", "extra_mac_registers", "off" },
+    { "e1000", "init-vet", "off" },
     { "virtio-pci", "x-disable-pcie", "on" },
     { "virtio-pci", "migrate-extra", "off" },
     { "fw_cfg_mem", "dma_enabled", "off" },
@@ -222,10 +241,13 @@ GlobalProperty hw_compat_2_3[] = {
     { "migration", "send-configuration", "off" },
     { "migration", "send-section-footer", "off" },
     { "migration", "store-global-state", "off" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_3_len = G_N_ELEMENTS(hw_compat_2_3);
 
-GlobalProperty hw_compat_2_2[] = {};
+GlobalProperty hw_compat_2_2[] = {
+    { "e1000", "init-vet", "off" },
+};
 const size_t hw_compat_2_2_len = G_N_ELEMENTS(hw_compat_2_2);
 
 GlobalProperty hw_compat_2_1[] = {
@@ -236,6 +258,7 @@ GlobalProperty hw_compat_2_1[] = {
     { "usb-mouse", "usb_version", "1" },
     { "usb-kbd", "usb_version", "1" },
     { "virtio-pci", "virtio-pci-bus-master-bug-migration", "on" },
+    { "e1000", "init-vet", "off" },
 };
 const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
 
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 4f75b44cfc..543c4fbb02 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -29,6 +29,7 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
+#include "net/eth.h"
 #include "net/net.h"
 #include "net/checksum.h"
 #include "sysemu/sysemu.h"
@@ -130,10 +131,13 @@ struct E1000State_st {
 #define E1000_FLAG_MIT_BIT 1
 #define E1000_FLAG_MAC_BIT 2
 #define E1000_FLAG_TSO_BIT 3
+#define E1000_FLAG_VET_BIT 4
 #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
 #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
 #define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT)
 #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT)
+#define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT)
+
     uint32_t compat_flags;
     bool received_tx_tso;
     bool use_tso_for_migration;
@@ -141,7 +145,7 @@ struct E1000State_st {
 };
 typedef struct E1000State_st E1000State;
 
-#define chkflag(x)     (s->compat_flags & E1000_FLAG_##x)
+#define chkflag(s, x)     (s->compat_flags & E1000_FLAG_##x)
 
 struct E1000BaseClass {
     PCIDeviceClass parent_class;
@@ -176,7 +180,7 @@ e1000_autoneg_done(E1000State *s)
 static bool
 have_autoneg(E1000State *s)
 {
-    return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
+    return chkflag(s, AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
 }
 
 static void
@@ -298,7 +302,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
         if (s->mit_timer_on) {
             return;
         }
-        if (chkflag(MIT)) {
+        if (chkflag(s, MIT)) {
             /* Compute the next mitigation delay according to pending
              * interrupts and the current values of RADV (provided
              * RDTR!=0), TADV and ITR.
@@ -386,6 +390,10 @@ static void e1000_reset(void *opaque)
     }
 
     e1000x_reset_mac_addr(d->nic, d->mac_reg, macaddr);
+
+    if (chkflag(d, VET)) {
+        d->mac_reg[VET] = ETH_P_VLAN;
+    }
 }
 
 static void
@@ -1397,7 +1405,7 @@ static int e1000_pre_save(void *opaque)
     }
 
     /* Decide which set of props to migrate in the main structure */
-    if (chkflag(TSO) || !s->use_tso_for_migration) {
+    if (chkflag(s, TSO) || !s->use_tso_for_migration) {
         /* Either we're migrating with the extra subsection, in which
          * case the mig_props is always 'props' OR
          * we've not got the subsection, but 'props' was the last
@@ -1418,7 +1426,7 @@ static int e1000_post_load(void *opaque, int version_id)
     E1000State *s = opaque;
     NetClientState *nc = qemu_get_queue(s->nic);
 
-    if (!chkflag(MIT)) {
+    if (!chkflag(s, MIT)) {
         s->mac_reg[ITR] = s->mac_reg[RDTR] = s->mac_reg[RADV] =
             s->mac_reg[TADV] = 0;
         s->mit_irq_level = false;
@@ -1461,21 +1469,21 @@ static bool e1000_mit_state_needed(void *opaque)
 {
     E1000State *s = opaque;
 
-    return chkflag(MIT);
+    return chkflag(s, MIT);
 }
 
 static bool e1000_full_mac_needed(void *opaque)
 {
     E1000State *s = opaque;
 
-    return chkflag(MAC);
+    return chkflag(s, MAC);
 }
 
 static bool e1000_tso_state_needed(void *opaque)
 {
     E1000State *s = opaque;
 
-    return chkflag(TSO);
+    return chkflag(s, TSO);
 }
 
 static const VMStateDescription vmstate_e1000_mit_state = {
@@ -1737,6 +1745,8 @@ static Property e1000_properties[] = {
                     compat_flags, E1000_FLAG_MAC_BIT, true),
     DEFINE_PROP_BIT("migrate_tso_props", E1000State,
                     compat_flags, E1000_FLAG_TSO_BIT, true),
+    DEFINE_PROP_BIT("init-vet", E1000State,
+                    compat_flags, E1000_FLAG_VET_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/3] hw/net: e1000e: Correct the initial value of VET register
  2021-07-21  4:15 [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng
@ 2021-07-21  4:15 ` Bin Meng
  2021-07-22  8:28   ` Jason Wang
  2021-07-21  4:15 ` [PATCH v3 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng
  2021-07-22  8:35 ` [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Jason Wang
  2 siblings, 1 reply; 5+ messages in thread
From: Bin Meng @ 2021-07-21  4:15 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Christina Wang, Markus Carlstedt

From: Christina Wang <christina.wang@windriver.com>

The initial value of VLAN Ether Type (VET) register is 0x8100, as per
the manual and real hardware.

While Linux e1000e driver always writes VET register to 0x8100, it is
not always the case for everyone. Drivers relying on the reset value
of VET won't be able to transmit and receive VLAN frames in QEMU.

Unlike e1000 in QEMU, e1000e uses a field 'vet' in "struct E1000Core"
to cache the value of VET register, but the cache only gets updated
when VET register is written. To always get a consistent VET value
no matter VET is written or remains its reset value, drop the 'vet'
field and use 'core->mac[VET]' directly.

Reported-by: Markus Carlstedt <markus.carlstedt@windriver.com>
Signed-off-by: Christina Wang <christina.wang@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v3:
- add a "init-vet" property for versioned machines

Changes in v2:
- keep the 'vet' field in "struct E1000Core" for migration compatibility

 hw/core/machine.c    | 21 +++++++++++++++++++++
 hw/net/e1000e.c      |  8 +++++++-
 hw/net/e1000e_core.c |  9 ++++-----
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 29982c1ef1..8355df69c7 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -42,6 +42,7 @@ GlobalProperty hw_compat_6_0[] = {
     { "i8042", "extended-state", "false"},
     { "nvme-ns", "eui64-default", "off"},
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
 
@@ -51,6 +52,7 @@ GlobalProperty hw_compat_5_2[] = {
     { "virtio-blk-device", "report-discard-granularity", "off" },
     { "virtio-net-pci", "vectors", "3"},
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
 
@@ -65,6 +67,7 @@ GlobalProperty hw_compat_5_1[] = {
     { "pl011", "migrate-clk", "off" },
     { "virtio-pci", "x-ats-page-aligned", "off"},
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
 
@@ -77,6 +80,7 @@ GlobalProperty hw_compat_5_0[] = {
     { "vmport", "x-cmds-v2", "off" },
     { "virtio-device", "x-disable-legacy-check", "true" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
 
@@ -94,12 +98,14 @@ GlobalProperty hw_compat_4_2[] = {
     { "fw_cfg", "acpi-mr-restore", "false" },
     { "virtio-device", "use-disabled-flag", "false" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
 
 GlobalProperty hw_compat_4_1[] = {
     { "virtio-pci", "x-pcie-flr-init", "off" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
 
@@ -113,6 +119,7 @@ GlobalProperty hw_compat_4_0[] = {
     { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
     { "pl031", "migrate-tick-offset", "false" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
@@ -131,11 +138,13 @@ GlobalProperty hw_compat_3_1[] = {
     { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
     { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
 
 GlobalProperty hw_compat_3_0[] = {
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
 
@@ -147,6 +156,7 @@ GlobalProperty hw_compat_2_12[] = {
     { "vmware-svga", "global-vmstate", "true" },
     { "qxl-vga", "global-vmstate", "true" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
 
@@ -156,6 +166,7 @@ GlobalProperty hw_compat_2_11[] = {
     { "vhost-user-blk-pci", "vectors", "2" },
     { "e1000", "migrate_tso_props", "off" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
 
@@ -163,6 +174,7 @@ GlobalProperty hw_compat_2_10[] = {
     { "virtio-mouse-device", "wheel-axis", "false" },
     { "virtio-tablet-device", "wheel-axis", "false" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
 
@@ -172,6 +184,7 @@ GlobalProperty hw_compat_2_9[] = {
     { "virtio-net-device", "x-mtu-bypass-backend", "off" },
     { "pcie-root-port", "x-migrate-msix", "false" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
 
@@ -187,6 +200,7 @@ GlobalProperty hw_compat_2_8[] = {
     { "cirrus-vga", "vgamem_mb", "8" },
     { "isa-cirrus-vga", "vgamem_mb", "8" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_8_len = G_N_ELEMENTS(hw_compat_2_8);
 
@@ -197,6 +211,7 @@ GlobalProperty hw_compat_2_7[] = {
     { "intel-iommu", "x-buggy-eim", "true" },
     { "virtio-pci", "x-ignore-backend-features", "on" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_7_len = G_N_ELEMENTS(hw_compat_2_7);
 
@@ -206,6 +221,7 @@ GlobalProperty hw_compat_2_6[] = {
     { "virtio-pci", "disable-modern", "on",  .optional = true },
     { "virtio-pci", "disable-legacy", "off", .optional = true },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6);
 
@@ -216,6 +232,7 @@ GlobalProperty hw_compat_2_5[] = {
     { "vmxnet3", "x-old-msi-offsets", "on" },
     { "vmxnet3", "x-disable-pcie", "on" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
 
@@ -224,6 +241,7 @@ GlobalProperty hw_compat_2_4[] = {
     { "virtio-blk-device", "scsi", "true", .optional = true },
     { "e1000", "extra_mac_registers", "off" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
     { "virtio-pci", "x-disable-pcie", "on" },
     { "virtio-pci", "migrate-extra", "off" },
     { "fw_cfg_mem", "dma_enabled", "off" },
@@ -242,11 +260,13 @@ GlobalProperty hw_compat_2_3[] = {
     { "migration", "send-section-footer", "off" },
     { "migration", "store-global-state", "off" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_3_len = G_N_ELEMENTS(hw_compat_2_3);
 
 GlobalProperty hw_compat_2_2[] = {
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_2_len = G_N_ELEMENTS(hw_compat_2_2);
 
@@ -259,6 +279,7 @@ GlobalProperty hw_compat_2_1[] = {
     { "usb-kbd", "usb_version", "1" },
     { "virtio-pci", "virtio-pci-bus-master-bug-migration", "on" },
     { "e1000", "init-vet", "off" },
+    { "e1000e", "init-vet", "off" },
 };
 const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
 
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index a8a77eca95..ac96f7665a 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -35,6 +35,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "net/eth.h"
 #include "net/net.h"
 #include "net/tap.h"
 #include "qemu/module.h"
@@ -79,7 +80,7 @@ struct E1000EState {
     bool disable_vnet;
 
     E1000ECore core;
-
+    bool init_vet;
 };
 
 #define E1000E_MMIO_IDX     0
@@ -527,6 +528,10 @@ static void e1000e_qdev_reset(DeviceState *dev)
     trace_e1000e_cb_qdev_reset();
 
     e1000e_core_reset(&s->core);
+
+    if (s->init_vet) {
+        s->core.mac[VET] = ETH_P_VLAN;
+    }
 }
 
 static int e1000e_pre_save(void *opaque)
@@ -666,6 +671,7 @@ static Property e1000e_properties[] = {
                         e1000e_prop_subsys_ven, uint16_t),
     DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, 0,
                         e1000e_prop_subsys, uint16_t),
+    DEFINE_PROP_BOOL("init-vet", E1000EState, init_vet, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index b75f2ab8fc..b4bf4ca2f1 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -731,7 +731,7 @@ e1000e_process_tx_desc(E1000ECore *core,
             if (e1000x_vlan_enabled(core->mac) &&
                 e1000x_is_vlan_txd(txd_lower)) {
                 net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt,
-                    le16_to_cpu(dp->upper.fields.special), core->vet);
+                    le16_to_cpu(dp->upper.fields.special), core->mac[VET]);
             }
             if (e1000e_tx_pkt_send(core, tx, queue_index)) {
                 e1000e_on_tx_done_update_stats(core, tx->tx_pkt);
@@ -1012,7 +1012,7 @@ e1000e_receive_filter(E1000ECore *core, const uint8_t *buf, int size)
 {
     uint32_t rctl = core->mac[RCTL];
 
-    if (e1000x_is_vlan_packet(buf, core->vet) &&
+    if (e1000x_is_vlan_packet(buf, core->mac[VET]) &&
         e1000x_vlan_rx_filter_enabled(core->mac)) {
         uint16_t vid = lduw_be_p(buf + 14);
         uint32_t vfta = ldl_le_p((uint32_t *)(core->mac + VFTA) +
@@ -1686,7 +1686,7 @@ e1000e_receive_iov(E1000ECore *core, const struct iovec *iov, int iovcnt)
     }
 
     net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
-                               e1000x_vlan_enabled(core->mac), core->vet);
+                               e1000x_vlan_enabled(core->mac), core->mac[VET]);
 
     e1000e_rss_parse_packet(core, core->rx_pkt, &rss_info);
     e1000e_rx_ring_init(core, &rxr, rss_info.queue);
@@ -2397,8 +2397,7 @@ static void
 e1000e_set_vet(E1000ECore *core, int index, uint32_t val)
 {
     core->mac[VET] = val & 0xffff;
-    core->vet = le16_to_cpu(core->mac[VET]);
-    trace_e1000e_vlan_vet(core->vet);
+    trace_e1000e_vlan_vet(core->mac[VET]);
 }
 
 static void
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor
  2021-07-21  4:15 [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng
  2021-07-21  4:15 ` [PATCH v3 2/3] hw/net: e1000e: " Bin Meng
@ 2021-07-21  4:15 ` Bin Meng
  2021-07-22  8:35 ` [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Jason Wang
  2 siblings, 0 replies; 5+ messages in thread
From: Bin Meng @ 2021-07-21  4:15 UTC (permalink / raw)
  To: Jason Wang, qemu-devel; +Cc: Bin Meng, Christina Wang, Markus Carlstedt

From: Christina Wang <christina.wang@windriver.com>

In the legacy RX descriptor mode, VLAN tag was saved to d->special
by e1000e_build_rx_metadata() in e1000e_write_lgcy_rx_descr(), but
it was then zeroed out again at the end of the call, which is wrong.

Fixes: c89d416a2b0f ("e1000e: Don't zero out buffer address in rx descriptor")
Reported-by: Markus Carlstedt <markus.carlstedt@windriver.com>
Signed-off-by: Christina Wang <christina.wang@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

(no changes since v1)

 hw/net/e1000e_core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index b4bf4ca2f1..8ae6fb7e14 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1285,7 +1285,6 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t *desc,
                              &d->special);
     d->errors = (uint8_t) (le32_to_cpu(status_flags) >> 24);
     d->status = (uint8_t) le32_to_cpu(status_flags);
-    d->special = 0;
 }
 
 static inline void
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 2/3] hw/net: e1000e: Correct the initial value of VET register
  2021-07-21  4:15 ` [PATCH v3 2/3] hw/net: e1000e: " Bin Meng
@ 2021-07-22  8:28   ` Jason Wang
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2021-07-22  8:28 UTC (permalink / raw)
  To: Bin Meng, qemu-devel; +Cc: Bin Meng, Christina Wang, Markus Carlstedt


在 2021/7/21 下午12:15, Bin Meng 写道:
> From: Christina Wang <christina.wang@windriver.com>
>
> The initial value of VLAN Ether Type (VET) register is 0x8100, as per
> the manual and real hardware.
>
> While Linux e1000e driver always writes VET register to 0x8100, it is
> not always the case for everyone. Drivers relying on the reset value
> of VET won't be able to transmit and receive VLAN frames in QEMU.
>
> Unlike e1000 in QEMU, e1000e uses a field 'vet' in "struct E1000Core"
> to cache the value of VET register, but the cache only gets updated
> when VET register is written. To always get a consistent VET value
> no matter VET is written or remains its reset value, drop the 'vet'
> field and use 'core->mac[VET]' directly.
>
> Reported-by: Markus Carlstedt <markus.carlstedt@windriver.com>
> Signed-off-by: Christina Wang <christina.wang@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v3:
> - add a "init-vet" property for versioned machines
>
> Changes in v2:
> - keep the 'vet' field in "struct E1000Core" for migration compatibility
>
>   hw/core/machine.c    | 21 +++++++++++++++++++++
>   hw/net/e1000e.c      |  8 +++++++-
>   hw/net/e1000e_core.c |  9 ++++-----
>   3 files changed, 32 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 29982c1ef1..8355df69c7 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -42,6 +42,7 @@ GlobalProperty hw_compat_6_0[] = {
>       { "i8042", "extended-state", "false"},
>       { "nvme-ns", "eui64-default", "off"},
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };


It looks to me doing compat in hw_compat_6_0 is sufficient.

The codes will do it automatically for the versions before 6.0.

E.g virt_machine_5_2_options() will call virt_machine_6_0_options() etc.

Other looks good.

Thanks


>   const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
>   
> @@ -51,6 +52,7 @@ GlobalProperty hw_compat_5_2[] = {
>       { "virtio-blk-device", "report-discard-granularity", "off" },
>       { "virtio-net-pci", "vectors", "3"},
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
>   
> @@ -65,6 +67,7 @@ GlobalProperty hw_compat_5_1[] = {
>       { "pl011", "migrate-clk", "off" },
>       { "virtio-pci", "x-ats-page-aligned", "off"},
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
>   
> @@ -77,6 +80,7 @@ GlobalProperty hw_compat_5_0[] = {
>       { "vmport", "x-cmds-v2", "off" },
>       { "virtio-device", "x-disable-legacy-check", "true" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
>   
> @@ -94,12 +98,14 @@ GlobalProperty hw_compat_4_2[] = {
>       { "fw_cfg", "acpi-mr-restore", "false" },
>       { "virtio-device", "use-disabled-flag", "false" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
>   
>   GlobalProperty hw_compat_4_1[] = {
>       { "virtio-pci", "x-pcie-flr-init", "off" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
>   
> @@ -113,6 +119,7 @@ GlobalProperty hw_compat_4_0[] = {
>       { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
>       { "pl031", "migrate-tick-offset", "false" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>   
> @@ -131,11 +138,13 @@ GlobalProperty hw_compat_3_1[] = {
>       { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
>       { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>   
>   GlobalProperty hw_compat_3_0[] = {
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
>   
> @@ -147,6 +156,7 @@ GlobalProperty hw_compat_2_12[] = {
>       { "vmware-svga", "global-vmstate", "true" },
>       { "qxl-vga", "global-vmstate", "true" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
>   
> @@ -156,6 +166,7 @@ GlobalProperty hw_compat_2_11[] = {
>       { "vhost-user-blk-pci", "vectors", "2" },
>       { "e1000", "migrate_tso_props", "off" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
>   
> @@ -163,6 +174,7 @@ GlobalProperty hw_compat_2_10[] = {
>       { "virtio-mouse-device", "wheel-axis", "false" },
>       { "virtio-tablet-device", "wheel-axis", "false" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
>   
> @@ -172,6 +184,7 @@ GlobalProperty hw_compat_2_9[] = {
>       { "virtio-net-device", "x-mtu-bypass-backend", "off" },
>       { "pcie-root-port", "x-migrate-msix", "false" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
>   
> @@ -187,6 +200,7 @@ GlobalProperty hw_compat_2_8[] = {
>       { "cirrus-vga", "vgamem_mb", "8" },
>       { "isa-cirrus-vga", "vgamem_mb", "8" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_8_len = G_N_ELEMENTS(hw_compat_2_8);
>   
> @@ -197,6 +211,7 @@ GlobalProperty hw_compat_2_7[] = {
>       { "intel-iommu", "x-buggy-eim", "true" },
>       { "virtio-pci", "x-ignore-backend-features", "on" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_7_len = G_N_ELEMENTS(hw_compat_2_7);
>   
> @@ -206,6 +221,7 @@ GlobalProperty hw_compat_2_6[] = {
>       { "virtio-pci", "disable-modern", "on",  .optional = true },
>       { "virtio-pci", "disable-legacy", "off", .optional = true },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6);
>   
> @@ -216,6 +232,7 @@ GlobalProperty hw_compat_2_5[] = {
>       { "vmxnet3", "x-old-msi-offsets", "on" },
>       { "vmxnet3", "x-disable-pcie", "on" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
>   
> @@ -224,6 +241,7 @@ GlobalProperty hw_compat_2_4[] = {
>       { "virtio-blk-device", "scsi", "true", .optional = true },
>       { "e1000", "extra_mac_registers", "off" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>       { "virtio-pci", "x-disable-pcie", "on" },
>       { "virtio-pci", "migrate-extra", "off" },
>       { "fw_cfg_mem", "dma_enabled", "off" },
> @@ -242,11 +260,13 @@ GlobalProperty hw_compat_2_3[] = {
>       { "migration", "send-section-footer", "off" },
>       { "migration", "store-global-state", "off" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_3_len = G_N_ELEMENTS(hw_compat_2_3);
>   
>   GlobalProperty hw_compat_2_2[] = {
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_2_len = G_N_ELEMENTS(hw_compat_2_2);
>   
> @@ -259,6 +279,7 @@ GlobalProperty hw_compat_2_1[] = {
>       { "usb-kbd", "usb_version", "1" },
>       { "virtio-pci", "virtio-pci-bus-master-bug-migration", "on" },
>       { "e1000", "init-vet", "off" },
> +    { "e1000e", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
>   
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index a8a77eca95..ac96f7665a 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -35,6 +35,7 @@
>   
>   #include "qemu/osdep.h"
>   #include "qemu/units.h"
> +#include "net/eth.h"
>   #include "net/net.h"
>   #include "net/tap.h"
>   #include "qemu/module.h"
> @@ -79,7 +80,7 @@ struct E1000EState {
>       bool disable_vnet;
>   
>       E1000ECore core;
> -
> +    bool init_vet;
>   };
>   
>   #define E1000E_MMIO_IDX     0
> @@ -527,6 +528,10 @@ static void e1000e_qdev_reset(DeviceState *dev)
>       trace_e1000e_cb_qdev_reset();
>   
>       e1000e_core_reset(&s->core);
> +
> +    if (s->init_vet) {
> +        s->core.mac[VET] = ETH_P_VLAN;
> +    }
>   }
>   
>   static int e1000e_pre_save(void *opaque)
> @@ -666,6 +671,7 @@ static Property e1000e_properties[] = {
>                           e1000e_prop_subsys_ven, uint16_t),
>       DEFINE_PROP_SIGNED("subsys", E1000EState, subsys, 0,
>                           e1000e_prop_subsys, uint16_t),
> +    DEFINE_PROP_BOOL("init-vet", E1000EState, init_vet, true),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index b75f2ab8fc..b4bf4ca2f1 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -731,7 +731,7 @@ e1000e_process_tx_desc(E1000ECore *core,
>               if (e1000x_vlan_enabled(core->mac) &&
>                   e1000x_is_vlan_txd(txd_lower)) {
>                   net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt,
> -                    le16_to_cpu(dp->upper.fields.special), core->vet);
> +                    le16_to_cpu(dp->upper.fields.special), core->mac[VET]);
>               }
>               if (e1000e_tx_pkt_send(core, tx, queue_index)) {
>                   e1000e_on_tx_done_update_stats(core, tx->tx_pkt);
> @@ -1012,7 +1012,7 @@ e1000e_receive_filter(E1000ECore *core, const uint8_t *buf, int size)
>   {
>       uint32_t rctl = core->mac[RCTL];
>   
> -    if (e1000x_is_vlan_packet(buf, core->vet) &&
> +    if (e1000x_is_vlan_packet(buf, core->mac[VET]) &&
>           e1000x_vlan_rx_filter_enabled(core->mac)) {
>           uint16_t vid = lduw_be_p(buf + 14);
>           uint32_t vfta = ldl_le_p((uint32_t *)(core->mac + VFTA) +
> @@ -1686,7 +1686,7 @@ e1000e_receive_iov(E1000ECore *core, const struct iovec *iov, int iovcnt)
>       }
>   
>       net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
> -                               e1000x_vlan_enabled(core->mac), core->vet);
> +                               e1000x_vlan_enabled(core->mac), core->mac[VET]);
>   
>       e1000e_rss_parse_packet(core, core->rx_pkt, &rss_info);
>       e1000e_rx_ring_init(core, &rxr, rss_info.queue);
> @@ -2397,8 +2397,7 @@ static void
>   e1000e_set_vet(E1000ECore *core, int index, uint32_t val)
>   {
>       core->mac[VET] = val & 0xffff;
> -    core->vet = le16_to_cpu(core->mac[VET]);
> -    trace_e1000e_vlan_vet(core->vet);
> +    trace_e1000e_vlan_vet(core->mac[VET]);
>   }
>   
>   static void



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register
  2021-07-21  4:15 [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng
  2021-07-21  4:15 ` [PATCH v3 2/3] hw/net: e1000e: " Bin Meng
  2021-07-21  4:15 ` [PATCH v3 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng
@ 2021-07-22  8:35 ` Jason Wang
  2 siblings, 0 replies; 5+ messages in thread
From: Jason Wang @ 2021-07-22  8:35 UTC (permalink / raw)
  To: Bin Meng, qemu-devel; +Cc: Bin Meng, Christina Wang, Markus Carlstedt


在 2021/7/21 下午12:15, Bin Meng 写道:
> From: Christina Wang <christina.wang@windriver.com>
>
> The initial value of VLAN Ether Type (VET) register is 0x8100, as per
> the manual and real hardware.
>
> While Linux e1000 driver always writes VET register to 0x8100, it is
> not always the case for everyone. Drivers relying on the reset value
> of VET won't be able to transmit and receive VLAN frames in QEMU.
>
> Reported-by: Markus Carlstedt <markus.carlstedt@windriver.com>
> Signed-off-by: Christina Wang <christina.wang@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v3:
> - add a "init-vet" property for versioned machines
>
>   hw/core/machine.c | 27 +++++++++++++++++++++++++--
>   hw/net/e1000.c    | 26 ++++++++++++++++++--------
>   2 files changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 775add0795..29982c1ef1 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -41,6 +41,7 @@ GlobalProperty hw_compat_6_0[] = {
>       { "gpex-pcihost", "allow-unmapped-accesses", "false" },
>       { "i8042", "extended-state", "false"},
>       { "nvme-ns", "eui64-default", "off"},
> +    { "e1000", "init-vet", "off" },


Doing this for 6.0 is sufficient. See patch 2.


>   };
>   const size_t hw_compat_6_0_len = G_N_ELEMENTS(hw_compat_6_0);
>   
> @@ -49,6 +50,7 @@ GlobalProperty hw_compat_5_2[] = {
>       { "PIIX4_PM", "smm-compat", "on"},
>       { "virtio-blk-device", "report-discard-granularity", "off" },
>       { "virtio-net-pci", "vectors", "3"},
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
>   
> @@ -62,6 +64,7 @@ GlobalProperty hw_compat_5_1[] = {
>       { "pvpanic", "events", "1"}, /* PVPANIC_PANICKED */
>       { "pl011", "migrate-clk", "off" },
>       { "virtio-pci", "x-ats-page-aligned", "off"},
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_1_len = G_N_ELEMENTS(hw_compat_5_1);
>   
> @@ -73,6 +76,7 @@ GlobalProperty hw_compat_5_0[] = {
>       { "vmport", "x-report-vmx-type", "off" },
>       { "vmport", "x-cmds-v2", "off" },
>       { "virtio-device", "x-disable-legacy-check", "true" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
>   
> @@ -89,11 +93,13 @@ GlobalProperty hw_compat_4_2[] = {
>       { "qxl-vga", "revision", "4" },
>       { "fw_cfg", "acpi-mr-restore", "false" },
>       { "virtio-device", "use-disabled-flag", "false" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_2_len = G_N_ELEMENTS(hw_compat_4_2);
>   
>   GlobalProperty hw_compat_4_1[] = {
>       { "virtio-pci", "x-pcie-flr-init", "off" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
>   
> @@ -106,6 +112,7 @@ GlobalProperty hw_compat_4_0[] = {
>       { "virtio-device", "use-started", "false" },
>       { "virtio-balloon-device", "qemu-4-0-config-size", "true" },
>       { "pl031", "migrate-tick-offset", "false" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>   
> @@ -123,10 +130,13 @@ GlobalProperty hw_compat_3_1[] = {
>       { "virtio-blk-device", "write-zeroes", "false" },
>       { "virtio-balloon-device", "qemu-4-0-config-size", "false" },
>       { "pcie-root-port-base", "disable-acs", "true" }, /* Added in 4.1 */
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>   
> -GlobalProperty hw_compat_3_0[] = {};
> +GlobalProperty hw_compat_3_0[] = {
> +    { "e1000", "init-vet", "off" },
> +};
>   const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
>   
>   GlobalProperty hw_compat_2_12[] = {
> @@ -136,6 +146,7 @@ GlobalProperty hw_compat_2_12[] = {
>       { "VGA", "global-vmstate", "true" },
>       { "vmware-svga", "global-vmstate", "true" },
>       { "qxl-vga", "global-vmstate", "true" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_12_len = G_N_ELEMENTS(hw_compat_2_12);
>   
> @@ -144,12 +155,14 @@ GlobalProperty hw_compat_2_11[] = {
>       { "virtio-blk-pci", "vectors", "2" },
>       { "vhost-user-blk-pci", "vectors", "2" },
>       { "e1000", "migrate_tso_props", "off" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_11_len = G_N_ELEMENTS(hw_compat_2_11);
>   
>   GlobalProperty hw_compat_2_10[] = {
>       { "virtio-mouse-device", "wheel-axis", "false" },
>       { "virtio-tablet-device", "wheel-axis", "false" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_10_len = G_N_ELEMENTS(hw_compat_2_10);
>   
> @@ -158,6 +171,7 @@ GlobalProperty hw_compat_2_9[] = {
>       { "intel-iommu", "pt", "off" },
>       { "virtio-net-device", "x-mtu-bypass-backend", "off" },
>       { "pcie-root-port", "x-migrate-msix", "false" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_9_len = G_N_ELEMENTS(hw_compat_2_9);
>   
> @@ -172,6 +186,7 @@ GlobalProperty hw_compat_2_8[] = {
>       { "virtio-pci", "x-pcie-pm-init", "off" },
>       { "cirrus-vga", "vgamem_mb", "8" },
>       { "isa-cirrus-vga", "vgamem_mb", "8" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_8_len = G_N_ELEMENTS(hw_compat_2_8);
>   
> @@ -181,6 +196,7 @@ GlobalProperty hw_compat_2_7[] = {
>       { "ioapic", "version", "0x11" },
>       { "intel-iommu", "x-buggy-eim", "true" },
>       { "virtio-pci", "x-ignore-backend-features", "on" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_7_len = G_N_ELEMENTS(hw_compat_2_7);
>   
> @@ -189,6 +205,7 @@ GlobalProperty hw_compat_2_6[] = {
>       /* Optional because not all virtio-pci devices support legacy mode */
>       { "virtio-pci", "disable-modern", "on",  .optional = true },
>       { "virtio-pci", "disable-legacy", "off", .optional = true },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_6_len = G_N_ELEMENTS(hw_compat_2_6);
>   
> @@ -198,6 +215,7 @@ GlobalProperty hw_compat_2_5[] = {
>       { "pvscsi", "x-disable-pcie", "on" },
>       { "vmxnet3", "x-old-msi-offsets", "on" },
>       { "vmxnet3", "x-disable-pcie", "on" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
>   
> @@ -205,6 +223,7 @@ GlobalProperty hw_compat_2_4[] = {
>       /* Optional because the 'scsi' property is Linux-only */
>       { "virtio-blk-device", "scsi", "true", .optional = true },
>       { "e1000", "extra_mac_registers", "off" },
> +    { "e1000", "init-vet", "off" },
>       { "virtio-pci", "x-disable-pcie", "on" },
>       { "virtio-pci", "migrate-extra", "off" },
>       { "fw_cfg_mem", "dma_enabled", "off" },
> @@ -222,10 +241,13 @@ GlobalProperty hw_compat_2_3[] = {
>       { "migration", "send-configuration", "off" },
>       { "migration", "send-section-footer", "off" },
>       { "migration", "store-global-state", "off" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_3_len = G_N_ELEMENTS(hw_compat_2_3);
>   
> -GlobalProperty hw_compat_2_2[] = {};
> +GlobalProperty hw_compat_2_2[] = {
> +    { "e1000", "init-vet", "off" },
> +};
>   const size_t hw_compat_2_2_len = G_N_ELEMENTS(hw_compat_2_2);
>   
>   GlobalProperty hw_compat_2_1[] = {
> @@ -236,6 +258,7 @@ GlobalProperty hw_compat_2_1[] = {
>       { "usb-mouse", "usb_version", "1" },
>       { "usb-kbd", "usb_version", "1" },
>       { "virtio-pci", "virtio-pci-bus-master-bug-migration", "on" },
> +    { "e1000", "init-vet", "off" },
>   };
>   const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
>   
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 4f75b44cfc..543c4fbb02 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -29,6 +29,7 @@
>   #include "hw/pci/pci.h"
>   #include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
> +#include "net/eth.h"
>   #include "net/net.h"
>   #include "net/checksum.h"
>   #include "sysemu/sysemu.h"
> @@ -130,10 +131,13 @@ struct E1000State_st {
>   #define E1000_FLAG_MIT_BIT 1
>   #define E1000_FLAG_MAC_BIT 2
>   #define E1000_FLAG_TSO_BIT 3
> +#define E1000_FLAG_VET_BIT 4
>   #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
>   #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
>   #define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT)
>   #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT)
> +#define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT)
> +
>       uint32_t compat_flags;
>       bool received_tx_tso;
>       bool use_tso_for_migration;
> @@ -141,7 +145,7 @@ struct E1000State_st {
>   };
>   typedef struct E1000State_st E1000State;
>   
> -#define chkflag(x)     (s->compat_flags & E1000_FLAG_##x)
> +#define chkflag(s, x)     (s->compat_flags & E1000_FLAG_##x)
>   
>   struct E1000BaseClass {
>       PCIDeviceClass parent_class;
> @@ -176,7 +180,7 @@ e1000_autoneg_done(E1000State *s)
>   static bool
>   have_autoneg(E1000State *s)
>   {
> -    return chkflag(AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
> +    return chkflag(s, AUTONEG) && (s->phy_reg[PHY_CTRL] & MII_CR_AUTO_NEG_EN);
>   }
>   
>   static void
> @@ -298,7 +302,7 @@ set_interrupt_cause(E1000State *s, int index, uint32_t val)
>           if (s->mit_timer_on) {
>               return;
>           }
> -        if (chkflag(MIT)) {
> +        if (chkflag(s, MIT)) {
>               /* Compute the next mitigation delay according to pending
>                * interrupts and the current values of RADV (provided
>                * RDTR!=0), TADV and ITR.
> @@ -386,6 +390,10 @@ static void e1000_reset(void *opaque)
>       }
>   
>       e1000x_reset_mac_addr(d->nic, d->mac_reg, macaddr);
> +
> +    if (chkflag(d, VET)) {
> +        d->mac_reg[VET] = ETH_P_VLAN;
> +    }
>   }
>   
>   static void
> @@ -1397,7 +1405,7 @@ static int e1000_pre_save(void *opaque)
>       }
>   
>       /* Decide which set of props to migrate in the main structure */
> -    if (chkflag(TSO) || !s->use_tso_for_migration) {
> +    if (chkflag(s, TSO) || !s->use_tso_for_migration) {
>           /* Either we're migrating with the extra subsection, in which
>            * case the mig_props is always 'props' OR
>            * we've not got the subsection, but 'props' was the last
> @@ -1418,7 +1426,7 @@ static int e1000_post_load(void *opaque, int version_id)
>       E1000State *s = opaque;
>       NetClientState *nc = qemu_get_queue(s->nic);
>   
> -    if (!chkflag(MIT)) {
> +    if (!chkflag(s, MIT)) {
>           s->mac_reg[ITR] = s->mac_reg[RDTR] = s->mac_reg[RADV] =
>               s->mac_reg[TADV] = 0;
>           s->mit_irq_level = false;
> @@ -1461,21 +1469,21 @@ static bool e1000_mit_state_needed(void *opaque)
>   {
>       E1000State *s = opaque;
>   
> -    return chkflag(MIT);
> +    return chkflag(s, MIT);
>   }
>   
>   static bool e1000_full_mac_needed(void *opaque)
>   {
>       E1000State *s = opaque;
>   
> -    return chkflag(MAC);
> +    return chkflag(s, MAC);
>   }
>   
>   static bool e1000_tso_state_needed(void *opaque)
>   {
>       E1000State *s = opaque;
>   
> -    return chkflag(TSO);
> +    return chkflag(s, TSO);
>   }


Let's introduce a e1000_vet_init_need(void *opaque)

Then we don't need to touch the other compatibility flags.

Thanks


>   
>   static const VMStateDescription vmstate_e1000_mit_state = {
> @@ -1737,6 +1745,8 @@ static Property e1000_properties[] = {
>                       compat_flags, E1000_FLAG_MAC_BIT, true),
>       DEFINE_PROP_BIT("migrate_tso_props", E1000State,
>                       compat_flags, E1000_FLAG_TSO_BIT, true),
> +    DEFINE_PROP_BIT("init-vet", E1000State,
> +                    compat_flags, E1000_FLAG_VET_BIT, true),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-22  8:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21  4:15 [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng
2021-07-21  4:15 ` [PATCH v3 2/3] hw/net: e1000e: " Bin Meng
2021-07-22  8:28   ` Jason Wang
2021-07-21  4:15 ` [PATCH v3 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng
2021-07-22  8:35 ` [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register Jason Wang

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.