Re: [PATCH 04/15] net: hbl_cn: QP state machine

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/17/24 16:18, Leon Romanovsky wrote:
> [Some people who received this message don't often get email from leon@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Thu, Jun 13, 2024 at 11:21:57AM +0300, Omer Shpigelman wrote:
>> Add a common QP state machine which handles the moving for a QP from one
>> state to another including performing necessary checks, draining
>> in-flight transactions, invalidating caches and error reporting.
>>
>> Signed-off-by: Omer Shpigelman <oshpigelman@xxxxxxxxx>
>> Co-developed-by: Abhilash K V <kvabhilash@xxxxxxxxx>
>> Signed-off-by: Abhilash K V <kvabhilash@xxxxxxxxx>
>> Co-developed-by: Andrey Agranovich <aagranovich@xxxxxxxxx>
>> Signed-off-by: Andrey Agranovich <aagranovich@xxxxxxxxx>
>> Co-developed-by: Bharat Jauhari <bjauhari@xxxxxxxxx>
>> Signed-off-by: Bharat Jauhari <bjauhari@xxxxxxxxx>
>> Co-developed-by: David Meriin <dmeriin@xxxxxxxxx>
>> Signed-off-by: David Meriin <dmeriin@xxxxxxxxx>
>> Co-developed-by: Sagiv Ozeri <sozeri@xxxxxxxxx>
>> Signed-off-by: Sagiv Ozeri <sozeri@xxxxxxxxx>
>> Co-developed-by: Zvika Yehudai <zyehudai@xxxxxxxxx>
>> Signed-off-by: Zvika Yehudai <zyehudai@xxxxxxxxx>
>> ---
>>  .../ethernet/intel/hbl_cn/common/hbl_cn_qp.c  | 480 +++++++++++++++++-
>>  1 file changed, 479 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
>> index 9ddc23bf8194..26ebdf448193 100644
>> --- a/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
>> +++ b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn_qp.c
>> @@ -6,8 +6,486 @@
> 
> <...>
> 
>> +/* The following table represents the (valid) operations that can be performed on
>> + * a QP in order to move it from one state to another
>> + * For example: a QP in RTR state can be moved to RTS state using the CN_QP_OP_RTR_2RTS
>> + * operation.
>> + */
>> +static const enum hbl_cn_qp_state_op qp_valid_state_op[CN_QP_NUM_STATE][CN_QP_NUM_STATE] = {
>> +     [CN_QP_STATE_RESET] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_INIT]      = CN_QP_OP_RST_2INIT,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_NOP,
>> +     },
>> +     [CN_QP_STATE_INIT] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_INIT]      = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_RTR]       = CN_QP_OP_INIT_2RTR,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_NOP,
>> +     },
>> +     [CN_QP_STATE_RTR] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_RTR]       = CN_QP_OP_RTR_2RTR,
>> +             [CN_QP_STATE_RTS]       = CN_QP_OP_RTR_2RTS,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_RTR_2QPD,
>> +     },
>> +     [CN_QP_STATE_RTS] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_RTS]       = CN_QP_OP_RTS_2RTS,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_RTS_2SQD,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_RTS_2QPD,
>> +             [CN_QP_STATE_SQERR]     = CN_QP_OP_RTS_2SQERR,
>> +     },
>> +     [CN_QP_STATE_SQD] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_SQD_2SQD,
>> +             [CN_QP_STATE_RTS]       = CN_QP_OP_SQD_2RTS,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_SQD_2QPD,
>> +             [CN_QP_STATE_SQERR]     = CN_QP_OP_SQD_2SQ_ERR,
>> +     },
>> +     [CN_QP_STATE_QPD] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_QPD]       = CN_QP_OP_NOP,
>> +             [CN_QP_STATE_RTR]       = CN_QP_OP_QPD_2RTR,
>> +     },
>> +     [CN_QP_STATE_SQERR] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +             [CN_QP_STATE_SQD]       = CN_QP_OP_SQ_ERR_2SQD,
>> +             [CN_QP_STATE_SQERR]     = CN_QP_OP_NOP,
>> +     },
>> +     [CN_QP_STATE_ERR] = {
>> +             [CN_QP_STATE_RESET]     = CN_QP_OP_2RESET,
>> +             [CN_QP_STATE_ERR]       = CN_QP_OP_2ERR,
>> +     }
>> +};
> 
> I don't understand why IBTA QP state machine is declared in ETH driver
> and not in IB driver.
> 

Implementing the actual transitions between the states requires full
knowledge of the HW e.g. when to flush, cache invalidation, timeouts.
Our IB driver is agnostic to the ASIC type by design. Note that more ASIC
generations are planned to be added and the IB driver should not be aware
of these additional HWs.
Hence we implemeted the QP state machine in the CN driver which is aware
of the actual HW.

>> +
> 
> <...>
> 
>> +             /* Release lock while we wait before retry.
>> +              * Note, we can assert that we are already locked.
>> +              */
>> +             port_funcs->cfg_unlock(cn_port);
>> +
>> +             msleep(20);
>> +
>> +             port_funcs->cfg_lock(cn_port);
> 
> lock/unlock through ops pointer doesn't look like a good idea.
> 

More ASIC generations will be added once we merge the current Gaudi2 code.
On other ASICs the locking granularity is different because some of the HW
resources are shared between different logical ports.
Hence it is was logical for us to implement it with a function pointer so
each ASIC specific code can implemnet the locking properly.

> Thanks




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux