netdev - Re: [PATCH net-next] ice: fix linking when CONFIG_PTP_1588

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]

Date: Fri, 22 Sep 2023 13:51:40 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jacob Keller <jacob.e.keller@...el.com>, <netdev@...r.kernel.org>, "Intel
 Wired LAN" <intel-wired-lan@...ts.osuosl.org>
CC: Anthony Nguyen <anthony.l.nguyen@...el.com>, Alexander Lobakin
	<aleksander.lobakin@...el.com>, David Miller <davem@...emloft.net>,
	"Arkadiusz Kubalewski" <arkadiusz.kubalewski@...el.com>, kernel test robot
	<lkp@...el.com>
Subject: Re: [PATCH net-next] ice: fix linking when CONFIG_PTP_1588_CLOCK=n

On 9/21/23 02:06, Jacob Keller wrote:
> The recent support for DPLL introduced by commit 8a3a565ff210 ("ice: add
> admin commands to access cgu configuration") and commit d7999f5ea64b ("ice:
> implement dpll interface to control cgu") broke linking the ice driver if
> CONFIG_PTP_1588_CLOCK=n:
> 
> ld: vmlinux.o: in function `ice_init_feature_support':
> (.text+0x8702b8): undefined reference to `ice_is_phy_rclk_present'
> ld: (.text+0x8702cd): undefined reference to `ice_is_cgu_present'
> ld: (.text+0x8702d9): undefined reference to `ice_is_clock_mux_present_e810t'
> ld: vmlinux.o: in function `ice_dpll_init_info_direct_pins':
> ice_dpll.c:(.text+0x894167): undefined reference to `ice_cgu_get_pin_freq_supp'
> ld: ice_dpll.c:(.text+0x894197): undefined reference to `ice_cgu_get_pin_name'
> ld: ice_dpll.c:(.text+0x8941a8): undefined reference to `ice_cgu_get_pin_type'
> ld: vmlinux.o: in function `ice_dpll_update_state':
> ice_dpll.c:(.text+0x894494): undefined reference to `ice_get_cgu_state'
> ld: vmlinux.o: in function `ice_dpll_init':
> (.text+0x8953d5): undefined reference to `ice_get_cgu_rclk_pin_info'
> 
> The first commit broke things by calling functions in
> ice_init_feature_support that are compiled as part of ice_ptp_hw.o,
> including:
> 
> * ice_is_phy_rclk_present
> * ice_is_clock_mux_present_e810t
> * ice_is_cgU_present
> 
> The second commit continued the break by calling several CGU functions
> defined in ice_ptp_hw.c in the DPLL code.
> Because the ice_dpll.c file is compiled unconditionally, it will not
> link when CONFIG_PTP_1588_CLOCK=n.
> 
> It might be possible to break this dependency and expose those functions
> without CONFIG_PTP_1588_CLOCK, but that is not clear to me.
> 
> For the DPLL case, simply compile ice_dpll.o only when we have
> CONFIG_PTP_1588_CLOCK. Add stub no-op implementation of ice_dpll_init() and
> ice_dpll_uninit() when CONFIG_PTP_1588_CLOCK=n into ice_dpll.h
> 
> The other functions are part of checking the netlist to see if hardware
> features are enabled. These checks don't really belong in ice_ptp_hw.c, and
> make more sense as part of the ice_common.c file. We already have
> ice_is_gps_in_netlist() in ice_common.c which is doing a similar check.
> 
> Move the functions into ice_common.c and rename them to have the similar
> postfix of "in_netlist()" to be more expressive of what they are actually
> checking.
> 
> This also makes the ice_find_netlist_node only called from within
> ice_common.c, so its safe to mark it static and stop declaring it in the
> ice_common.h header as well.
> 
> Fixes: 8a3a565ff210 ("ice: add admin commands to access cgu configuration")
> Fixes: d7999f5ea64b ("ice: implement dpll interface to control cgu")
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202309191214.TaYEct4H-lkp@intel.com
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> ---
> This is an alternative approach to fixing the same compilation errors of the
> ice driver compared to [1]. It does not include the fix for idpf which I
> have no issue with.
> 
> I prefer this over the stubs for the functions in ice_ptp_hw.h, and I had
> proposed these a while ago as part of [2], but the DPLL code merged first
> and had apparently duplicated some of the work.
> 
> [1]: https://lore.kernel.org/netdev/20230920180745.1607563-1-aleksander.lobakin@intel.com/T/#mfeeb599a95334e78dba08330a6bd8edee7843fbb
> [2]: https://lore.kernel.org/netdev/20230918212814.435688-1-anthony.l.nguyen@intel.com/
> 
>   drivers/net/ethernet/intel/ice/Makefile     |  5 +-
>   drivers/net/ethernet/intel/ice/ice_common.c | 66 ++++++++++++++++++++-
>   drivers/net/ethernet/intel/ice/ice_common.h |  6 +-
>   drivers/net/ethernet/intel/ice/ice_dpll.h   |  6 +-
>   drivers/net/ethernet/intel/ice/ice_lib.c    |  6 +-
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 66 ---------------------
>   drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  3 -
>   7 files changed, 76 insertions(+), 82 deletions(-)
> 

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>