| 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
| ||
|
Message-ID: <b7702efc-9994-4656-9d4e-29c2c8145ab3@linux.dev>
Date: Wed, 12 Nov 2025 12:48:38 +0000
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Jiawen Wu <jiawenwu@...stnetic.com>, netdev@...r.kernel.org,
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>,
Russell King <linux@...linux.org.uk>, Simon Horman <horms@...nel.org>,
Jacob Keller <jacob.e.keller@...el.com>
Cc: Mengyuan Lou <mengyuanlou@...-swift.com>
Subject: Re: [PATCH net-next 5/5] net: txgbe: support getting module EEPROM by
page
On 12/11/2025 05:58, Jiawen Wu wrote:
> Getting module EEPROM has been supported in TXGBE SP devices, since SFP
> driver has already implemented it.
>
> Now add support to read module EEPROM for AML devices. Towards this, add
> a new firmware mailbox command to get the page data.
>
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
> .../net/ethernet/wangxun/txgbe/txgbe_aml.c | 37 +++++++++++++++++++
> .../net/ethernet/wangxun/txgbe/txgbe_aml.h | 3 ++
> .../ethernet/wangxun/txgbe/txgbe_ethtool.c | 30 +++++++++++++++
> .../net/ethernet/wangxun/txgbe/txgbe_type.h | 11 ++++++
> 4 files changed, 81 insertions(+)
>
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> index 3b6ea456fbf7..12900abfa91a 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> @@ -73,6 +73,43 @@ int txgbe_test_hostif(struct wx *wx)
> WX_HI_COMMAND_TIMEOUT, true);
> }
>
> +int txgbe_read_eeprom_hostif(struct wx *wx,
> + struct txgbe_hic_i2c_read *buffer,
> + u32 length, u8 *data)
> +{
> + u32 buf_size = sizeof(struct txgbe_hic_i2c_read) - sizeof(u8);
> + u32 total_len = buf_size + length;
> + u32 dword_len, value, i;
> + u8 local_data[256];
> + int err;
> +
> + if (total_len > sizeof(local_data))
> + return -EINVAL;
if it's really possible? SFF pages are 128 bytes, you reserve 256 bytes
of local buffer. What are you protecting from?
> +
> + buffer->hdr.cmd = FW_READ_EEPROM_CMD;
> + buffer->hdr.buf_len = sizeof(struct txgbe_hic_i2c_read) -
> + sizeof(struct wx_hic_hdr);
> + buffer->hdr.cmd_or_resp.cmd_resv = FW_CEM_CMD_RESERVED;
> +
> + err = wx_host_interface_command(wx, (u32 *)buffer,
> + sizeof(struct txgbe_hic_i2c_read),
> + WX_HI_COMMAND_TIMEOUT, false);
> + if (err != 0)
> + return err;
> +
> + dword_len = (total_len + 3) / 4;
round_up()?
> +
> + for (i = 0; i < dword_len; i++) {
> + value = rd32a(wx, WX_FW2SW_MBOX, i);
> + le32_to_cpus(&value);
> +
> + memcpy(&local_data[i * 4], &value, 4);
> + }
the logic here is not clear from the first read of the code. effectively
in the reply you have the same txgbe_hic_i2c_read struct but without
data field, which is obviously VLA, but then you simply skip the result
of read of txgbe_hic_i2c_read and only provide the real data back to the
caller. Maybe you can organize the code the way it can avoid double copying?
> +
> + memcpy(data, &local_data[buf_size], length);
> + return 0;
> +}
> +
> static int txgbe_identify_module_hostif(struct wx *wx,
> struct txgbe_hic_get_module_info *buffer)
> {
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.h
> index 7c8fa48e68d3..4f6df0ee860b 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.h
> @@ -7,6 +7,9 @@
> void txgbe_gpio_init_aml(struct wx *wx);
> irqreturn_t txgbe_gpio_irq_handler_aml(int irq, void *data);
> int txgbe_test_hostif(struct wx *wx);
> +int txgbe_read_eeprom_hostif(struct wx *wx,
> + struct txgbe_hic_i2c_read *buffer,
> + u32 length, u8 *data);
> int txgbe_set_phy_link(struct wx *wx);
> int txgbe_identify_module(struct wx *wx);
> void txgbe_setup_link(struct wx *wx);
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> index f553ec5f8238..1f60121fe73c 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> @@ -10,6 +10,7 @@
> #include "../libwx/wx_lib.h"
> #include "txgbe_type.h"
> #include "txgbe_fdir.h"
> +#include "txgbe_aml.h"
> #include "txgbe_ethtool.h"
>
> int txgbe_get_link_ksettings(struct net_device *netdev,
> @@ -534,6 +535,34 @@ static int txgbe_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
> return ret;
> }
>
> +static int
> +txgbe_get_module_eeprom_by_page(struct net_device *netdev,
> + const struct ethtool_module_eeprom *page_data,
> + struct netlink_ext_ack *extack)
> +{
> + struct wx *wx = netdev_priv(netdev);
> + struct txgbe_hic_i2c_read buffer;
> + int err;
> +
> + if (!test_bit(WX_FLAG_SWFW_RING, wx->flags))
> + return -EOPNOTSUPP;
> +
> + buffer.length = (__force u32)cpu_to_be32(page_data->length);
> + buffer.offset = (__force u32)cpu_to_be32(page_data->offset);
maybe txgbe_hic_i2c_read::length and txgbe_hic_i2c_read::offset should
be __be32 ?
> + buffer.page = page_data->page;
> + buffer.bank = page_data->bank;
> + buffer.i2c_address = page_data->i2c_address;
> +
> + err = txgbe_read_eeprom_hostif(wx, &buffer, page_data->length,
> + page_data->data);
> + if (err) {
> + wx_err(wx, "Failed to read module EEPROM\n");
> + return err;
> + }
> +
> + return page_data->length;
> +}
> +
> static const struct ethtool_ops txgbe_ethtool_ops = {
> .supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ |
> @@ -568,6 +597,7 @@ static const struct ethtool_ops txgbe_ethtool_ops = {
> .set_msglevel = wx_set_msglevel,
> .get_ts_info = wx_get_ts_info,
> .get_ts_stats = wx_get_ptp_stats,
> + .get_module_eeprom_by_page = txgbe_get_module_eeprom_by_page,
> };
>
> void txgbe_set_ethtool_ops(struct net_device *netdev)
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> index e72edb9ef084..3d1bec39d74c 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_type.h
> @@ -353,6 +353,7 @@ void txgbe_do_reset(struct net_device *netdev);
> #define FW_PHY_GET_LINK_CMD 0xC0
> #define FW_PHY_SET_LINK_CMD 0xC1
> #define FW_GET_MODULE_INFO_CMD 0xC5
> +#define FW_READ_EEPROM_CMD 0xC6
>
> struct txgbe_sff_id {
> u8 identifier; /* A0H 0x00 */
> @@ -394,6 +395,16 @@ struct txgbe_hic_ephy_getlink {
> u8 resv[6];
> };
>
> +struct txgbe_hic_i2c_read {
> + struct wx_hic_hdr hdr;
> + u32 offset;
> + u32 length;
> + u8 page;
> + u8 bank;
> + u8 i2c_address;
> + u8 data;
> +};
you removed struct txgbe_hic_i2c_read in the patch 2 just to introduce
it in this patch?
> +
> #define NODE_PROP(_NAME, _PROP) \
> (const struct software_node) { \
> .name = _NAME, \
Powered by blists - more mailing lists