Re: [PATCH net-next v10 11/19] net/mlx5e: Support PSP offload functionality

[Saeed Mahameed <saeed@kernel.org>]

On 28 Aug 09:29, Daniel Zahka wrote:
>From: Raed Salem <raeds@nvidia.com>
>
>Add PSP offload related IFC structs, layouts, and enumerations. Implement
>.set_config and .rx_spi_alloc PSP device operations. Driver does not need
>to make use of the .set_config operation. Stub .assoc_add and .assoc_del
>PSP operations.
>
>Introduce the MLX5_EN_PSP configuration option for enabling PSP offload
>support on mlx5 devices.
>
>Signed-off-by: Raed Salem <raeds@nvidia.com>
>Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
>Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>---
>
>Notes:
>    v7:
>    - use flexible array declaration instead of 0-length array declaration
>      in struct mlx5_ifc_psp_gen_spi_out_bits
>    v4:
>    - remove unneeded psp.c/psp.h files
>    - remove unneeded struct psp_key_spi usage
>    v1:
>    - https://lore.kernel.org/netdev/20240510030435.120935-10-kuba@kernel.org/
>
> .../net/ethernet/mellanox/mlx5/core/Kconfig   |  11 ++
> .../net/ethernet/mellanox/mlx5/core/Makefile  |   2 +
> drivers/net/ethernet/mellanox/mlx5/core/en.h  |   3 +
> .../ethernet/mellanox/mlx5/core/en/params.c   |   4 +-
> .../mellanox/mlx5/core/en_accel/psp.c         | 140 ++++++++++++++++++
> .../mellanox/mlx5/core/en_accel/psp.h         |  47 ++++++
> .../mellanox/mlx5/core/en_accel/psp_offload.c |  44 ++++++
> .../net/ethernet/mellanox/mlx5/core/en_main.c |   9 ++
> drivers/net/ethernet/mellanox/mlx5/core/fw.c  |   6 +
> .../net/ethernet/mellanox/mlx5/core/main.c    |   1 +
> .../mellanox/mlx5/core/steering/hws/definer.c |   2 +-
> include/linux/mlx5/device.h                   |   4 +
> include/linux/mlx5/mlx5_ifc.h                 |  95 +++++++++++-
> 13 files changed, 361 insertions(+), 7 deletions(-)
> create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.h
> create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp_offload.c
>

[...]

>
>+struct mlx5_ifc_psp_cap_bits {
>+	u8         reserved_at_0[0x1];

The below cap is not set or checked in the whole series:
The only occurrence is this definition.

$ git grep psp_crypto_offload tmp/psp
tmp/psp:include/linux/mlx5/mlx5_ifc.h:  u8 psp_crypto_offload[0x1]; /* Set by the driver */

This should be at least checked in mlx5_is_psp_device();

>+	u8         psp_crypto_offload[0x1]; /* Set by the driver */

This comment is not true, the cap is advertised by FW on a psp
capable device. Nothing is needed from driver.

On CX7 and CX8 (FW already PSP capable, without this series):

$ mlx5ctl 0000:17:00.0 cap -i cmd_hca_cap  | grep psp
         psp_old: 0x0 (0)
         psp: 0x1 (1)

$ mlx5ctl 0000:17:00.0 cap -i psp_cap
Node: psp_cap
         psp_crypto_offload: 0x1 (1)
                             ^^^^^^^ (Advertised by FW already)
         psp_crypto_esp_aes_gcm_256_encrypt: 0x1 (1)
         psp_crypto_esp_aes_gcm_128_encrypt: 0x1 (1)
         psp_crypto_esp_aes_gcm_256_decrypt: 0x1 (1)
         psp_crypto_esp_aes_gcm_128_decrypt: 0x1 (1)
         log_max_num_of_psp_spi: 0xb (11)

On Cx6-LX (Crypto, but not psp capable):
$ mlx5ctl 0000:97:00.0 cap -i cmd_hca_cap | grep -E "psp|crypto"
         psp_old: 0x0 (0)
         psp: 0x0 (0)
         crypto: 0x1 (1)

$ mlx5ctl 0000:97:00.0 cap -i psp_cap
Error : opcode 0, syndrome 0x3d6c79 fw status 3 status 0
query cap (0x1e) failed opcode 0x100 opmod 0x3d

I will clean this up as part of my mlx5-next PR, my cleanup will cause a
conflict when re-basing this series on top of the PR+netdev, so just take
my changes "current" to resolve the conflict.

>+	u8         reserved_at_2[0x1];
>+	u8         psp_crypto_esp_aes_gcm_256_encrypt[0x1];
>+	u8         psp_crypto_esp_aes_gcm_128_encrypt[0x1];
>+	u8         psp_crypto_esp_aes_gcm_256_decrypt[0x1];
>+	u8         psp_crypto_esp_aes_gcm_128_decrypt[0x1];
>+	u8         reserved_at_7[0x4];
>+	u8         log_max_num_of_psp_spi[0x5];
>+	u8         reserved_at_10[0x7f0];
>+};