netdev - RE: [net-next 15/15] ice: add 1G SGMII PHY type

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: <BY5PR11MB445222D4DF27B02D7DA7E621E0770@BY5PR11MB4452.namprd11.prod.outlook.com>
Date:   Fri, 24 Jul 2020 00:07:42 +0000
From:   "Stillwell Jr, Paul M" <paul.m.stillwell.jr@...el.com>
To:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "Bowers, AndrewX" <andrewx.bowers@...el.com>
Subject: RE: [net-next 15/15] ice: add 1G SGMII PHY type

Ack, thanks!

Paul

> -----Original Message-----
> From: Nguyen, Anthony L <anthony.l.nguyen@...el.com>
> Sent: Thursday, July 23, 2020 4:47 PM
> To: davem@...emloft.net
> Cc: Stillwell Jr, Paul M <paul.m.stillwell.jr@...el.com>; netdev@...r.kernel.org;
> nhorman@...hat.com; sassmann@...hat.com; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@...el.com>; Nguyen, Anthony L
> <anthony.l.nguyen@...el.com>; Bowers, AndrewX
> <andrewx.bowers@...el.com>
> Subject: [net-next 15/15] ice: add 1G SGMII PHY type
> 
> From: Paul M Stillwell Jr <paul.m.stillwell.jr@...el.com>
> 
> There isn't a case for 1G SGMII in ice_get_media_type() so add the handling for
> it.
> 
> Also handle the special case where some direct attach cables may report that
> they support 1G SGMII, but that is erroneous since SGMII is supposed to be a
> backplane media type (between a MAC and a PHY). If the driver doesn't handle
> this special case then a user could see the 'Port' in ethtool change from 'Direct
> attach Copper' to 'Backplane' when they have forced the speed to 1G, but the
> cable hasn't changed.
> 
> Lastly, change ice_aq_get_phy_caps() to save the module_type info if the
> function was called with ICE_AQC_REPORT_TOPO_CAP. This call uses the media
> information to populate the module_type. If no media is present then the values
> in module_type will be 0.
> 
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@...el.com>
> Tested-by: Andrew Bowers <andrewx.bowers@...el.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  1 +
>  drivers/net/ethernet/intel/ice/ice_common.c     | 17 ++++++++++++++---
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 4315a784b975..b363e0223670 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -993,6 +993,7 @@ struct ice_aqc_get_phy_caps_data {
>  	u8 module_type[ICE_MODULE_TYPE_TOTAL_BYTE];
>  #define ICE_AQC_MOD_TYPE_BYTE0_SFP_PLUS			0xA0
>  #define ICE_AQC_MOD_TYPE_BYTE0_QSFP_PLUS		0x80
> +#define ICE_AQC_MOD_TYPE_IDENT				1
>  #define ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_PASSIVE	BIT(0)
>  #define ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_ACTIVE	BIT(1)
>  #define ICE_AQC_MOD_TYPE_BYTE1_10G_BASE_SR		BIT(4)
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index bb9952038efa..c72cc77b8d67 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -194,6 +194,8 @@ ice_aq_get_phy_caps(struct ice_port_info *pi, bool
> qual_mods, u8 report_mode,
>  	if (!status && report_mode == ICE_AQC_REPORT_TOPO_CAP) {
>  		pi->phy.phy_type_low = le64_to_cpu(pcaps->phy_type_low);
>  		pi->phy.phy_type_high = le64_to_cpu(pcaps->phy_type_high);
> +		memcpy(pi->phy.link_info.module_type, &pcaps-
> >module_type,
> +		       sizeof(pi->phy.link_info.module_type));
>  	}
> 
>  	return status;
> @@ -266,6 +268,18 @@ static enum ice_media_type
> ice_get_media_type(struct ice_port_info *pi)
>  		return ICE_MEDIA_UNKNOWN;
> 
>  	if (hw_link_info->phy_type_low) {
> +		/* 1G SGMII is a special case where some DA cable PHYs
> +		 * may show this as an option when it really shouldn't
> +		 * be since SGMII is meant to be between a MAC and a PHY
> +		 * in a backplane. Try to detect this case and handle it
> +		 */
> +		if (hw_link_info->phy_type_low ==
> ICE_PHY_TYPE_LOW_1G_SGMII &&
> +		    (hw_link_info->module_type[ICE_AQC_MOD_TYPE_IDENT]
> ==
> +		    ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_ACTIVE ||
> +		    hw_link_info->module_type[ICE_AQC_MOD_TYPE_IDENT] ==
> +		    ICE_AQC_MOD_TYPE_BYTE1_SFP_PLUS_CU_PASSIVE))
> +			return ICE_MEDIA_DA;
> +
>  		switch (hw_link_info->phy_type_low) {
>  		case ICE_PHY_TYPE_LOW_1000BASE_SX:
>  		case ICE_PHY_TYPE_LOW_1000BASE_LX:
> @@ -2647,9 +2661,6 @@ enum ice_status ice_update_link_info(struct
> ice_port_info *pi)
> 
>  		status = ice_aq_get_phy_caps(pi, false,
> ICE_AQC_REPORT_TOPO_CAP,
>  					     pcaps, NULL);
> -		if (!status)
> -			memcpy(li->module_type, &pcaps->module_type,
> -			       sizeof(li->module_type));
> 
>  		devm_kfree(ice_hw_to_dev(hw), pcaps);
>  	}
> --
> 2.26.2