net: e1000: Correct the initial value of VET register

Jason Wang Thu, 22 Jul 2021 01:35:50 -0700


在 2021/7/21 下午12:15, Bin Meng 写道:

From: Christina Wang <christina.w...@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.carlst...@windriver.com>
Signed-off-by: Christina Wang <christina.w...@windriver.com>
Signed-off-by: Bin Meng <bin.m...@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(),
  };

Re: [PATCH v3 1/3] hw/net: e1000: Correct the initial value of VET register

Reply via email to