linux-kernel - Re: [PATCH net-next v7 4/7] stmmac: intel: configure SerDes according to the interface mode

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: <ceb6cee6-9cfb-4363-9d23-dfd21f78d486@linux.intel.com>
Date: Fri, 7 Feb 2025 17:53:37 +0800
From: Choong Yong Liang <yong.liang.choong@...ux.intel.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Simon Horman <horms@...nel.org>, Jose Abreu <joabreu@...opsys.com>,
 Jose Abreu <Jose.Abreu@...opsys.com>,
 David E Box <david.e.box@...ux.intel.com>,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 "H . Peter Anvin" <hpa@...or.com>,
 Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
 David E Box <david.e.box@...el.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>, Maxime Coquelin
 <mcoquelin.stm32@...il.com>, Alexandre Torgue
 <alexandre.torgue@...s.st.com>, Jiawen Wu <jiawenwu@...stnetic.com>,
 Mengyuan Lou <mengyuanlou@...-swift.com>,
 Heiner Kallweit <hkallweit1@...il.com>, Russell King
 <linux@...linux.org.uk>, Hans de Goede <hdegoede@...hat.com>,
 Richard Cochran <richardcochran@...il.com>,
 Serge Semin <fancer.lancer@...il.com>, x86@...nel.org,
 LKML <linux-kernel@...r.kernel.org>, Netdev <netdev@...r.kernel.org>,
 platform-driver-x86@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net-next v7 4/7] stmmac: intel: configure SerDes according
 to the interface mode

On 6/2/2025 9:31 pm, Ilpo Järvinen wrote:
>> +static int intel_tsn_lane_is_available(struct net_device *ndev,
>> +				       struct intel_priv_data *intel_priv)
>> +{
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	struct pmc_ipc_cmd tmp = {};
>> +	u32 rbuf[4] = {};
>> +	int ret = 0, i, j;
>> +	const int max_fia_regs = 5;
>> +
>> +	tmp.cmd = IPC_SOC_REGISTER_ACCESS;
>> +	tmp.sub_cmd = IPC_SOC_SUB_CMD_READ;
>> +
>> +	for (i = 0; i < max_fia_regs; i++) {
> 
> Usually, defines are used for true consts.
> 
Hi Ilpo,
Thank you for your feedback. I used const int max_fia_regs = 5; to leverage 
type safety and scope control in modern C. However, I understand that 
#define is a common practice. Please let me know if you prefer I switch to 
#define for consistency.

>> +static int intel_mac_finish(struct net_device *ndev,
>> +			    void *intel_data,
>> +			    unsigned int mode,
>> +			    phy_interface_t interface)
>> +{
>> +	struct intel_priv_data *intel_priv = intel_data;
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	const struct pmc_serdes_regs *regs;
>> +	int max_regs = 0;
>> +	int ret = 0;
>> +
>> +	ret = intel_tsn_lane_is_available(ndev, intel_priv);
>> +	if (ret < 0) {
>> +		netdev_info(priv->dev, "No TSN lane available to set the registers.\n");
>> +		return ret;
>> +	}
>> +
>> +	if (interface == PHY_INTERFACE_MODE_2500BASEX) {
>> +		regs = intel_priv->pid_2p5g.regs;
>> +		max_regs = intel_priv->pid_2p5g.num_regs;
>> +	} else {
>> +		regs = intel_priv->pid_1g.regs;
>> +		max_regs = intel_priv->pid_1g.num_regs;
>> +	}
>> +
>> +	ret = intel_set_reg_access(regs, max_regs);
>> +	if (ret < 0)
>> +		return ret;
> 
> This looks much cleaner now, thanks the update.
> 
> However, the intel_priv fields you introduced are not setup until patch
> 6/7? Will this cause NULL ptr deref issues in between the two changes? By
> introducing the reg arrays in this patch but only use them after patch 6,
> you'll also get unused variable warnings out of them in between the
> changes which is unacceptable.
> 
Thank you for pointing out the potential issues with the intel_priv fields. 
I will move the changes from patch 6 into this patch to avoid NULL pointer 
de-reference issues and unused variable warnings. This will ensure 
everything is properly set up and used within the same patch.