Re: [PATCH net] r8169: fix rtl8125b dmar pte write access not set error

[Heiner Kallweit <hkallweit1@gmail.com>]

On 04.10.2022 10:10, Chunhao Lin wrote:
> When close device, rx will be enabled if wol is enabeld. When open device
> it will cause rx to dma to wrong address after pci_set_master().
> 
Hi Hau,

I never experienced this problem. Is it an edge case that can occur under
specific circumstances?

> In this patch, driver will disable tx/rx when close device. If wol is
> eanbled only enable rx filter and disable rxdv_gate to let hardware
> can receive packet to fifo but not to dma it.
> 
> Fixes: 120068481405 ("r8169: fix failing WoL")
> Signed-off-by: Chunhao Lin <hau@realtek.com>
> ---
>  drivers/net/ethernet/realtek/r8169_main.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 1b7fdb4f056b..c09cfbe1d3f0 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -2239,6 +2239,9 @@ static void rtl_wol_enable_rx(struct rtl8169_private *tp)
>  	if (tp->mac_version >= RTL_GIGA_MAC_VER_25)
>  		RTL_W32(tp, RxConfig, RTL_R32(tp, RxConfig) |
>  			AcceptBroadcast | AcceptMulticast | AcceptMyPhys);
> +
> +	if (tp->mac_version >= RTL_GIGA_MAC_VER_40)
> +		RTL_W32(tp, MISC, RTL_R32(tp, MISC) & ~RXDV_GATED_EN);

In the commit title you reference RTL8125b only, but the actual change
affects all chip versions from RTL8168g. So either title or patch need to be
adjusted. Is the actual issue restricted to RTL8125b (hw issue?) or can it
occur on all chip versions that use RXDV_GATED_EN?

>  }
>  
>  static void rtl_prepare_power_down(struct rtl8169_private *tp)
> @@ -3981,7 +3984,7 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
>  	netdev_reset_queue(tp->dev);
>  }
>  
> -static void rtl8169_cleanup(struct rtl8169_private *tp, bool going_down)
> +static void rtl8169_cleanup(struct rtl8169_private *tp)
>  {
>  	napi_disable(&tp->napi);
>  
> @@ -3993,9 +3996,6 @@ static void rtl8169_cleanup(struct rtl8169_private *tp, bool going_down)
>  
>  	rtl_rx_close(tp);
>  
> -	if (going_down && tp->dev->wol_enabled)
> -		goto no_reset;
> -

Here you change the behavior for various other chip versions too. This should not be done
in a fix, even if it should be safe.

>  	switch (tp->mac_version) {
>  	case RTL_GIGA_MAC_VER_28:
>  	case RTL_GIGA_MAC_VER_31:
> @@ -4016,7 +4016,7 @@ static void rtl8169_cleanup(struct rtl8169_private *tp, bool going_down)
>  	}
>  
>  	rtl_hw_reset(tp);
> -no_reset:
> +
>  	rtl8169_tx_clear(tp);
>  	rtl8169_init_ring_indexes(tp);
>  }
> @@ -4027,7 +4027,7 @@ static void rtl_reset_work(struct rtl8169_private *tp)
>  
>  	netif_stop_queue(tp->dev);
>  
> -	rtl8169_cleanup(tp, false);
> +	rtl8169_cleanup(tp);
>  
>  	for (i = 0; i < NUM_RX_DESC; i++)
>  		rtl8169_mark_to_asic(tp->RxDescArray + i);
> @@ -4715,7 +4715,7 @@ static void rtl8169_down(struct rtl8169_private *tp)
>  	pci_clear_master(tp->pci_dev);
>  	rtl_pci_commit(tp);
>  
> -	rtl8169_cleanup(tp, true);
> +	rtl8169_cleanup(tp);
>  	rtl_disable_exit_l1(tp);
>  	rtl_prepare_power_down(tp);
>  }