netdev - Re: [PATCH] r8169: enable ASPM on Dell platforms

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: <rqeme247cojqejerkedcj7m6t6zglks3pe2wcro3xvprit6npt@s4ymo5357hiv>
Date: Mon, 15 Sep 2025 09:37:37 +0800
From: "Chia-Lin Kao (AceLan)" <acelan.kao@...onical.com>
To: Heiner Kallweit <hkallweit1@...il.com>
Cc: nic_swsd@...ltek.com, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, "Wang, Crag" <Crag.Wang@...l.com>, 
	"Chen, Alan" <Alan.Chen6@...l.com>, "Alex Shen@...l" <Yijun.Shen@...l.com>
Subject: Re: [PATCH] r8169: enable ASPM on Dell platforms

On Fri, Sep 12, 2025 at 05:30:52PM +0200, Heiner Kallweit wrote:
> On 9/12/2025 9:29 AM, Chia-Lin Kao (AceLan) wrote:
> > Enable PCIe ASPM for RTL8169 NICs on Dell platforms that have been
> > verified to work reliably with this power management feature. The
> > r8169 driver traditionally disables ASPM to prevent random link
> > failures and system hangs on problematic hardware.
> > 
> > Dell has validated these product families to work correctly with
> > RTL NIC ASPM and commits to addressing any ASPM-related issues
> > with RTL hardware in collaboration with Realtek.
> > 
> > This change enables ASPM for the following Dell product families:
> > - Alienware
> > - Dell Laptops/Pro Laptops/Pro Max Laptops
> > - Dell Desktops/Pro Desktops/Pro Max Desktops
> > - Dell Pro Rugged Laptops
> > 
> I'd like to avoid DMI-based whitelists in kernel code. If more system
> vendors do it the same way, then this becomes hard to maintain.
I totally understand your point; I also don’t like constantly adding DMI
info to the list. But this list isn’t for a single product name, it’s a
product family that covers a series of products, and it probably won’t
change anytime soon.

> There is already a mechanism for vendors to flag that they successfully
> tested ASPM. See c217ab7a3961 ("r8169: enable ASPM L1.2 if system vendor
> flags it as safe").
Right, but writing the flag is not applicable for Dell manufacturing
processes.

> Last but not least ASPM can be (re-)enabled from userspace, using sysfs.
That doesn't sound like a good solution to push the list to userspace.

Dell has already been working with Canonical for more than a decade to
ship their products with r8169 ASPM enabled. Dell has also had lengthy
discussions with Realtek to have this feature enabled by default in the
r8169 driver. I think this is a good opportunity for Dell to work with
Realtek to spot bugs and refine the r8169 driver.

BTW, I found the dmi.h header file is missing in the patch, so I
submitted a v2 patch.
https://lore.kernel.org/lkml/20250915013555.365230-1-acelan.kao@canonical.com/T/#u
> 
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@...onical.com>
> > ---
> >  drivers/net/ethernet/realtek/r8169_main.c | 29 +++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> > index 9c601f271c02..63e83cf071de 100644
> > --- a/drivers/net/ethernet/realtek/r8169_main.c
> > +++ b/drivers/net/ethernet/realtek/r8169_main.c
> > @@ -5366,6 +5366,32 @@ static void rtl_init_mac_address(struct rtl8169_private *tp)
> >  	rtl_rar_set(tp, mac_addr);
> >  }
> >  
> > +bool rtl_aspm_new_dell_platforms(void)
> > +{
> > +	const char *family = dmi_get_system_info(DMI_PRODUCT_FAMILY);
> > +	static const char * const dell_product_families[] = {
> > +		"Alienware",
> > +		"Dell Laptops",
> > +		"Dell Pro Laptops",
> > +		"Dell Pro Max Laptops",
> > +		"Dell Desktops",
> > +		"Dell Pro Desktops",
> > +		"Dell Pro Max Desktops",
> > +		"Dell Pro Rugged Laptops"
> > +	};
> > +	int i;
> > +
> > +	if (!family)
> > +		return false;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dell_product_families); i++) {
> > +		if (str_has_prefix(family, dell_product_families[i]))
> > +			return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  /* register is set if system vendor successfully tested ASPM 1.2 */
> >  static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
> >  {
> > @@ -5373,6 +5399,9 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
> >  	    r8168_mac_ocp_read(tp, 0xc0b2) & 0xf)
> >  		return true;
> >  
> > +	if (rtl_aspm_new_dell_platforms())
> > +		return true;
> > +
> >  	return false;
> >  }
> >  
>