[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-7.2 v3 1/3] rtl8139: avoid clobbering tx descriptor bits
From: |
Jason Wang |
Subject: |
Re: [PATCH for-7.2 v3 1/3] rtl8139: avoid clobbering tx descriptor bits |
Date: |
Mon, 21 Nov 2022 12:16:02 +0800 |
On Fri, Nov 18, 2022 at 12:56 AM Stefan Hajnoczi <stefanha@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).
>
> Use a separate local variable for Tx Status and preserve Tx Descriptor
> in txdw0.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Jason Wang <jasowang@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
>