> -----Original Message----- > From: Keller, Jacob E <[email protected]> > Sent: Tuesday, May 5, 2026 1:42 AM > To: Korba, Przemyslaw <[email protected]>; > [email protected] > Cc: [email protected]; Nguyen, Anthony L <[email protected]>; > Kitszel, Przemyslaw <[email protected]>; > Loktionov, Aleksandr <[email protected]>; Kubalewski, Arkadiusz > <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next] ice: add SBQ posted writes > with non-posted support for CGU > > On 4/15/2026 4:27 AM, Przemyslaw Korba wrote: > > From: Karol Kolacinski <[email protected]> > > > > Sideband queue (SBQ) is a HW queue with very short completion time. All > > SBQ writes were posted by default, which means that the driver did not > > have to wait for completion from the neighbor device, because there was > > none. This introduced unnecessary delays, where only those delays were > > "ensuring" that the command is "completed" and this was a potential race > > condition. > > > > Add the possibility to perform non-posted writes where it's necessary to > > wait for completion, instead of relying on fake completion from the FW, > > where only the delays are guarding the writes. > > > > Flush the SBQ by reading address 0 from the PHY 0 before issuing SYNC > > command to ensure that writes to all PHYs were completed and skip SBQ > > message completion if it's posted. > > > > To analyze if delays are gone, look for and compare time spent in > > ice_sq_send_cmd — posted writes should return immediately after the wr32. > > That can be done for example by adjusting phc time with phc_ctl on E830 > > device, for less than 2 seconds to use this new mechanism. Without it, > > command below will fail. > > > > Reproduction steps: > > phc_ctl eth13 adj 1 > > phc_ctl[4478170.994]: adjusted clock by 1.000000 seconds > > > > Check trace for timing for comparisions: > > echo ice_sbq_send_cmd > /sys/kernel/debug/tracing/set_ftrace_filter > > echo function_graph > /sys/kernel/debug/tracing/current_tracer > > cat /sys/kernel/debug/tracing/trace > > > > Tested on: > > - Intel E830 NIC (FW version 1.00) > > - Kernel 6.19.0+ > > > > Signed-off-by: Karol Kolacinski <[email protected]> > > Signed-off-by: Przemyslaw Korba <[email protected]> > > Reviewed-by: Aleksandr Loktionov <[email protected]> > > Reviewed-by: Arkadiusz Kubalewski <[email protected]> > > --- > > This doesn't appear to apply clean to the tip of Intel Wired LAN > dev-queue, nor to net-next/main... > > > drivers/net/ethernet/intel/ice/ice_common.c | 18 ++++-- > > drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 64 ++++++++++++-------- > > drivers/net/ethernet/intel/ice/ice_sbq_cmd.h | 5 +- > > 3 files changed, 53 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_common.c > > b/drivers/net/ethernet/intel/ice/ice_common.c > > index f84990996530..2cd3d6d450a9 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_common.c > > +++ b/drivers/net/ethernet/intel/ice/ice_common.c > > @@ -1777,23 +1777,29 @@ int ice_sbq_rw_reg(struct ice_hw *hw, struct > > ice_sbq_msg_input *in, u16 flags) > > msg.msg_addr_low = cpu_to_le16(in->msg_addr_low); > > msg.msg_addr_high = cpu_to_le32(in->msg_addr_high); > > > > - if (in->opcode) > > + switch (in->opcode) { > > + case ice_sbq_msg_wr_p: > > + case ice_sbq_msg_wr_np: > > msg.data = cpu_to_le32(in->data); > > - else > > + break; > > + case ice_sbq_msg_rd: > > /* data read comes back in completion, so shorten the struct by > > * sizeof(msg.data) > > */ > > msg_len -= sizeof(msg.data); > > + break; > > + default: > > + return -EINVAL; > > + } > > > > - if (in->opcode == ice_sbq_msg_wr) > > - cd.posted = 1; > > It looks like this code in the upstream version doesn't have the cd > structure on this function. > > > + cd.posted = in->opcode == ice_sbq_msg_wr_p; > > > > It looks like this is based on top of "ice: fix posted write support for > sideband queue operations"? That was dropped from the queue because of > our discussion that you would re-submit a fixed version. > > Since that didn't get applied, this won't apply clean either. Do you > still want the part that fixes E830 to go to net? Or do you just want to > implement posted writes all together in one patch? > > Either way, could you please re-submit the work either as 2 patches or > as a single combined patch? > > Thanks, > Jake
Hi, thank you for review 😊 I thin iwl-net will be a better place for this patch, since we need it backported to older kernels as well. I will re-upload it to net as one patch.
