* [PATCH net-next 1/9] ice: pass VSI pointer into ice_vc_isvalid_q_id
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 2/9] ice: remove unnecessary duplicate checks for VF VSI ID Tony Nguyen
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel, Rafal Romanowski
From: Jacob Keller <jacob.e.keller@intel•com>
The ice_vc_isvalid_q_id() function takes a VSI index and a queue ID. It
looks up the VSI from its index, and then validates that the queue number
is valid for that VSI.
The VSI ID passed is typically a VSI index from the VF. This VSI number is
validated by the PF to ensure that it matches the VSI associated with the
VF already.
In every flow where ice_vc_isvalid_q_id() is called, the PF driver already
has a pointer to the VSI associated with the VF. This pointer is obtained
using ice_get_vf_vsi(), rather than looking up the VSI using the index sent
by the VF.
Since we already know which VSI to operate on, we can modify
ice_vc_isvalid_q_id() to take a VSI pointer instead of a VSI index. Pass
the VSI we found from ice_get_vf_vsi() instead of re-doing the lookup. This
removes some unnecessary computation and scanning of the VSI list.
It also removes the last place where the driver directly used the VSI
number from the VF. This will pave the way for refactoring to communicate
relative VSI numbers to the VF instead of absolute numbers from the PF
space.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 22 +++++++++----------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index c925813ec9ca..3dd4e5af0b6a 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -562,17 +562,15 @@ bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id)
/**
* ice_vc_isvalid_q_id
- * @vf: pointer to the VF info
- * @vsi_id: VSI ID
+ * @vsi: VSI to check queue ID against
* @qid: VSI relative queue ID
*
* check for the valid queue ID
*/
-static bool ice_vc_isvalid_q_id(struct ice_vf *vf, u16 vsi_id, u8 qid)
+static bool ice_vc_isvalid_q_id(struct ice_vsi *vsi, u8 qid)
{
- struct ice_vsi *vsi = ice_find_vsi(vf->pf, vsi_id);
/* allocated Tx and Rx queues should be always equal for VF VSI */
- return (vsi && (qid < vsi->alloc_txq));
+ return qid < vsi->alloc_txq;
}
/**
@@ -1330,7 +1328,7 @@ static int ice_vc_ena_qs_msg(struct ice_vf *vf, u8 *msg)
*/
q_map = vqs->rx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1352,7 +1350,7 @@ static int ice_vc_ena_qs_msg(struct ice_vf *vf, u8 *msg)
q_map = vqs->tx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1457,7 +1455,7 @@ static int ice_vc_dis_qs_msg(struct ice_vf *vf, u8 *msg)
q_map = vqs->tx_queues;
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1483,7 +1481,7 @@ static int ice_vc_dis_qs_msg(struct ice_vf *vf, u8 *msg)
bitmap_zero(vf->rxq_ena, ICE_MAX_RSS_QS_PER_VF);
} else if (q_map) {
for_each_set_bit(vf_q_id, &q_map, ICE_MAX_RSS_QS_PER_VF) {
- if (!ice_vc_isvalid_q_id(vf, vqs->vsi_id, vf_q_id)) {
+ if (!ice_vc_isvalid_q_id(vsi, vf_q_id)) {
v_ret = VIRTCHNL_STATUS_ERR_PARAM;
goto error_param;
}
@@ -1539,7 +1537,7 @@ ice_cfg_interrupt(struct ice_vf *vf, struct ice_vsi *vsi, u16 vector_id,
for_each_set_bit(vsi_q_id_idx, &qmap, ICE_MAX_RSS_QS_PER_VF) {
vsi_q_id = vsi_q_id_idx;
- if (!ice_vc_isvalid_q_id(vf, vsi->vsi_num, vsi_q_id))
+ if (!ice_vc_isvalid_q_id(vsi, vsi_q_id))
return VIRTCHNL_STATUS_ERR_PARAM;
q_vector->num_ring_rx++;
@@ -1553,7 +1551,7 @@ ice_cfg_interrupt(struct ice_vf *vf, struct ice_vsi *vsi, u16 vector_id,
for_each_set_bit(vsi_q_id_idx, &qmap, ICE_MAX_RSS_QS_PER_VF) {
vsi_q_id = vsi_q_id_idx;
- if (!ice_vc_isvalid_q_id(vf, vsi->vsi_num, vsi_q_id))
+ if (!ice_vc_isvalid_q_id(vsi, vsi_q_id))
return VIRTCHNL_STATUS_ERR_PARAM;
q_vector->num_ring_tx++;
@@ -1710,7 +1708,7 @@ static int ice_vc_cfg_qs_msg(struct ice_vf *vf, u8 *msg)
qpi->txq.headwb_enabled ||
!ice_vc_isvalid_ring_len(qpi->txq.ring_len) ||
!ice_vc_isvalid_ring_len(qpi->rxq.ring_len) ||
- !ice_vc_isvalid_q_id(vf, qci->vsi_id, qpi->txq.queue_id)) {
+ !ice_vc_isvalid_q_id(vsi, qpi->txq.queue_id)) {
goto error_param;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 2/9] ice: remove unnecessary duplicate checks for VF VSI ID
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 1/9] ice: pass VSI pointer into ice_vc_isvalid_q_id Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 3/9] ice: use relative VSI index for VFs instead of PF VSI number Tony Nguyen
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel, Rafal Romanowski
From: Jacob Keller <jacob.e.keller@intel•com>
The ice_vc_fdir_param_check() function validates that the VSI ID of the
virtchnl flow director command matches the VSI number of the VF. This is
already checked by the call to ice_vc_isvalid_vsi_id() immediately
following this.
This check is unnecessary since ice_vc_isvalid_vsi_id() already confirms
this by checking that the VSI ID can locate the VSI associated with the VF
structure.
Furthermore, a following change is going to refactor the ice driver to
report VSI IDs using a relative index for each VF instead of reporting the
PF VSI number. This additional check would break that logic since it
enforces that the VSI ID matches the VSI number.
Since this check duplicates the logic in ice_vc_isvalid_vsi_id() and gets
in the way of refactoring that logic, remove it.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
index f001553e1a1a..8e4ff3af86c6 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_fdir.c
@@ -94,9 +94,6 @@ ice_vc_fdir_param_check(struct ice_vf *vf, u16 vsi_id)
if (!(vf->driver_caps & VIRTCHNL_VF_OFFLOAD_FDIR_PF))
return -EINVAL;
- if (vsi_id != vf->lan_vsi_num)
- return -EINVAL;
-
if (!ice_vc_isvalid_vsi_id(vf, vsi_id))
return -EINVAL;
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 3/9] ice: use relative VSI index for VFs instead of PF VSI number
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 1/9] ice: pass VSI pointer into ice_vc_isvalid_q_id Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 2/9] ice: remove unnecessary duplicate checks for VF VSI ID Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 4/9] ice: remove vf->lan_vsi_num field Tony Nguyen
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel, Rafal Romanowski
From: Jacob Keller <jacob.e.keller@intel•com>
When initializing over virtchnl, the PF is required to pass a VSI ID to the
VF as part of its capabilities exchange. The VF driver reports this value
back to the PF in a variety of commands. The PF driver validates that this
value matches the value it sent to the VF.
Some hardware families such as the E700 series could use this value when
reading RSS registers or communicating directly with firmware over the
Admin Queue.
However, E800 series hardware does not support any of these interfaces and
the VF's only use for this value is to report it back to the PF. Thus,
there is no requirement that this value be an actual VSI ID value of any
kind.
The PF driver already does not trust that the VF sends it a real VSI ID.
The VSI structure is always looked up from the VF structure. The PF does
validate that the VSI ID provided matches a VSI associated with the VF, but
otherwise does not use the VSI ID for any purpose.
Instead of reporting the VSI number relative to the PF space, report a
fixed value of 1. When communicating with the VF over virtchnl, validate
that the VSI number is returned appropriately.
This avoids leaking information about the firmware of the PF state.
Currently the ice driver only supplies a VF with a single VSI. However, it
appears that virtchnl has some support for allowing multiple VSIs. I did
not attempt to implement this. However, space is left open to allow further
relative indexes if additional VSIs are provided in future feature
development. For this reason, keep the ice_vc_isvalid_vsi_id function in
place to allow extending it for multiple VSIs in the future.
This change will also simplify handling of live migration in a future
series. Since we no longer will provide a real VSI number to the VF, there
will be no need to keep track of this number when migrating to a new host.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_virtchnl.c | 9 ++-------
drivers/net/ethernet/intel/ice/ice_virtchnl.h | 9 +++++++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.c b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
index 3dd4e5af0b6a..6b88b9bfb51e 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.c
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.c
@@ -506,7 +506,7 @@ static int ice_vc_get_vf_res_msg(struct ice_vf *vf, u8 *msg)
vfres->rss_lut_size = ICE_LUT_VSI_SIZE;
vfres->max_mtu = ice_vc_get_max_frame_size(vf);
- vfres->vsi_res[0].vsi_id = vf->lan_vsi_num;
+ vfres->vsi_res[0].vsi_id = ICE_VF_VSI_ID;
vfres->vsi_res[0].vsi_type = VIRTCHNL_VSI_SRIOV;
vfres->vsi_res[0].num_queue_pairs = vsi->num_txq;
ether_addr_copy(vfres->vsi_res[0].default_mac_addr,
@@ -552,12 +552,7 @@ static void ice_vc_reset_vf_msg(struct ice_vf *vf)
*/
bool ice_vc_isvalid_vsi_id(struct ice_vf *vf, u16 vsi_id)
{
- struct ice_pf *pf = vf->pf;
- struct ice_vsi *vsi;
-
- vsi = ice_find_vsi(pf, vsi_id);
-
- return (vsi && (vsi->vf == vf));
+ return vsi_id == ICE_VF_VSI_ID;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl.h b/drivers/net/ethernet/intel/ice/ice_virtchnl.h
index 60dfbe05980a..3a4115869153 100644
--- a/drivers/net/ethernet/intel/ice/ice_virtchnl.h
+++ b/drivers/net/ethernet/intel/ice/ice_virtchnl.h
@@ -19,6 +19,15 @@
#define ICE_MAX_MACADDR_PER_VF 18
#define ICE_FLEX_DESC_RXDID_MAX_NUM 64
+/* VFs only get a single VSI. For ice hardware, the VF does not need to know
+ * its VSI index. However, the virtchnl interface requires a VSI number,
+ * mainly due to legacy hardware.
+ *
+ * Since the VF doesn't need this information, report a static value to the VF
+ * instead of leaking any information about the PF or hardware setup.
+ */
+#define ICE_VF_VSI_ID 1
+
struct ice_virtchnl_ops {
int (*get_ver_msg)(struct ice_vf *vf, u8 *msg);
int (*get_vf_res_msg)(struct ice_vf *vf, u8 *msg);
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 4/9] ice: remove vf->lan_vsi_num field
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (2 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 3/9] ice: use relative VSI index for VFs instead of PF VSI number Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 5/9] ice: rename ice_write_* functions to ice_pack_ctx_* Tony Nguyen
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel, Rafal Romanowski
From: Jacob Keller <jacob.e.keller@intel•com>
The lan_vsi_num field of the VF structure is no longer used for any
purpose. Remove it.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_sriov.c | 1 -
drivers/net/ethernet/intel/ice/ice_vf_lib.c | 10 +---------
drivers/net/ethernet/intel/ice/ice_vf_lib.h | 5 -----
3 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c b/drivers/net/ethernet/intel/ice/ice_sriov.c
index a94a1c48c3de..a2f8dbe3cb82 100644
--- a/drivers/net/ethernet/intel/ice/ice_sriov.c
+++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
@@ -240,7 +240,6 @@ static struct ice_vsi *ice_vf_vsi_setup(struct ice_vf *vf)
}
vf->lan_vsi_idx = vsi->idx;
- vf->lan_vsi_num = vsi->vsi_num;
return vsi;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
index 2ffdae9a82df..21d26e19338a 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
@@ -280,12 +280,6 @@ int ice_vf_reconfig_vsi(struct ice_vf *vf)
return err;
}
- /* Update the lan_vsi_num field since it might have been changed. The
- * PF lan_vsi_idx number remains the same so we don't need to change
- * that.
- */
- vf->lan_vsi_num = vsi->vsi_num;
-
return 0;
}
@@ -315,7 +309,6 @@ static int ice_vf_rebuild_vsi(struct ice_vf *vf)
* vf->lan_vsi_idx
*/
vsi->vsi_num = ice_get_hw_vsi_num(&pf->hw, vsi->idx);
- vf->lan_vsi_num = vsi->vsi_num;
return 0;
}
@@ -1315,13 +1308,12 @@ int ice_vf_init_host_cfg(struct ice_vf *vf, struct ice_vsi *vsi)
}
/**
- * ice_vf_invalidate_vsi - invalidate vsi_idx/vsi_num to remove VSI access
+ * ice_vf_invalidate_vsi - invalidate vsi_idx to remove VSI access
* @vf: VF to remove access to VSI for
*/
void ice_vf_invalidate_vsi(struct ice_vf *vf)
{
vf->lan_vsi_idx = ICE_NO_VSI;
- vf->lan_vsi_num = ICE_NO_VSI;
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.h b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
index 0cc9034065c5..fec16919ec19 100644
--- a/drivers/net/ethernet/intel/ice/ice_vf_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.h
@@ -109,11 +109,6 @@ struct ice_vf {
u8 spoofchk:1;
u8 link_forced:1;
u8 link_up:1; /* only valid if VF link is forced */
- /* VSI indices - actual VSI pointers are maintained in the PF structure
- * When assigned, these will be non-zero, because VSI 0 is always
- * the main LAN VSI for the PF.
- */
- u16 lan_vsi_num; /* ID as used by firmware */
unsigned int min_tx_rate; /* Minimum Tx bandwidth limit in Mbps */
unsigned int max_tx_rate; /* Maximum Tx bandwidth limit in Mbps */
DECLARE_BITMAP(vf_states, ICE_VF_STATES_NBITS); /* VF runtime states */
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 5/9] ice: rename ice_write_* functions to ice_pack_ctx_*
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (3 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 4/9] ice: remove vf->lan_vsi_num field Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 6/9] ice: use GENMASK instead of BIT(n) - 1 in pack functions Tony Nguyen
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel
From: Jacob Keller <jacob.e.keller@intel•com>
In ice_common.c there are 4 functions used for converting the unpacked
software Tx and Rx context structure data into the packed format used by
hardware. These functions have extremely generic names:
* ice_write_byte
* ice_write_word
* ice_write_dword
* ice_write_qword
When I saw these function names my first thought was "write what? to
where?". Understanding what these functions do requires looking at the
implementation details. The functions take bits from an unpacked structure
and copy them into the packed layout used by hardware.
As part of live migration, we will want functions which perform the inverse
operation of reading bits from the packed layout and copying them into the
unpacked format. Naming these as "ice_read_byte", etc would be very
confusing since they appear to write data.
In preparation for adding this new inverse operation, rename the existing
functions to use the prefix "ice_pack_ctx_". This makes it clear that they
perform the bit packing while copying from the unpacked software context
structure to the packed hardware context.
The inverse operations can then neatly be named ice_unpack_ctx_*, clearly
indicating they perform the bit unpacking while copying from the packed
hardware context to the unpacked software context structure.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 56 ++++++++++-----------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 9266f25a9978..3622079cb506 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4362,13 +4362,13 @@ ice_aq_add_rdma_qsets(struct ice_hw *hw, u8 num_qset_grps,
/* End of FW Admin Queue command wrappers */
/**
- * ice_write_byte - write a byte to a packed context structure
- * @src_ctx: the context structure to read from
- * @dest_ctx: the context to be written to
- * @ce_info: a description of the struct to be filled
+ * ice_pack_ctx_byte - write a byte to a packed context structure
+ * @src_ctx: unpacked source context structure
+ * @dest_ctx: packed destination context data
+ * @ce_info: context element description
*/
-static void
-ice_write_byte(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
+static void ice_pack_ctx_byte(u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info)
{
u8 src_byte, dest_byte, mask;
u8 *from, *dest;
@@ -4401,13 +4401,13 @@ ice_write_byte(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
}
/**
- * ice_write_word - write a word to a packed context structure
- * @src_ctx: the context structure to read from
- * @dest_ctx: the context to be written to
- * @ce_info: a description of the struct to be filled
+ * ice_pack_ctx_word - write a word to a packed context structure
+ * @src_ctx: unpacked source context structure
+ * @dest_ctx: packed destination context data
+ * @ce_info: context element description
*/
-static void
-ice_write_word(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
+static void ice_pack_ctx_word(u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info)
{
u16 src_word, mask;
__le16 dest_word;
@@ -4444,13 +4444,13 @@ ice_write_word(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
}
/**
- * ice_write_dword - write a dword to a packed context structure
- * @src_ctx: the context structure to read from
- * @dest_ctx: the context to be written to
- * @ce_info: a description of the struct to be filled
+ * ice_pack_ctx_dword - write a dword to a packed context structure
+ * @src_ctx: unpacked source context structure
+ * @dest_ctx: packed destination context data
+ * @ce_info: context element description
*/
-static void
-ice_write_dword(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
+static void ice_pack_ctx_dword(u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info)
{
u32 src_dword, mask;
__le32 dest_dword;
@@ -4495,13 +4495,13 @@ ice_write_dword(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
}
/**
- * ice_write_qword - write a qword to a packed context structure
- * @src_ctx: the context structure to read from
- * @dest_ctx: the context to be written to
- * @ce_info: a description of the struct to be filled
+ * ice_pack_ctx_qword - write a qword to a packed context structure
+ * @src_ctx: unpacked source context structure
+ * @dest_ctx: packed destination context data
+ * @ce_info: context element description
*/
-static void
-ice_write_qword(u8 *src_ctx, u8 *dest_ctx, const struct ice_ctx_ele *ce_info)
+static void ice_pack_ctx_qword(u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info)
{
u64 src_qword, mask;
__le64 dest_qword;
@@ -4570,16 +4570,16 @@ ice_set_ctx(struct ice_hw *hw, u8 *src_ctx, u8 *dest_ctx,
}
switch (ce_info[f].size_of) {
case sizeof(u8):
- ice_write_byte(src_ctx, dest_ctx, &ce_info[f]);
+ ice_pack_ctx_byte(src_ctx, dest_ctx, &ce_info[f]);
break;
case sizeof(u16):
- ice_write_word(src_ctx, dest_ctx, &ce_info[f]);
+ ice_pack_ctx_word(src_ctx, dest_ctx, &ce_info[f]);
break;
case sizeof(u32):
- ice_write_dword(src_ctx, dest_ctx, &ce_info[f]);
+ ice_pack_ctx_dword(src_ctx, dest_ctx, &ce_info[f]);
break;
case sizeof(u64):
- ice_write_qword(src_ctx, dest_ctx, &ce_info[f]);
+ ice_pack_ctx_qword(src_ctx, dest_ctx, &ce_info[f]);
break;
default:
return -EINVAL;
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 6/9] ice: use GENMASK instead of BIT(n) - 1 in pack functions
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (4 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 5/9] ice: rename ice_write_* functions to ice_pack_ctx_* Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 7/9] ice: cleanup line splitting for context set functions Tony Nguyen
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel,
Pucha Himasekhar Reddy
From: Jacob Keller <jacob.e.keller@intel•com>
The functions used to pack the Tx and Rx context into the hardware format
rely on using BIT() and then subtracting 1 to get a bitmask. These
functions even have a comment about how x86 machines can't use this method
for certain widths because the SHL instructions will not work properly.
The Linux kernel already provides the GENMASK macro for generating a
suitable bitmask. Further, GENMASK is capable of generating the mask
including the shift_width. Since width is the total field width, take care
to subtract one to get the final bit position.
Since we now include the shifted bits as part of the mask, shift the source
value first before applying the mask.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel•com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 44 ++++-----------------
1 file changed, 8 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 3622079cb506..e9e4a8ef7904 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4379,14 +4379,11 @@ static void ice_pack_ctx_byte(u8 *src_ctx, u8 *dest_ctx,
/* prepare the bits and mask */
shift_width = ce_info->lsb % 8;
- mask = (u8)(BIT(ce_info->width) - 1);
+ mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
src_byte = *from;
- src_byte &= mask;
-
- /* shift to correct alignment */
- mask <<= shift_width;
src_byte <<= shift_width;
+ src_byte &= mask;
/* get the current bits from the target bit string */
dest = dest_ctx + (ce_info->lsb / 8);
@@ -4419,17 +4416,14 @@ static void ice_pack_ctx_word(u8 *src_ctx, u8 *dest_ctx,
/* prepare the bits and mask */
shift_width = ce_info->lsb % 8;
- mask = BIT(ce_info->width) - 1;
+ mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
/* don't swizzle the bits until after the mask because the mask bits
* will be in a different bit position on big endian machines
*/
src_word = *(u16 *)from;
- src_word &= mask;
-
- /* shift to correct alignment */
- mask <<= shift_width;
src_word <<= shift_width;
+ src_word &= mask;
/* get the current bits from the target bit string */
dest = dest_ctx + (ce_info->lsb / 8);
@@ -4462,25 +4456,14 @@ static void ice_pack_ctx_dword(u8 *src_ctx, u8 *dest_ctx,
/* prepare the bits and mask */
shift_width = ce_info->lsb % 8;
-
- /* if the field width is exactly 32 on an x86 machine, then the shift
- * operation will not work because the SHL instructions count is masked
- * to 5 bits so the shift will do nothing
- */
- if (ce_info->width < 32)
- mask = BIT(ce_info->width) - 1;
- else
- mask = (u32)~0;
+ mask = GENMASK(ce_info->width - 1 + shift_width, shift_width);
/* don't swizzle the bits until after the mask because the mask bits
* will be in a different bit position on big endian machines
*/
src_dword = *(u32 *)from;
- src_dword &= mask;
-
- /* shift to correct alignment */
- mask <<= shift_width;
src_dword <<= shift_width;
+ src_dword &= mask;
/* get the current bits from the target bit string */
dest = dest_ctx + (ce_info->lsb / 8);
@@ -4513,25 +4496,14 @@ static void ice_pack_ctx_qword(u8 *src_ctx, u8 *dest_ctx,
/* prepare the bits and mask */
shift_width = ce_info->lsb % 8;
-
- /* if the field width is exactly 64 on an x86 machine, then the shift
- * operation will not work because the SHL instructions count is masked
- * to 6 bits so the shift will do nothing
- */
- if (ce_info->width < 64)
- mask = BIT_ULL(ce_info->width) - 1;
- else
- mask = (u64)~0;
+ mask = GENMASK_ULL(ce_info->width - 1 + shift_width, shift_width);
/* don't swizzle the bits until after the mask because the mask bits
* will be in a different bit position on big endian machines
*/
src_qword = *(u64 *)from;
- src_qword &= mask;
-
- /* shift to correct alignment */
- mask <<= shift_width;
src_qword <<= shift_width;
+ src_qword &= mask;
/* get the current bits from the target bit string */
dest = dest_ctx + (ce_info->lsb / 8);
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 7/9] ice: cleanup line splitting for context set functions
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (5 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 6/9] ice: use GENMASK instead of BIT(n) - 1 in pack functions Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 8/9] ice: do not disable Tx queues twice in ice_down() Tony Nguyen
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Jacob Keller, anthony.l.nguyen, Przemek Kitszel
From: Jacob Keller <jacob.e.keller@intel•com>
The indentation for ice_set_ctx and ice_write_rxq_ctx breaks the function
name after the return type. This style of breaking is used a lot throughout
the ice driver, even in cases where its not actually helpful for
readability. We no longer prefer this style of line splitting in the
driver, and new code is avoiding it.
Normally, I would leave this alone unless the actual function contents or
description needed updating. However, a future change is going to add
inverse functions for converting packed context to unpacked context
structures. To keep this code uniform with the existing set functions, fix
up the style to the modern format of keeping the type on the same line.
Signed-off-by: Jacob Keller <jacob.e.keller@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 12 +++++-------
drivers/net/ethernet/intel/ice/ice_common.h | 10 ++++------
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index e9e4a8ef7904..7d9315ffae57 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1399,9 +1399,8 @@ static const struct ice_ctx_ele ice_rlan_ctx_info[] = {
* it to HW register space and enables the hardware to prefetch descriptors
* instead of only fetching them on demand
*/
-int
-ice_write_rxq_ctx(struct ice_hw *hw, struct ice_rlan_ctx *rlan_ctx,
- u32 rxq_index)
+int ice_write_rxq_ctx(struct ice_hw *hw, struct ice_rlan_ctx *rlan_ctx,
+ u32 rxq_index)
{
u8 ctx_buf[ICE_RXQ_CTX_SZ] = { 0 };
@@ -4522,11 +4521,10 @@ static void ice_pack_ctx_qword(u8 *src_ctx, u8 *dest_ctx,
* @hw: pointer to the hardware structure
* @src_ctx: pointer to a generic non-packed context structure
* @dest_ctx: pointer to memory for the packed structure
- * @ce_info: a description of the structure to be transformed
+ * @ce_info: List of Rx context elements
*/
-int
-ice_set_ctx(struct ice_hw *hw, u8 *src_ctx, u8 *dest_ctx,
- const struct ice_ctx_ele *ce_info)
+int ice_set_ctx(struct ice_hw *hw, u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info)
{
int f;
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 32fd10de620c..ffb22c7ce28b 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -53,9 +53,8 @@ int ice_get_caps(struct ice_hw *hw);
void ice_set_safe_mode_caps(struct ice_hw *hw);
-int
-ice_write_rxq_ctx(struct ice_hw *hw, struct ice_rlan_ctx *rlan_ctx,
- u32 rxq_index);
+int ice_write_rxq_ctx(struct ice_hw *hw, struct ice_rlan_ctx *rlan_ctx,
+ u32 rxq_index);
int
ice_aq_get_rss_lut(struct ice_hw *hw, struct ice_aq_get_set_rss_lut_params *get_params);
@@ -72,9 +71,8 @@ bool ice_check_sq_alive(struct ice_hw *hw, struct ice_ctl_q_info *cq);
int ice_aq_q_shutdown(struct ice_hw *hw, bool unloading);
void ice_fill_dflt_direct_cmd_desc(struct ice_aq_desc *desc, u16 opcode);
extern const struct ice_ctx_ele ice_tlan_ctx_info[];
-int
-ice_set_ctx(struct ice_hw *hw, u8 *src_ctx, u8 *dest_ctx,
- const struct ice_ctx_ele *ce_info);
+int ice_set_ctx(struct ice_hw *hw, u8 *src_ctx, u8 *dest_ctx,
+ const struct ice_ctx_ele *ce_info);
extern struct mutex ice_global_cfg_lock_sw;
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 8/9] ice: do not disable Tx queues twice in ice_down()
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (6 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 7/9] ice: cleanup line splitting for context set functions Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-04 21:29 ` [PATCH net-next 9/9] ice: avoid unnecessary devm_ usage Tony Nguyen
2024-03-06 11:30 ` [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) patchwork-bot+netdevbpf
9 siblings, 0 replies; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Maciej Fijalkowski, anthony.l.nguyen, Pucha Himasekhar Reddy
From: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
ice_down() clears QINT_TQCTL_CAUSE_ENA_M bit twice, which is not
necessary. First clearing happens in ice_vsi_dis_irq() and second in
ice_vsi_stop_tx_ring() - remove the first one.
While at it, make ice_vsi_dis_irq() static as ice_down() is the only
current caller of it.
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel•com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_lib.c | 55 -----------------------
drivers/net/ethernet/intel/ice/ice_lib.h | 2 -
drivers/net/ethernet/intel/ice/ice_main.c | 44 ++++++++++++++++++
3 files changed, 44 insertions(+), 57 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 59e8a2572985..48e4c33a3550 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -2719,61 +2719,6 @@ void ice_dis_vsi(struct ice_vsi *vsi, bool locked)
}
}
-/**
- * ice_vsi_dis_irq - Mask off queue interrupt generation on the VSI
- * @vsi: the VSI being un-configured
- */
-void ice_vsi_dis_irq(struct ice_vsi *vsi)
-{
- struct ice_pf *pf = vsi->back;
- struct ice_hw *hw = &pf->hw;
- u32 val;
- int i;
-
- /* disable interrupt causation from each queue */
- if (vsi->tx_rings) {
- ice_for_each_txq(vsi, i) {
- if (vsi->tx_rings[i]) {
- u16 reg;
-
- reg = vsi->tx_rings[i]->reg_idx;
- val = rd32(hw, QINT_TQCTL(reg));
- val &= ~QINT_TQCTL_CAUSE_ENA_M;
- wr32(hw, QINT_TQCTL(reg), val);
- }
- }
- }
-
- if (vsi->rx_rings) {
- ice_for_each_rxq(vsi, i) {
- if (vsi->rx_rings[i]) {
- u16 reg;
-
- reg = vsi->rx_rings[i]->reg_idx;
- val = rd32(hw, QINT_RQCTL(reg));
- val &= ~QINT_RQCTL_CAUSE_ENA_M;
- wr32(hw, QINT_RQCTL(reg), val);
- }
- }
- }
-
- /* disable each interrupt */
- ice_for_each_q_vector(vsi, i) {
- if (!vsi->q_vectors[i])
- continue;
- wr32(hw, GLINT_DYN_CTL(vsi->q_vectors[i]->reg_idx), 0);
- }
-
- ice_flush(hw);
-
- /* don't call synchronize_irq() for VF's from the host */
- if (vsi->type == ICE_VSI_VF)
- return;
-
- ice_for_each_q_vector(vsi, i)
- synchronize_irq(vsi->q_vectors[i]->irq.virq);
-}
-
/**
* __ice_queue_set_napi - Set the napi instance for the queue
* @dev: device to which NAPI and queue belong
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.h b/drivers/net/ethernet/intel/ice/ice_lib.h
index b5a1ed7cc4b1..9cd23afe5f15 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_lib.h
@@ -110,8 +110,6 @@ void
ice_write_qrxflxp_cntxt(struct ice_hw *hw, u16 pf_q, u32 rxdid, u32 prio,
bool ena_ts);
-void ice_vsi_dis_irq(struct ice_vsi *vsi);
-
void ice_vsi_free_irq(struct ice_vsi *vsi);
void ice_vsi_free_rx_rings(struct ice_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 8f73ba77e835..062b4ea8e2c3 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -7001,6 +7001,50 @@ static void ice_napi_disable_all(struct ice_vsi *vsi)
}
}
+/**
+ * ice_vsi_dis_irq - Mask off queue interrupt generation on the VSI
+ * @vsi: the VSI being un-configured
+ */
+static void ice_vsi_dis_irq(struct ice_vsi *vsi)
+{
+ struct ice_pf *pf = vsi->back;
+ struct ice_hw *hw = &pf->hw;
+ u32 val;
+ int i;
+
+ /* disable interrupt causation from each Rx queue; Tx queues are
+ * handled in ice_vsi_stop_tx_ring()
+ */
+ if (vsi->rx_rings) {
+ ice_for_each_rxq(vsi, i) {
+ if (vsi->rx_rings[i]) {
+ u16 reg;
+
+ reg = vsi->rx_rings[i]->reg_idx;
+ val = rd32(hw, QINT_RQCTL(reg));
+ val &= ~QINT_RQCTL_CAUSE_ENA_M;
+ wr32(hw, QINT_RQCTL(reg), val);
+ }
+ }
+ }
+
+ /* disable each interrupt */
+ ice_for_each_q_vector(vsi, i) {
+ if (!vsi->q_vectors[i])
+ continue;
+ wr32(hw, GLINT_DYN_CTL(vsi->q_vectors[i]->reg_idx), 0);
+ }
+
+ ice_flush(hw);
+
+ /* don't call synchronize_irq() for VF's from the host */
+ if (vsi->type == ICE_VSI_VF)
+ return;
+
+ ice_for_each_q_vector(vsi, i)
+ synchronize_irq(vsi->q_vectors[i]->irq.virq);
+}
+
/**
* ice_down - Shutdown the connection
* @vsi: The VSI being stopped
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 9/9] ice: avoid unnecessary devm_ usage
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (7 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 8/9] ice: do not disable Tx queues twice in ice_down() Tony Nguyen
@ 2024-03-04 21:29 ` Tony Nguyen
2024-03-05 12:18 ` Simon Horman
2024-03-06 11:30 ` [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) patchwork-bot+netdevbpf
9 siblings, 1 reply; 12+ messages in thread
From: Tony Nguyen @ 2024-03-04 21:29 UTC (permalink / raw)
To: davem, kuba, pabeni, edumazet, netdev
Cc: Maciej Fijalkowski, anthony.l.nguyen, Przemek Kitszel,
Pucha Himasekhar Reddy
From: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
1. pcaps are free'd right after AQ routines are done, no need for
devm_'s
2. a test frame for loopback test in ethtool -t is destroyed at the end
of the test so we don't need devm_ here either.
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel•com> (A Contingent worker at Intel)
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
---
drivers/net/ethernet/intel/ice/ice_common.c | 34 +++++++-------------
drivers/net/ethernet/intel/ice/ice_ethtool.c | 10 ++----
2 files changed, 14 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 7d9315ffae57..4d8111aeb0ff 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -1002,9 +1002,9 @@ static void ice_get_itr_intrl_gran(struct ice_hw *hw)
*/
int ice_init_hw(struct ice_hw *hw)
{
- struct ice_aqc_get_phy_caps_data *pcaps;
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
+ void *mac_buf __free(kfree);
u16 mac_buf_len;
- void *mac_buf;
int status;
/* Set MAC type based on DeviceID */
@@ -1082,7 +1082,7 @@ int ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_sched;
- pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL);
+ pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
if (!pcaps) {
status = -ENOMEM;
goto err_unroll_sched;
@@ -1092,7 +1092,6 @@ int ice_init_hw(struct ice_hw *hw)
status = ice_aq_get_phy_caps(hw->port_info, false,
ICE_AQC_REPORT_TOPO_CAP_MEDIA, pcaps,
NULL);
- devm_kfree(ice_hw_to_dev(hw), pcaps);
if (status)
dev_warn(ice_hw_to_dev(hw), "Get PHY capabilities failed status = %d, continuing anyway\n",
status);
@@ -1119,18 +1118,15 @@ int ice_init_hw(struct ice_hw *hw)
/* Get MAC information */
/* A single port can report up to two (LAN and WoL) addresses */
- mac_buf = devm_kcalloc(ice_hw_to_dev(hw), 2,
- sizeof(struct ice_aqc_manage_mac_read_resp),
- GFP_KERNEL);
- mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp);
-
+ mac_buf = kcalloc(2, sizeof(struct ice_aqc_manage_mac_read_resp),
+ GFP_KERNEL);
if (!mac_buf) {
status = -ENOMEM;
goto err_unroll_fltr_mgmt_struct;
}
+ mac_buf_len = 2 * sizeof(struct ice_aqc_manage_mac_read_resp);
status = ice_aq_manage_mac_read(hw, mac_buf, mac_buf_len, NULL);
- devm_kfree(ice_hw_to_dev(hw), mac_buf);
if (status)
goto err_unroll_fltr_mgmt_struct;
@@ -3276,19 +3272,14 @@ int ice_update_link_info(struct ice_port_info *pi)
return status;
if (li->link_info & ICE_AQ_MEDIA_AVAILABLE) {
- struct ice_aqc_get_phy_caps_data *pcaps;
- struct ice_hw *hw;
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
- hw = pi->hw;
- pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps),
- GFP_KERNEL);
+ pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
if (!pcaps)
return -ENOMEM;
status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_TOPO_CAP_MEDIA,
pcaps, NULL);
-
- devm_kfree(ice_hw_to_dev(hw), pcaps);
}
return status;
@@ -3429,8 +3420,8 @@ ice_cfg_phy_fc(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
int
ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update)
{
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
struct ice_aqc_set_phy_cfg_data cfg = { 0 };
- struct ice_aqc_get_phy_caps_data *pcaps;
struct ice_hw *hw;
int status;
@@ -3440,7 +3431,7 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update)
*aq_failures = 0;
hw = pi->hw;
- pcaps = devm_kzalloc(ice_hw_to_dev(hw), sizeof(*pcaps), GFP_KERNEL);
+ pcaps = kzalloc(sizeof(*pcaps), GFP_KERNEL);
if (!pcaps)
return -ENOMEM;
@@ -3492,7 +3483,6 @@ ice_set_fc(struct ice_port_info *pi, u8 *aq_failures, bool ena_auto_link_update)
}
out:
- devm_kfree(ice_hw_to_dev(hw), pcaps);
return status;
}
@@ -3571,7 +3561,7 @@ int
ice_cfg_phy_fec(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
enum ice_fec_mode fec)
{
- struct ice_aqc_get_phy_caps_data *pcaps;
+ struct ice_aqc_get_phy_caps_data *pcaps __free(kfree);
struct ice_hw *hw;
int status;
@@ -3640,8 +3630,6 @@ ice_cfg_phy_fec(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
}
out:
- kfree(pcaps);
-
return status;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 3cc364a4d682..bb912155676e 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -802,7 +802,7 @@ static int ice_lbtest_create_frame(struct ice_pf *pf, u8 **ret_data, u16 size)
if (!pf)
return -EINVAL;
- data = devm_kzalloc(ice_pf_to_dev(pf), size, GFP_KERNEL);
+ data = kzalloc(size, GFP_KERNEL);
if (!data)
return -ENOMEM;
@@ -945,11 +945,9 @@ static u64 ice_loopback_test(struct net_device *netdev)
int num_frames, valid_frames;
struct ice_tx_ring *tx_ring;
struct ice_rx_ring *rx_ring;
- struct device *dev;
- u8 *tx_frame;
+ u8 *tx_frame __free(kfree);
int i;
- dev = ice_pf_to_dev(pf);
netdev_info(netdev, "loopback test\n");
test_vsi = ice_lb_vsi_setup(pf, pf->hw.port_info);
@@ -994,7 +992,7 @@ static u64 ice_loopback_test(struct net_device *netdev)
for (i = 0; i < num_frames; i++) {
if (ice_diag_send(tx_ring, tx_frame, ICE_LB_FRAME_SIZE)) {
ret = 8;
- goto lbtest_free_frame;
+ goto remove_mac_filters;
}
}
@@ -1004,8 +1002,6 @@ static u64 ice_loopback_test(struct net_device *netdev)
else if (valid_frames != num_frames)
ret = 10;
-lbtest_free_frame:
- devm_kfree(dev, tx_frame);
remove_mac_filters:
if (ice_fltr_remove_mac(test_vsi, broadcast, ICE_FWD_TO_VSI))
netdev_err(netdev, "Could not remove MAC filter for the test VSI\n");
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next 9/9] ice: avoid unnecessary devm_ usage
2024-03-04 21:29 ` [PATCH net-next 9/9] ice: avoid unnecessary devm_ usage Tony Nguyen
@ 2024-03-05 12:18 ` Simon Horman
0 siblings, 0 replies; 12+ messages in thread
From: Simon Horman @ 2024-03-05 12:18 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, netdev, Maciej Fijalkowski,
Przemek Kitszel, Pucha Himasekhar Reddy
On Mon, Mar 04, 2024 at 01:29:30PM -0800, Tony Nguyen wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
>
> 1. pcaps are free'd right after AQ routines are done, no need for
> devm_'s
> 2. a test frame for loopback test in ethtool -t is destroyed at the end
> of the test so we don't need devm_ here either.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel•com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel•com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel•com> (A Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel•com>
FWIIW, I did look over this patch and it looks good to me.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice)
2024-03-04 21:29 [PATCH net-next 0/9][pull request] Intel Wired LAN Driver Updates 2024-03-04 (ice) Tony Nguyen
` (8 preceding siblings ...)
2024-03-04 21:29 ` [PATCH net-next 9/9] ice: avoid unnecessary devm_ usage Tony Nguyen
@ 2024-03-06 11:30 ` patchwork-bot+netdevbpf
9 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-06 11:30 UTC (permalink / raw)
To: Tony Nguyen; +Cc: davem, kuba, pabeni, edumazet, netdev
Hello:
This series was applied to netdev/net-next.git (main)
by Tony Nguyen <anthony.l.nguyen@intel•com>:
On Mon, 4 Mar 2024 13:29:21 -0800 you wrote:
> This series contains updates to ice driver only.
>
> Jake changes the driver to use relative VSI index for VF VSIs as the VF
> driver has no direct use of the VSI number on ice hardware. He also
> reworks some Tx/Rx functions to clarify their uses, cleans up some style
> issues, and utilizes kernel helper functions.
>
> [...]
Here is the summary with links:
- [net-next,1/9] ice: pass VSI pointer into ice_vc_isvalid_q_id
https://git.kernel.org/netdev/net-next/c/a21605993dd5
- [net-next,2/9] ice: remove unnecessary duplicate checks for VF VSI ID
https://git.kernel.org/netdev/net-next/c/363f689600dd
- [net-next,3/9] ice: use relative VSI index for VFs instead of PF VSI number
https://git.kernel.org/netdev/net-next/c/11fbb1bfb5bc
- [net-next,4/9] ice: remove vf->lan_vsi_num field
https://git.kernel.org/netdev/net-next/c/1cf94cbfc61b
- [net-next,5/9] ice: rename ice_write_* functions to ice_pack_ctx_*
https://git.kernel.org/netdev/net-next/c/1260b45dbe2d
- [net-next,6/9] ice: use GENMASK instead of BIT(n) - 1 in pack functions
https://git.kernel.org/netdev/net-next/c/a45d1bf516c0
- [net-next,7/9] ice: cleanup line splitting for context set functions
https://git.kernel.org/netdev/net-next/c/979c2c049fbe
- [net-next,8/9] ice: do not disable Tx queues twice in ice_down()
https://git.kernel.org/netdev/net-next/c/d5926e01e373
- [net-next,9/9] ice: avoid unnecessary devm_ usage
https://git.kernel.org/netdev/net-next/c/90f821d72e11
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread