Re: [PATCH net-next, 5/6] net/mlx5: Add HV VHCA control agent

Mark Bloch
2019-08-14T20:42:31Z

On 8/14/19 12:09 PM, Haiyang Zhang wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Control agent is responsible over of the control block (ID 0). It should
> update the PF via this block about every capability change. In addition,
> upon block 0 invalidate, it should activate all other supported agents
> with data requests from the PF.
>
> Upon agent create/destroy, the invalidate callback of the control agent
> is being called in order to update the PF driver about this change.
>
> The control agent is an integral part of HV VHCA and will be created
> and destroy as part of the HV VHCA init/cleanup flow.
>
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c | 122 ++++++++++++++++++++-
> .../net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h | 1 +
> 2 files changed, 121 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> index b2eebdf..3c7fffa 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.c
> @@ -110,22 +110,131 @@ void mlx5_hv_vhca_invalidate(void *context, u64 block_mask)
> queue_work(hv_vhca->work_queue, &work->invalidate_work);
> }
>
> +#define AGENT_MASK(type) (type ? BIT(type - 1) : 0 /* control */)
> +
> +static void mlx5_hv_vhca_agents_control(struct mlx5_hv_vhca *hv_vhca,
> + struct mlx5_hv_vhca_control_block *block)
> +{
> + int i;
> +
> + for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
> + struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
> +
> + if (!agent || !agent->control)
> + continue;
> +
> + if (!(AGENT_MASK(agent->type) & block->control))
> + continue;
> +
> + agent->control(agent, block);
> + }
> +}
> +
> +static void mlx5_hv_vhca_capabilities(struct mlx5_hv_vhca *hv_vhca,
> + u32 *capabilities)
> +{
> + int i;
> +
> + for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) {
> + struct mlx5_hv_vhca_agent *agent = hv_vhca->agents[i];
> +
> + if (agent)
> + *capabilities |= AGENT_MASK(agent->type);
> + }
> +}
> +
> +static void
> +mlx5_hv_vhca_control_agent_invalidate(struct mlx5_hv_vhca_agent *agent,
> + u64 block_mask)
> +{
> + struct mlx5_hv_vhca *hv_vhca = agent->hv_vhca;
> + struct mlx5_core_dev *dev = hv_vhca->dev;
> + struct mlx5_hv_vhca_control_block *block;
> + u32 capabilities = 0;
> + int err;
> +
> + block = kzalloc(sizeof(*block), GFP_KERNEL);
> + if (!block)
> + return;
> +
> + err = mlx5_hv_read_config(dev, block, sizeof(*block), 0);
> + if (err)
> + goto free_block;
> +
> + mlx5_hv_vhca_capabilities(hv_vhca, &capabilities);
> +
> + /* In case no capabilities, send empty block in return */
> + if (!capabilities) {
> + memset(block, 0, sizeof(*block));
> + goto write;
> + }
> +
> + if (block->capabilities != capabilities)
> + block->capabilities = capabilities;
> +
> + if (block->control & ~capabilities)
> + goto free_block;
> +
> + mlx5_hv_vhca_agents_control(hv_vhca, block);
> + block->command_ack = block->command;
> +
> +write:
> + mlx5_hv_write_config(dev, block, sizeof(*block), 0);
> +
> +free_block:
> + kfree(block);
> +}
> +
> +static struct mlx5_hv_vhca_agent *
> +mlx5_hv_vhca_control_agent_create(struct mlx5_hv_vhca *hv_vhca)
> +{
> + return mlx5_hv_vhca_agent_create(hv_vhca, MLX5_HV_VHCA_AGENT_CONTROL,
> + NULL,
> + mlx5_hv_vhca_control_agent_invalidate,
> + NULL, NULL);
> +}
> +
> +static void mlx5_hv_vhca_control_agent_destroy(struct mlx5_hv_vhca_agent *agent)
> +{
> + mlx5_hv_vhca_agent_destroy(agent);
> +}
> +
> int mlx5_hv_vhca_init(struct mlx5_hv_vhca *hv_vhca)
> {
> + struct mlx5_hv_vhca_agent *agent;
> + int err;
> +
> if (IS_ERR_OR_NULL(hv_vhca))
> return IS_ERR_OR_NULL(hv_vhca);
>
> - return mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
> - mlx5_hv_vhca_invalidate);
> + err = mlx5_hv_register_invalidate(hv_vhca->dev, hv_vhca,
> + mlx5_hv_vhca_invalidate);
> + if (err)
> + return err;
> +
> + agent = mlx5_hv_vhca_control_agent_create(hv_vhca);
> + if (IS_ERR_OR_NULL(agent)) {
> + mlx5_hv_unregister_invalidate(hv_vhca->dev);
> + return IS_ERR_OR_NULL(agent);
> + }
> +
> + hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL] = agent;
> +
> + return 0;
> }
>
> void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
> {
> + struct mlx5_hv_vhca_agent *agent;
> int i;
>
> if (IS_ERR_OR_NULL(hv_vhca))
> return;
>
> + agent = hv_vhca->agents[MLX5_HV_VHCA_AGENT_CONTROL];
> + if (!IS_ERR_OR_NULL(agent))
> + mlx5_hv_vhca_control_agent_destroy(agent);

Can the agent be err ptr here?

> +
> mutex_lock(&hv_vhca->agents_lock);
> for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++)
> WARN_ON(hv_vhca->agents[i]);

With the comment above in mind, here you check only for not null

> @@ -135,6 +244,11 @@ void mlx5_hv_vhca_cleanup(struct mlx5_hv_vhca *hv_vhca)
> mlx5_hv_unregister_invalidate(hv_vhca->dev);
> }
>
> +static void mlx5_hv_vhca_agents_update(struct mlx5_hv_vhca *hv_vhca)
> +{
> + mlx5_hv_vhca_invalidate(hv_vhca, BIT(MLX5_HV_VHCA_AGENT_CONTROL));
> +}
> +
> struct mlx5_hv_vhca_agent *
> mlx5_hv_vhca_agent_create(struct mlx5_hv_vhca *hv_vhca,
> enum mlx5_hv_vhca_agent_type type,
> @@ -168,6 +282,8 @@ struct mlx5_hv_vhca_agent *
> hv_vhca->agents[type] = agent;
> mutex_unlock(&hv_vhca->agents_lock);
>
> + mlx5_hv_vhca_agents_update(hv_vhca);
> +
> return agent;
> }
>
> @@ -189,6 +305,8 @@ void mlx5_hv_vhca_agent_destroy(struct mlx5_hv_vhca_agent *agent)
> agent->cleanup(agent);
>
> kfree(agent);
> +
> + mlx5_hv_vhca_agents_update(hv_vhca);
> }
>
> static int mlx5_hv_vhca_data_block_prepare(struct mlx5_hv_vhca_agent *agent,
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> index fa7ee85..6f4bfb1 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/hv_vhca.h
> @@ -12,6 +12,7 @@
> struct mlx5_hv_vhca_control_block;
>
> enum mlx5_hv_vhca_agent_type {
> + MLX5_HV_VHCA_AGENT_CONTROL = 0,

No need to start value

> MLX5_HV_VHCA_AGENT_MAX = 32,
> };
>
>

Mark