| 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
| ||
|
Date: Mon, 30 Nov 2020 14:31:05 -0800
From: Alexander Duyck <alexander.duyck@...il.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Mario Limonciello <mario.limonciello@...l.com>,
Netdev <netdev@...r.kernel.org>,
Stefan Assmann <sassmann@...hat.com>,
"Neftin, Sasha" <sasha.neftin@...el.com>,
Yijun Shen <Yijun.shen@...l.com>
Subject: Re: [net-next 2/4] e1000e: Add Dell's Comet Lake systems into s0ix heuristics
On Mon, Nov 30, 2020 at 1:32 PM Tony Nguyen <anthony.l.nguyen@...el.com> wrote:
>
> From: Mario Limonciello <mario.limonciello@...l.com>
>
> Dell's Comet Lake Latitude and Precision systems containing i219LM are
> properly configured and should use the s0ix flows.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@...l.com>
> Tested-by: Yijun Shen <Yijun.shen@...l.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
> drivers/net/ethernet/intel/Kconfig | 1 +
> drivers/net/ethernet/intel/e1000e/param.c | 80 ++++++++++++++++++++++-
> 2 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
> index 5aa86318ed3e..280af47d74d2 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -58,6 +58,7 @@ config E1000
> config E1000E
> tristate "Intel(R) PRO/1000 PCI-Express Gigabit Ethernet support"
> depends on PCI && (!SPARC32 || BROKEN)
> + depends on DMI
> select CRC32
> imply PTP_1588_CLOCK
> help
Is DMI the only way we can identify systems that want to enable S0ix
states? I'm just wondering if we could identify these parts using a 4
tuple device ID or if the DMI ID is the only way we can do this?
> diff --git a/drivers/net/ethernet/intel/e1000e/param.c b/drivers/net/ethernet/intel/e1000e/param.c
> index 56316b797521..d05f55201541 100644
> --- a/drivers/net/ethernet/intel/e1000e/param.c
> +++ b/drivers/net/ethernet/intel/e1000e/param.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright(c) 1999 - 2018 Intel Corporation. */
>
> +#include <linux/dmi.h>
> #include <linux/netdevice.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> @@ -201,6 +202,80 @@ static const struct e1000e_me_supported me_supported[] = {
> {0}
> };
>
> +static const struct dmi_system_id s0ix_supported_systems[] = {
> + {
> + /* Dell Latitude 5310 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "099F"),
> + },
> + },
> + {
> + /* Dell Latitude 5410 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09A0"),
> + },
> + },
> + {
> + /* Dell Latitude 5410 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C9"),
> + },
> + },
> + {
> + /* Dell Latitude 5510 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09A1"),
> + },
> + },
> + {
> + /* Dell Precision 3550 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09A2"),
> + },
> + },
> + {
> + /* Dell Latitude 5411 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C0"),
> + },
> + },
> + {
> + /* Dell Latitude 5511 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C1"),
> + },
> + },
> + {
> + /* Dell Precision 3551 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C2"),
> + },
> + },
> + {
> + /* Dell Precision 7550 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C3"),
> + },
> + },
> + {
> + /* Dell Precision 7750 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> + DMI_MATCH(DMI_PRODUCT_SKU, "09C4"),
> + },
> + },
> + { }
> +};
> +
So which "product" are we verifying here? Are these SKU values for the
platform or for the NIC? Just wondering if this is something we could
retrieve via PCI as I mentioned or if this is something that can only
be retrieved via DMI.
> static bool e1000e_check_me(u16 device_id)
> {
> struct e1000e_me_supported *id;
> @@ -599,8 +674,11 @@ void e1000e_check_options(struct e1000_adapter *adapter)
> }
>
> if (enabled == S0IX_HEURISTICS) {
> + /* check for allowlist of systems */
> + if (dmi_check_system(s0ix_supported_systems))
> + enabled = S0IX_FORCE_ON;
> /* default to off for ME configurations */
> - if (e1000e_check_me(hw->adapter->pdev->device))
> + else if (e1000e_check_me(hw->adapter->pdev->device))
> enabled = S0IX_FORCE_OFF;
> }
>
Is there really a need to set it to SOIX_FORCE_ON when the if
statement below this section will essentially treat it as though it is
set that way anyway? Seems like we only really need to just do a
(!dmi_check_system() && e1000e_check_me()) in the code block that is
setting SOIX_FORCE_OFF rather than bothering to even set enabled to
SOIX_FORCE_ON.
Powered by blists - more mailing lists