Hello, Thanks for the patch!
The problem of infinite loop indeed exists in e1000e, however it is different from the one fixed in e1000. Please find my comments inline. ~Dmitry > On 7 Feb 2017, at 11:43 AM, Li Qiang <[email protected]> wrote: > > From: Li Qiang <[email protected]> > > This issue is the same as e1000 network card in this commit: > e1000: eliminate infinite loops on out-of-bounds transfer start. > > Signed-off-by: Li Qiang <[email protected]> > --- > hw/net/e1000e_core.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c > index 2b11499..53f2b1d 100644 > --- a/hw/net/e1000e_core.c > +++ b/hw/net/e1000e_core.c > @@ -914,7 +914,8 @@ e1000e_start_xmit(E1000ECore *core, const E1000E_TxRing > *txr) > struct e1000_tx_desc desc; > bool ide = false; > const E1000E_RingInfo *txi = txr->i; > - uint32_t cause = E1000_ICS_TXQE; > + uint32_t tdh_start = core->mac[txi->dh], cause = E1000_ICS_TXQE; > + > > if (!(core->mac[TCTL] & E1000_TCTL_EN)) { > trace_e1000e_tx_disabled(); > @@ -933,6 +934,14 @@ e1000e_start_xmit(E1000ECore *core, const E1000E_TxRing > *txr) > cause |= e1000e_txdesc_writeback(core, base, &desc, &ide, txi->idx); > > e1000e_ring_advance(core, txi, 1); > + > + /* > + * The following avoid infinite loop, just as the e1000 > + */ > + if (core->mac[txi->dh] == tdh_start || > + tdh_start >= core->mac[txi->dlen] / E1000_RING_DESC_LEN) { > + break; > + } Part of this validity check, namely tdh_start >= core->mac[txi->dlen] / E1000_RING_DESC_LEN) already done by e1000e_ring_advance(), therefore not needed here. The second part - full wraparound protection is relevant, but it fixes more symptoms than the cause. The only possible cause for full wraparound is r->dt (tail) value bigger or equal to r->dlen (length), so I would suggest to check for this in e1000e_ring_empty() and cover both TX and RX cases at once. > } > > if (!ide || !e1000e_intrmgr_delay_tx_causes(core, &cause)) { > @@ -1500,6 +1509,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct > NetRxPkt *pkt, > size_t desc_size; > size_t desc_offset = 0; > size_t iov_ofs = 0; > + uint32_t rdh_start; > > struct iovec *iov = net_rx_pkt_get_iovec(pkt); > size_t size = net_rx_pkt_get_total_len(pkt); > @@ -1509,6 +1519,7 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct > NetRxPkt *pkt, > bool do_ps = e1000e_do_ps(core, pkt, &ps_hdr_len); > > rxi = rxr->i; > + rdh_start = core->mac[rxi->dh]; > > do { > hwaddr ba[MAX_PS_BUFFERS]; > @@ -1605,6 +1616,10 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct > NetRxPkt *pkt, > e1000e_ring_advance(core, rxi, > core->rx_desc_len / E1000_MIN_RX_DESC_LEN); > > + if (core->mac[rxi->dh] == rdh_start || > + rdh_start >= core->mac[rxi->dlen] / E1000_RING_DESC_LEN) { > + break; > + } > } while (desc_offset < total_size); > > e1000e_update_rx_stats(core, size, total_size); > -- > 1.8.3.1 >
