netdev - Re: [PATCH] [PATCH net] tg3: power down device only on SYSTEM_POWER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACKFLi=0eRFGfGXBihLKwZaifKJAVWR5tK9HM82pyjDNA2r7pw@mail.gmail.com>
Date: Thu, 2 Nov 2023 10:49:36 -0700
From: Michael Chan <michael.chan@...adcom.com>
To: Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc: George Shuklin <george.shuklin@...il.com>, netdev@...r.kernel.org, 
	Andrew Gospodarek <andrew.gospodarek@...adcom.com>
Subject: Re: [PATCH] [PATCH net] tg3: power down device only on SYSTEM_POWER_OFF

On Thu, Nov 2, 2023 at 10:11 AM Pavan Chebbi <pavan.chebbi@...adcom.com> wrote:
>
> On Thu, Nov 2, 2023 at 7:44 PM George Shuklin <george.shuklin@...il.com> wrote:
> >
> > I'm right now with dell support, and what they asked is to 'try this on
> > supported distros', which at newest are 5.15. I'll try to bypass their
> > L1 with Ubuntu + HWE to get to 6+ versions...
> >
> > I was able to reproduce hanging at reboot there (without ACPI messages),
> > and patching helps there too.
> >
> OK. I am not too sure what we should do. The change as such looks fine to me.
> Of course, the patch needs proper tags (tree.fixes, cc ...)
>
> @Michael : Do you have any suggestions on this?

I think that this logic:

if (system_state == SYSTEM_POWER_OFF)
        tg3_power_down(tp);

is correct in the shutdown method.  Many other drivers do the same
thing.  tg3_power_down() is just to enable WoL if it should be enabled
and to put the device in D3hot state.

The original commit to change this (2ca1c94ce0b6 tg3: Disable tg3
device on system reboot to avoid triggering AER) claims that calling
tg3_power_down() unconditionally is to avoid getting spurious MSI.
But the dev_close() call in tg3_shutdown() should have shut everything
down including MSI.  So I don't think it is necessary to call
tg3_power_down() unconditionally.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4209 bytes)