[PATCH 1/3] net: designware: Disable DMA on removal

[Simon Glass]

Hi Jonas,

On Sun, 6 Apr 2025 at 20:50, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2025-04-06 00:12, Simon Glass wrote:
> > At present, removing the device frees struct dw_eth_dev but does not
> > disable DMA. When DMA is active, packets can be received even if the
> > driver is not active.
> >
> > While it is possible that the memory used by the struct may remain
> > untouched after removal, any other allocation may reuse that memory
> > and result in DMA writing to random addresses.
> >
> > In most case U-Boot does this removal in announce_and_cleanup()
> > immediately before jumping to the OS. No further allocations are done
> > after that point.
> >
> > But with EFI_LOADER this function is not called before jumping to the
> > EFI app. U-Boot continues to run while GRUB starts and finishes, as well
> > as while Linux is starting up. During this time, devices cannot be
> > removed, as they are in use. U-Boot completes the removal when
> > efi_exit_boot_services() is called. It seems that this function does
> > more allocations after calling dm_remove_devices_active() although I
> > can't figure out where.
> >
> > Fix this by disabling DMA when the driver is removed.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reported-by: Christian Kohlschütter <christian at kohlschutter.com>
> > ---
> >
> >  drivers/net/designware.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/designware.c b/drivers/net/designware.c
> > index 07b0f49ef58..eebf14bd51a 100644
> > --- a/drivers/net/designware.c
> > +++ b/drivers/net/designware.c
> > @@ -809,6 +809,7 @@ static int designware_eth_remove(struct udevice *dev)
> >  {
> >       struct dw_eth_dev *priv = dev_get_priv(dev);
> >
> > +     _dw_eth_halt(priv);
>
> This should not really be needed, eth-uclass already call the stop() ops
> that in turn call _dw_eth_halt() from the eth-uclass pre_remove() ops.
>
> So ethernet should already have halted when remove() is called, calling
> it here will probably only hide any real issue.

Yes that seems right, thanks.

Regards,
Simon