| Message ID | 589996bb.02482e0a.10629.5b0e@mx.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <liq3ea@gmail.com> wrote: > > From: Li Qiang <liqiang6-s@360.cn> > > 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 <liqiang6-s@360.cn> > --- > 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 >
Hello, 2017-02-08 16:38 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>: > 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. > > If I read the code correctly, I think this issue is the same. Could you please explain more? Thanks. > ~Dmitry > > > On 7 Feb 2017, at 11:43 AM, Li Qiang <liq3ea@gmail.com> wrote: > > > > From: Li Qiang <liqiang6-s@360.cn> > > > > 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 <liqiang6-s@360.cn> > > --- > > 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. > > e1000e_ring_advance() check the added r->dh. Not the original one. > 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. > > Do you mean 'core->mac[r->dt] >= core->mac[r->dlen]' indicate an empty ring? > > } > > > > 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 > > > >
> On 8 Feb 2017, at 11:30 AM, Li Qiang <liq3ea@gmail.com> wrote: > > Hello, > > 2017-02-08 16:38 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com <mailto:dmitry@daynix.com>>: > 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. > > > If I read the code correctly, I think this issue is the same. Could you please explain more? Thanks. I meant that code changes needed to fix it are slightly different. > > ~Dmitry > > > On 7 Feb 2017, at 11:43 AM, Li Qiang <liq3ea@gmail.com <mailto:liq3ea@gmail.com>> wrote: > > > > From: Li Qiang <liqiang6-s@360.cn <mailto:liqiang6-s@360.cn>> > > > > 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 <liqiang6-s@360.cn <mailto:liqiang6-s@360.cn>> > > --- > > 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. > > > e1000e_ring_advance() check the added r->dh. Not the original one. Oh, I see now. Right, it does not make sense to check for wraparound in case original TDH was out of bounds. Now I see there is one more problem - in case TDH is out of the ring, e1000e_ring_head_descr() called from e1000e_start_xmit() will do out of bounds read, so TDH should be validated prior to reading the descriptor as well. Similar problem exists on RX also. Besides that, as I see now, e1000e_ring_empty() is not called on RX so validation should be done separately for TX and RX rings. A function like e1000e_ring_validate() is needed... > > 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. > > > Do you mean 'core->mac[r->dt] >= core->mac[r->dlen]' indicate an empty ring? > > > } > > > > 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 > >
Hello Dmitry, 2017-02-08 18:01 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>: > > On 8 Feb 2017, at 11:30 AM, Li Qiang <liq3ea@gmail.com> wrote: > > Hello, > > 2017-02-08 16:38 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>: > >> 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. >> >> > If I read the code correctly, I think this issue is the same. Could you > please explain more? Thanks. > > > I meant that code changes needed to fix it are slightly different. > Got. > > > >> ~Dmitry >> >> > On 7 Feb 2017, at 11:43 AM, Li Qiang <liq3ea@gmail.com> wrote: >> > >> > From: Li Qiang <liqiang6-s@360.cn> >> > >> > 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 <liqiang6-s@360.cn> >> > --- >> > 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. >> >> > e1000e_ring_advance() check the added r->dh. Not the original one. > > > Oh, I see now. Right, it does not make sense to check for wraparound in > case original TDH was out of bounds. > > Now I see there is one more problem - in case TDH is out of the > ring, e1000e_ring_head_descr() > called from e1000e_start_xmit() will do out of bounds read, so TDH should > be validated prior to > reading the descriptor as well. > > IIUC you mean pci_dma_read will do oob read, right? I think this is not a issue as this is reading data from guest And guest can provide arbitrary data. For example, just provide 10 bytes buffer, but tell the qemu it has 1000. > Similar problem exists on RX also. > > Besides that, as I see now, e1000e_ring_empty() is not called on RX > so validation should be done separately for TX and RX rings. > > A function like e1000e_ring_validate() is needed... > > > >> 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. >> >> > Do you mean 'core->mac[r->dt] >= core->mac[r->dlen]' indicate an empty > ring? > > > > So I still confused by this 'full wrapparound', any more explain? > > } >> > >> > 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 >> > >> > >
> On 8 Feb 2017, at 12:17 PM, Li Qiang <liq3ea@gmail.com> wrote: > > Hello Dmitry, > > > 2017-02-08 18:01 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>: > >> On 8 Feb 2017, at 11:30 AM, Li Qiang <liq3ea@gmail.com> wrote: >> >> Hello, >> >> 2017-02-08 16:38 GMT+08:00 Dmitry Fleytman <dmitry@daynix.com>: >> 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. >> >> >> If I read the code correctly, I think this issue is the same. Could you please explain more? Thanks. > > I meant that code changes needed to fix it are slightly different. > > Got. > > >> >> ~Dmitry >> >> > On 7 Feb 2017, at 11:43 AM, Li Qiang <liq3ea@gmail.com> wrote: >> > >> > From: Li Qiang <liqiang6-s@360.cn> >> > >> > 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 <liqiang6-s@360.cn> >> > --- >> > 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. >> >> >> e1000e_ring_advance() check the added r->dh. Not the original one. > > Oh, I see now. Right, it does not make sense to check for wraparound in case original TDH was out of bounds. > > Now I see there is one more problem - in case TDH is out of the ring, e1000e_ring_head_descr() > called from e1000e_start_xmit() will do out of bounds read, so TDH should be validated prior to > reading the descriptor as well. > > > IIUC you mean pci_dma_read will do oob read, right? I think this is not a issue as this is reading data from guest > And guest can provide arbitrary data. For example, just provide 10 bytes buffer, but tell the qemu it has 1000. Agree. > > Similar problem exists on RX also. > > Besides that, as I see now, e1000e_ring_empty() is not called on RX > so validation should be done separately for TX and RX rings. > > A function like e1000e_ring_validate() is needed... > >> >> 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. >> >> >> Do you mean 'core->mac[r->dt] >= core->mac[r->dlen]' indicate an empty ring? >> > > So I still confused by this 'full wrapparound', any more explain? Yes, I would indicate an empty ring when r->dt in invalid. That would do the job. Having e1000e_ring_empty() fixed, you might call it from e1000e_write_packet_to_guest() to fix RX issues as well. > >> > } >> > >> > 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 >> >
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; + } } 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);