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] [day] [month] [year] [list]

Message-ID: <CALs4sv3BTPo-FxNfJJwtNYoMti45sai3VonhKC9Cyp9khJegwQ@mail.gmail.com>
Date: Fri, 3 Nov 2023 09:18:36 +0530
From: Pavan Chebbi <pavan.chebbi@...adcom.com>
To: Michael Chan <michael.chan@...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 11:19 PM Michael Chan <michael.chan@...adcom.com> wrote:
>
> 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.

Agree. So let us have this change in. George, can you pls spin a v2
with proper tagging/addressing done to the patch.
Please see https://docs.kernel.org/process/submitting-patches.html for
help. Thanks.

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