On Sun, 20 Nov 2022 at 23:17, Jason Wang <jasow...@redhat.com> wrote: > > On Fri, Nov 18, 2022 at 12:56 AM Stefan Hajnoczi <stefa...@redhat.com> wrote: > > > > The device turns the Tx Descriptor into a Tx Status descriptor after > > fully reading the descriptor. This involves clearing Tx Own (bit 31) to > > indicate that the driver has ownership of the descriptor again as well > > as several other bits. > > > > The code keeps the first dword of the Tx Descriptor in the txdw0 local > > variable. txdw0 is reused to build the first word of the Tx Status > > descriptor. Later on the code uses txdw0 again, incorrectly assuming > > that it still contains the first dword of the Tx Descriptor. The tx > > offloading code misbehaves because it sees bogus bits in txdw0. > > (This is only noticeable with patch 2).
Yes, although the large_send_mss variable is already junk because some bits have been cleared: int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK; Luckily it's not used yet, aside from DPRINTF(). > > > > > Use a separate local variable for Tx Status and preserve Tx Descriptor > > in txdw0. > > > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > > Acked-by: Jason Wang <jasow...@redhat.com> > > > --- > > hw/net/rtl8139.c | 17 ++++++++++------- > > 1 file changed, 10 insertions(+), 7 deletions(-) > > > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > > index e6643e3c9d..ffef3789b5 100644 > > --- a/hw/net/rtl8139.c > > +++ b/hw/net/rtl8139.c > > @@ -2027,18 +2027,21 @@ static int rtl8139_cplus_transmit_one(RTL8139State > > *s) > > s->currCPlusTxDesc = 0; > > } > > > > + /* Build the Tx Status Descriptor */ > > + uint32_t tx_status = txdw0; > > + > > /* transfer ownership to target */ > > - txdw0 &= ~CP_TX_OWN; > > + tx_status &= ~CP_TX_OWN; > > > > /* reset error indicator bits */ > > - txdw0 &= ~CP_TX_STATUS_UNF; > > - txdw0 &= ~CP_TX_STATUS_TES; > > - txdw0 &= ~CP_TX_STATUS_OWC; > > - txdw0 &= ~CP_TX_STATUS_LNKF; > > - txdw0 &= ~CP_TX_STATUS_EXC; > > + tx_status &= ~CP_TX_STATUS_UNF; > > + tx_status &= ~CP_TX_STATUS_TES; > > + tx_status &= ~CP_TX_STATUS_OWC; > > + tx_status &= ~CP_TX_STATUS_LNKF; > > + tx_status &= ~CP_TX_STATUS_EXC; > > > > /* update ring data */ > > - val = cpu_to_le32(txdw0); > > + val = cpu_to_le32(tx_status); > > pci_dma_write(d, cplus_tx_ring_desc, (uint8_t *)&val, 4); > > > > /* Now decide if descriptor being processed is holding the last > > segment of packet */ > > -- > > 2.38.1 > > > >