diff mbox series

[iwl-next,v10,05/14] iavf: negotiate PTP capabilities

Message ID 20240821121539.374343-6-wojciech.drewek@intel.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series Add support for Rx timestamping for both ice and iavf drivers | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Wojciech Drewek Aug. 21, 2024, 12:15 p.m. UTC
From: Jacob Keller <jacob.e.keller@intel.com>

Add a new extended capabilities negotiation to exchange information from
the PF about what PTP capabilities are supported by this VF. This
requires sending a VIRTCHNL_OP_1588_PTP_GET_CAPS message, and waiting
for the response from the PF. Handle this early on during the VF
initialization.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
---
 drivers/net/ethernet/intel/iavf/iavf.h        | 17 ++++-
 drivers/net/ethernet/intel/iavf/iavf_main.c   | 60 ++++++++++++++++
 drivers/net/ethernet/intel/iavf/iavf_ptp.h    |  9 +++
 drivers/net/ethernet/intel/iavf/iavf_types.h  | 36 ++++++++++
 .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 72 +++++++++++++++++++
 5 files changed, 192 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/iavf/iavf_ptp.h
 create mode 100644 drivers/net/ethernet/intel/iavf/iavf_types.h

Comments

Alexander Lobakin Aug. 21, 2024, 2:06 p.m. UTC | #1
From: Wojciech Drewek <wojciech.drewek@intel.com>
Date: Wed, 21 Aug 2024 14:15:30 +0200

> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Add a new extended capabilities negotiation to exchange information from
> the PF about what PTP capabilities are supported by this VF. This
> requires sending a VIRTCHNL_OP_1588_PTP_GET_CAPS message, and waiting
> for the response from the PF. Handle this early on during the VF
> initialization.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf.h        | 17 ++++-
>  drivers/net/ethernet/intel/iavf/iavf_main.c   | 60 ++++++++++++++++
>  drivers/net/ethernet/intel/iavf/iavf_ptp.h    |  9 +++
>  drivers/net/ethernet/intel/iavf/iavf_types.h  | 36 ++++++++++
>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 72 +++++++++++++++++++
>  5 files changed, 192 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/ethernet/intel/iavf/iavf_ptp.h
>  create mode 100644 drivers/net/ethernet/intel/iavf/iavf_types.h
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
> index f1506b3d01ce..871431bed64a 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
> @@ -40,6 +40,7 @@
>  #include "iavf_txrx.h"
>  #include "iavf_fdir.h"
>  #include "iavf_adv_rss.h"
> +#include "iavf_types.h"
>  #include <linux/bitmap.h>
>  
>  #define DEFAULT_DEBUG_LEVEL_SHIFT 3
> @@ -338,13 +339,16 @@ struct iavf_adapter {
>  #define IAVF_FLAG_AQ_ENABLE_STAG_VLAN_INSERTION		BIT_ULL(37)
>  #define IAVF_FLAG_AQ_DISABLE_STAG_VLAN_INSERTION	BIT_ULL(38)
>  #define IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS		BIT_ULL(39)
> +#define IAVF_FLAG_AQ_GET_PTP_CAPS			BIT_ULL(40)
> +#define IAVF_FLAG_AQ_SEND_PTP_CMD			BIT_ULL(41)
>  
>  	/* AQ messages that must be sent after IAVF_FLAG_AQ_GET_CONFIG, in
>  	 * order to negotiated extended capabilities.
>  	 */
>  #define IAVF_FLAG_AQ_EXTENDED_CAPS			\
>  	(IAVF_FLAG_AQ_GET_OFFLOAD_VLAN_V2_CAPS |	\
> -	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS)
> +	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS |		\
> +	 IAVF_FLAG_AQ_GET_PTP_CAPS)
>  
>  	/* flags for processing extended capability messages during
>  	 * __IAVF_INIT_EXTENDED_CAPS. Each capability exchange requires
> @@ -358,12 +362,16 @@ struct iavf_adapter {
>  #define IAVF_EXTENDED_CAP_RECV_VLAN_V2			BIT_ULL(1)
>  #define IAVF_EXTENDED_CAP_SEND_RXDID			BIT_ULL(2)
>  #define IAVF_EXTENDED_CAP_RECV_RXDID			BIT_ULL(3)
> +#define IAVF_EXTENDED_CAP_SEND_PTP			BIT_ULL(4)
> +#define IAVF_EXTENDED_CAP_RECV_PTP			BIT_ULL(5)
>  
>  #define IAVF_EXTENDED_CAPS				\
>  	(IAVF_EXTENDED_CAP_SEND_VLAN_V2 |		\
>  	 IAVF_EXTENDED_CAP_RECV_VLAN_V2 |		\
>  	 IAVF_EXTENDED_CAP_SEND_RXDID |			\
> -	 IAVF_EXTENDED_CAP_RECV_RXDID)
> +	 IAVF_EXTENDED_CAP_RECV_RXDID |			\
> +	 IAVF_EXTENDED_CAP_SEND_PTP |			\
> +	 IAVF_EXTENDED_CAP_RECV_PTP)
>  
>  	/* Lock to prevent possible clobbering of
>  	 * current_netdev_promisc_flags
> @@ -423,6 +431,8 @@ struct iavf_adapter {
>  			     VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF)
>  #define IAVF_RXDID_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
>  			       VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC)
> +#define IAVF_PTP_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
> +			      VIRTCHNL_VF_CAP_PTP)

Bah, should've mentioned that where you introduce IAVF_RXDID_ALLOWED().
I realize that the macros added previously are indented with spaces, but
it's not sorta correct style for the kernel. Maybe you'd indent both new
macros (RXDID and PTP) with tabs? You can also break the line different
way if you want, like

#define IAVF_PTP_ALLOWED(a)					\
	((a)->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP)

Looks more clear than breaking it after the '&'.

>  	struct virtchnl_vf_resource *vf_res; /* incl. all VSIs */
>  	struct virtchnl_vsi_resource *vsi_res; /* our LAN VSI */
>  	struct virtchnl_version_info pf_version;
> @@ -430,6 +440,7 @@ struct iavf_adapter {
>  		       ((_a)->pf_version.minor == 1))
>  	struct virtchnl_vlan_caps vlan_v2_caps;
>  	u64 supp_rxdids;
> +	struct iavf_ptp ptp;
>  	u16 msg_enable;
>  	struct iavf_eth_stats current_stats;
>  	struct iavf_vsi vsi;

[...]

> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.h b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
> new file mode 100644
> index 000000000000..65678e76c34f
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2024 Intel Corporation. */
> +
> +#ifndef _IAVF_PTP_H_
> +#define _IAVF_PTP_H_
> +
> +#include "iavf_types.h"
> +
> +#endif /* _IAVF_PTP_H_ */
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_types.h b/drivers/net/ethernet/intel/iavf/iavf_types.h
> new file mode 100644
> index 000000000000..6b7029a1a5a7
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/iavf/iavf_types.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2024 Intel Corporation. */
> +
> +#ifndef _IAVF_TYPES_H_
> +#define _IAVF_TYPES_H_
> +
> +#include "iavf_types.h"
> +
> +#include <linux/avf/virtchnl.h>
> +#include <linux/ptp_clock_kernel.h>

Oh well. I initially asked to introduce iavf_types.h to not bloat
iavf.h, but now types.h includes big ptp_clock_kernel.h :z
When I was reviewing PTP for idpf, I proposed to make this iavf_ptp in
iavf_adapter a pointer and allocate it dynamically, so that iavf.h
wouldn't need to include anything PTP-related at all. This way you
wouldn't need iavf_types.h.
What do you think?

> +
> +/* structure used to queue PTP commands for processing */
> +struct iavf_ptp_aq_cmd {
> +	struct list_head list;
> +	enum virtchnl_ops v_opcode:16;
> +	u16 msglen;
> +	u8 msg[] __counted_by(msglen);
> +};
> +
> +/* fields used for PTP support */

Redundant comment I'd say.

> +struct iavf_ptp {
> +	wait_queue_head_t phc_time_waitqueue;
> +	struct virtchnl_ptp_caps hw_caps;
> +	struct ptp_clock_info info;
> +	struct ptp_clock *clock;
> +	struct list_head aq_cmds;
> +	u64 cached_phc_time;
> +	unsigned long cached_phc_updated;
> +	/* Lock protecting access to the AQ command list */
> +	struct mutex aq_cmd_lock;
> +	struct kernel_hwtstamp_config hwtstamp_config;
> +	bool initialized:1;
> +	bool phc_time_ready:1;
> +};
> +
> +#endif /* _IAVF_TYPES_H_ */

[...]

> @@ -307,6 +343,38 @@ int iavf_get_vf_supported_rxdids(struct iavf_adapter *adapter)
>  	return 0;
>  }
>  
> +int iavf_get_vf_ptp_caps(struct iavf_adapter *adapter)
> +{
> +	struct virtchnl_ptp_caps caps = {};
> +	struct iavf_hw *hw = &adapter->hw;
> +	struct iavf_arq_event_info event;
> +	enum virtchnl_ops op;
> +	enum iavf_status err;
> +
> +	event.msg_buf = (u8 *)&caps;
> +	event.buf_len = sizeof(caps);
> +
> +	while (1) {
> +		/* When the AQ is empty, iavf_clean_arq_element will return
> +		 * nonzero and this loop will terminate.
> +		 */
> +		err = iavf_clean_arq_element(hw, &event, NULL);
> +		if (err != IAVF_SUCCESS)
> +			return err;
> +		op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);

This cast is not needed.

> +		if (op == VIRTCHNL_OP_1588_PTP_GET_CAPS)
> +			break;

Same comments as to one of the previous patches -- you can declare @op
inside the loop and also take into consideration that cpu_to_le32(const)
is faster than le32_to_cpu(var) on BE.

> +	}
> +
> +	err = le32_to_cpu(event.desc.cookie_low);
> +	if (err)
> +		return err;
> +
> +	memcpy(&adapter->ptp.hw_caps, &caps, sizeof(caps));
> +
> +	return 0;
> +}
> +
>  /**
>   * iavf_configure_queues
>   * @adapter: adapter structure
> @@ -2423,6 +2491,10 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
>  		memcpy(&adapter->supp_rxdids, msg,
>  		       min_t(u16, msglen, sizeof(adapter->supp_rxdids)));
>  		break;
> +	case VIRTCHNL_OP_1588_PTP_GET_CAPS:
> +		memcpy(&adapter->ptp.hw_caps, msg,
> +		       min_t(u16, msglen, sizeof(adapter->ptp.hw_caps)));

Same as to one of the previous patches -- I'd avoid partial copying and
check the msglen first to be the same as this sizeof().

> +		break;
>  	case VIRTCHNL_OP_ENABLE_QUEUES:
>  		/* enable transmits */
>  		iavf_irq_enable(adapter, true);

Thanks,
Olek
Wojciech Drewek Aug. 26, 2024, 12:43 p.m. UTC | #2
On 21.08.2024 16:06, Alexander Lobakin wrote:
> From: Wojciech Drewek <wojciech.drewek@intel.com>
> Date: Wed, 21 Aug 2024 14:15:30 +0200
> 
>> From: Jacob Keller <jacob.e.keller@intel.com>
>>
>> Add a new extended capabilities negotiation to exchange information from
>> the PF about what PTP capabilities are supported by this VF. This
>> requires sending a VIRTCHNL_OP_1588_PTP_GET_CAPS message, and waiting
>> for the response from the PF. Handle this early on during the VF
>> initialization.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
>> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
>> Reviewed-by: Simon Horman <horms@kernel.org>
>> Co-developed-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
>> Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com>
>> Signed-off-by: Wojciech Drewek <wojciech.drewek@intel.com>
>> ---
>>  drivers/net/ethernet/intel/iavf/iavf.h        | 17 ++++-
>>  drivers/net/ethernet/intel/iavf/iavf_main.c   | 60 ++++++++++++++++
>>  drivers/net/ethernet/intel/iavf/iavf_ptp.h    |  9 +++
>>  drivers/net/ethernet/intel/iavf/iavf_types.h  | 36 ++++++++++
>>  .../net/ethernet/intel/iavf/iavf_virtchnl.c   | 72 +++++++++++++++++++
>>  5 files changed, 192 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/net/ethernet/intel/iavf/iavf_ptp.h
>>  create mode 100644 drivers/net/ethernet/intel/iavf/iavf_types.h
>>
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
>> index f1506b3d01ce..871431bed64a 100644
>> --- a/drivers/net/ethernet/intel/iavf/iavf.h
>> +++ b/drivers/net/ethernet/intel/iavf/iavf.h
>> @@ -40,6 +40,7 @@
>>  #include "iavf_txrx.h"
>>  #include "iavf_fdir.h"
>>  #include "iavf_adv_rss.h"
>> +#include "iavf_types.h"
>>  #include <linux/bitmap.h>
>>  
>>  #define DEFAULT_DEBUG_LEVEL_SHIFT 3
>> @@ -338,13 +339,16 @@ struct iavf_adapter {
>>  #define IAVF_FLAG_AQ_ENABLE_STAG_VLAN_INSERTION		BIT_ULL(37)
>>  #define IAVF_FLAG_AQ_DISABLE_STAG_VLAN_INSERTION	BIT_ULL(38)
>>  #define IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS		BIT_ULL(39)
>> +#define IAVF_FLAG_AQ_GET_PTP_CAPS			BIT_ULL(40)
>> +#define IAVF_FLAG_AQ_SEND_PTP_CMD			BIT_ULL(41)
>>  
>>  	/* AQ messages that must be sent after IAVF_FLAG_AQ_GET_CONFIG, in
>>  	 * order to negotiated extended capabilities.
>>  	 */
>>  #define IAVF_FLAG_AQ_EXTENDED_CAPS			\
>>  	(IAVF_FLAG_AQ_GET_OFFLOAD_VLAN_V2_CAPS |	\
>> -	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS)
>> +	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS |		\
>> +	 IAVF_FLAG_AQ_GET_PTP_CAPS)
>>  
>>  	/* flags for processing extended capability messages during
>>  	 * __IAVF_INIT_EXTENDED_CAPS. Each capability exchange requires
>> @@ -358,12 +362,16 @@ struct iavf_adapter {
>>  #define IAVF_EXTENDED_CAP_RECV_VLAN_V2			BIT_ULL(1)
>>  #define IAVF_EXTENDED_CAP_SEND_RXDID			BIT_ULL(2)
>>  #define IAVF_EXTENDED_CAP_RECV_RXDID			BIT_ULL(3)
>> +#define IAVF_EXTENDED_CAP_SEND_PTP			BIT_ULL(4)
>> +#define IAVF_EXTENDED_CAP_RECV_PTP			BIT_ULL(5)
>>  
>>  #define IAVF_EXTENDED_CAPS				\
>>  	(IAVF_EXTENDED_CAP_SEND_VLAN_V2 |		\
>>  	 IAVF_EXTENDED_CAP_RECV_VLAN_V2 |		\
>>  	 IAVF_EXTENDED_CAP_SEND_RXDID |			\
>> -	 IAVF_EXTENDED_CAP_RECV_RXDID)
>> +	 IAVF_EXTENDED_CAP_RECV_RXDID |			\
>> +	 IAVF_EXTENDED_CAP_SEND_PTP |			\
>> +	 IAVF_EXTENDED_CAP_RECV_PTP)
>>  
>>  	/* Lock to prevent possible clobbering of
>>  	 * current_netdev_promisc_flags
>> @@ -423,6 +431,8 @@ struct iavf_adapter {
>>  			     VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF)
>>  #define IAVF_RXDID_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
>>  			       VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC)
>> +#define IAVF_PTP_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
>> +			      VIRTCHNL_VF_CAP_PTP)
> 
> Bah, should've mentioned that where you introduce IAVF_RXDID_ALLOWED().
> I realize that the macros added previously are indented with spaces, but
> it's not sorta correct style for the kernel. Maybe you'd indent both new
> macros (RXDID and PTP) with tabs? You can also break the line different
> way if you want, like
> 
> #define IAVF_PTP_ALLOWED(a)					\
> 	((a)->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP)
> 
> Looks more clear than breaking it after the '&'.

sure

> 
>>  	struct virtchnl_vf_resource *vf_res; /* incl. all VSIs */
>>  	struct virtchnl_vsi_resource *vsi_res; /* our LAN VSI */
>>  	struct virtchnl_version_info pf_version;
>> @@ -430,6 +440,7 @@ struct iavf_adapter {
>>  		       ((_a)->pf_version.minor == 1))
>>  	struct virtchnl_vlan_caps vlan_v2_caps;
>>  	u64 supp_rxdids;
>> +	struct iavf_ptp ptp;
>>  	u16 msg_enable;
>>  	struct iavf_eth_stats current_stats;
>>  	struct iavf_vsi vsi;
> 
> [...]
> 
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.h b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
>> new file mode 100644
>> index 000000000000..65678e76c34f
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
>> @@ -0,0 +1,9 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2024 Intel Corporation. */
>> +
>> +#ifndef _IAVF_PTP_H_
>> +#define _IAVF_PTP_H_
>> +
>> +#include "iavf_types.h"
>> +
>> +#endif /* _IAVF_PTP_H_ */
>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_types.h b/drivers/net/ethernet/intel/iavf/iavf_types.h
>> new file mode 100644
>> index 000000000000..6b7029a1a5a7
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/iavf/iavf_types.h
>> @@ -0,0 +1,36 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2024 Intel Corporation. */
>> +
>> +#ifndef _IAVF_TYPES_H_
>> +#define _IAVF_TYPES_H_
>> +
>> +#include "iavf_types.h"
>> +
>> +#include <linux/avf/virtchnl.h>
>> +#include <linux/ptp_clock_kernel.h>
> 
> Oh well. I initially asked to introduce iavf_types.h to not bloat
> iavf.h, but now types.h includes big ptp_clock_kernel.h :z
> When I was reviewing PTP for idpf, I proposed to make this iavf_ptp in
> iavf_adapter a pointer and allocate it dynamically, so that iavf.h
> wouldn't need to include anything PTP-related at all. This way you
> wouldn't need iavf_types.h.
> What do you think?

I can try make it happen

> 
>> +
>> +/* structure used to queue PTP commands for processing */
>> +struct iavf_ptp_aq_cmd {
>> +	struct list_head list;
>> +	enum virtchnl_ops v_opcode:16;
>> +	u16 msglen;
>> +	u8 msg[] __counted_by(msglen);
>> +};
>> +
>> +/* fields used for PTP support */
> 
> Redundant comment I'd say.

Agree

> 
>> +struct iavf_ptp {
>> +	wait_queue_head_t phc_time_waitqueue;
>> +	struct virtchnl_ptp_caps hw_caps;
>> +	struct ptp_clock_info info;
>> +	struct ptp_clock *clock;
>> +	struct list_head aq_cmds;
>> +	u64 cached_phc_time;
>> +	unsigned long cached_phc_updated;
>> +	/* Lock protecting access to the AQ command list */
>> +	struct mutex aq_cmd_lock;
>> +	struct kernel_hwtstamp_config hwtstamp_config;
>> +	bool initialized:1;
>> +	bool phc_time_ready:1;
>> +};
>> +
>> +#endif /* _IAVF_TYPES_H_ */
> 
> [...]
> 
>> @@ -307,6 +343,38 @@ int iavf_get_vf_supported_rxdids(struct iavf_adapter *adapter)
>>  	return 0;
>>  }
>>  
>> +int iavf_get_vf_ptp_caps(struct iavf_adapter *adapter)
>> +{
>> +	struct virtchnl_ptp_caps caps = {};
>> +	struct iavf_hw *hw = &adapter->hw;
>> +	struct iavf_arq_event_info event;
>> +	enum virtchnl_ops op;
>> +	enum iavf_status err;
>> +
>> +	event.msg_buf = (u8 *)&caps;
>> +	event.buf_len = sizeof(caps);
>> +
>> +	while (1) {
>> +		/* When the AQ is empty, iavf_clean_arq_element will return
>> +		 * nonzero and this loop will terminate.
>> +		 */
>> +		err = iavf_clean_arq_element(hw, &event, NULL);
>> +		if (err != IAVF_SUCCESS)
>> +			return err;
>> +		op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
> 
> This cast is not needed.
> 
>> +		if (op == VIRTCHNL_OP_1588_PTP_GET_CAPS)
>> +			break;
> 
> Same comments as to one of the previous patches -- you can declare @op
> inside the loop and also take into consideration that cpu_to_le32(const)
> is faster than le32_to_cpu(var) on BE.

As in previous patch I'll use iavf_poll_virtchnl_msg. After that
your comments won't apply

> 
>> +	}
>> +
>> +	err = le32_to_cpu(event.desc.cookie_low);
>> +	if (err)
>> +		return err;
>> +
>> +	memcpy(&adapter->ptp.hw_caps, &caps, sizeof(caps));
>> +
>> +	return 0;
>> +}
>> +
>>  /**
>>   * iavf_configure_queues
>>   * @adapter: adapter structure
>> @@ -2423,6 +2491,10 @@ void iavf_virtchnl_completion(struct iavf_adapter *adapter,
>>  		memcpy(&adapter->supp_rxdids, msg,
>>  		       min_t(u16, msglen, sizeof(adapter->supp_rxdids)));
>>  		break;
>> +	case VIRTCHNL_OP_1588_PTP_GET_CAPS:
>> +		memcpy(&adapter->ptp.hw_caps, msg,
>> +		       min_t(u16, msglen, sizeof(adapter->ptp.hw_caps)));
> 
> Same as to one of the previous patches -- I'd avoid partial copying and
> check the msglen first to be the same as this sizeof().

Sure

> 
>> +		break;
>>  	case VIRTCHNL_OP_ENABLE_QUEUES:
>>  		/* enable transmits */
>>  		iavf_irq_enable(adapter, true);
> 
> Thanks,
> Olek
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h
index f1506b3d01ce..871431bed64a 100644
--- a/drivers/net/ethernet/intel/iavf/iavf.h
+++ b/drivers/net/ethernet/intel/iavf/iavf.h
@@ -40,6 +40,7 @@ 
 #include "iavf_txrx.h"
 #include "iavf_fdir.h"
 #include "iavf_adv_rss.h"
+#include "iavf_types.h"
 #include <linux/bitmap.h>
 
 #define DEFAULT_DEBUG_LEVEL_SHIFT 3
@@ -338,13 +339,16 @@  struct iavf_adapter {
 #define IAVF_FLAG_AQ_ENABLE_STAG_VLAN_INSERTION		BIT_ULL(37)
 #define IAVF_FLAG_AQ_DISABLE_STAG_VLAN_INSERTION	BIT_ULL(38)
 #define IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS		BIT_ULL(39)
+#define IAVF_FLAG_AQ_GET_PTP_CAPS			BIT_ULL(40)
+#define IAVF_FLAG_AQ_SEND_PTP_CMD			BIT_ULL(41)
 
 	/* AQ messages that must be sent after IAVF_FLAG_AQ_GET_CONFIG, in
 	 * order to negotiated extended capabilities.
 	 */
 #define IAVF_FLAG_AQ_EXTENDED_CAPS			\
 	(IAVF_FLAG_AQ_GET_OFFLOAD_VLAN_V2_CAPS |	\
-	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS)
+	 IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS |		\
+	 IAVF_FLAG_AQ_GET_PTP_CAPS)
 
 	/* flags for processing extended capability messages during
 	 * __IAVF_INIT_EXTENDED_CAPS. Each capability exchange requires
@@ -358,12 +362,16 @@  struct iavf_adapter {
 #define IAVF_EXTENDED_CAP_RECV_VLAN_V2			BIT_ULL(1)
 #define IAVF_EXTENDED_CAP_SEND_RXDID			BIT_ULL(2)
 #define IAVF_EXTENDED_CAP_RECV_RXDID			BIT_ULL(3)
+#define IAVF_EXTENDED_CAP_SEND_PTP			BIT_ULL(4)
+#define IAVF_EXTENDED_CAP_RECV_PTP			BIT_ULL(5)
 
 #define IAVF_EXTENDED_CAPS				\
 	(IAVF_EXTENDED_CAP_SEND_VLAN_V2 |		\
 	 IAVF_EXTENDED_CAP_RECV_VLAN_V2 |		\
 	 IAVF_EXTENDED_CAP_SEND_RXDID |			\
-	 IAVF_EXTENDED_CAP_RECV_RXDID)
+	 IAVF_EXTENDED_CAP_RECV_RXDID |			\
+	 IAVF_EXTENDED_CAP_SEND_PTP |			\
+	 IAVF_EXTENDED_CAP_RECV_PTP)
 
 	/* Lock to prevent possible clobbering of
 	 * current_netdev_promisc_flags
@@ -423,6 +431,8 @@  struct iavf_adapter {
 			     VIRTCHNL_VF_OFFLOAD_ADV_RSS_PF)
 #define IAVF_RXDID_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
 			       VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC)
+#define IAVF_PTP_ALLOWED(a) ((a)->vf_res->vf_cap_flags & \
+			      VIRTCHNL_VF_CAP_PTP)
 	struct virtchnl_vf_resource *vf_res; /* incl. all VSIs */
 	struct virtchnl_vsi_resource *vsi_res; /* our LAN VSI */
 	struct virtchnl_version_info pf_version;
@@ -430,6 +440,7 @@  struct iavf_adapter {
 		       ((_a)->pf_version.minor == 1))
 	struct virtchnl_vlan_caps vlan_v2_caps;
 	u64 supp_rxdids;
+	struct iavf_ptp ptp;
 	u16 msg_enable;
 	struct iavf_eth_stats current_stats;
 	struct iavf_vsi vsi;
@@ -569,6 +580,8 @@  int iavf_get_vf_vlan_v2_caps(struct iavf_adapter *adapter);
 int iavf_send_vf_offload_vlan_v2_msg(struct iavf_adapter *adapter);
 int iavf_send_vf_supported_rxdids_msg(struct iavf_adapter *adapter);
 int iavf_get_vf_supported_rxdids(struct iavf_adapter *adapter);
+int iavf_send_vf_ptp_caps_msg(struct iavf_adapter *adapter);
+int iavf_get_vf_ptp_caps(struct iavf_adapter *adapter);
 void iavf_set_queue_vlan_tag_loc(struct iavf_adapter *adapter);
 u16 iavf_get_num_vlans_added(struct iavf_adapter *adapter);
 void iavf_irq_enable(struct iavf_adapter *adapter, bool flush);
diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index df905b2dea28..c26f6fc5250b 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -2083,6 +2083,8 @@  static int iavf_process_aq_command(struct iavf_adapter *adapter)
 		return iavf_send_vf_offload_vlan_v2_msg(adapter);
 	if (adapter->aq_required & IAVF_FLAG_AQ_GET_SUPPORTED_RXDIDS)
 		return iavf_send_vf_supported_rxdids_msg(adapter);
+	if (adapter->aq_required & IAVF_FLAG_AQ_GET_PTP_CAPS)
+		return iavf_send_vf_ptp_caps_msg(adapter);
 	if (adapter->aq_required & IAVF_FLAG_AQ_DISABLE_QUEUES) {
 		iavf_disable_queues(adapter);
 		return 0;
@@ -2653,6 +2655,55 @@  static void iavf_init_recv_supported_rxdids(struct iavf_adapter *adapter)
 	iavf_change_state(adapter, __IAVF_INIT_FAILED);
 }
 
+/**
+ * iavf_init_send_ptp_caps - part of querying for extended PTP capabilities
+ * @adapter: board private structure
+ *
+ * Function processes send of the request for 1588 PTP capabilities to the PF.
+ * Must clear IAVF_EXTENDED_CAP_SEND_PTP if the message is not sent, e.g.
+ * due to the PF not negotiating VIRTCHNL_VF_PTP_CAP
+ */
+static void iavf_init_send_ptp_caps(struct iavf_adapter *adapter)
+{
+	if (iavf_send_vf_ptp_caps_msg(adapter) == -EOPNOTSUPP) {
+		/* PF does not support VIRTCHNL_VF_PTP_CAP. In this case, we
+		 * did not send the capability exchange message and do not
+		 * expect a response.
+		 */
+		adapter->extended_caps &= ~IAVF_EXTENDED_CAP_RECV_PTP;
+	}
+
+	/* We sent the message, so move on to the next step */
+	adapter->extended_caps &= ~IAVF_EXTENDED_CAP_SEND_PTP;
+}
+
+/**
+ * iavf_init_recv_ptp_caps - part of querying for supported PTP capabilities
+ * @adapter: board private structure
+ *
+ * Function processes receipt of the PTP capabilities supported on this VF.
+ **/
+static void iavf_init_recv_ptp_caps(struct iavf_adapter *adapter)
+{
+	memset(&adapter->ptp.hw_caps, 0, sizeof(adapter->ptp.hw_caps));
+
+	if (iavf_get_vf_ptp_caps(adapter))
+		goto err;
+
+	/* We've processed the PF response to the VIRTCHNL_OP_1588_PTP_GET_CAPS
+	 * message we sent previously.
+	 */
+	adapter->extended_caps &= ~IAVF_EXTENDED_CAP_RECV_PTP;
+	return;
+
+err:
+	/* We didn't receive a reply. Make sure we try sending again when
+	 * __IAVF_INIT_FAILED attempts to recover.
+	 */
+	adapter->extended_caps |= IAVF_EXTENDED_CAP_SEND_PTP;
+	iavf_change_state(adapter, __IAVF_INIT_FAILED);
+}
+
 /**
  * iavf_init_process_extended_caps - Part of driver startup
  * @adapter: board private structure
@@ -2686,6 +2737,15 @@  static void iavf_init_process_extended_caps(struct iavf_adapter *adapter)
 		return;
 	}
 
+	/* Process capability exchange for PTP features */
+	if (adapter->extended_caps & IAVF_EXTENDED_CAP_SEND_PTP) {
+		iavf_init_send_ptp_caps(adapter);
+		return;
+	} else if (adapter->extended_caps & IAVF_EXTENDED_CAP_RECV_PTP) {
+		iavf_init_recv_ptp_caps(adapter);
+		return;
+	}
+
 	/* When we reach here, no further extended capabilities exchanges are
 	 * necessary, so we finally transition into __IAVF_INIT_CONFIG_ADAPTER
 	 */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_ptp.h b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
new file mode 100644
index 000000000000..65678e76c34f
--- /dev/null
+++ b/drivers/net/ethernet/intel/iavf/iavf_ptp.h
@@ -0,0 +1,9 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2024 Intel Corporation. */
+
+#ifndef _IAVF_PTP_H_
+#define _IAVF_PTP_H_
+
+#include "iavf_types.h"
+
+#endif /* _IAVF_PTP_H_ */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_types.h b/drivers/net/ethernet/intel/iavf/iavf_types.h
new file mode 100644
index 000000000000..6b7029a1a5a7
--- /dev/null
+++ b/drivers/net/ethernet/intel/iavf/iavf_types.h
@@ -0,0 +1,36 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2024 Intel Corporation. */
+
+#ifndef _IAVF_TYPES_H_
+#define _IAVF_TYPES_H_
+
+#include "iavf_types.h"
+
+#include <linux/avf/virtchnl.h>
+#include <linux/ptp_clock_kernel.h>
+
+/* structure used to queue PTP commands for processing */
+struct iavf_ptp_aq_cmd {
+	struct list_head list;
+	enum virtchnl_ops v_opcode:16;
+	u16 msglen;
+	u8 msg[] __counted_by(msglen);
+};
+
+/* fields used for PTP support */
+struct iavf_ptp {
+	wait_queue_head_t phc_time_waitqueue;
+	struct virtchnl_ptp_caps hw_caps;
+	struct ptp_clock_info info;
+	struct ptp_clock *clock;
+	struct list_head aq_cmds;
+	u64 cached_phc_time;
+	unsigned long cached_phc_updated;
+	/* Lock protecting access to the AQ command list */
+	struct mutex aq_cmd_lock;
+	struct kernel_hwtstamp_config hwtstamp_config;
+	bool initialized:1;
+	bool phc_time_ready:1;
+};
+
+#endif /* _IAVF_TYPES_H_ */
diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
index bbd0eb06acd3..63ea30a20576 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
@@ -148,6 +148,7 @@  int iavf_send_vf_config_msg(struct iavf_adapter *adapter)
 	       VIRTCHNL_VF_OFFLOAD_CRC |
 	       VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM |
 	       VIRTCHNL_VF_OFFLOAD_REQ_QUEUES |
+	       VIRTCHNL_VF_CAP_PTP |
 	       VIRTCHNL_VF_OFFLOAD_ADQ |
 	       VIRTCHNL_VF_OFFLOAD_USO |
 	       VIRTCHNL_VF_OFFLOAD_FDIR_PF |
@@ -190,6 +191,41 @@  int iavf_send_vf_supported_rxdids_msg(struct iavf_adapter *adapter)
 				NULL, 0);
 }
 
+/**
+ * iavf_send_vf_ptp_caps_msg - Send request for PTP capabilities
+ * @adapter: private adapter structure
+ *
+ * Send the VIRTCHNL_OP_1588_PTP_GET_CAPS command to the PF to request the PTP
+ * capabilities available to this device. This includes the following
+ * potential access:
+ *
+ * * READ_PHC - access to read the PTP hardware clock time
+ * * RX_TSTAMP - access to request Rx timestamps on all received packets
+ *
+ * The PF will reply with the same opcode a filled out copy of the
+ * virtchnl_ptp_caps structure which defines the specifics of which features
+ * are accessible to this device.
+ *
+ * Return: 0 if success, error code otherwise.
+ */
+int iavf_send_vf_ptp_caps_msg(struct iavf_adapter *adapter)
+{
+	struct virtchnl_ptp_caps hw_caps = {
+		.caps = VIRTCHNL_1588_PTP_CAP_READ_PHC |
+			VIRTCHNL_1588_PTP_CAP_RX_TSTAMP
+	};
+
+	adapter->aq_required &= ~IAVF_FLAG_AQ_GET_PTP_CAPS;
+
+	if (!IAVF_PTP_ALLOWED(adapter))
+		return -EOPNOTSUPP;
+
+	adapter->current_op = VIRTCHNL_OP_1588_PTP_GET_CAPS;
+
+	return iavf_send_pf_msg(adapter, VIRTCHNL_OP_1588_PTP_GET_CAPS,
+				(u8 *)&hw_caps, sizeof(hw_caps));
+}
+
 /**
  * iavf_validate_num_queues
  * @adapter: adapter structure
@@ -307,6 +343,38 @@  int iavf_get_vf_supported_rxdids(struct iavf_adapter *adapter)
 	return 0;
 }
 
+int iavf_get_vf_ptp_caps(struct iavf_adapter *adapter)
+{
+	struct virtchnl_ptp_caps caps = {};
+	struct iavf_hw *hw = &adapter->hw;
+	struct iavf_arq_event_info event;
+	enum virtchnl_ops op;
+	enum iavf_status err;
+
+	event.msg_buf = (u8 *)&caps;
+	event.buf_len = sizeof(caps);
+
+	while (1) {
+		/* When the AQ is empty, iavf_clean_arq_element will return
+		 * nonzero and this loop will terminate.
+		 */
+		err = iavf_clean_arq_element(hw, &event, NULL);
+		if (err != IAVF_SUCCESS)
+			return err;
+		op = (enum virtchnl_ops)le32_to_cpu(event.desc.cookie_high);
+		if (op == VIRTCHNL_OP_1588_PTP_GET_CAPS)
+			break;
+	}
+
+	err = le32_to_cpu(event.desc.cookie_low);
+	if (err)
+		return err;
+
+	memcpy(&adapter->ptp.hw_caps, &caps, sizeof(caps));
+
+	return 0;
+}
+
 /**
  * iavf_configure_queues
  * @adapter: adapter structure
@@ -2423,6 +2491,10 @@  void iavf_virtchnl_completion(struct iavf_adapter *adapter,
 		memcpy(&adapter->supp_rxdids, msg,
 		       min_t(u16, msglen, sizeof(adapter->supp_rxdids)));
 		break;
+	case VIRTCHNL_OP_1588_PTP_GET_CAPS:
+		memcpy(&adapter->ptp.hw_caps, msg,
+		       min_t(u16, msglen, sizeof(adapter->ptp.hw_caps)));
+		break;
 	case VIRTCHNL_OP_ENABLE_QUEUES:
 		/* enable transmits */
 		iavf_irq_enable(adapter, true);