[Intel-wired-lan] [PATCH iwl-next v3] igc: Add MQPRIO offload support

2024-06-21 Thread Kurt Kanzenbach
Add support for offloading MQPRIO. The hardware has four priorities as well
as four queues. Each queue must be a assigned with a unique priority.

However, the priorities are only considered in TSN Tx mode. There are two
TSN Tx modes. In case of MQPRIO the Qbv capability is not required.
Therefore, use the legacy TSN Tx mode, which performs strict priority
arbitration.

Example for mqprio with hardware offload:

|tc qdisc replace dev ${INTERFACE} handle 100 parent root mqprio num_tc 4 \
|   map 0 0 0 0 0 1 2 3 0 0 0 0 0 0 0 0 \
|   queues 1@0 1@1 1@2 1@3 \
|   hw 1

The mqprio Qdisc also allows to configure the `preemptible_tcs'. However,
frame preemption is not supported yet.

Tested on Intel i225 and implemented by following data sheet section 7.5.2,
Transmit Scheduling.

Signed-off-by: Kurt Kanzenbach 
---
Changes in v3:
- Use FIELD_PREP for Tx ARB (Simon)
- Add helper for Tx ARB configuration (Simon)
- Limit ethtool_set_channels when mqprio is enabled (Jian)
- Link to v2: 
https://lore.kernel.org/r/20240212-igc_mqprio-v2-1-587924e6b...@linutronix.de

Changes in v2:
- Improve changelog (Paul Menzel)
- Link to v1: 
https://lore.kernel.org/r/20240212-igc_mqprio-v1-1-7aed95b73...@linutronix.de
---
 drivers/net/ethernet/intel/igc/igc.h | 10 +++-
 drivers/net/ethernet/intel/igc/igc_defines.h | 11 +
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  4 ++
 drivers/net/ethernet/intel/igc/igc_main.c| 69 +++
 drivers/net/ethernet/intel/igc/igc_regs.h|  2 +
 drivers/net/ethernet/intel/igc/igc_tsn.c | 70 +++-
 6 files changed, 163 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h 
b/drivers/net/ethernet/intel/igc/igc.h
index 8b14c029eda1..b31cd2d7120d 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -260,6 +260,10 @@ struct igc_adapter {
 */
spinlock_t qbv_tx_lock;
 
+   bool strict_priority_enable;
+   u8 num_tc;
+   u16 queue_per_tc[IGC_MAX_TX_QUEUES];
+
/* OS defined structs */
struct pci_dev *pdev;
/* lock for statistics */
@@ -383,9 +387,11 @@ extern char igc_driver_name[];
 #define IGC_FLAG_RX_LEGACY BIT(16)
 #define IGC_FLAG_TSN_QBV_ENABLED   BIT(17)
 #define IGC_FLAG_TSN_QAV_ENABLED   BIT(18)
+#define IGC_FLAG_TSN_LEGACY_ENABLEDBIT(19)
 
-#define IGC_FLAG_TSN_ANY_ENABLED \
-   (IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED)
+#define IGC_FLAG_TSN_ANY_ENABLED   \
+   (IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED |  \
+IGC_FLAG_TSN_LEGACY_ENABLED)
 
 #define IGC_FLAG_RSS_FIELD_IPV4_UDPBIT(6)
 #define IGC_FLAG_RSS_FIELD_IPV6_UDPBIT(7)
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h 
b/drivers/net/ethernet/intel/igc/igc_defines.h
index 5f92b3c7c3d4..58f6631bfdd5 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -4,6 +4,8 @@
 #ifndef _IGC_DEFINES_H_
 #define _IGC_DEFINES_H_
 
+#include 
+
 /* Number of Transmit and Receive Descriptors must be a multiple of 8 */
 #define REQ_TX_DESCRIPTOR_MULTIPLE 8
 #define REQ_RX_DESCRIPTOR_MULTIPLE 8
@@ -547,6 +549,15 @@
 
 #define IGC_MAX_SR_QUEUES  2
 
+#define IGC_TXARB_TXQ_PRIO_0_MASK  GENMASK(1, 0)
+#define IGC_TXARB_TXQ_PRIO_1_MASK  GENMASK(3, 2)
+#define IGC_TXARB_TXQ_PRIO_2_MASK  GENMASK(5, 4)
+#define IGC_TXARB_TXQ_PRIO_3_MASK  GENMASK(7, 6)
+#define IGC_TXARB_TXQ_PRIO_0(x)
FIELD_PREP(IGC_TXARB_TXQ_PRIO_0_MASK, (x))
+#define IGC_TXARB_TXQ_PRIO_1(x)
FIELD_PREP(IGC_TXARB_TXQ_PRIO_1_MASK, (x))
+#define IGC_TXARB_TXQ_PRIO_2(x)
FIELD_PREP(IGC_TXARB_TXQ_PRIO_2_MASK, (x))
+#define IGC_TXARB_TXQ_PRIO_3(x)
FIELD_PREP(IGC_TXARB_TXQ_PRIO_3_MASK, (x))
+
 /* Receive Checksum Control */
 #define IGC_RXCSUM_CRCOFL  0x0800   /* CRC32 offload enable */
 #define IGC_RXCSUM_PCSD0x2000   /* packet checksum 
disabled */
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c 
b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 0cd2bd695db1..d436472f3388 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1540,6 +1540,10 @@ static int igc_ethtool_set_channels(struct net_device 
*netdev,
if (ch->other_count != NON_Q_VECTORS)
return -EINVAL;
 
+   /* Do not allow channel reconfiguration when mqprio is enabled */
+   if (adapter->strict_priority_enable)
+   return -EINVAL;
+
/* Verify the number of channels doesn't exceed hw limits */
max_combined = igc_get_max_rss_queues(adapter);
if (count > max_combined)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c 
b/drivers/net/ethernet/intel/igc/igc_main.c
index 87b655b839c1..7a027ef96e93 100644
--- a/drivers/net/ethernet/intel/igc/igc

[Intel-wired-lan] [RFC PATCH iwl-next v1 1/4] ice: Introduce ice_get_phy_model() wrapper

2024-06-21 Thread Sergey Temerkhanov
Introduce ice_get_phy_model() to improve code readability

Reviewed-by: Przemek Kitszel 
Signed-off-by: Sergey Temerkhanov 
---
 drivers/net/ethernet/intel/ice/ice.h|  5 +
 drivers/net/ethernet/intel/ice/ice_ptp.c| 18 -
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 22 ++---
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h 
b/drivers/net/ethernet/intel/ice/ice.h
index 4c563b0d57ac..b43719ee8324 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -1046,5 +1046,10 @@ static inline void ice_clear_rdma_cap(struct ice_pf *pf)
clear_bit(ICE_FLAG_RDMA_ENA, pf->flags);
 }
 
+static inline enum ice_phy_model ice_get_phy_model(const struct ice_hw *hw)
+{
+   return hw->ptp.phy_model;
+}
+
 extern const struct xdp_metadata_ops ice_xdp_md_ops;
 #endif /* _ICE_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c 
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 0fb5f78ce3db..57e1e5a5da4a 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -1372,7 +1372,7 @@ ice_ptp_port_phy_stop(struct ice_ptp_port *ptp_port)
 
mutex_lock(&ptp_port->ps_lock);
 
-   switch (hw->ptp.phy_model) {
+   switch (ice_get_phy_model(hw)) {
case ICE_PHY_ETH56G:
err = ice_stop_phy_timer_eth56g(hw, port, true);
break;
@@ -1418,7 +1418,7 @@ ice_ptp_port_phy_restart(struct ice_ptp_port *ptp_port)
 
mutex_lock(&ptp_port->ps_lock);
 
-   switch (hw->ptp.phy_model) {
+   switch (ice_get_phy_model(hw)) {
case ICE_PHY_ETH56G:
err = ice_start_phy_timer_eth56g(hw, port);
break;
@@ -1486,7 +1486,7 @@ void ice_ptp_link_change(struct ice_pf *pf, u8 port, bool 
linkup)
/* Update cached link status for this port immediately */
ptp_port->link_up = linkup;
 
-   switch (hw->ptp.phy_model) {
+   switch (ice_get_phy_model(hw)) {
case ICE_PHY_E810:
/* Do not reconfigure E810 PHY */
return;
@@ -1519,7 +1519,7 @@ static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, 
bool ena, u32 threshold)
 
ice_ptp_reset_ts_memory(hw);
 
-   switch (hw->ptp.phy_model) {
+   switch (ice_get_phy_model(hw)) {
case ICE_PHY_ETH56G: {
int port;
 
@@ -1558,7 +1558,7 @@ static int ice_ptp_cfg_phy_interrupt(struct ice_pf *pf, 
bool ena, u32 threshold)
case ICE_PHY_UNSUP:
default:
dev_warn(dev, "%s: Unexpected PHY model %d\n", __func__,
-hw->ptp.phy_model);
+ice_get_phy_model(hw));
return -EOPNOTSUPP;
}
 }
@@ -1999,7 +1999,7 @@ ice_ptp_settime64(struct ptp_clock_info *info, const 
struct timespec64 *ts)
/* For Vernier mode on E82X, we need to recalibrate after new settime.
 * Start with marking timestamps as invalid.
 */
-   if (hw->ptp.phy_model == ICE_PHY_E82X) {
+   if (ice_get_phy_model(hw) == ICE_PHY_E82X) {
err = ice_ptp_clear_phy_offset_ready_e82x(hw);
if (err)
dev_warn(ice_pf_to_dev(pf), "Failed to mark timestamps 
as invalid before settime\n");
@@ -2023,7 +2023,7 @@ ice_ptp_settime64(struct ptp_clock_info *info, const 
struct timespec64 *ts)
ice_ptp_enable_all_clkout(pf);
 
/* Recalibrate and re-enable timestamp blocks for E822/E823 */
-   if (hw->ptp.phy_model == ICE_PHY_E82X)
+   if (ice_get_phy_model(hw) == ICE_PHY_E82X)
ice_ptp_restart_all_phy(pf);
 exit:
if (err) {
@@ -3144,7 +3144,7 @@ static int ice_ptp_init_port(struct ice_pf *pf, struct 
ice_ptp_port *ptp_port)
 
mutex_init(&ptp_port->ps_lock);
 
-   switch (hw->ptp.phy_model) {
+   switch (ice_get_phy_model(hw)) {
case ICE_PHY_ETH56G:
return ice_ptp_init_tx_eth56g(pf, &ptp_port->tx,
  ptp_port->port_num);
@@ -3242,7 +3242,7 @@ static void ice_ptp_remove_auxbus_device(struct ice_pf 
*pf)
  */
 static void ice_ptp_init_tx_interrupt_mode(struct ice_pf *pf)
 {
-   switch (pf->hw.ptp.phy_model) {
+   switch (ice_get_phy_model(&pf->hw)) {
case ICE_PHY_E82X:
/* E822 based PHY has the clock owner process the interrupt
 * for all ports.
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c 
b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 1e9a4ccd0ea2..73a6788f9990 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -806,7 +806,7 @@ static u32 ice_ptp_tmr_cmd_to_port_reg(struct ice_hw *hw,
/* Certain hardware families share the same register values for the
 * port register and source timer register.
 */
-   switch (hw->ptp.phy_model) {
+   switch 

[Intel-wired-lan] [RFC PATCH iwl-next v1 0/4] Replace auxbus with ice_adapter in the PTP support code

2024-06-21 Thread Sergey Temerkhanov
This series replaces multiple aux buses and devices used in
the PTP support code with struct ice_adapter holding the necessary
shared data

Patches 1,2 add convenience wrappers
Patch 3 does the main refactoring
Patch 4 finalizes the refactoring

Sergey Temerkhanov (4):
  ice: Introduce ice_get_phy_model() wrapper
  ice: Add ice_get_ctrl_ptp() wrapper to simplify the code
  ice: Use ice_adapter for PTP shared data instead of auxdev
  ice: Drop auxbus use for PTP to finalize ice_adapter move

 drivers/net/ethernet/intel/ice/ice.h |   5 +
 drivers/net/ethernet/intel/ice/ice_adapter.c |   6 +
 drivers/net/ethernet/intel/ice/ice_adapter.h |  21 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c | 338 ---
 drivers/net/ethernet/intel/ice/ice_ptp.h |  24 +-
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c  |  22 +-
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h  |   5 +
 7 files changed, 111 insertions(+), 310 deletions(-)

-- 
2.43.0



[Intel-wired-lan] [RFC PATCH iwl-next v1 2/4] ice: Add ice_get_ctrl_ptp() wrapper to simplify the code

2024-06-21 Thread Sergey Temerkhanov
Add ice_get_ctrl_ptp() wrapper to simplify the PTP support code
in the functions that do not use ctrl_pf directly

Reviewed-by: Przemek Kitszel 
Signed-off-by: Sergey Temerkhanov 
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c 
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 57e1e5a5da4a..a2578bc2af54 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -16,6 +16,18 @@ static const struct ptp_pin_desc ice_pin_desc_e810t[] = {
{ "U.FL2", UFL2, PTP_PF_NONE, 2, { 0, } },
 };
 
+static struct ice_pf *ice_get_ctrl_pf(struct ice_pf *pf)
+{
+   return !pf->adapter ? NULL : pf->adapter->ctrl_pf;
+}
+
+static struct ice_ptp *ice_get_ctrl_ptp(struct ice_pf *pf)
+{
+   struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
+
+   return !ctrl_pf ? NULL : &ctrl_pf->ptp;
+}
+
 /**
  * ice_get_sma_config_e810t
  * @hw: pointer to the hw struct
-- 
2.43.0



[Intel-wired-lan] [RFC PATCH iwl-next v1 3/4] ice: Use ice_adapter for PTP shared data instead of auxdev

2024-06-21 Thread Sergey Temerkhanov
- Use struct ice_adapter to hold shared PTP data and control PTP
related actions instead of auxbus. This allows significant code
simplification and faster access to the container fields used in
the PTP support code.

- Move the PTP port list to the ice_adapter container to simplify
the code and avoid race conditions which could occur due to the
synchronous nature of the initialization/access and
certain memory saving can be achieved by moving PTP data into
the ice_adapter itself.

Reviewed-by: Przemek Kitszel 
Signed-off-by: Sergey Temerkhanov 
---
 drivers/net/ethernet/intel/ice/ice_adapter.c |  6 ++
 drivers/net/ethernet/intel/ice/ice_adapter.h | 21 +-
 drivers/net/ethernet/intel/ice/ice_ptp.c | 79 +++-
 drivers/net/ethernet/intel/ice/ice_ptp.h | 24 +-
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h  |  5 ++
 5 files changed, 92 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c 
b/drivers/net/ethernet/intel/ice/ice_adapter.c
index 52d15ef7f4b1..f9d40e9f0c60 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -39,11 +39,17 @@ static struct ice_adapter *ice_adapter_new(void)
spin_lock_init(&adapter->ptp_gltsyn_time_lock);
refcount_set(&adapter->refcount, 1);
 
+   mutex_init(&adapter->ports.lock);
+   INIT_LIST_HEAD(&adapter->ports.ports);
+
return adapter;
 }
 
 static void ice_adapter_free(struct ice_adapter *adapter)
 {
+   WARN_ON(!list_empty(&adapter->ports.ports));
+   mutex_destroy(&adapter->ports.lock);
+
kfree(adapter);
 }
 
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h 
b/drivers/net/ethernet/intel/ice/ice_adapter.h
index 9d11014ec02f..45a42109ad3b 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -4,22 +4,41 @@
 #ifndef _ICE_ADAPTER_H_
 #define _ICE_ADAPTER_H_
 
+#include 
 #include 
 #include 
 
 struct pci_dev;
+struct ice_pf;
+
+/**
+ * struct ice_port_list - data used to store the list of adapter ports
+ *
+ * This structure contains data used to maintain a list of adapter ports
+ *
+ * @ports: list of ports
+ * @lock: protect access to the ports list
+ */
+struct ice_port_list {
+   struct list_head ports;
+   /* To synchronize the ports list operations */
+   struct mutex lock;
+};
 
 /**
  * struct ice_adapter - PCI adapter resources shared across PFs
  * @ptp_gltsyn_time_lock: Spinlock protecting access to the GLTSYN_TIME
  *register of the PTP clock.
  * @refcount: Reference count. struct ice_pf objects hold the references.
+ * @ctrl_pf: Control PF of the adapter
  */
 struct ice_adapter {
/* For access to the GLTSYN_TIME register */
spinlock_t ptp_gltsyn_time_lock;
-
refcount_t refcount;
+
+   struct ice_pf *ctrl_pf;
+   struct ice_port_list ports;
 };
 
 struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev);
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c 
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index a2578bc2af54..acabd84d6d52 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -821,8 +821,8 @@ static enum ice_tx_tstamp_work 
ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
struct ice_ptp_port *port;
unsigned int i;
 
-   mutex_lock(&pf->ptp.ports_owner.lock);
-   list_for_each_entry(port, &pf->ptp.ports_owner.ports, list_member) {
+   mutex_lock(&pf->adapter->ports.lock);
+   list_for_each_entry(port, &pf->adapter->ports.ports, list_node) {
struct ice_ptp_tx *tx = &port->tx;
 
if (!tx || !tx->init)
@@ -830,7 +830,7 @@ static enum ice_tx_tstamp_work 
ice_ptp_tx_tstamp_owner(struct ice_pf *pf)
 
ice_ptp_process_tx_tstamp(tx);
}
-   mutex_unlock(&pf->ptp.ports_owner.lock);
+   mutex_unlock(&pf->adapter->ports.lock);
 
for (i = 0; i < ICE_GET_QUAD_NUM(pf->hw.ptp.num_lports); i++) {
u64 tstamp_ready;
@@ -995,7 +995,7 @@ ice_ptp_flush_all_tx_tracker(struct ice_pf *pf)
 {
struct ice_ptp_port *port;
 
-   list_for_each_entry(port, &pf->ptp.ports_owner.ports, list_member)
+   list_for_each_entry(port, &pf->adapter->ports.ports, list_node)
ice_ptp_flush_tx_tracker(ptp_port_to_pf(port), &port->tx);
 }
 
@@ -1592,10 +1592,10 @@ static void ice_ptp_restart_all_phy(struct ice_pf *pf)
 {
struct list_head *entry;
 
-   list_for_each(entry, &pf->ptp.ports_owner.ports) {
+   list_for_each(entry, &pf->adapter->ports.ports) {
struct ice_ptp_port *port = list_entry(entry,
   struct ice_ptp_port,
-  list_member);
+  list_node);
 
if (port->link_up)
ice_pt

[Intel-wired-lan] [RFC PATCH iwl-next v1 4/4] ice: Drop auxbus use for PTP to finalize ice_adapter move

2024-06-21 Thread Sergey Temerkhanov
Drop unused auxbus/auxdev support from the PTP code due to
move to the ice_adapter.

Reviewed-by: Przemek Kitszel 
Signed-off-by: Sergey Temerkhanov 
---
 drivers/net/ethernet/intel/ice/ice_ptp.c | 265 ---
 1 file changed, 265 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c 
b/drivers/net/ethernet/intel/ice/ice_ptp.c
index acabd84d6d52..6f1fd11f9249 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -2880,189 +2880,6 @@ static void ice_ptp_cleanup_pf(struct ice_pf *pf)
mutex_unlock(&pf->adapter->ports.lock);
}
 }
-/**
- * ice_ptp_aux_dev_to_aux_pf - Get auxiliary PF handle for the auxiliary device
- * @aux_dev: auxiliary device to get the auxiliary PF for
- */
-static struct ice_pf *
-ice_ptp_aux_dev_to_aux_pf(struct auxiliary_device *aux_dev)
-{
-   struct ice_ptp_port *aux_port;
-   struct ice_ptp *aux_ptp;
-
-   aux_port = container_of(aux_dev, struct ice_ptp_port, aux_dev);
-   aux_ptp = container_of(aux_port, struct ice_ptp, port);
-
-   return container_of(aux_ptp, struct ice_pf, ptp);
-}
-
-/**
- * ice_ptp_aux_dev_to_owner_pf - Get PF handle for the auxiliary device
- * @aux_dev: auxiliary device to get the PF for
- */
-static struct ice_pf *
-ice_ptp_aux_dev_to_owner_pf(struct auxiliary_device *aux_dev)
-{
-   struct ice_ptp_port_owner *ports_owner;
-   struct auxiliary_driver *aux_drv;
-   struct ice_ptp *owner_ptp;
-
-   if (!aux_dev->dev.driver)
-   return NULL;
-
-   aux_drv = to_auxiliary_drv(aux_dev->dev.driver);
-   ports_owner = container_of(aux_drv, struct ice_ptp_port_owner,
-  aux_driver);
-   owner_ptp = container_of(ports_owner, struct ice_ptp, ports_owner);
-   return container_of(owner_ptp, struct ice_pf, ptp);
-}
-
-/**
- * ice_ptp_auxbus_probe - Probe auxiliary devices
- * @aux_dev: PF's auxiliary device
- * @id: Auxiliary device ID
- */
-static int ice_ptp_auxbus_probe(struct auxiliary_device *aux_dev,
-   const struct auxiliary_device_id *id)
-{
-   struct ice_pf *owner_pf = ice_ptp_aux_dev_to_owner_pf(aux_dev);
-   struct ice_pf *aux_pf = ice_ptp_aux_dev_to_aux_pf(aux_dev);
-
-   if (WARN_ON(!owner_pf))
-   return -ENODEV;
-
-   INIT_LIST_HEAD(&aux_pf->ptp.port.list_member);
-   mutex_lock(&owner_pf->ptp.ports_owner.lock);
-   list_add(&aux_pf->ptp.port.list_member,
-&owner_pf->ptp.ports_owner.ports);
-   mutex_unlock(&owner_pf->ptp.ports_owner.lock);
-
-   return 0;
-}
-
-/**
- * ice_ptp_auxbus_remove - Remove auxiliary devices from the bus
- * @aux_dev: PF's auxiliary device
- */
-static void ice_ptp_auxbus_remove(struct auxiliary_device *aux_dev)
-{
-   struct ice_pf *owner_pf = ice_ptp_aux_dev_to_owner_pf(aux_dev);
-   struct ice_pf *aux_pf = ice_ptp_aux_dev_to_aux_pf(aux_dev);
-
-   mutex_lock(&owner_pf->ptp.ports_owner.lock);
-   list_del(&aux_pf->ptp.port.list_member);
-   mutex_unlock(&owner_pf->ptp.ports_owner.lock);
-}
-
-/**
- * ice_ptp_auxbus_shutdown
- * @aux_dev: PF's auxiliary device
- */
-static void ice_ptp_auxbus_shutdown(struct auxiliary_device *aux_dev)
-{
-   /* Doing nothing here, but handle to auxbus driver must be satisfied */
-}
-
-/**
- * ice_ptp_auxbus_suspend
- * @aux_dev: PF's auxiliary device
- * @state: power management state indicator
- */
-static int
-ice_ptp_auxbus_suspend(struct auxiliary_device *aux_dev, pm_message_t state)
-{
-   /* Doing nothing here, but handle to auxbus driver must be satisfied */
-   return 0;
-}
-
-/**
- * ice_ptp_auxbus_resume
- * @aux_dev: PF's auxiliary device
- */
-static int ice_ptp_auxbus_resume(struct auxiliary_device *aux_dev)
-{
-   /* Doing nothing here, but handle to auxbus driver must be satisfied */
-   return 0;
-}
-
-/**
- * ice_ptp_auxbus_create_id_table - Create auxiliary device ID table
- * @pf: Board private structure
- * @name: auxiliary bus driver name
- */
-static struct auxiliary_device_id *
-ice_ptp_auxbus_create_id_table(struct ice_pf *pf, const char *name)
-{
-   struct auxiliary_device_id *ids;
-
-   /* Second id left empty to terminate the array */
-   ids = devm_kcalloc(ice_pf_to_dev(pf), 2,
-  sizeof(struct auxiliary_device_id), GFP_KERNEL);
-   if (!ids)
-   return NULL;
-
-   snprintf(ids[0].name, sizeof(ids[0].name), "ice.%s", name);
-
-   return ids;
-}
-
-/**
- * ice_ptp_register_auxbus_driver - Register PTP auxiliary bus driver
- * @pf: Board private structure
- */
-static int ice_ptp_register_auxbus_driver(struct ice_pf *pf)
-{
-   struct auxiliary_driver *aux_driver;
-   struct ice_ptp *ptp;
-   struct device *dev;
-   char *name;
-   int err;
-
-   ptp = &pf->ptp;
-   dev = ice_pf_to_dev(pf);
-   aux_driver = &ptp->ports_owner.aux_driver;
-   INIT_LIST_H

Re: [Intel-wired-lan] [RFC net-next 5/9] ice: apply XDP offloading fixup when building skb

2024-06-21 Thread Alexander Lobakin
From: Yan Zhai 
Date: Thu, 20 Jun 2024 15:19:22 -0700

> Add a common point to transfer offloading info from XDP context to skb.
> 
> Signed-off-by: Yan Zhai 
> Signed-off-by: Jesper Dangaard Brouer 
> ---
>  drivers/net/ethernet/intel/ice/ice_txrx.c | 2 ++
>  drivers/net/ethernet/intel/ice/ice_xsk.c  | 6 +-
>  include/net/xdp_sock_drv.h| 2 +-
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c 
> b/drivers/net/ethernet/intel/ice/ice_txrx.c
> index 8bb743f78fcb..a247306837ed 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> @@ -1222,6 +1222,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int 
> budget)
>  
>   hard_start = page_address(rx_buf->page) + 
> rx_buf->page_offset -
>offset;
> + xdp_init_buff_minimal(xdp);

Two lines below, you have this:

xdp_buff_clear_frags_flag(xdp);

Which clears frags bit in xdp->flags. I.e. since you always clear flags
here, this call becomes redundant.
But I'd say that `xdp->flags = 0` really wants to be moved from
xdp_init_buff() to xdp_prepare_buff().

>   xdp_prepare_buff(xdp, hard_start, offset, size, 
> !!offset);
>  #if (PAGE_SIZE > 4096)
>   /* At larger PAGE_SIZE, frame_sz depend on len size */
> @@ -1287,6 +1288,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int 
> budget)
>  
>   /* populate checksum, VLAN, and protocol */
>   ice_process_skb_fields(rx_ring, rx_desc, skb);
> + xdp_buff_fixup_skb_offloading(xdp, skb);
>  
>   ice_trace(clean_rx_irq_indicate, rx_ring, rx_desc, skb);
>   /* send completed skb up the stack */
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c 
> b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index a65955eb23c0..367658acaab8 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -845,8 +845,10 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int 
> budget)
>   xdp_prog = READ_ONCE(rx_ring->xdp_prog);
>   xdp_ring = rx_ring->xdp_ring;
>  
> - if (ntc != rx_ring->first_desc)
> + if (ntc != rx_ring->first_desc) {
>   first = *ice_xdp_buf(rx_ring, rx_ring->first_desc);
> + xdp_init_buff_minimal(first);

xdp_buff_set_size() always clears flags, this is redundant.

> + }
>  
>   while (likely(total_rx_packets < (unsigned int)budget)) {
>   union ice_32b_rx_flex_desc *rx_desc;
> @@ -920,6 +922,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int 
> budget)
>   break;
>   }
>  
> + xdp = first;
>   first = NULL;
>   rx_ring->first_desc = ntc;
>  
> @@ -934,6 +937,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int 
> budget)
>   vlan_tci = ice_get_vlan_tci(rx_desc);
>  
>   ice_process_skb_fields(rx_ring, rx_desc, skb);
> + xdp_buff_fixup_skb_offloading(xdp, skb);
>   ice_receive_skb(rx_ring, skb, vlan_tci);
>   }
>  
> diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> index 0a5dca2b2b3f..02243dc064c2 100644
> --- a/include/net/xdp_sock_drv.h
> +++ b/include/net/xdp_sock_drv.h
> @@ -181,7 +181,7 @@ static inline void xsk_buff_set_size(struct xdp_buff 
> *xdp, u32 size)
>   xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
>   xdp->data_meta = xdp->data;
>   xdp->data_end = xdp->data + size;
> - xdp->flags = 0;
> + xdp_init_buff_minimal(xdp);

Why is this done in the patch prefixed with "ice:"?

>  }
>  
>  static inline dma_addr_t xsk_buff_raw_get_dma(struct xsk_buff_pool *pool,

Thanks,
Olek


Re: [Intel-wired-lan] [PATCH iwl-next v3] igc: Add MQPRIO offload support

2024-06-21 Thread Wojciech Drewek



On 21.06.2024 09:25, Kurt Kanzenbach wrote:
> Add support for offloading MQPRIO. The hardware has four priorities as well
> as four queues. Each queue must be a assigned with a unique priority.
> 
> However, the priorities are only considered in TSN Tx mode. There are two
> TSN Tx modes. In case of MQPRIO the Qbv capability is not required.
> Therefore, use the legacy TSN Tx mode, which performs strict priority
> arbitration.
> 
> Example for mqprio with hardware offload:
> 
> |tc qdisc replace dev ${INTERFACE} handle 100 parent root mqprio num_tc 4 \
> |   map 0 0 0 0 0 1 2 3 0 0 0 0 0 0 0 0 \
> |   queues 1@0 1@1 1@2 1@3 \
> |   hw 1
> 
> The mqprio Qdisc also allows to configure the `preemptible_tcs'. However,
> frame preemption is not supported yet.
> 
> Tested on Intel i225 and implemented by following data sheet section 7.5.2,
> Transmit Scheduling.
> 
> Signed-off-by: Kurt Kanzenbach 
> ---

Thank you!
Reviewed-by: Wojciech Drewek 

> Changes in v3:
> - Use FIELD_PREP for Tx ARB (Simon)
> - Add helper for Tx ARB configuration (Simon)
> - Limit ethtool_set_channels when mqprio is enabled (Jian)
> - Link to v2: 
> https://lore.kernel.org/r/20240212-igc_mqprio-v2-1-587924e6b...@linutronix.de
> 
> Changes in v2:
> - Improve changelog (Paul Menzel)
> - Link to v1: 
> https://lore.kernel.org/r/20240212-igc_mqprio-v1-1-7aed95b73...@linutronix.de
> ---
>  drivers/net/ethernet/intel/igc/igc.h | 10 +++-
>  drivers/net/ethernet/intel/igc/igc_defines.h | 11 +
>  drivers/net/ethernet/intel/igc/igc_ethtool.c |  4 ++
>  drivers/net/ethernet/intel/igc/igc_main.c| 69 +++
>  drivers/net/ethernet/intel/igc/igc_regs.h|  2 +
>  drivers/net/ethernet/intel/igc/igc_tsn.c | 70 
> +++-
>  6 files changed, 163 insertions(+), 3 deletions(-)
> 

<...>


Re: [Intel-wired-lan] [PATCH v2 iwl-net 3/3] ice: Reject pin requests with unsupported flags

2024-06-21 Thread Simon Horman
On Thu, Jun 20, 2024 at 02:27:10PM +0200, Karol Kolacinski wrote:
> From: Jacob Keller 
> 
> The driver receives requests for configuring pins via the .enable
> callback of the PTP clock object. These requests come into the driver
> with flags which modify the requested behavior from userspace. Current
> implementation in ice does not reject flags that it doesn't support.
> This causes the driver to incorrectly apply requests with such flags as
> PTP_PEROUT_DUTY_CYCLE, or any future flags added by the kernel which it
> is not yet aware of.
> 
> Fix this by properly validating flags in both ice_ptp_cfg_perout and
> ice_ptp_cfg_extts. Ensure that we check by bit-wise negating supported
> flags rather than just checking and rejecting known un-supported flags.
> This is preferable, as it ensures better compatibility with future
> kernels.
> 
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel 
> Signed-off-by: Jacob Keller 
> Signed-off-by: Karol Kolacinski 
> ---
> V1 -> V2: adjusted indentation and added NULL config pointer check

Thanks for the update, this version looks good to me.

Reviewed-by: Simon Horman 



Re: [Intel-wired-lan] [PATCH v2 iwl-net 2/3] ice: Don't process extts if PTP is disabled

2024-06-21 Thread Simon Horman
On Thu, Jun 20, 2024 at 02:27:09PM +0200, Karol Kolacinski wrote:
> From: Jacob Keller 
> 
> The ice_ptp_extts_event() function can race with ice_ptp_release() and
> result in a NULL pointer dereference which leads to a kernel panic.
> 
> Panic occurs because the ice_ptp_extts_event() function calls
> ptp_clock_event() with a NULL pointer. The ice driver has already
> released the PTP clock by the time the interrupt for the next external
> timestamp event occurs.
> 
> To fix this, modify the ice_ptp_extts_event() function to check the
> PTP state and bail early if PTP is not ready.
> 
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel 
> Signed-off-by: Jacob Keller 
> Signed-off-by: Karol Kolacinski 
> ---
> V1 -> V2: removed unnecessary hunk of code and adjusted commit message

Thanks for the update.

Reviewed-by: Simon Horman 



Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/3] ice: Fix improper extts handling

2024-06-21 Thread Simon Horman
On Thu, Jun 20, 2024 at 02:27:08PM +0200, Karol Kolacinski wrote:
> From: Milena Olech 
> 
> Extts events are disabled and enabled by the application ts2phc.
> However, in case where the driver is removed when the application is
> running, channel remains enabled. As a result, in the next run of the
> app, two channels are enabled and the information "extts on unexpected
> channel" is printed to the user.
> 
> To avoid that, extts events shall be disabled when PTP is released.
> 
> Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> Reviewed-by: Przemek Kitszel 
> Co-developed-by: Jacob Keller 
> Signed-off-by: Jacob Keller 
> Signed-off-by: Milena Olech 
> Signed-off-by: Karol Kolacinski 
> ---
> V1 -> V2: removed extra space and fixed return value in
>   ice_ptp_gpio_enable_e823()

Reviewed-by: Simon Horman 



Re: [Intel-wired-lan] [PATCH iwl-net v2 1/1] e1000e: fix force smbus during suspend flow

2024-06-21 Thread Brandt, Todd E
I just built and tested your patch on the latest 6.10.0-rc3 tip. It seems to 
have fixed the issue on three of our machines, but the issue still occurs on 
our Meteor Lake SDV board (otcpl-mtl-s).

[  130.302511] e1000e: EEE TX LPI TIMER: 0011
[  130.390014] e1000e :80:1f.6: PM: pci_pm_suspend(): e1000e_pm_suspend 
[e1000e] returns -2
[  130.390033] e1000e :80:1f.6: PM: dpm_run_callback(): pci_pm_suspend 
returns -2
[  130.390039] e1000e :80:1f.6: PM: failed to suspend async: error -2
[  130.574807] PM: suspend of devices aborted after 293.955 msecs
[  130.574817] PM: start suspend of devices aborted after 376.596 msecs
[  130.574820] PM: Some devices failed to suspend, or early wake event detected

$> lspci -nn -s 80:1f.6
80:1f.6 Ethernet controller [0200]: Intel Corporation Device [8086:550d]

-Original Message-
From: Lifshits, Vitaly  
Sent: Wednesday, June 19, 2024 11:37 PM
To: intel-wired-lan@osuosl.org
Cc: hui.w...@canonical.com; Lifshits, Vitaly ; 
Brandt, Todd E ; Dieter Mummenschanz 

Subject: [PATCH iwl-net v2 1/1] e1000e: fix force smbus during suspend flow

Commit 861e8086029e ("e1000e: move force SMBUS from enable ulp function to 
avoid PHY loss issue") resolved a PHY access loss during suspend on Meteor Lake 
consumer platforms, but it affected corporate systems incorrectly.

A better fix, working for both consumer and corporate systems, was proposed in 
commit bfd546a552e1 ("e1000e: move force SMBUS near the end of enable_ulp 
function"). However, it introduced a regression on older devices, such as 
[8086:15B8], [8086:15F9], [8086:15BE].

This patch aims to fix the secondary regression, by limiting the scope of the 
changes to Meteor Lake platforms only.

Fixes: bfd546a552e1 ("e1000e: move force SMBUS near the end of enable_ulp 
function")
Reported-by: Todd Brandt 
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218940
Reported-by: Dieter Mummenschanz 
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218936
Signed-off-by: Vitaly Lifshits 
--
v2: enhance the function description and address community comments
v1: initial version
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 73 +++--
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 2e98a2a0bead..86d4ae95b45a 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -137,6 +137,7 @@ static void e1000_gate_hw_phy_config_ich8lan(struct 
e1000_hw *hw, bool gate);  static s32 e1000_disable_ulp_lpt_lp(struct e1000_hw 
*hw, bool force);  static s32 e1000_setup_copper_link_pch_lpt(struct e1000_hw 
*hw);  static s32 e1000_oem_bits_config_ich8lan(struct e1000_hw *hw, bool 
d0_state);
+static s32 e1000e_force_smbus(struct e1000_hw *hw);
 
 static inline u16 __er16flash(struct e1000_hw *hw, unsigned long reg)  { @@ 
-1108,6 +1109,46 @@ static s32 e1000_platform_pm_pch_lpt(struct e1000_hw *hw, 
bool link)
return 0;
 }
 
+/**
+ *  e1000e_force_smbus - Force interfaces to transition to SMBUS mode.
+ *  @hw: pointer to the HW structure
+ *
+ *  Force the MAC and the PHY to SMBUS mode. Assumes semaphore already
+ *  acquired.
+ *
+ * Return: 0 on success, negative errno on failure.
+ **/
+static s32 e1000e_force_smbus(struct e1000_hw *hw) {
+   u16 smb_ctrl = 0;
+   u32 ctrl_ext;
+   s32 ret_val;
+
+   /* Switching PHY interface always returns MDI error
+* so disable retry mechanism to avoid wasting time
+*/
+   e1000e_disable_phy_retry(hw);
+
+   /* Force SMBus mode in the PHY */
+   ret_val = e1000_read_phy_reg_hv_locked(hw, CV_SMB_CTRL, &smb_ctrl);
+   if (ret_val) {
+   e1000e_enable_phy_retry(hw);
+   return ret_val;
+   }
+
+   smb_ctrl |= CV_SMB_CTRL_FORCE_SMBUS;
+   e1000_write_phy_reg_hv_locked(hw, CV_SMB_CTRL, smb_ctrl);
+
+   e1000e_enable_phy_retry(hw);
+
+   /* Force SMBus mode in the MAC */
+   ctrl_ext = er32(CTRL_EXT);
+   ctrl_ext |= E1000_CTRL_EXT_FORCE_SMBUS;
+   ew32(CTRL_EXT, ctrl_ext);
+
+   return 0;
+}
+
 /**
  *  e1000_enable_ulp_lpt_lp - configure Ultra Low Power mode for LynxPoint-LP
  *  @hw: pointer to the HW structure
@@ -1165,6 +1206,14 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool 
to_sx)
if (ret_val)
goto out;
 
+   if (hw->mac.type != e1000_pch_mtp) {
+   ret_val = e1000e_force_smbus(hw);
+   if (ret_val) {
+   e_dbg("Failed to force SMBUS: %d\n", ret_val);
+   goto release;
+   }
+   }
+
/* Si workaround for ULP entry flow on i127/rev6 h/w.  Enable
 * LPLU and disable Gig speed when entering ULP
 */
@@ -1225,27 +1274,11 @@ s32 e1000_enable_ulp_lpt_lp(struct e1000_hw *hw, bool 
to_sx)
}
 
 release:
-   /* Switching PHY interface always returns MDI

Re: [Intel-wired-lan] [PATCH iwl-next v7 09/12] iavf: refactor iavf_clean_rx_irq to support legacy and flex descriptors

2024-06-21 Thread Alexander Lobakin
From: Alexander Lobakin 
Date: Wed, 12 Jun 2024 14:33:17 +0200

> From: Jacob Keller 
> Date: Tue, 11 Jun 2024 13:52:57 -0700
> 
>>
>>
>> On 6/11/2024 4:47 AM, Alexander Lobakin wrote:
>>> From: Mateusz Polchlopek 
>>> Date: Tue,  4 Jun 2024 09:13:57 -0400
>>>
 From: Jacob Keller 

 Using VIRTCHNL_VF_OFFLOAD_FLEX_DESC, the iAVF driver is capable of
 negotiating to enable the advanced flexible descriptor layout. Add the
 flexible NIC layout (RXDID=2) as a member of the Rx descriptor union.

[...]

Why is this taken into the next queue if I asked for changes and there's
v8 in development?

Thanks,
Olek


Re: [Intel-wired-lan] [PATCH iwl-next v7 09/12] iavf: refactor iavf_clean_rx_irq to support legacy and flex descriptors

2024-06-21 Thread Tony Nguyen




On 6/21/2024 7:21 AM, Alexander Lobakin wrote:

From: Alexander Lobakin 
Date: Wed, 12 Jun 2024 14:33:17 +0200


From: Jacob Keller 
Date: Tue, 11 Jun 2024 13:52:57 -0700




On 6/11/2024 4:47 AM, Alexander Lobakin wrote:

From: Mateusz Polchlopek 
Date: Tue,  4 Jun 2024 09:13:57 -0400


From: Jacob Keller 

Using VIRTCHNL_VF_OFFLOAD_FLEX_DESC, the iAVF driver is capable of
negotiating to enable the advanced flexible descriptor layout. Add the
flexible NIC layout (RXDID=2) as a member of the Rx descriptor union.


[...]

Why is this taken into the next queue if I asked for changes and there's
v8 in development?


This was applied before I returned, however, I believe the patches were 
applied before your comments were received. Since they were already 
applied, I left them on the tree by request [1] (while waiting for v8). 
There were other issues reported after this though so I recently dropped 
the series off the tree.


Thanks,
Tony

[1] 
https://lore.kernel.org/intel-wired-lan/70458c52-75ef-4876-a4a3-c042c52ec...@intel.com/


Re: [Intel-wired-lan] [RFC net-next 5/9] ice: apply XDP offloading fixup when building skb

2024-06-21 Thread Yan Zhai
On Fri, Jun 21, 2024 at 4:22 AM Alexander Lobakin
 wrote:
>
> From: Yan Zhai 
> Date: Thu, 20 Jun 2024 15:19:22 -0700
>
> > Add a common point to transfer offloading info from XDP context to skb.
> >
> > Signed-off-by: Yan Zhai 
> > Signed-off-by: Jesper Dangaard Brouer 
> > ---
> >  drivers/net/ethernet/intel/ice/ice_txrx.c | 2 ++
> >  drivers/net/ethernet/intel/ice/ice_xsk.c  | 6 +-
> >  include/net/xdp_sock_drv.h| 2 +-
> >  3 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c 
> > b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > index 8bb743f78fcb..a247306837ed 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> > @@ -1222,6 +1222,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int 
> > budget)
> >
> >   hard_start = page_address(rx_buf->page) + 
> > rx_buf->page_offset -
> >offset;
> > + xdp_init_buff_minimal(xdp);
>
> Two lines below, you have this:
>
> xdp_buff_clear_frags_flag(xdp);
>
> Which clears frags bit in xdp->flags. I.e. since you always clear flags
> here, this call becomes redundant.
> But I'd say that `xdp->flags = 0` really wants to be moved from
> xdp_init_buff() to xdp_prepare_buff().
>
You are right, there is some redundancy here. I will fix it if people
feel good about the use case in general :)


> >   xdp_prepare_buff(xdp, hard_start, offset, size, 
> > !!offset);
> >  #if (PAGE_SIZE > 4096)
> >   /* At larger PAGE_SIZE, frame_sz depend on len size */
> > @@ -1287,6 +1288,7 @@ int ice_clean_rx_irq(struct ice_rx_ring *rx_ring, int 
> > budget)
> >
> >   /* populate checksum, VLAN, and protocol */
> >   ice_process_skb_fields(rx_ring, rx_desc, skb);
> > + xdp_buff_fixup_skb_offloading(xdp, skb);
> >
> >   ice_trace(clean_rx_irq_indicate, rx_ring, rx_desc, skb);
> >   /* send completed skb up the stack */
> > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c 
> > b/drivers/net/ethernet/intel/ice/ice_xsk.c
> > index a65955eb23c0..367658acaab8 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> > @@ -845,8 +845,10 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, 
> > int budget)
> >   xdp_prog = READ_ONCE(rx_ring->xdp_prog);
> >   xdp_ring = rx_ring->xdp_ring;
> >
> > - if (ntc != rx_ring->first_desc)
> > + if (ntc != rx_ring->first_desc) {
> >   first = *ice_xdp_buf(rx_ring, rx_ring->first_desc);
> > + xdp_init_buff_minimal(first);
>
> xdp_buff_set_size() always clears flags, this is redundant.
>
> > + }
> >
> >   while (likely(total_rx_packets < (unsigned int)budget)) {
> >   union ice_32b_rx_flex_desc *rx_desc;
> > @@ -920,6 +922,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, 
> > int budget)
> >   break;
> >   }
> >
> > + xdp = first;
> >   first = NULL;
> >   rx_ring->first_desc = ntc;
> >
> > @@ -934,6 +937,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, 
> > int budget)
> >   vlan_tci = ice_get_vlan_tci(rx_desc);
> >
> >   ice_process_skb_fields(rx_ring, rx_desc, skb);
> > + xdp_buff_fixup_skb_offloading(xdp, skb);
> >   ice_receive_skb(rx_ring, skb, vlan_tci);
> >   }
> >
> > diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> > index 0a5dca2b2b3f..02243dc064c2 100644
> > --- a/include/net/xdp_sock_drv.h
> > +++ b/include/net/xdp_sock_drv.h
> > @@ -181,7 +181,7 @@ static inline void xsk_buff_set_size(struct xdp_buff 
> > *xdp, u32 size)
> >   xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
> >   xdp->data_meta = xdp->data;
> >   xdp->data_end = xdp->data + size;
> > - xdp->flags = 0;
> > + xdp_init_buff_minimal(xdp);
>
> Why is this done in the patch prefixed with "ice:"?
>
Good catch, this should be moved to the previous patch.

thanks
Yan

> >  }
> >
> >  static inline dma_addr_t xsk_buff_raw_get_dma(struct xsk_buff_pool *pool,
>
> Thanks,
> Olek


Re: [Intel-wired-lan] [intel-next 1/2] net/i40e: link NAPI instances to queues and IRQs

2024-06-21 Thread Joe Damato
On Mon, Apr 15, 2024 at 09:37:09AM -0700, Tony Nguyen wrote:
> 
> 
> On 4/13/2024 12:24 PM, Joe Damato wrote:
> > On Thu, Apr 11, 2024 at 04:02:37PM -0700, Nambiar, Amritha wrote:
> > > On 4/10/2024 4:43 PM, Joe Damato wrote:
> > > > On Wed, Apr 10, 2024 at 02:10:52AM -0700, Nambiar, Amritha wrote:
> > > > > On 4/9/2024 9:39 PM, Joe Damato wrote:
> > > > > > Make i40e compatible with the newly added netlink queue GET APIs.
> > > > > > 
> > > > > > $ ./cli.py --spec ../../../Documentation/netlink/specs/netdev.yaml \
> > > > > >  --do queue-get --json '{"ifindex": 3, "id": 1, "type": "rx"}'
> > > > > > 
> > > > > > {'id': 1, 'ifindex': 3, 'napi-id': 162, 'type': 'rx'}
> > > > > > 
> > > > > > $ ./cli.py --spec ../../../Documentation/netlink/specs/netdev.yaml \
> > > > > >  --do napi-get --json '{"id": 162}'
> > > > > > 
> > > > > > {'id': 162, 'ifindex': 3, 'irq': 136}
> > > > > > 
> > > > > > The above output suggests that irq 136 was allocated for queue 1, 
> > > > > > which has
> > > > > > a NAPI ID of 162.
> > > > > > 
> > > > > > To double check this is correct, the IRQ to queue mapping can be 
> > > > > > verified
> > > > > > by checking /proc/interrupts:
> > > > > > 
> > > > > > $ cat /proc/interrupts  | grep 136\: | \
> > > > > >  awk '{print "irq: " $1 " name " $76}'
> > > > > > 
> > > > > > irq: 136: name i40e-vlan300-TxRx-1
> > > > > > 
> > > > > > Suggests that queue 1 has IRQ 136, as expected.
> > > > > > 
> > > > > > Signed-off-by: Joe Damato 
> > > > > > ---
> > > > > > drivers/net/ethernet/intel/i40e/i40e.h  |  2 +
> > > > > > drivers/net/ethernet/intel/i40e/i40e_main.c | 58 
> > > > > > +
> > > > > > drivers/net/ethernet/intel/i40e/i40e_txrx.c |  4 ++
> > > > > > 3 files changed, 64 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e.h 
> > > > > > b/drivers/net/ethernet/intel/i40e/i40e.h
> > > > > > index 2fbabcdb5bb5..5900ed5c7170 100644
> > > > > > --- a/drivers/net/ethernet/intel/i40e/i40e.h
> > > > > > +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> > > > > > @@ -1267,6 +1267,8 @@ int i40e_ioctl(struct net_device *netdev, 
> > > > > > struct ifreq *ifr, int cmd);
> > > > > > int i40e_open(struct net_device *netdev);
> > > > > > int i40e_close(struct net_device *netdev);
> > > > > > int i40e_vsi_open(struct i40e_vsi *vsi);
> > > > > > +void i40e_queue_set_napi(struct i40e_vsi *vsi, unsigned int 
> > > > > > queue_index,
> > > > > > +enum netdev_queue_type type, struct 
> > > > > > napi_struct *napi);
> > > > > > void i40e_vlan_stripping_disable(struct i40e_vsi *vsi);
> > > > > > int i40e_add_vlan_all_mac(struct i40e_vsi *vsi, s16 vid);
> > > > > > int i40e_vsi_add_vlan(struct i40e_vsi *vsi, u16 vid);
> > > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
> > > > > > b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > > > > index 0bdcdea0be3e..6384a0c73a05 100644
> > > > > > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > > > > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > > > > > @@ -3448,6 +3448,58 @@ static struct xsk_buff_pool 
> > > > > > *i40e_xsk_pool(struct i40e_ring *ring)
> > > > > > return xsk_get_pool_from_qid(ring->vsi->netdev, qid);
> > > > > > }
> > > > > > +/**
> > > > > > + * __i40e_queue_set_napi - Set the napi instance for the queue
> > > > > > + * @dev: device to which NAPI and queue belong
> > > > > > + * @queue_index: Index of queue
> > > > > > + * @type: queue type as RX or TX
> > > > > > + * @napi: NAPI context
> > > > > > + * @locked: is the rtnl_lock already held
> > > > > > + *
> > > > > > + * Set the napi instance for the queue. Caller indicates the lock 
> > > > > > status.
> > > > > > + */
> > > > > > +static void
> > > > > > +__i40e_queue_set_napi(struct net_device *dev, unsigned int 
> > > > > > queue_index,
> > > > > > + enum netdev_queue_type type, struct napi_struct 
> > > > > > *napi,
> > > > > > + bool locked)
> > > > > > +{
> > > > > > +   if (!locked)
> > > > > > +   rtnl_lock();
> > > > > > +   netif_queue_set_napi(dev, queue_index, type, napi);
> > > > > > +   if (!locked)
> > > > > > +   rtnl_unlock();
> > > > > > +}
> > > > > > +
> > > > > > +/**
> > > > > > + * i40e_queue_set_napi - Set the napi instance for the queue
> > > > > > + * @vsi: VSI being configured
> > > > > > + * @queue_index: Index of queue
> > > > > > + * @type: queue type as RX or TX
> > > > > > + * @napi: NAPI context
> > > > > > + *
> > > > > > + * Set the napi instance for the queue. The rtnl lock state is 
> > > > > > derived from the
> > > > > > + * execution path.
> > > > > > + */
> > > > > > +void
> > > > > > +i40e_queue_set_napi(struct i40e_vsi *vsi, unsigned int queue_index,
> > > > > > +   enum netdev_queue_type type, struct napi_struct 
> > > > > > *napi)
> > > > > > +{
> > > > > > +   struct i40e_pf *pf = vsi->back;
> > > > > > +
> >

Re: [Intel-wired-lan] [PATCH iwl-next v3 3/3] ice: Implement driver functionality to dump serdes equalizer values

2024-06-21 Thread Samal, Anil
Hi Simon, 
Serdes equalizer parameters are raw hex values and it is not useful to end 
user. It is only useful to engineer who is familiar with serdes/ physical lane. 
So we decided not to implement it as new ethtool option. 

Thanks
Anil Kumar Samal  

-Original Message-
From: Simon Horman  
Sent: Monday, June 17, 2024 7:53 AM
To: Jakub Kicinski 
Cc: Samal, Anil ; intel-wired-...@lists.osuosl.org; 
net...@vger.kernel.org; Pepiak, Leszek ; Kitszel, 
Przemyslaw ; Czapnik, Lukasz 
; Nguyen, Anthony L ; 
Brandeburg, Jesse 
Subject: Re: [PATCH iwl-next v3 3/3] ice: Implement driver functionality to 
dump serdes equalizer values

On Fri, Jun 14, 2024 at 05:55:59PM -0700, Jakub Kicinski wrote:
> On Fri, 14 Jun 2024 05:58:17 -0700 Anil Samal wrote:
> > To debug link issues in the field, serdes Tx/Rx equalizer values 
> > help to determine the health of serdes lane.
> > 
> > Extend 'ethtool -d' option to dump serdes Tx/Rx equalizer.
> > The following list of equalizer param is supported
> > a. rx_equalization_pre2
> > b. rx_equalization_pre1
> > c. rx_equalization_post1
> > d. rx_equalization_bflf
> > e. rx_equalization_bfhf
> > f. rx_equalization_drate
> > g. tx_equalization_pre1
> > h. tx_equalization_pre3
> > i. tx_equalization_atten
> > j. tx_equalization_post1
> > k. tx_equalization_pre2
> 
> I'd be tempted to create a dedicated way to dump vendor specific 
> signal quality indicators (both for Ethernet and PCIe). Feels little 
> cleaner than appending to a flat mixed-purpose dump. But either way is 
> fine by me, TBH. Much better than vendor tools poking into the BAR...

+1

In particular, I agree that either way ethtool -d is better than than vendor 
tools poking into the BAR.
Because the Kernel can mediate access to the hardware and see the data.