| Message ID | 1565809632-39138-6-git-send-email-haiyangz@microsoft.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add software backchannel and mlx5e HV VHCA stats | expand |
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
On 8/14/2019 11:41 PM, Mark Bloch wrote: > > > 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? Only NULL, will fix. > >> + >> 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 Comment above was right... after fixing it, all is aligned here. > >> @@ -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 I find it more easy to read when having the value explicitly. If you or Saeed has a strong opinion against it, this can be easily fixed. > >> MLX5_HV_VHCA_AGENT_MAX = 32, >> }; >> >> > > Mark >
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); + mutex_lock(&hv_vhca->agents_lock); for (i = 0; i < MLX5_HV_VHCA_AGENT_MAX; i++) WARN_ON(hv_vhca->agents[i]); @@ -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, MLX5_HV_VHCA_AGENT_MAX = 32, };