[Intel-wired-lan] [PATCH] ice: implement snapshot for device capabilities
Keller, Jacob E
jacob.e.keller at intel.com
Thu Jun 18 17:37:15 UTC 2020
> -----Original Message-----
> From: Keller, Jacob E <jacob.e.keller at intel.com>
> Sent: Tuesday, June 16, 2020 5:06 PM
> To: Intel Wired LAN <intel-wired-lan at lists.osuosl.org>
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher at intel.com>; Keller, Jacob E
> <jacob.e.keller at intel.com>
> Subject: [PATCH] ice: implement snapshot for device capabilities
>
> Add a new devlink region used for capturing a snapshot of the device
> capabilities buffer which is reported by the firmware over the AdminQ.
> This information can useful in debugging driver and firmware
> interactions.
>
> Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 1 +
> drivers/net/ethernet/intel/ice/ice_common.c | 66 +++++++++++++++-----
> drivers/net/ethernet/intel/ice/ice_common.h | 3 +
> drivers/net/ethernet/intel/ice/ice_devlink.c | 57 +++++++++++++++++
> 4 files changed, 113 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index a4cda8212e64..7486d010a619 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -373,6 +373,7 @@ struct ice_pf {
> struct devlink_port devlink_port;
>
> struct devlink_region *nvm_region;
> + struct devlink_region *devcaps_region;
>
> /* OS reserved IRQ details */
> struct msix_entry *msix_entries;
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index bce0e1281168..e3cb7bd1ecab 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -1823,20 +1823,27 @@ ice_parse_caps(struct ice_hw *hw, void *buf, u32
> cap_count,
> }
>
> /**
> - * ice_aq_discover_caps - query function/device capabilities
> + * ice_aq_list_caps - query function/device capabilities
> * @hw: pointer to the HW struct
> - * @buf: a virtual buffer to hold the capabilities
> - * @buf_size: Size of the virtual buffer
> - * @cap_count: cap count needed if AQ err==ENOMEM
> - * @opc: capabilities type to discover - pass in the command opcode
> + * @buf: a buffer to hold the capabilities
> + * @buf_size: size of the buffer
> + * @cap_count: if not NULL, set to the number of capabilities reported
> + * @opc: capabilities type to discover, device or function
> * @cd: pointer to command details structure or NULL
> *
> - * Get the function(0x000a)/device(0x000b) capabilities description from
> - * the firmware.
> + * Get the function (0x000A) or device (0x000B) capabilities description from
> + * firmware and store it in the buffer.
> + *
> + * If the cap_count pointer is not NULL, then it is set to the number of
> + * capabilities firmware will report. Note that if the buffer size is too
> + * small, it is possible the command will return ICE_AQ_ERR_ENOMEM. The
> + * cap_count will still be updated in this case. It is recommended that the
> + * buffer size be set to ICE_AQ_MAX_BUF_LEN (the largest possible buffer that
> + * firmware could return) to avoid this.
> */
> -static enum ice_status
> -ice_aq_discover_caps(struct ice_hw *hw, void *buf, u16 buf_size, u32
> *cap_count,
> - enum ice_adminq_opc opc, struct ice_sq_cd *cd)
> +enum ice_status
> +ice_aq_list_caps(struct ice_hw *hw, void *buf, u16 buf_size, u32 *cap_count,
> + enum ice_adminq_opc opc, struct ice_sq_cd *cd)
> {
> struct ice_aqc_list_caps *cmd;
> struct ice_aq_desc desc;
> @@ -1849,12 +1856,43 @@ ice_aq_discover_caps(struct ice_hw *hw, void *buf,
> u16 buf_size, u32 *cap_count,
> return ICE_ERR_PARAM;
>
> ice_fill_dflt_direct_cmd_desc(&desc, opc);
> -
> status = ice_aq_send_cmd(hw, &desc, buf, buf_size, cd);
> - if (!status)
> - ice_parse_caps(hw, buf, le32_to_cpu(cmd->count), opc);
> - else if (hw->adminq.sq_last_status == ICE_AQ_RC_ENOMEM)
> +
> + if (cap_count)
> *cap_count = le32_to_cpu(cmd->count);
> +
> + return status;
> +}
> +
> +/**
> + * ice_aq_discover_caps - query function/device capabilities
> + * @hw: pointer to the HW struct
> + * @buf: a virtual buffer to hold the capabilities
> + * @buf_size: Size of the virtual buffer
> + * @cap_count: cap count needed if AQ err==ENOMEM
> + * @opc: capabilities type to discover - pass in the command opcode
> + * @cd: pointer to command details structure or NULL
> + *
> + * Get the function(0x000a)/device(0x000b) capabilities description from
> + * the firmware.
> + *
> + * NOTE: this function has the side effect of updating the hw->dev_caps or
> + * hw->func_caps by way of calling ice_parse_caps.
> + */
> +static enum ice_status
> +ice_aq_discover_caps(struct ice_hw *hw, void *buf, u16 buf_size, u32
> *cap_count,
> + enum ice_adminq_opc opc, struct ice_sq_cd *cd)
> +{
> + u32 local_cap_count = 0;
> + enum ice_status status;
> +
> + status = ice_aq_list_caps(hw, buf, buf_size, &local_cap_count,
> + opc, cd);
> + if (!status)
> + ice_parse_caps(hw, buf, local_cap_count, opc);
> + else if (hw->adminq.sq_last_status == ICE_AQ_RC_ENOMEM)
> + *cap_count = local_cap_count;
> +
> return status;
> }
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.h
> b/drivers/net/ethernet/intel/ice/ice_common.h
> index 9b9e50d2398b..e72529e9f022 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.h
> +++ b/drivers/net/ethernet/intel/ice/ice_common.h
> @@ -87,6 +87,9 @@ enum ice_status
> ice_aq_get_phy_caps(struct ice_port_info *pi, bool qual_mods, u8
> report_mode,
> struct ice_aqc_get_phy_caps_data *caps,
> struct ice_sq_cd *cd);
> +enum ice_status
> +ice_aq_list_caps(struct ice_hw *hw, void *buf, u16 buf_size, u32 *cap_count,
> + enum ice_adminq_opc opc, struct ice_sq_cd *cd);
> void
> ice_update_phy_type(u64 *phy_type_low, u64 *phy_type_high,
> u16 link_speeds_bitmap);
> diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c
> b/drivers/net/ethernet/intel/ice/ice_devlink.c
> index a73d06e06b5d..10efe6c5914b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_devlink.c
> +++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
> @@ -397,12 +397,60 @@ static int ice_devlink_nvm_snapshot(struct devlink
> *devlink,
> return 0;
> }
>
> +/**
> + * ice_devlink_devcaps_snapshot - Capture snapshot of device capabilities
> + * @devlink: the devlink instance
> + * @extack: extended ACK response structure
> + * @data: on exit points to snapshot data buffer
> + *
> + * This function is called in response to the DEVLINK_CMD_REGION_TRIGGER
> for
> + * the device-caps devlink region. It captures a snapshot of the device
> + * capabilities reported by firmware.
> + *
> + * @returns zero on success, and updates the data pointer. Returns a non-zero
> + * error code on failure.
> + */
> +static int
> +ice_devlink_devcaps_snapshot(struct devlink *devlink,
> + struct netlink_ext_ack *extack, u8 **data)
> +{
> + struct ice_pf *pf = devlink_priv(devlink);
> + struct device *dev = ice_pf_to_dev(pf);
> + struct ice_hw *hw = &pf->hw;
> + enum ice_status status;
> + void *devcaps;
> +
> + devcaps = vzalloc(ICE_AQ_MAX_BUF_LEN);
> + if (!devcaps)
> + return -ENOMEM;
> +
> + status = ice_aq_list_caps(hw, devcaps, ICE_AQ_MAX_BUF_LEN, NULL,
> + ice_aqc_opc_list_dev_caps, NULL);
> + if (status) {
> + dev_dbg(dev, "ice_aq_list_caps: failed to read device capabilities,
> err %d aq_err %d\n",
> + status, hw->adminq.sq_last_status);
> + NL_SET_ERR_MSG_MOD(extack, "Failed to read device
> capabilities");
> + vfree(devcaps);
> + return -EIO;
> + }
> +
> + *data = (u8 *)devcaps;
> +
> + return 0;
> +}
> +
> static const struct devlink_region_ops ice_nvm_region_ops = {
> .name = "nvm-flash",
> .destructor = vfree,
> .snapshot = ice_devlink_nvm_snapshot,
> };
>
> +static const struct devlink_region_ops ice_devcaps_region_ops = {
> + .name = "device-caps",
> + .destructor = vfree,
> + .snapshot = ice_devlink_devcaps_snapshot,
> +};
> +
> /**
> * ice_devlink_init_regions - Initialize devlink regions
> * @pf: the PF device structure
> @@ -424,6 +472,15 @@ void ice_devlink_init_regions(struct ice_pf *pf)
> PTR_ERR(pf->nvm_region));
> pf->nvm_region = NULL;
> }
> +
> + pf->devcaps_region = devlink_region_create(devlink,
> + &ice_devcaps_region_ops, 10,
> + ICE_AQ_MAX_BUF_LEN);
> + if (IS_ERR(pf->devcaps_region)) {
> + dev_err(dev, "failed to create device-caps devlink region, err
> %ld\n",
> + PTR_ERR(pf->devcaps_region));
> + pf->devcaps_region = NULL;
> + }
This patch has a flaw, in that it fails to destroy the region during shutdown. I'll be sending a v2 soon.
Thanks,
Jake
> }
>
> /**
> --
> 2.25.2
More information about the Intel-wired-lan
mailing list