* [PATCH v4 1/3] hw/net: e1000: Correct the initial value of VET register @ 2021-07-23 7:55 Bin Meng 2021-07-23 7:55 ` [PATCH v4 2/3] hw/net: e1000e: " Bin Meng 2021-07-23 7:55 ` [PATCH v4 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng 0 siblings, 2 replies; 5+ messages in thread From: Bin Meng @ 2021-07-23 7:55 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 v4: - Only keep hw_compat_6_0[] changes - Use e1000_vet_init_need() instead of touching other flags Changes in v3: - add a "init-vet" property for versioned machines hw/core/machine.c | 1 + hw/net/e1000.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/hw/core/machine.c b/hw/core/machine.c index 775add0795..f98a797e43 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); diff --git a/hw/net/e1000.c b/hw/net/e1000.c index 4f75b44cfc..a30546c5d5 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; @@ -361,6 +365,13 @@ e1000_autoneg_timer(void *opaque) } } +static bool e1000_vet_init_need(void *opaque) +{ + E1000State *s = opaque; + + return chkflag(VET); +} + static void e1000_reset(void *opaque) { E1000State *d = opaque; @@ -386,6 +397,10 @@ static void e1000_reset(void *opaque) } e1000x_reset_mac_addr(d->nic, d->mac_reg, macaddr); + + if (e1000_vet_init_need(d)) { + d->mac_reg[VET] = ETH_P_VLAN; + } } static void @@ -1737,6 +1752,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 v4 2/3] hw/net: e1000e: Correct the initial value of VET register 2021-07-23 7:55 [PATCH v4 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng @ 2021-07-23 7:55 ` Bin Meng 2021-07-30 1:21 ` Bin Meng 2021-07-23 7:55 ` [PATCH v4 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng 1 sibling, 1 reply; 5+ messages in thread From: Bin Meng @ 2021-07-23 7:55 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 v4: - Only keep hw_compat_6_0[] changes 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 | 1 + hw/net/e1000e.c | 8 +++++++- hw/net/e1000e_core.c | 9 ++++----- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index f98a797e43..943974d411 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); 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
* Re: [PATCH v4 2/3] hw/net: e1000e: Correct the initial value of VET register 2021-07-23 7:55 ` [PATCH v4 2/3] hw/net: e1000e: " Bin Meng @ 2021-07-30 1:21 ` Bin Meng 2021-08-02 4:05 ` Jason Wang 0 siblings, 1 reply; 5+ messages in thread From: Bin Meng @ 2021-07-30 1:21 UTC (permalink / raw) To: Jason Wang, qemu-devel@nongnu.org Developers Hi Jason, On Fri, Jul 23, 2021 at 3:55 PM Bin Meng <bmeng.cn@gmail.com> wrote: > > 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 v4: > - Only keep hw_compat_6_0[] changes > > 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 | 1 + > hw/net/e1000e.c | 8 +++++++- > hw/net/e1000e_core.c | 9 ++++----- > 3 files changed, 12 insertions(+), 6 deletions(-) > Will this series be in 6.1? Regards, Bin ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 2/3] hw/net: e1000e: Correct the initial value of VET register 2021-07-30 1:21 ` Bin Meng @ 2021-08-02 4:05 ` Jason Wang 0 siblings, 0 replies; 5+ messages in thread From: Jason Wang @ 2021-08-02 4:05 UTC (permalink / raw) To: Bin Meng, qemu-devel@nongnu.org Developers 在 2021/7/30 上午9:21, Bin Meng 写道: > Hi Jason, > > On Fri, Jul 23, 2021 at 3:55 PM Bin Meng <bmeng.cn@gmail.com> wrote: >> 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 v4: >> - Only keep hw_compat_6_0[] changes >> >> 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 | 1 + >> hw/net/e1000e.c | 8 +++++++- >> hw/net/e1000e_core.c | 9 ++++----- >> 3 files changed, 12 insertions(+), 6 deletions(-) >> > Will this series be in 6.1? > > Regards, > Bin Yes, I've applied them. Thanks ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor 2021-07-23 7:55 [PATCH v4 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng 2021-07-23 7:55 ` [PATCH v4 2/3] hw/net: e1000e: " Bin Meng @ 2021-07-23 7:55 ` Bin Meng 1 sibling, 0 replies; 5+ messages in thread From: Bin Meng @ 2021-07-23 7:55 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
end of thread, other threads:[~2021-08-02 4:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-23 7:55 [PATCH v4 1/3] hw/net: e1000: Correct the initial value of VET register Bin Meng 2021-07-23 7:55 ` [PATCH v4 2/3] hw/net: e1000e: " Bin Meng 2021-07-30 1:21 ` Bin Meng 2021-08-02 4:05 ` Jason Wang 2021-07-23 7:55 ` [PATCH v4 3/3] hw/net: e1000e: Don't zero out the VLAN tag in the legacy RX descriptor Bin Meng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).