[Top][All Lists]

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

Re: [PATCH 2/3] hw/net: e1000e: Correct the initial value of VET registe

From: Jason Wang
Subject: Re: [PATCH 2/3] hw/net: e1000e: Correct the initial value of VET register
Date: Fri, 2 Jul 2021 15:01:04 +0800
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

在 2021/7/2 下午2:12, Bin Meng 写道:
On Fri, Jul 2, 2021 at 1:47 PM Jason Wang <jasowang@redhat.com> wrote:

在 2021/7/2 下午12:43, Bin Meng 写道:
On Fri, Jul 2, 2021 at 11:29 AM Jason Wang <jasowang@redhat.com> wrote:
在 2021/7/1 下午5:46, 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>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

    hw/net/e1000e_core.h |  2 --
    hw/net/e1000e.c      |  6 ++----
    hw/net/e1000e_core.c | 11 ++++++-----
    3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
index 4ddb4d2c39..07d722bc68 100644
--- a/hw/net/e1000e_core.h
+++ b/hw/net/e1000e_core.h
@@ -105,8 +105,6 @@ struct E1000Core {
        uint32_t itr_guest_value;
        uint32_t eitr_guest_value[E1000E_MSIX_VEC_NUM];

-    uint16_t vet;
        uint8_t permanent_mac[ETH_ALEN];

        NICState *owner_nic;
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index a8a77eca95..1797e4a7cb 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -602,8 +602,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[]) {
@@ -645,8 +645,6 @@ static const VMStateDescription e1000e_vmstate = {
            VMSTATE_UINT32_ARRAY(core.eitr_guest_value, E1000EState,

-        VMSTATE_UINT16(core.vet, E1000EState),
This is not the suggested way. We'd better not bump version in this case.

How about update vet during post_load?
But core.vet is removed in this patch. Not sure how to handle this?

Keep using core.vet, sync core.vet with mac[VET] during post_load.

But keeping using core.vet in the e1000e_core.c will cause mismatch
with mac[VET] as the commit message says.

We can still keep the 'vet' field in the "struct E1000Core", and keep
the "VMSTATE_UINT16(core.vet, E1000EState)" here, but it is useless in
the new code.

The point is to unbreak migration to old version.



reply via email to

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