On 2022/9/9 10:40, Jason Wang wrote:
On Sat, Aug 27, 2022 at 12:06 AM Ding Hui <ding...@sangfor.com.cn> wrote:Like commit 034d00d48581 ("e1000: set RX descriptor status in a separate operation"), there is also same issue in e1000e, which would cause lost packets or stop sending packets to VM with DPDK. Do similar fix in e1000e. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/402 Signed-off-by: Ding Hui <ding...@sangfor.com.cn> --- hw/net/e1000e_core.c | 54 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 208e3e0d79..b8038e4014 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -1364,6 +1364,58 @@ struct NetRxPkt *pkt, const E1000E_RSSInfo *rss_info, } } +static inline void +e1000e_pci_dma_write_rx_desc(E1000ECore *core, dma_addr_t addr, + uint8_t *desc, dma_addr_t len) +{ + PCIDevice *dev = core->owner; + + if (e1000e_rx_use_legacy_descriptor(core)) { + struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc; + size_t offset = offsetof(struct e1000_rx_desc, status); + typeof(d->status) status = d->status; + + d->status &= ~E1000_RXD_STAT_DD; + pci_dma_write(dev, addr, desc, len); + + if (status & E1000_RXD_STAT_DD) { + d->status = status; + pci_dma_write(dev, addr + offset, &status, sizeof(status)); + } + } else { + if (core->mac[RCTL] & E1000_RCTL_DTYP_PS) { + union e1000_rx_desc_packet_split *d = + (union e1000_rx_desc_packet_split *) desc; + size_t offset = offsetof(union e1000_rx_desc_packet_split, + wb.middle.status_error); + typeof(d->wb.middle.status_error) status = + d->wb.middle.status_error;Any reason to use typeof here? Its type is known to be uint32_t?
My intention was using exact type same with struct member status_error, which is indeed uint32_t now. If the type of status_error in struct be changed in some case, we do not need to change everywhere.
Maybe I worry too much, the struct is related to h/w spec, so it is unlikely be changed in the future.
Should I send v2 to use uint32_t directly? I'm also OK with it.
+ + d->wb.middle.status_error &= ~E1000_RXD_STAT_DD; + pci_dma_write(dev, addr, desc, len); + + if (status & E1000_RXD_STAT_DD) { + d->wb.middle.status_error = status; + pci_dma_write(dev, addr + offset, &status, sizeof(status)); + } + } else { + union e1000_rx_desc_extended *d = + (union e1000_rx_desc_extended *) desc; + size_t offset = offsetof(union e1000_rx_desc_extended, + wb.upper.status_error); + typeof(d->wb.upper.status_error) status = d->wb.upper.status_error;So did here. Thanks+ + d->wb.upper.status_error &= ~E1000_RXD_STAT_DD; + pci_dma_write(dev, addr, desc, len); + + if (status & E1000_RXD_STAT_DD) { + d->wb.upper.status_error = status; + pci_dma_write(dev, addr + offset, &status, sizeof(status)); + } + } + } +} + typedef struct e1000e_ba_state_st { uint16_t written[MAX_PS_BUFFERS]; uint8_t cur_idx; @@ -1600,7 +1652,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt, e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL, rss_info, do_ps ? ps_hdr_len : 0, &bastate.written); - pci_dma_write(d, base, &desc, core->rx_desc_len); + e1000e_pci_dma_write_rx_desc(core, base, desc, core->rx_desc_len); e1000e_ring_advance(core, rxi, core->rx_desc_len / E1000_MIN_RX_DESC_LEN); -- 2.17.1
-- Thanks, - Ding Hui
