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

[Hau <hau@realtek.com>]

> On 17.10.2022 19:23, Hau wrote:
> >> On 13.10.2022 08:04, Hau wrote:
> >>>> On 12.10.2022 09:59, Hau wrote:
> >>>>>>
> >>>>>> 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().
> >>>>>>>
> >>>>>>> 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);
> >>>>>>
> >>>>>> Is this correct anyway? Supposedly you want to set this bit to
> >>>>>> disable
> >> DMA.
> >>>>>>
> >>>>> If wol is enabled, driver need to disable hardware rxdv_gate for
> >>>>> receiving
> >>>> packets.
> >>>>>
> >>>> OK, I see. But why disable it here? I see no scenario where
> >>>> rxdv_gate would be enabled when we get here.
> >>>>
> >>> rxdv_gate will be enabled in rtl8169_cleanup(). When suspend or
> >>> close and wol is enabled driver will call rtl8169_down() ->
> >>> rtl8169_cleanup()->
> >> rtl_prepare_power_down()-> rtl_wol_enable_rx().
> >>> So disabled rxdv_gate in rtl_wol_enable_rx() for receiving packets.
> >>>
> >> rtl8169_cleanup() skips the call to rtl_enable_rxdvgate() when being
> >> called from
> >> rtl8169_down() and wol is enabled. This means rxdv gate is still disabled.
> >>
> > Yes, it will keep rxdv_gate disable. But it will also keep tx/rx on.
> > If OS have an  unexpected reboot hardware  may dma to invalid memory
> > address. If possible I prefer to keep tx/rx off when exit driver control.
> >
> 
> When you say "keep tx/rx off", do you refer to the rxconfig bits in register
> RxConfig, or to CmdTxEnb and CmdRxEnb in ChipCmd?
> 
> If we talk about the first option, then my guess would be:
> According to rtl_wol_enable_rx() the rx config bits are required for WoL to
> work on certain chip versions. With the introduction of rxdvgate this changed
> and setting these bits isn't needed any longer.
> I tested on RTL8168h and WoL worked w/o the Accept bits set in RxConfig.
> Please confirm or correct my understanding.
> 
> 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); }
> 
> 
We expect wol  packet should be filtered by Accept bits set in RxConfig. 
But for magic packet wakeup it seems it does not perform as expected. 
We need some time to figure this out. Once we have any update we will let you know.

 ------Please consider the environment before printing this e-mail.