netdev - Re: [PATCH 1/3] tg3: Avoid setting power.can_wakeup for devices that cannot wake up

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: <20110210204844.GA4158@mcarlson.broadcom.com>
Date:	Thu, 10 Feb 2011 12:48:44 -0800
From:	"Matt Carlson" <mcarlson@...adcom.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"David Miller" <davem@...emloft.net>,
	"Matthew Carlson" <mcarlson@...adcom.com>,
	"Michael Chan" <mchan@...adcom.com>,
	"Linux PM mailing list" <linux-pm@...ts.linux-foundation.org>,
	"Thomas Fjellstrom" <thomas@...llstrom.ca>,
	"Jay Cliburn" <jcliburn@...il.com>,
	"Chris Snook" <chris.snook@...il.com>,
	"Jie Yang" <jie.yang@...eros.com>
Subject: Re: [PATCH 1/3] tg3: Avoid setting power.can_wakeup for devices
 that cannot wake up

On Thu, Feb 10, 2011 at 08:53:09AM -0800, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@...k.pl>
> 
> The tg3 driver uses device_init_wakeup() in such a way that the
> device's power.can_wakeup flag may be set even though the PCI
> subsystem cleared it before, in which case the device cannot wake
> up the system from sleep states.  Modify the driver to only change
> the power.can_wakeup flag if the device is not capable of generating
> wakeup signals.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
> ---
>  drivers/net/tg3.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/net/tg3.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/tg3.c
> +++ linux-2.6/drivers/net/tg3.c
> @@ -12403,9 +12403,11 @@ static void __devinit tg3_get_eeprom_hw_
>  			tp->tg3_flags3 |= TG3_FLG3_RGMII_EXT_IBND_TX_EN;
>  	}
>  done:
> -	device_init_wakeup(&tp->pdev->dev, tp->tg3_flags & TG3_FLAG_WOL_CAP);
> -	device_set_wakeup_enable(&tp->pdev->dev,
> +	if (tp->tg3_flags & TG3_FLAG_WOL_CAP)
> +		device_set_wakeup_enable(&tp->pdev->dev,
>  				 tp->tg3_flags & TG3_FLAG_WOL_ENABLE);
> +	else
> +		device_set_wakeup_capable(&tp->pdev->dev, false);

I did this because I couldn't see where 'can_wakeup' gets set.  I don't
see a call to device_init_wakeup() that would be relevant to tg3
devices.  I do see a couple calls to device_set_wakeup_capable() in
acpi/glue.c and acpi/scan.c.  Is that the place?

>  }
>  
>  static int __devinit tg3_issue_otp_command(struct tg3 *tp, u32 cmd)

This is something I was always curious about too.  The TG3_FLAG_WOL_CAP
tracks whether or not the device can handle WOL.  Is it safe to do away
with this flag and lean on the 'can_wakeup' flag instead?  My concern is
that some other part of the system might enable that flag after
tg3_get_invariants() has run.  If that happens and the device isn't
really WOL capable bad things can occur.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html