* [net 2/7] net/mlx5: DR, Remove redundant vport number from action
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Alex Vesker, Maor Gottlieb,
Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Alex Vesker <valex@mellanox.com>
The vport number is part of the vport_cap, there is no reason
to store in a separate variable on the vport.
Fixes: 9db810ed2d37 ("net/mlx5: DR, Expose steering action functionality")
Signed-off-by: Alex Vesker <valex@mellanox.com>
Reviewed-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c | 4 ++--
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h | 1 -
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
index 7d81a7735de5..b74b7d0f6590 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
@@ -615,7 +615,7 @@ static int dr_action_handle_cs_recalc(struct mlx5dr_domain *dmn,
* that recalculates the CS and forwards to the vport.
*/
ret = mlx5dr_domain_cache_get_recalc_cs_ft_addr(dest_action->vport.dmn,
- dest_action->vport.num,
+ dest_action->vport.caps->num,
final_icm_addr);
if (ret) {
mlx5dr_err(dmn, "Failed to get FW cs recalc flow table\n");
@@ -744,7 +744,7 @@ int mlx5dr_actions_build_ste_arr(struct mlx5dr_matcher *matcher,
dest_action = action;
if (rx_rule) {
/* Loopback on WIRE vport is not supported */
- if (action->vport.num == WIRE_PORT)
+ if (action->vport.caps->num == WIRE_PORT)
goto out_invalid_arg;
attr.final_icm_addr = action->vport.caps->icm_address_rx;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
index a37ee6359be2..78f899fb3305 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
@@ -745,7 +745,6 @@ struct mlx5dr_action {
struct {
struct mlx5dr_domain *dmn;
struct mlx5dr_cmd_vport_cap *caps;
- u32 num;
} vport;
struct {
u32 vlan_hdr; /* tpid_pcp_dei_vid */
--
2.21.0
^ permalink raw reply related
* [net 1/7] net/mlx5: DR, Fix SW steering HW bits and definitions
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Yevgeny Kliteynik, Alex Vesker,
Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Yevgeny Kliteynik <kliteyn@mellanox.com>
Fix wrong reserved bits offsets.
Fixes: 97b5484ed608 ("net/mlx5: Add HW bits and definitions required for SW steering")
Signed-off-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
Reviewed-by: Alex Vesker <valex@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
include/linux/mlx5/mlx5_ifc.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index a487b681b516..138c50d5a353 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -282,7 +282,6 @@ enum {
MLX5_CMD_OP_ALLOC_MODIFY_HEADER_CONTEXT = 0x940,
MLX5_CMD_OP_DEALLOC_MODIFY_HEADER_CONTEXT = 0x941,
MLX5_CMD_OP_QUERY_MODIFY_HEADER_CONTEXT = 0x942,
- MLX5_CMD_OP_SYNC_STEERING = 0xb00,
MLX5_CMD_OP_FPGA_CREATE_QP = 0x960,
MLX5_CMD_OP_FPGA_MODIFY_QP = 0x961,
MLX5_CMD_OP_FPGA_QUERY_QP = 0x962,
@@ -296,6 +295,7 @@ enum {
MLX5_CMD_OP_DESTROY_UCTX = 0xa06,
MLX5_CMD_OP_CREATE_UMEM = 0xa08,
MLX5_CMD_OP_DESTROY_UMEM = 0xa0a,
+ MLX5_CMD_OP_SYNC_STEERING = 0xb00,
MLX5_CMD_OP_MAX
};
@@ -487,7 +487,7 @@ union mlx5_ifc_gre_key_bits {
struct mlx5_ifc_fte_match_set_misc_bits {
u8 gre_c_present[0x1];
- u8 reserved_auto1[0x1];
+ u8 reserved_at_1[0x1];
u8 gre_k_present[0x1];
u8 gre_s_present[0x1];
u8 source_vhca_port[0x4];
@@ -5054,50 +5054,50 @@ struct mlx5_ifc_query_hca_cap_in_bits {
struct mlx5_ifc_other_hca_cap_bits {
u8 roce[0x1];
- u8 reserved_0[0x27f];
+ u8 reserved_at_1[0x27f];
};
struct mlx5_ifc_query_other_hca_cap_out_bits {
u8 status[0x8];
- u8 reserved_0[0x18];
+ u8 reserved_at_8[0x18];
u8 syndrome[0x20];
- u8 reserved_1[0x40];
+ u8 reserved_at_40[0x40];
struct mlx5_ifc_other_hca_cap_bits other_capability;
};
struct mlx5_ifc_query_other_hca_cap_in_bits {
u8 opcode[0x10];
- u8 reserved_0[0x10];
+ u8 reserved_at_10[0x10];
- u8 reserved_1[0x10];
+ u8 reserved_at_20[0x10];
u8 op_mod[0x10];
- u8 reserved_2[0x10];
+ u8 reserved_at_40[0x10];
u8 function_id[0x10];
- u8 reserved_3[0x20];
+ u8 reserved_at_60[0x20];
};
struct mlx5_ifc_modify_other_hca_cap_out_bits {
u8 status[0x8];
- u8 reserved_0[0x18];
+ u8 reserved_at_8[0x18];
u8 syndrome[0x20];
- u8 reserved_1[0x40];
+ u8 reserved_at_40[0x40];
};
struct mlx5_ifc_modify_other_hca_cap_in_bits {
u8 opcode[0x10];
- u8 reserved_0[0x10];
+ u8 reserved_at_10[0x10];
- u8 reserved_1[0x10];
+ u8 reserved_at_20[0x10];
u8 op_mod[0x10];
- u8 reserved_2[0x10];
+ u8 reserved_at_40[0x10];
u8 function_id[0x10];
u8 field_select[0x20];
--
2.21.0
^ permalink raw reply related
* [net 3/7] net/mlx5: DR, Fix getting incorrect prev node in ste_free
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Alex Vesker, Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Alex Vesker <valex@mellanox.com>
When we free an STE and the STE is in the middle of collision
list, the prev_ste was obtained incorrectly from the list.
To avoid such issues list_entry calls replaced with standard list API.
Fixes: 26d688e33f88 ("net/mlx5: DR, Add Steering entry (STE) utilities")
Signed-off-by: Alex Vesker <valex@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../mellanox/mlx5/core/steering/dr_matcher.c | 10 ++++------
.../ethernet/mellanox/mlx5/core/steering/dr_rule.c | 2 +-
.../ethernet/mellanox/mlx5/core/steering/dr_ste.c | 14 +++++---------
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
index 01008cd66f75..9c2c25356dd0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
@@ -458,13 +458,11 @@ static int dr_matcher_add_to_tbl(struct mlx5dr_matcher *matcher)
prev_matcher = NULL;
if (next_matcher && !first)
- prev_matcher = list_entry(next_matcher->matcher_list.prev,
- struct mlx5dr_matcher,
- matcher_list);
+ prev_matcher = list_prev_entry(next_matcher, matcher_list);
else if (!first)
- prev_matcher = list_entry(tbl->matcher_list.prev,
- struct mlx5dr_matcher,
- matcher_list);
+ prev_matcher = list_last_entry(&tbl->matcher_list,
+ struct mlx5dr_matcher,
+ matcher_list);
if (dmn->type == MLX5DR_DOMAIN_TYPE_FDB ||
dmn->type == MLX5DR_DOMAIN_TYPE_NIC_RX) {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c
index 3bc3f66b8fa8..4187f2b112b8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_rule.c
@@ -18,7 +18,7 @@ static int dr_rule_append_to_miss_list(struct mlx5dr_ste *new_last_ste,
struct mlx5dr_ste *last_ste;
/* The new entry will be inserted after the last */
- last_ste = list_entry(miss_list->prev, struct mlx5dr_ste, miss_list_node);
+ last_ste = list_last_entry(miss_list, struct mlx5dr_ste, miss_list_node);
WARN_ON(!last_ste);
ste_info_last = kzalloc(sizeof(*ste_info_last), GFP_KERNEL);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
index 6b0af64536d8..95b7221f5730 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
@@ -429,12 +429,9 @@ static void dr_ste_remove_middle_ste(struct mlx5dr_ste *ste,
struct mlx5dr_ste *prev_ste;
u64 miss_addr;
- prev_ste = list_entry(mlx5dr_ste_get_miss_list(ste)->prev, struct mlx5dr_ste,
- miss_list_node);
- if (!prev_ste) {
- WARN_ON(true);
+ prev_ste = list_prev_entry(ste, miss_list_node);
+ if (WARN_ON(!prev_ste))
return;
- }
miss_addr = mlx5dr_ste_get_miss_addr(ste->hw_ste);
mlx5dr_ste_set_miss_addr(prev_ste->hw_ste, miss_addr);
@@ -461,8 +458,8 @@ void mlx5dr_ste_free(struct mlx5dr_ste *ste,
struct mlx5dr_ste_htbl *stats_tbl;
LIST_HEAD(send_ste_list);
- first_ste = list_entry(mlx5dr_ste_get_miss_list(ste)->next,
- struct mlx5dr_ste, miss_list_node);
+ first_ste = list_first_entry(mlx5dr_ste_get_miss_list(ste),
+ struct mlx5dr_ste, miss_list_node);
stats_tbl = first_ste->htbl;
/* Two options:
@@ -479,8 +476,7 @@ void mlx5dr_ste_free(struct mlx5dr_ste *ste,
if (last_ste == first_ste)
next_ste = NULL;
else
- next_ste = list_entry(ste->miss_list_node.next,
- struct mlx5dr_ste, miss_list_node);
+ next_ste = list_next_entry(ste, miss_list_node);
if (!next_ste) {
/* One and only entry in the list */
--
2.21.0
^ permalink raw reply related
* [net 4/7] net/mlx5: DR, Allow matching on vport based on vhca_id
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Alaa Hleihel, Alex Vesker, Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Alaa Hleihel <alaa@mellanox.com>
In case source_eswitch_owner_vhca_id is given as a match,
the source_vport (vhca_id) will be set in case vhca_id_valid.
This will allow matching on peer vports, vports that belong
to the other pf.
Fixes: 26d688e33f88 ("net/mlx5: DR, Add Steering entry (STE) utilities")
Signed-off-by: Alaa Hleihel <alaa@mellanox.com>
Signed-off-by: Alex Vesker <valex@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../mellanox/mlx5/core/steering/dr_matcher.c | 3 +-
.../mellanox/mlx5/core/steering/dr_ste.c | 36 ++++++++++++++++---
.../mellanox/mlx5/core/steering/dr_types.h | 6 ++--
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
index 9c2c25356dd0..67dea7698fc9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_matcher.c
@@ -230,8 +230,7 @@ static int dr_matcher_set_ste_builders(struct mlx5dr_matcher *matcher,
(dmn->type == MLX5DR_DOMAIN_TYPE_FDB ||
dmn->type == MLX5DR_DOMAIN_TYPE_NIC_RX)) {
ret = mlx5dr_ste_build_src_gvmi_qpn(&sb[idx++], &mask,
- &dmn->info.caps,
- inner, rx);
+ dmn, inner, rx);
if (ret)
return ret;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
index 95b7221f5730..4efe1b0be4a8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_ste.c
@@ -837,6 +837,8 @@ static void dr_ste_copy_mask_misc(char *mask, struct mlx5dr_match_misc *spec)
spec->source_sqn = MLX5_GET(fte_match_set_misc, mask, source_sqn);
spec->source_port = MLX5_GET(fte_match_set_misc, mask, source_port);
+ spec->source_eswitch_owner_vhca_id = MLX5_GET(fte_match_set_misc, mask,
+ source_eswitch_owner_vhca_id);
spec->outer_second_prio = MLX5_GET(fte_match_set_misc, mask, outer_second_prio);
spec->outer_second_cfi = MLX5_GET(fte_match_set_misc, mask, outer_second_cfi);
@@ -2250,11 +2252,18 @@ static int dr_ste_build_src_gvmi_qpn_bit_mask(struct mlx5dr_match_param *value,
{
struct mlx5dr_match_misc *misc_mask = &value->misc;
- if (misc_mask->source_port != 0xffff)
+ /* Partial misc source_port is not supported */
+ if (misc_mask->source_port && misc_mask->source_port != 0xffff)
+ return -EINVAL;
+
+ /* Partial misc source_eswitch_owner_vhca_id is not supported */
+ if (misc_mask->source_eswitch_owner_vhca_id &&
+ misc_mask->source_eswitch_owner_vhca_id != 0xffff)
return -EINVAL;
DR_STE_SET_MASK(src_gvmi_qp, bit_mask, source_gvmi, misc_mask, source_port);
DR_STE_SET_MASK(src_gvmi_qp, bit_mask, source_qp, misc_mask, source_sqn);
+ misc_mask->source_eswitch_owner_vhca_id = 0;
return 0;
}
@@ -2266,17 +2275,33 @@ static int dr_ste_build_src_gvmi_qpn_tag(struct mlx5dr_match_param *value,
struct dr_hw_ste_format *hw_ste = (struct dr_hw_ste_format *)hw_ste_p;
struct mlx5dr_match_misc *misc = &value->misc;
struct mlx5dr_cmd_vport_cap *vport_cap;
+ struct mlx5dr_domain *dmn = sb->dmn;
+ struct mlx5dr_cmd_caps *caps;
u8 *tag = hw_ste->tag;
DR_STE_SET_TAG(src_gvmi_qp, tag, source_qp, misc, source_sqn);
- vport_cap = mlx5dr_get_vport_cap(sb->caps, misc->source_port);
+ if (sb->vhca_id_valid) {
+ /* Find port GVMI based on the eswitch_owner_vhca_id */
+ if (misc->source_eswitch_owner_vhca_id == dmn->info.caps.gvmi)
+ caps = &dmn->info.caps;
+ else if (dmn->peer_dmn && (misc->source_eswitch_owner_vhca_id ==
+ dmn->peer_dmn->info.caps.gvmi))
+ caps = &dmn->peer_dmn->info.caps;
+ else
+ return -EINVAL;
+ } else {
+ caps = &dmn->info.caps;
+ }
+
+ vport_cap = mlx5dr_get_vport_cap(caps, misc->source_port);
if (!vport_cap)
return -EINVAL;
if (vport_cap->vport_gvmi)
MLX5_SET(ste_src_gvmi_qp, tag, source_gvmi, vport_cap->vport_gvmi);
+ misc->source_eswitch_owner_vhca_id = 0;
misc->source_port = 0;
return 0;
@@ -2284,17 +2309,20 @@ static int dr_ste_build_src_gvmi_qpn_tag(struct mlx5dr_match_param *value,
int mlx5dr_ste_build_src_gvmi_qpn(struct mlx5dr_ste_build *sb,
struct mlx5dr_match_param *mask,
- struct mlx5dr_cmd_caps *caps,
+ struct mlx5dr_domain *dmn,
bool inner, bool rx)
{
int ret;
+ /* Set vhca_id_valid before we reset source_eswitch_owner_vhca_id */
+ sb->vhca_id_valid = mask->misc.source_eswitch_owner_vhca_id;
+
ret = dr_ste_build_src_gvmi_qpn_bit_mask(mask, sb->bit_mask);
if (ret)
return ret;
sb->rx = rx;
- sb->caps = caps;
+ sb->dmn = dmn;
sb->inner = inner;
sb->lu_type = MLX5DR_STE_LU_TYPE_SRC_GVMI_AND_QP;
sb->byte_mask = dr_ste_conv_bit_to_byte_mask(sb->bit_mask);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
index 78f899fb3305..1cb3769d4e3c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_types.h
@@ -180,6 +180,8 @@ void mlx5dr_send_fill_and_append_ste_send_info(struct mlx5dr_ste *ste, u16 size,
struct mlx5dr_ste_build {
u8 inner:1;
u8 rx:1;
+ u8 vhca_id_valid:1;
+ struct mlx5dr_domain *dmn;
struct mlx5dr_cmd_caps *caps;
u8 lu_type;
u16 byte_mask;
@@ -331,7 +333,7 @@ void mlx5dr_ste_build_register_1(struct mlx5dr_ste_build *sb,
bool inner, bool rx);
int mlx5dr_ste_build_src_gvmi_qpn(struct mlx5dr_ste_build *sb,
struct mlx5dr_match_param *mask,
- struct mlx5dr_cmd_caps *caps,
+ struct mlx5dr_domain *dmn,
bool inner, bool rx);
void mlx5dr_ste_build_empty_always_hit(struct mlx5dr_ste_build *sb, bool rx);
@@ -453,7 +455,7 @@ struct mlx5dr_match_misc {
u32 gre_c_present:1;
/* Source port.;0xffff determines wire port */
u32 source_port:16;
- u32 reserved_auto2:16;
+ u32 source_eswitch_owner_vhca_id:16;
/* VLAN ID of first VLAN tag the inner header of the incoming packet.
* Valid only when inner_second_cvlan_tag ==1 or inner_second_svlan_tag ==1
*/
--
2.21.0
^ permalink raw reply related
* [net 6/7] net/mlx5e: Fix traffic duplication in ethtool steering
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Saeed Mahameed, Maor Gottlieb
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
Before this patch, when adding multiple ethtool steering rules with
identical classification, the driver used to append the new destination
to the already existing hw rule, which caused the hw to forward the
traffic to all destinations (rx queues).
Here we avoid this by setting the "no append" mlx5 fs core flag when
adding a new ethtool rule.
Fixes: 6dc6071cfcde ("net/mlx5e: Add ethtool flow steering support")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
index eed7101e8bb7..acd946f2ddbe 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_fs_ethtool.c
@@ -399,10 +399,10 @@ add_ethtool_flow_rule(struct mlx5e_priv *priv,
struct mlx5_flow_table *ft,
struct ethtool_rx_flow_spec *fs)
{
+ struct mlx5_flow_act flow_act = { .flags = FLOW_ACT_NO_APPEND };
struct mlx5_flow_destination *dst = NULL;
- struct mlx5_flow_act flow_act = {0};
- struct mlx5_flow_spec *spec;
struct mlx5_flow_handle *rule;
+ struct mlx5_flow_spec *spec;
int err = 0;
spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
--
2.21.0
^ permalink raw reply related
* [net 5/7] net/mlx5: Add device ID of upcoming BlueField-2
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev@vger.kernel.org, Bodong Wang, Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Bodong Wang <bodong@mellanox.com>
Add the device ID of upcoming BlueField-2 integrated ConnectX-6 Dx
network controller. Its VFs will be using the generic VF device ID:
0x101e "ConnectX Family mlx5Gen Virtual Function".
Fixes: 2e9d3e83ab82 ("net/mlx5: Update the list of the PCI supported devices")
Signed-off-by: Bodong Wang <bodong@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 9648c2297803..e47dd7c1b909 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1568,6 +1568,7 @@ static const struct pci_device_id mlx5_core_pci_table[] = {
{ PCI_VDEVICE(MELLANOX, 0x101e), MLX5_PCI_DEV_IS_VF}, /* ConnectX Family mlx5Gen Virtual Function */
{ PCI_VDEVICE(MELLANOX, 0xa2d2) }, /* BlueField integrated ConnectX-5 network controller */
{ PCI_VDEVICE(MELLANOX, 0xa2d3), MLX5_PCI_DEV_IS_VF}, /* BlueField integrated ConnectX-5 network controller VF */
+ { PCI_VDEVICE(MELLANOX, 0xa2d6) }, /* BlueField-2 integrated ConnectX-6 Dx network controller */
{ 0, }
};
--
2.21.0
^ permalink raw reply related
* [net 7/7] net/mlx5e: Fix matching on tunnel addresses type
From: Saeed Mahameed @ 2019-09-24 9:41 UTC (permalink / raw)
To: David S. Miller
Cc: netdev@vger.kernel.org, Dmytro Linkin, Vlad Buslov, Eli Britstein,
Roi Dayan, Saeed Mahameed
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
From: Dmytro Linkin <dmitrolin@mellanox.com>
In mlx5 parse_tunnel_attr() function dispatch on encap IP address type
is performed by directly checking flow_rule_match_key() on
FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS, and then on
FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS. However, since those are stored in
union, first check is always true if any type of encap address is set,
which leads to IPv6 tunnel encap address being parsed as IPv4 by mlx5.
Determine correct IP address type by checking control key first and if
it set, take address type from match.key->addr_type.
Fixes: d1bda7eecd88 ("net/mlx5e: Allow matching only enc_key_id/enc_dst_port for decapsulation action")
Signed-off-by: Dmytro Linkin <dmitrolin@mellanox.com>
Reviewed-by: Vlad Buslov <vladbu@mellanox.com>
Reviewed-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 89 +++++++++++--------
1 file changed, 53 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index da7555fdb4d5..3e78a727f3e6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1664,46 +1664,63 @@ static int parse_tunnel_attr(struct mlx5e_priv *priv,
return err;
}
- if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IPV4_ADDRS)) {
- struct flow_match_ipv4_addrs match;
+ if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_CONTROL)) {
+ struct flow_match_control match;
+ u16 addr_type;
- flow_rule_match_enc_ipv4_addrs(rule, &match);
- MLX5_SET(fte_match_set_lyr_2_4, headers_c,
- src_ipv4_src_ipv6.ipv4_layout.ipv4,
- ntohl(match.mask->src));
- MLX5_SET(fte_match_set_lyr_2_4, headers_v,
- src_ipv4_src_ipv6.ipv4_layout.ipv4,
- ntohl(match.key->src));
-
- MLX5_SET(fte_match_set_lyr_2_4, headers_c,
- dst_ipv4_dst_ipv6.ipv4_layout.ipv4,
- ntohl(match.mask->dst));
- MLX5_SET(fte_match_set_lyr_2_4, headers_v,
- dst_ipv4_dst_ipv6.ipv4_layout.ipv4,
- ntohl(match.key->dst));
-
- MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c, ethertype);
- MLX5_SET(fte_match_set_lyr_2_4, headers_v, ethertype, ETH_P_IP);
- } else if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IPV6_ADDRS)) {
- struct flow_match_ipv6_addrs match;
+ flow_rule_match_enc_control(rule, &match);
+ addr_type = match.key->addr_type;
- flow_rule_match_enc_ipv6_addrs(rule, &match);
- memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_c,
- src_ipv4_src_ipv6.ipv6_layout.ipv6),
- &match.mask->src, MLX5_FLD_SZ_BYTES(ipv6_layout, ipv6));
- memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
- src_ipv4_src_ipv6.ipv6_layout.ipv6),
- &match.key->src, MLX5_FLD_SZ_BYTES(ipv6_layout, ipv6));
+ /* For tunnel addr_type used same key id`s as for non-tunnel */
+ if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
+ struct flow_match_ipv4_addrs match;
- memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_c,
- dst_ipv4_dst_ipv6.ipv6_layout.ipv6),
- &match.mask->dst, MLX5_FLD_SZ_BYTES(ipv6_layout, ipv6));
- memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
- dst_ipv4_dst_ipv6.ipv6_layout.ipv6),
- &match.key->dst, MLX5_FLD_SZ_BYTES(ipv6_layout, ipv6));
+ flow_rule_match_enc_ipv4_addrs(rule, &match);
+ MLX5_SET(fte_match_set_lyr_2_4, headers_c,
+ src_ipv4_src_ipv6.ipv4_layout.ipv4,
+ ntohl(match.mask->src));
+ MLX5_SET(fte_match_set_lyr_2_4, headers_v,
+ src_ipv4_src_ipv6.ipv4_layout.ipv4,
+ ntohl(match.key->src));
- MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c, ethertype);
- MLX5_SET(fte_match_set_lyr_2_4, headers_v, ethertype, ETH_P_IPV6);
+ MLX5_SET(fte_match_set_lyr_2_4, headers_c,
+ dst_ipv4_dst_ipv6.ipv4_layout.ipv4,
+ ntohl(match.mask->dst));
+ MLX5_SET(fte_match_set_lyr_2_4, headers_v,
+ dst_ipv4_dst_ipv6.ipv4_layout.ipv4,
+ ntohl(match.key->dst));
+
+ MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c,
+ ethertype);
+ MLX5_SET(fte_match_set_lyr_2_4, headers_v, ethertype,
+ ETH_P_IP);
+ } else if (addr_type == FLOW_DISSECTOR_KEY_IPV6_ADDRS) {
+ struct flow_match_ipv6_addrs match;
+
+ flow_rule_match_enc_ipv6_addrs(rule, &match);
+ memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_c,
+ src_ipv4_src_ipv6.ipv6_layout.ipv6),
+ &match.mask->src, MLX5_FLD_SZ_BYTES(ipv6_layout,
+ ipv6));
+ memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
+ src_ipv4_src_ipv6.ipv6_layout.ipv6),
+ &match.key->src, MLX5_FLD_SZ_BYTES(ipv6_layout,
+ ipv6));
+
+ memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_c,
+ dst_ipv4_dst_ipv6.ipv6_layout.ipv6),
+ &match.mask->dst, MLX5_FLD_SZ_BYTES(ipv6_layout,
+ ipv6));
+ memcpy(MLX5_ADDR_OF(fte_match_set_lyr_2_4, headers_v,
+ dst_ipv4_dst_ipv6.ipv6_layout.ipv6),
+ &match.key->dst, MLX5_FLD_SZ_BYTES(ipv6_layout,
+ ipv6));
+
+ MLX5_SET_TO_ONES(fte_match_set_lyr_2_4, headers_c,
+ ethertype);
+ MLX5_SET(fte_match_set_lyr_2_4, headers_v, ethertype,
+ ETH_P_IPV6);
+ }
}
if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ENC_IP)) {
--
2.21.0
^ permalink raw reply related
* Re: [pull request][net 0/7] Mellanox, mlx5 fixes 2019-09-24
From: Saeed Mahameed @ 2019-09-24 9:51 UTC (permalink / raw)
To: davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190924094047.15915-1-saeedm@mellanox.com>
On Tue, 2019-09-24 at 09:41 +0000, Saeed Mahameed wrote:
> Hi Dave,
>
> This series introduces some fixes to mlx5 driver.
> For more information please see tag log below.
>
> Please pull and let me know if there is any problem.
>
> For -stable v4.10:
correction: 4.20, NOT 4.10.
> ('net/mlx5e: Fix traffic duplication in ethtool steering')
>
> For -stable v4.19:
> ('net/mlx5: Add device ID of upcoming BlueField-2')
>
> For -stable v5.3:
> ('net/mlx5e: Fix matching on tunnel addresses type')
>
> Thanks,
> Saeed.
>
> ---
> The following changes since commit
> 34b4688425d9841a19a15fa6ae2bfc12a372650f:
>
> net: dsa: Use the correct style for SPDX License Identifier (2019-
> 09-22 15:25:08 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git
> tags/mlx5-fixes-2019-09-24
>
> for you to fetch changes up to
> fe1587a7de94912ed75ba5ddbfabf0741f9f8239:
>
> net/mlx5e: Fix matching on tunnel addresses type (2019-09-24
> 12:38:08 +0300)
>
> ----------------------------------------------------------------
> mlx5-fixes-2019-09-24
>
> ----------------------------------------------------------------
> Alaa Hleihel (1):
> net/mlx5: DR, Allow matching on vport based on vhca_id
>
> Alex Vesker (2):
> net/mlx5: DR, Remove redundant vport number from action
> net/mlx5: DR, Fix getting incorrect prev node in ste_free
>
> Bodong Wang (1):
> net/mlx5: Add device ID of upcoming BlueField-2
>
> Dmytro Linkin (1):
> net/mlx5e: Fix matching on tunnel addresses type
>
> Saeed Mahameed (1):
> net/mlx5e: Fix traffic duplication in ethtool steering
>
> Yevgeny Kliteynik (1):
> net/mlx5: DR, Fix SW steering HW bits and definitions
>
> .../ethernet/mellanox/mlx5/core/en_fs_ethtool.c | 4 +-
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 89
> +++++++++++++---------
> drivers/net/ethernet/mellanox/mlx5/core/main.c | 1 +
> .../mellanox/mlx5/core/steering/dr_action.c | 4 +-
> .../mellanox/mlx5/core/steering/dr_matcher.c | 13 ++--
> .../ethernet/mellanox/mlx5/core/steering/dr_rule.c | 2 +-
> .../ethernet/mellanox/mlx5/core/steering/dr_ste.c | 50 ++++++++----
> .../mellanox/mlx5/core/steering/dr_types.h | 7 +-
> include/linux/mlx5/mlx5_ifc.h | 28 +++----
> 9 files changed, 119 insertions(+), 79 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/6] mdev: class id support
From: Jason Wang @ 2019-09-24 10:11 UTC (permalink / raw)
To: Alex Williamson
Cc: kvm, linux-s390, linux-kernel, dri-devel, intel-gfx,
intel-gvt-dev, kwankhede, mst, tiwei.bie, virtualization, netdev,
cohuck, maxime.coquelin, cunming.liang, zhihong.wang, rob.miller,
xiao.w.wang, haotian.wang, zhenyuw, zhi.a.wang, jani.nikula,
joonas.lahtinen, rodrigo.vivi, airlied, daniel, farman, pasic,
sebott, oberpar, heiko.carstens, gor, borntraeger, akrowiak,
freude, lingshan.zhu, idos, eperezma, lulu, parav
In-Reply-To: <20190923100529.54568ad8@x1.home>
On 2019/9/24 上午12:05, Alex Williamson wrote:
> On Mon, 23 Sep 2019 21:03:26 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> Mdev bus only supports vfio driver right now, so it doesn't implement
>> match method. But in the future, we may add drivers other than vfio,
>> one example is virtio-mdev[1] driver. This means we need to add device
>> class id support in bus match method to pair the mdev device and mdev
>> driver correctly.
>>
>> So this patch adds id_table to mdev_driver and class_id for mdev
>> parent with the match method for mdev bus.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Documentation/driver-api/vfio-mediated-device.rst | 7 +++++--
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
>> drivers/s390/cio/vfio_ccw_ops.c | 2 +-
>> drivers/s390/crypto/vfio_ap_ops.c | 3 ++-
>> drivers/vfio/mdev/mdev_core.c | 14 ++++++++++++--
>> drivers/vfio/mdev/mdev_driver.c | 14 ++++++++++++++
>> drivers/vfio/mdev/mdev_private.h | 1 +
>> drivers/vfio/mdev/vfio_mdev.c | 6 ++++++
>> include/linux/mdev.h | 7 ++++++-
>> include/linux/mod_devicetable.h | 8 ++++++++
>> samples/vfio-mdev/mbochs.c | 2 +-
>> samples/vfio-mdev/mdpy.c | 2 +-
>> samples/vfio-mdev/mtty.c | 2 +-
>> 13 files changed, 59 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
>> index 25eb7d5b834b..0e052072e1d8 100644
>> --- a/Documentation/driver-api/vfio-mediated-device.rst
>> +++ b/Documentation/driver-api/vfio-mediated-device.rst
>> @@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
>> * @probe: called when new device created
>> * @remove: called when device removed
>> * @driver: device driver structure
>> + * @id_table: the ids serviced by this driver.
>> */
>> struct mdev_driver {
>> const char *name;
>> int (*probe) (struct device *dev);
>> void (*remove) (struct device *dev);
>> struct device_driver driver;
>> + const struct mdev_class_id *id_table;
>> };
>>
>> A mediated bus driver for mdev should use this structure in the function calls
>> @@ -116,7 +118,7 @@ to register and unregister itself with the core driver:
>> * Register::
>>
>> extern int mdev_register_driver(struct mdev_driver *drv,
>> - struct module *owner);
>> + struct module *owner);
>>
>> * Unregister::
>>
>> @@ -163,7 +165,8 @@ A driver should use the mdev_parent_ops structure in the function call to
>> register itself with the mdev core driver::
>>
>> extern int mdev_register_device(struct device *dev,
>> - const struct mdev_parent_ops *ops);
>> + const struct mdev_parent_ops *ops,
>> + u8 class_id);
>>
>> However, the mdev_parent_ops structure is not required in the function call
>> that a driver should use to unregister itself with the mdev core driver::
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 23aa3e50cbf8..19d51a35f019 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -1625,7 +1625,7 @@ static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops)
>> return -EFAULT;
>> intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
>>
>> - return mdev_register_device(dev, &intel_vgpu_ops);
>> + return mdev_register_vfio_device(dev, &intel_vgpu_ops);
>> }
>>
>> static void kvmgt_host_exit(struct device *dev)
>> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
>> index f0d71ab77c50..246ff0f80944 100644
>> --- a/drivers/s390/cio/vfio_ccw_ops.c
>> +++ b/drivers/s390/cio/vfio_ccw_ops.c
>> @@ -588,7 +588,7 @@ static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
>>
>> int vfio_ccw_mdev_reg(struct subchannel *sch)
>> {
>> - return mdev_register_device(&sch->dev, &vfio_ccw_mdev_ops);
>> + return mdev_register_vfio_device(&sch->dev, &vfio_ccw_mdev_ops);
>> }
>>
>> void vfio_ccw_mdev_unreg(struct subchannel *sch)
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>> index 5c0f53c6dde7..7487fc39d2c5 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -1295,7 +1295,8 @@ int vfio_ap_mdev_register(void)
>> {
>> atomic_set(&matrix_dev->available_instances, MAX_ZDEV_ENTRIES_EXT);
>>
>> - return mdev_register_device(&matrix_dev->device, &vfio_ap_matrix_ops);
>> + return mdev_register_vfio_device(&matrix_dev->device,
>> + &vfio_ap_matrix_ops);
>> }
>>
>> void vfio_ap_mdev_unregister(void)
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> index b558d4cfd082..a02c256a3514 100644
>> --- a/drivers/vfio/mdev/mdev_core.c
>> +++ b/drivers/vfio/mdev/mdev_core.c
>> @@ -135,11 +135,14 @@ static int mdev_device_remove_cb(struct device *dev, void *data)
>> * mdev_register_device : Register a device
>> * @dev: device structure representing parent device.
>> * @ops: Parent device operation structure to be registered.
>> + * @id: device id.
>> *
>> * Add device to list of registered parent devices.
>> * Returns a negative value on error, otherwise 0.
>> */
>> -int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>> +int mdev_register_device(struct device *dev,
>> + const struct mdev_parent_ops *ops,
>> + u8 class_id)
>> {
>> int ret;
>> struct mdev_parent *parent;
>> @@ -175,6 +178,7 @@ int mdev_register_device(struct device *dev, const struct mdev_parent_ops *ops)
>>
>> parent->dev = dev;
>> parent->ops = ops;
>> + parent->class_id = class_id;
>>
> I don't think we want to tie the class_id to the parent. mdev parent
> devices can create various types of devices, some might be virtio, some
> might be vfio. I think the cover letter even suggests that's a
> direction these virtio devices might be headed. It seems the class
> should be on the resulting device itself. That also suggests that at
> the parent we cannot have a single device_ops, the ops used will depend
> on the type of device created. Perhaps that means we need vfio_ops
> alongside virtio_ops, rather than a common device_ops. Thanks,
>
> Alex
>
Yes, will do it in V2.
Thanks
^ permalink raw reply
* Re: [PATCH net v3] net/sched: cls_api: Fix nooffloaddevcnt counter when indr block call success
From: Or Gerlitz @ 2019-09-24 10:33 UTC (permalink / raw)
To: wenxu
Cc: John Hurley, Pieter Jansen van Vuuren, Oz Shlomo, Jakub Kicinski,
David Miller, Linux Netdev List, Roi Dayan, Paul Blakey,
Jiri Pirko
In-Reply-To: <9797258f-0377-daad-e827-67713d3fba9c@ucloud.cn>
On Mon, Sep 23, 2019 at 5:21 PM wenxu <wenxu@ucloud.cn> wrote:
> 在 2019/9/23 17:42, John Hurley 写道:
> > On Mon, Sep 23, 2019 at 5:20 AM wenxu <wenxu@ucloud.cn> wrote:
> >> Hi John & Jakub
> >>
> >> There are some limitations for indirect tc callback work with skip_sw ?
> >>
> > Hi Wenxu,
> > This is not really a limitation.
> > As Or points out, indirect block offload is not supposed to work with skip_sw.
> > Indirect offload allows us to hook onto existing kernel devices (for
> > TC events we may which to offload) that are out of the control of the
> > offload driver and, therefore, should always accept software path
> > rules.
> > For example, the vxlan driver does not implement a setup_tc ndo so it
> > does not expect to run rules in hw - it should always handle
> > associated rules in the software datapath as a minimum.
> > I think accepting skip_sw rules for devices with no in-built concept
> > of hardware offload would be wrong.
> > Do you have a use case that requires skip_sw rules for such devices?
> When we use ovs to control the tc offload. The ovs kernel already provide the software
> path rules so maybe user don't want other soft path.
this (programming the rule to both tc and ovs kernel DPs) is a choice made by
the ovs user-space code and could change later. Actually, for complex use
case such as connection tracking, there might be cases when multiple tables
are used, when the 1st packet/s would jump before hw to sw TC tables, so
using skip_sw will likely not to work and we'll get bug reports. The production
TC configuration we use/recommend with for overlay networks e-switching
is "both" (== "none", i.e none of skip_sw or skip_hw).
> And with skip_sw it can be easily distinguish offloaded and non-offloaded rules.
per tc rule, the kernel reports on offloaded vs not offloaded
packet/bytes, so the
info is there. In the system level, I don't think it's good that on
point A in time
we blocked skip_sw rules for tunnel devices and at point B we enabled it for no
real/good reason, I vote for blocking it again, since as Pieter
explained disallowing
this is not consistent with the kernel SW model on the receiving side.
^ permalink raw reply
* [PATCH] openvswitch: change type of UPCALL_PID attribute to NLA_UNSPEC
From: Li RongQing @ 2019-09-24 11:11 UTC (permalink / raw)
To: netdev, pshelar
userspace openvswitch patch "(dpif-linux: Implement the API
functions to allow multiple handler threads read upcall)"
changes its type from U32 to UNSPEC, but leave the kernel
unchanged
and after kernel 6e237d099fac "(netlink: Relax attr validation
for fixed length types)", this bug is exposed by the below
warning
[ 57.215841] netlink: 'ovs-vswitchd': attribute type 5 has an invalid length.
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/openvswitch/datapath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index dde9d762edee..f30e406fbec5 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -2294,7 +2294,7 @@ static const struct nla_policy vport_policy[OVS_VPORT_ATTR_MAX + 1] = {
[OVS_VPORT_ATTR_STATS] = { .len = sizeof(struct ovs_vport_stats) },
[OVS_VPORT_ATTR_PORT_NO] = { .type = NLA_U32 },
[OVS_VPORT_ATTR_TYPE] = { .type = NLA_U32 },
- [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_U32 },
+ [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NLA_UNSPEC },
[OVS_VPORT_ATTR_OPTIONS] = { .type = NLA_NESTED },
[OVS_VPORT_ATTR_IFINDEX] = { .type = NLA_U32 },
[OVS_VPORT_ATTR_NETNSID] = { .type = NLA_S32 },
--
2.16.2
^ permalink raw reply related
* Re: [PATCH 1/6] mdev: class id support
From: Jason Wang @ 2019-09-24 11:20 UTC (permalink / raw)
To: Parav Pandit, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, tiwei.bie@intel.com
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
cohuck@redhat.com, maxime.coquelin@redhat.com,
cunming.liang@intel.com, zhihong.wang@intel.com,
rob.miller@broadcom.com, xiao.w.wang@intel.com,
haotian.wang@sifive.com, zhenyuw@linux.intel.com,
zhi.a.wang@intel.com, jani.nikula@linux.intel.com,
joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
pasic@linux.ibm.com, sebott@linux.ibm.com, oberpar@linux.ibm.com,
heiko.carstens@de.ibm.com, gor@linux.ibm.com,
borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
freude@linux.ibm.com, lingshan.zhu@intel.com, Ido Shamay,
eperezma@redhat.com, lulu@redhat.com
In-Reply-To: <AM0PR05MB486675D15C5C25F689BFB77AD1850@AM0PR05MB4866.eurprd05.prod.outlook.com>
On 2019/9/24 上午4:58, Parav Pandit wrote:
> Hi Jason,
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, September 23, 2019 8:03 AM
>> To: kvm@vger.kernel.org; linux-s390@vger.kernel.org; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; intel-
>> gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
>> kwankhede@nvidia.com; alex.williamson@redhat.com; mst@redhat.com;
>> tiwei.bie@intel.com
>> Cc: virtualization@lists.linux-foundation.org; netdev@vger.kernel.org;
>> cohuck@redhat.com; maxime.coquelin@redhat.com;
>> cunming.liang@intel.com; zhihong.wang@intel.com;
>> rob.miller@broadcom.com; xiao.w.wang@intel.com;
>> haotian.wang@sifive.com; zhenyuw@linux.intel.com; zhi.a.wang@intel.com;
>> jani.nikula@linux.intel.com; joonas.lahtinen@linux.intel.com;
>> rodrigo.vivi@intel.com; airlied@linux.ie; daniel@ffwll.ch;
>> farman@linux.ibm.com; pasic@linux.ibm.com; sebott@linux.ibm.com;
>> oberpar@linux.ibm.com; heiko.carstens@de.ibm.com; gor@linux.ibm.com;
>> borntraeger@de.ibm.com; akrowiak@linux.ibm.com; freude@linux.ibm.com;
>> lingshan.zhu@intel.com; Ido Shamay <idos@mellanox.com>;
>> eperezma@redhat.com; lulu@redhat.com; Parav Pandit
>> <parav@mellanox.com>; Jason Wang <jasowang@redhat.com>
>> Subject: [PATCH 1/6] mdev: class id support
>>
>> Mdev bus only supports vfio driver right now, so it doesn't implement match
>> method. But in the future, we may add drivers other than vfio, one example is
>> virtio-mdev[1] driver. This means we need to add device class id support in bus
>> match method to pair the mdev device and mdev driver correctly.
>>
>> So this patch adds id_table to mdev_driver and class_id for mdev parent with
>> the match method for mdev bus.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Documentation/driver-api/vfio-mediated-device.rst | 7 +++++--
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
>> drivers/s390/cio/vfio_ccw_ops.c | 2 +-
>> drivers/s390/crypto/vfio_ap_ops.c | 3 ++-
>> drivers/vfio/mdev/mdev_core.c | 14 ++++++++++++--
>> drivers/vfio/mdev/mdev_driver.c | 14 ++++++++++++++
>> drivers/vfio/mdev/mdev_private.h | 1 +
>> drivers/vfio/mdev/vfio_mdev.c | 6 ++++++
>> include/linux/mdev.h | 7 ++++++-
>> include/linux/mod_devicetable.h | 8 ++++++++
>> samples/vfio-mdev/mbochs.c | 2 +-
>> samples/vfio-mdev/mdpy.c | 2 +-
>> samples/vfio-mdev/mtty.c | 2 +-
>> 13 files changed, 59 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
>> b/Documentation/driver-api/vfio-mediated-device.rst
>> index 25eb7d5b834b..0e052072e1d8 100644
>> --- a/Documentation/driver-api/vfio-mediated-device.rst
>> +++ b/Documentation/driver-api/vfio-mediated-device.rst
>> @@ -102,12 +102,14 @@ structure to represent a mediated device's driver::
>> * @probe: called when new device created
>> * @remove: called when device removed
>> * @driver: device driver structure
>> + * @id_table: the ids serviced by this driver.
> No full stop at the end.
Will fix this.
>
>> */
>> struct mdev_driver {
>> const char *name;
>> int (*probe) (struct device *dev);
>> void (*remove) (struct device *dev);
>> struct device_driver driver;
>> + const struct mdev_class_id *id_table;
>> };
>>
>> A mediated bus driver for mdev should use this structure in the function calls
>> @@ -116,7 +118,7 @@ to register and unregister itself with the core driver:
>> * Register::
>>
>> extern int mdev_register_driver(struct mdev_driver *drv,
>> - struct module *owner);
>> + struct module *owner);
>>
> Unrelated change in this patch.
Yes, will fix.
>
>> * Unregister::
>>
>> @@ -163,7 +165,8 @@ A driver should use the mdev_parent_ops structure in
>> the function call to register itself with the mdev core driver::
>>
>> extern int mdev_register_device(struct device *dev,
>> - const struct mdev_parent_ops *ops);
>> + const struct mdev_parent_ops *ops,
>> + u8 class_id);
>>
> Cover letter from Change-V2 says that it class_id changed from " use u16 instead u8 for class id"
> But it is still u8 here?
Right, let me fix it in v2.
>
>> However, the mdev_parent_ops structure is not required in the function call
>> that a driver should use to unregister itself with the mdev core driver::
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 23aa3e50cbf8..19d51a35f019 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -1625,7 +1625,7 @@ static int kvmgt_host_init(struct device *dev, void
>> *gvt, const void *ops)
>> return -EFAULT;
>> intel_vgpu_ops.supported_type_groups = kvm_vgpu_type_groups;
>>
>> - return mdev_register_device(dev, &intel_vgpu_ops);
>> + return mdev_register_vfio_device(dev, &intel_vgpu_ops);
>> }
>>
>> static void kvmgt_host_exit(struct device *dev) diff --git
>> a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index
>> f0d71ab77c50..246ff0f80944 100644
>> --- a/drivers/s390/cio/vfio_ccw_ops.c
>> +++ b/drivers/s390/cio/vfio_ccw_ops.c
>> @@ -588,7 +588,7 @@ static const struct mdev_parent_ops
>> vfio_ccw_mdev_ops = {
>>
>> int vfio_ccw_mdev_reg(struct subchannel *sch) {
>> - return mdev_register_device(&sch->dev, &vfio_ccw_mdev_ops);
>> + return mdev_register_vfio_device(&sch->dev, &vfio_ccw_mdev_ops);
>> }
>>
>> void vfio_ccw_mdev_unreg(struct subchannel *sch) diff --git
>> a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
>> index 5c0f53c6dde7..7487fc39d2c5 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -1295,7 +1295,8 @@ int vfio_ap_mdev_register(void) {
>> atomic_set(&matrix_dev->available_instances,
>> MAX_ZDEV_ENTRIES_EXT);
>>
>> - return mdev_register_device(&matrix_dev->device,
>> &vfio_ap_matrix_ops);
>> + return mdev_register_vfio_device(&matrix_dev->device,
>> + &vfio_ap_matrix_ops);
>> }
>>
>> void vfio_ap_mdev_unregister(void)
>> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
>> index b558d4cfd082..a02c256a3514 100644
>> --- a/drivers/vfio/mdev/mdev_core.c
>> +++ b/drivers/vfio/mdev/mdev_core.c
>> @@ -135,11 +135,14 @@ static int mdev_device_remove_cb(struct device
>> *dev, void *data)
>> * mdev_register_device : Register a device
>> * @dev: device structure representing parent device.
>> * @ops: Parent device operation structure to be registered.
>> + * @id: device id.
>> *
> It device id here, but in below its class_id. Please make them uniform.
Yes, will fix.
>> * Add device to list of registered parent devices.
>> * Returns a negative value on error, otherwise 0.
>> */
>> -int mdev_register_device(struct device *dev, const struct mdev_parent_ops
>> *ops)
>> +int mdev_register_device(struct device *dev,
>> + const struct mdev_parent_ops *ops,
>> + u8 class_id)
>> {
> u16 class_id?
Yes it is.
>
>> int ret;
>> struct mdev_parent *parent;
>> @@ -175,6 +178,7 @@ int mdev_register_device(struct device *dev, const
>> struct mdev_parent_ops *ops)
>>
>> parent->dev = dev;
>> parent->ops = ops;
>> + parent->class_id = class_id;
>>
>> if (!mdev_bus_compat_class) {
>> mdev_bus_compat_class =
>> class_compat_register("mdev_bus");
>> @@ -208,7 +212,13 @@ int mdev_register_device(struct device *dev, const
>> struct mdev_parent_ops *ops)
>> put_device(dev);
>> return ret;
>> }
>> -EXPORT_SYMBOL(mdev_register_device);
>> +
>> +int mdev_register_vfio_device(struct device *dev,
>> + const struct mdev_parent_ops *ops) {
>> + return mdev_register_device(dev, ops, MDEV_ID_VFIO); }
>> +EXPORT_SYMBOL(mdev_register_vfio_device);
>>
>> /*
>> * mdev_unregister_device : Unregister a parent device diff --git
>> a/drivers/vfio/mdev/mdev_driver.c b/drivers/vfio/mdev/mdev_driver.c index
>> 0d3223aee20b..b70bbebc9dd3 100644
>> --- a/drivers/vfio/mdev/mdev_driver.c
>> +++ b/drivers/vfio/mdev/mdev_driver.c
>> @@ -69,8 +69,22 @@ static int mdev_remove(struct device *dev)
>> return 0;
>> }
>>
>> +static int mdev_match(struct device *dev, struct device_driver *drv) {
>> + unsigned int i;
>> + struct mdev_device *mdev = to_mdev_device(dev);
>> + struct mdev_driver *mdrv = to_mdev_driver(drv);
>> + const struct mdev_class_id *ids = mdrv->id_table;
>> +
>> + for (i = 0; ids[i].id; i++)
>> + if (ids[i].id == mdev->parent->class_id)
>> + return 1;
>> + return 0;
>> +}
>> +
>> struct bus_type mdev_bus_type = {
>> .name = "mdev",
>> + .match = mdev_match,
>> .probe = mdev_probe,
>> .remove = mdev_remove,
>> };
>> diff --git a/drivers/vfio/mdev/mdev_private.h
>> b/drivers/vfio/mdev/mdev_private.h
>> index 7d922950caaf..e58b07c866b1 100644
>> --- a/drivers/vfio/mdev/mdev_private.h
>> +++ b/drivers/vfio/mdev/mdev_private.h
>> @@ -22,6 +22,7 @@ struct mdev_parent {
>> struct list_head type_list;
>> /* Synchronize device creation/removal with parent unregistration */
>> struct rw_semaphore unreg_sem;
>> + u8 class_id;
>> };
>>
>> struct mdev_device {
>> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
>> index 30964a4e0a28..fd2a4d9a3f26 100644
>> --- a/drivers/vfio/mdev/vfio_mdev.c
>> +++ b/drivers/vfio/mdev/vfio_mdev.c
>> @@ -120,10 +120,16 @@ static void vfio_mdev_remove(struct device *dev)
>> vfio_del_group_dev(dev);
>> }
>>
>> +static struct mdev_class_id id_table[] = {
>> + { MDEV_ID_VFIO },
>> + { 0 },
>> +};
>> +
>> static struct mdev_driver vfio_mdev_driver = {
>> .name = "vfio_mdev",
>> .probe = vfio_mdev_probe,
>> .remove = vfio_mdev_remove,
>> + .id_table = id_table,
>> };
>>
>> static int __init vfio_mdev_init(void)
>> diff --git a/include/linux/mdev.h b/include/linux/mdev.h index
>> 0ce30ca78db0..3ebae310f599 100644
>> --- a/include/linux/mdev.h
>> +++ b/include/linux/mdev.h
>> @@ -118,6 +118,7 @@ struct mdev_type_attribute mdev_type_attr_##_name
>> = \
>> * @probe: called when new device created
>> * @remove: called when device removed
>> * @driver: device driver structure
>> + * @id_table: the ids serviced by this driver.
>> *
>> **/
>> struct mdev_driver {
>> @@ -125,6 +126,7 @@ struct mdev_driver {
>> int (*probe)(struct device *dev);
>> void (*remove)(struct device *dev);
>> struct device_driver driver;
>> + const struct mdev_class_id *id_table;
>> };
>>
>> #define to_mdev_driver(drv) container_of(drv, struct mdev_driver, driver)
>> @@ -135,7 +137,8 @@ const guid_t *mdev_uuid(struct mdev_device *mdev);
>>
>> extern struct bus_type mdev_bus_type;
>>
>> -int mdev_register_device(struct device *dev, const struct mdev_parent_ops
>> *ops);
>> +int mdev_register_vfio_device(struct device *dev,
>> + const struct mdev_parent_ops *ops);
>> void mdev_unregister_device(struct device *dev);
>>
>> int mdev_register_driver(struct mdev_driver *drv, struct module *owner);
>> @@ -145,4 +148,6 @@ struct device *mdev_parent_dev(struct mdev_device
>> *mdev); struct device *mdev_dev(struct mdev_device *mdev); struct
>> mdev_device *mdev_from_dev(struct device *dev);
>>
>> +#define MDEV_ID_VFIO 1 /* VFIO device */
>> +
> Instead of define, please put them as enum
>
> enum mdev_class/device_id {
> MDEV_ID_VFIO = 1,
> /* New entries must be added here */
> };
Ok.
>
>> #endif /* MDEV_H */
>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>> index 5714fd35a83c..f32c6e44fb1a 100644
>> --- a/include/linux/mod_devicetable.h
>> +++ b/include/linux/mod_devicetable.h
>> @@ -821,4 +821,12 @@ struct wmi_device_id {
>> const void *context;
>> };
>>
>> +/**
>> + * struct mdev_class_id - MDEV device class identifier
>> + * @id: Used to identify a specific class of device, e.g vfio-mdev device.
>> + */
>> +struct mdev_class_id {
>> + __u16 id;
>> +};
> This is u16 as I guess you wanted as opposed to u8 in other places in patch.
Right.
Thanks
>> +
>> #endif /* LINUX_MOD_DEVICETABLE_H */
>> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index
>> ac5c8c17b1ff..71a4469be85d 100644
>> --- a/samples/vfio-mdev/mbochs.c
>> +++ b/samples/vfio-mdev/mbochs.c
>> @@ -1468,7 +1468,7 @@ static int __init mbochs_dev_init(void)
>> if (ret)
>> goto failed2;
>>
>> - ret = mdev_register_device(&mbochs_dev, &mdev_fops);
>> + ret = mdev_register_vfio_device(&mbochs_dev, &mdev_fops);
>> if (ret)
>> goto failed3;
>>
>> diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index
>> cc86bf6566e4..d3029dd27d91 100644
>> --- a/samples/vfio-mdev/mdpy.c
>> +++ b/samples/vfio-mdev/mdpy.c
>> @@ -775,7 +775,7 @@ static int __init mdpy_dev_init(void)
>> if (ret)
>> goto failed2;
>>
>> - ret = mdev_register_device(&mdpy_dev, &mdev_fops);
>> + ret = mdev_register_vfio_device(&mdpy_dev, &mdev_fops);
>> if (ret)
>> goto failed3;
>>
>> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index
>> 92e770a06ea2..744c88a6b22c 100644
>> --- a/samples/vfio-mdev/mtty.c
>> +++ b/samples/vfio-mdev/mtty.c
>> @@ -1468,7 +1468,7 @@ static int __init mtty_dev_init(void)
>> if (ret)
>> goto failed2;
>>
>> - ret = mdev_register_device(&mtty_dev.dev, &mdev_fops);
>> + ret = mdev_register_vfio_device(&mtty_dev.dev, &mdev_fops);
>> if (ret)
>> goto failed3;
>>
>> --
>> 2.19.1
^ permalink raw reply
* Re: [PATCH 1/6] mdev: class id support
From: Jason Wang @ 2019-09-24 11:23 UTC (permalink / raw)
To: Parav Pandit, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, tiwei.bie@intel.com
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
cohuck@redhat.com, maxime.coquelin@redhat.com,
cunming.liang@intel.com, zhihong.wang@intel.com,
rob.miller@broadcom.com, xiao.w.wang@intel.com,
haotian.wang@sifive.com, zhenyuw@linux.intel.com,
zhi.a.wang@intel.com, jani.nikula@linux.intel.com,
joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
pasic@linux.ibm.com, sebott@linux.ibm.com, oberpar@linux.ibm.com,
heiko.carstens@de.ibm.com, gor@linux.ibm.com,
borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
freude@linux.ibm.com, lingshan.zhu@intel.com, Ido Shamay,
eperezma@redhat.com, lulu@redhat.com
In-Reply-To: <AM0PR05MB486657BB8E48F744D219CF9BD1850@AM0PR05MB4866.eurprd05.prod.outlook.com>
On 2019/9/24 上午5:02, Parav Pandit wrote:
> Hi Jason,
>
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, September 23, 2019 8:03 AM
>> To: kvm@vger.kernel.org; linux-s390@vger.kernel.org; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; intel-
>> gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
>> kwankhede@nvidia.com; alex.williamson@redhat.com; mst@redhat.com;
>> tiwei.bie@intel.com
>> Cc: virtualization@lists.linux-foundation.org; netdev@vger.kernel.org;
>> cohuck@redhat.com; maxime.coquelin@redhat.com;
>> cunming.liang@intel.com; zhihong.wang@intel.com;
>> rob.miller@broadcom.com; xiao.w.wang@intel.com;
>> haotian.wang@sifive.com; zhenyuw@linux.intel.com; zhi.a.wang@intel.com;
>> jani.nikula@linux.intel.com; joonas.lahtinen@linux.intel.com;
>> rodrigo.vivi@intel.com; airlied@linux.ie; daniel@ffwll.ch;
>> farman@linux.ibm.com; pasic@linux.ibm.com; sebott@linux.ibm.com;
>> oberpar@linux.ibm.com; heiko.carstens@de.ibm.com; gor@linux.ibm.com;
>> borntraeger@de.ibm.com; akrowiak@linux.ibm.com; freude@linux.ibm.com;
>> lingshan.zhu@intel.com; Ido Shamay <idos@mellanox.com>;
>> eperezma@redhat.com; lulu@redhat.com; Parav Pandit
>> <parav@mellanox.com>; Jason Wang <jasowang@redhat.com>
>> Subject: [PATCH 1/6] mdev: class id support
>>
>> Mdev bus only supports vfio driver right now, so it doesn't implement match
>> method. But in the future, we may add drivers other than vfio, one example is
>> virtio-mdev[1] driver. This means we need to add device class id support in bus
>> match method to pair the mdev device and mdev driver correctly.
>>
>> So this patch adds id_table to mdev_driver and class_id for mdev parent with
>> the match method for mdev bus.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Documentation/driver-api/vfio-mediated-device.rst | 7 +++++--
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
>> drivers/s390/cio/vfio_ccw_ops.c | 2 +-
>> drivers/s390/crypto/vfio_ap_ops.c | 3 ++-
>> drivers/vfio/mdev/mdev_core.c | 14 ++++++++++++--
>> drivers/vfio/mdev/mdev_driver.c | 14 ++++++++++++++
>> drivers/vfio/mdev/mdev_private.h | 1 +
>> drivers/vfio/mdev/vfio_mdev.c | 6 ++++++
>> include/linux/mdev.h | 7 ++++++-
>> include/linux/mod_devicetable.h | 8 ++++++++
>> samples/vfio-mdev/mbochs.c | 2 +-
>> samples/vfio-mdev/mdpy.c | 2 +-
>> samples/vfio-mdev/mtty.c | 2 +-
>> 13 files changed, 59 insertions(+), 11 deletions(-)
>>
> You additionally need modpost support for id table integration to modifo, modprobe and other tools.
> A small patch similar to this one [1] is needed.
> Please include in the series.
>
> [1] https://lore.kernel.org/patchwork/patch/1046991/
My understanding is this could be done on top and may require uevent
support for the bus. I can try to implement this in V2.
Thanks
>
>
^ permalink raw reply
* Re: [PATCH 2/6] mdev: introduce device specific ops
From: Jason Wang @ 2019-09-24 11:23 UTC (permalink / raw)
To: Michael S. Tsirkin, kbuild test robot
Cc: kbuild-all, kvm, linux-s390, linux-kernel, dri-devel, intel-gfx,
intel-gvt-dev, kwankhede, alex.williamson, tiwei.bie, sebott,
airlied, heiko.carstens, virtualization, rob.miller, lulu,
eperezma, pasic, borntraeger, haotian.wang, zhi.a.wang, farman,
idos, gor, cunming.liang, rodrigo.vivi, xiao.w.wang, freude,
parav, zhihong.wang, akrowiak, netdev, cohuck, oberpar,
maxime.coquelin, lingshan.zhu
In-Reply-To: <20190923113538-mutt-send-email-mst@kernel.org>
On 2019/9/23 下午11:36, Michael S. Tsirkin wrote:
> On Mon, Sep 23, 2019 at 11:20:12PM +0800, kbuild test robot wrote:
>> Hi Jason,
>>
>> I love your patch! Yet something to improve:
>>
>> [auto build test ERROR on linus/master]
>> [cannot apply to v5.3 next-20190920]
>> [if your patch is applied to the wrong git tree, please drop us a note to help
>> improve the system. BTW, we also suggest to use '--base' option to specify the
>> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>>
>> url: https://github.com/0day-ci/linux/commits/Jason-Wang/mdev-based-hardware-virtio-offloading-support/20190923-210738
>> config: ia64-allmodconfig (attached as .config)
>> compiler: ia64-linux-gcc (GCC) 7.4.0
>> reproduce:
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # save the attached .config to linux build tree
>> GCC_VERSION=7.4.0 make.cross ARCH=ia64
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <lkp@intel.com>
>>
>> All error/warnings (new ones prefixed by >>):
> Looks like a bunch of includes missing. I guess they happen to
> be pulled in on some platforms but not others - ideally
> headers should be self contained though, pulling in
> all their dependencies.
Right, will fix in V2.
Thanks
>
>
>> In file included from include/linux/vfio_mdev.h:10:0,
>> from <command-line>:0:
>>>> include/linux/mdev.h:25:34: warning: 'struct device' declared inside parameter list will not be visible outside of this definition or declaration
>> int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);
>> ^~~~~~
>>>> include/linux/mdev.h:62:27: warning: 'struct kobject' declared inside parameter list will not be visible outside of this definition or declaration
>> int (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> ^~~~~~~
>>>> include/linux/mdev.h:69:19: error: field 'attr' has incomplete type
>> struct attribute attr;
>> ^~~~
>> include/linux/mdev.h:70:25: warning: 'struct kobject' declared inside parameter list will not be visible outside of this definition or declaration
>> ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf);
>> ^~~~~~~
>> include/linux/mdev.h:71:26: warning: 'struct kobject' declared inside parameter list will not be visible outside of this definition or declaration
>> ssize_t (*store)(struct kobject *kobj, struct device *dev,
>> ^~~~~~~
>>>> include/linux/mdev.h:98:23: error: field 'driver' has incomplete type
>> struct device_driver driver;
>> ^~~~~~
>>>> include/linux/mdev.h:106:7: error: unknown type name 'guid_t'
>> const guid_t *mdev_uuid(struct mdev_device *mdev);
>> ^~~~~~
>> In file included from <command-line>:0:0:
>>>> include/linux/vfio_mdev.h:50:47: warning: 'struct vm_area_struct' declared inside parameter list will not be visible outside of this definition or declaration
>> int (*mmap)(struct mdev_device *mdev, struct vm_area_struct *vma);
>> ^~~~~~~~~~~~~~
>>
>> vim +/attr +69 include/linux/mdev.h
>>
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 14
>> 8ac13175cbe985 Lu Baolu 2019-04-12 15 /*
>> 8ac13175cbe985 Lu Baolu 2019-04-12 16 * Called by the parent device driver to set the device which represents
>> 8ac13175cbe985 Lu Baolu 2019-04-12 17 * this mdev in iommu protection scope. By default, the iommu device is
>> 8ac13175cbe985 Lu Baolu 2019-04-12 18 * NULL, that indicates using vendor defined isolation.
>> 8ac13175cbe985 Lu Baolu 2019-04-12 19 *
>> 8ac13175cbe985 Lu Baolu 2019-04-12 20 * @dev: the mediated device that iommu will isolate.
>> 8ac13175cbe985 Lu Baolu 2019-04-12 21 * @iommu_device: a pci device which represents the iommu for @dev.
>> 8ac13175cbe985 Lu Baolu 2019-04-12 22 *
>> 8ac13175cbe985 Lu Baolu 2019-04-12 23 * Return 0 for success, otherwise negative error value.
>> 8ac13175cbe985 Lu Baolu 2019-04-12 24 */
>> 8ac13175cbe985 Lu Baolu 2019-04-12 @25 int mdev_set_iommu_device(struct device *dev, struct device *iommu_device);
>> 8ac13175cbe985 Lu Baolu 2019-04-12 26
>> 8ac13175cbe985 Lu Baolu 2019-04-12 27 struct device *mdev_get_iommu_device(struct device *dev);
>> 8ac13175cbe985 Lu Baolu 2019-04-12 28
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 29 /**
>> 42930553a7c11f Alex Williamson 2016-12-30 30 * struct mdev_parent_ops - Structure to be registered for each parent device to
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 31 * register the device to mdev module.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 32 *
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 33 * @owner: The module owner.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 34 * @dev_attr_groups: Attributes of the parent device.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 35 * @mdev_attr_groups: Attributes of the mediated device.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 36 * @supported_type_groups: Attributes to define supported types. It is mandatory
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 37 * to provide supported types.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 38 * @create: Called to allocate basic resources in parent device's
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 39 * driver for a particular mediated device. It is
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 40 * mandatory to provide create ops.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 41 * @kobj: kobject of type for which 'create' is called.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 42 * @mdev: mdev_device structure on of mediated device
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 43 * that is being created
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 44 * Returns integer: success (0) or error (< 0)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 45 * @remove: Called to free resources in parent device's driver for a
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 46 * a mediated device. It is mandatory to provide 'remove'
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 47 * ops.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 48 * @mdev: mdev_device device structure which is being
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 49 * destroyed
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 50 * Returns integer: success (0) or error (< 0)
>> 0baad8a6f6fefa Jason Wang 2019-09-23 51 * @device_ops: Device specific emulation callback.
>> 0baad8a6f6fefa Jason Wang 2019-09-23 52 *
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 53 * Parent device that support mediated device should be registered with mdev
>> 42930553a7c11f Alex Williamson 2016-12-30 54 * module with mdev_parent_ops structure.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 55 **/
>> 42930553a7c11f Alex Williamson 2016-12-30 56 struct mdev_parent_ops {
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 57 struct module *owner;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 58 const struct attribute_group **dev_attr_groups;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 59 const struct attribute_group **mdev_attr_groups;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 60 struct attribute_group **supported_type_groups;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 61
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 @62 int (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 63 int (*remove)(struct mdev_device *mdev);
>> 0baad8a6f6fefa Jason Wang 2019-09-23 64 const void *device_ops;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 65 };
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 66
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 67 /* interface for exporting mdev supported type attributes */
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 68 struct mdev_type_attribute {
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 @69 struct attribute attr;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 @70 ssize_t (*show)(struct kobject *kobj, struct device *dev, char *buf);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 71 ssize_t (*store)(struct kobject *kobj, struct device *dev,
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 72 const char *buf, size_t count);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 73 };
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 74
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 75 #define MDEV_TYPE_ATTR(_name, _mode, _show, _store) \
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 76 struct mdev_type_attribute mdev_type_attr_##_name = \
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 77 __ATTR(_name, _mode, _show, _store)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 78 #define MDEV_TYPE_ATTR_RW(_name) \
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 79 struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_RW(_name)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 80 #define MDEV_TYPE_ATTR_RO(_name) \
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 81 struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_RO(_name)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 82 #define MDEV_TYPE_ATTR_WO(_name) \
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 83 struct mdev_type_attribute mdev_type_attr_##_name = __ATTR_WO(_name)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 84
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 85 /**
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 86 * struct mdev_driver - Mediated device driver
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 87 * @name: driver name
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 88 * @probe: called when new device created
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 89 * @remove: called when device removed
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 90 * @driver: device driver structure
>> 6294ee8e0b5153 Jason Wang 2019-09-23 91 * @id_table: the ids serviced by this driver.
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 92 *
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 93 **/
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 94 struct mdev_driver {
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 95 const char *name;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 96 int (*probe)(struct device *dev);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 97 void (*remove)(struct device *dev);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 @98 struct device_driver driver;
>> 6294ee8e0b5153 Jason Wang 2019-09-23 99 const struct mdev_class_id *id_table;
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 100 };
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 101
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 102 #define to_mdev_driver(drv) container_of(drv, struct mdev_driver, driver)
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 103
>> 50732af3b65691 Parav Pandit 2019-04-30 104 void *mdev_get_drvdata(struct mdev_device *mdev);
>> 50732af3b65691 Parav Pandit 2019-04-30 105 void mdev_set_drvdata(struct mdev_device *mdev, void *data);
>> 50732af3b65691 Parav Pandit 2019-04-30 @106 const guid_t *mdev_uuid(struct mdev_device *mdev);
>> 7b96953bc640b6 Kirti Wankhede 2016-11-17 107
>>
>> :::::: The code at line 69 was first introduced by commit
>> :::::: 7b96953bc640b6b25665fe17ffca4b668b371f14 vfio: Mediated device Core driver
>>
>> :::::: TO: Kirti Wankhede <kwankhede@nvidia.com>
>> :::::: CC: Alex Williamson <alex.williamson@redhat.com>
>>
>> ---
>> 0-DAY kernel test infrastructure Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply
* Re: [PATCH 2/6] mdev: introduce device specific ops
From: Jason Wang @ 2019-09-24 11:26 UTC (permalink / raw)
To: Parav Pandit, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, tiwei.bie@intel.com
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
cohuck@redhat.com, maxime.coquelin@redhat.com,
cunming.liang@intel.com, zhihong.wang@intel.com,
rob.miller@broadcom.com, xiao.w.wang@intel.com,
haotian.wang@sifive.com, zhenyuw@linux.intel.com,
zhi.a.wang@intel.com, jani.nikula@linux.intel.com,
joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
pasic@linux.ibm.com, sebott@linux.ibm.com, oberpar@linux.ibm.com,
heiko.carstens@de.ibm.com, gor@linux.ibm.com,
borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
freude@linux.ibm.com, lingshan.zhu@intel.com, Ido Shamay,
eperezma@redhat.com, lulu@redhat.com
In-Reply-To: <AM0PR05MB4866D870687C7EA7190A91B2D1850@AM0PR05MB4866.eurprd05.prod.outlook.com>
On 2019/9/24 上午6:59, Parav Pandit wrote:
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, September 23, 2019 8:03 AM
>> To: kvm@vger.kernel.org; linux-s390@vger.kernel.org; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; intel-
>> gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
>> kwankhede@nvidia.com; alex.williamson@redhat.com; mst@redhat.com;
>> tiwei.bie@intel.com
>> Cc: virtualization@lists.linux-foundation.org; netdev@vger.kernel.org;
>> cohuck@redhat.com; maxime.coquelin@redhat.com;
>> cunming.liang@intel.com; zhihong.wang@intel.com;
>> rob.miller@broadcom.com; xiao.w.wang@intel.com;
>> haotian.wang@sifive.com; zhenyuw@linux.intel.com; zhi.a.wang@intel.com;
>> jani.nikula@linux.intel.com; joonas.lahtinen@linux.intel.com;
>> rodrigo.vivi@intel.com; airlied@linux.ie; daniel@ffwll.ch;
>> farman@linux.ibm.com; pasic@linux.ibm.com; sebott@linux.ibm.com;
>> oberpar@linux.ibm.com; heiko.carstens@de.ibm.com; gor@linux.ibm.com;
>> borntraeger@de.ibm.com; akrowiak@linux.ibm.com; freude@linux.ibm.com;
>> lingshan.zhu@intel.com; Ido Shamay <idos@mellanox.com>;
>> eperezma@redhat.com; lulu@redhat.com; Parav Pandit
>> <parav@mellanox.com>; Jason Wang <jasowang@redhat.com>
>> Subject: [PATCH 2/6] mdev: introduce device specific ops
>>
>> Currently, except for the create and remove. The rest of mdev_parent_ops is
>> designed for vfio-mdev driver only and may not help for kernel mdev driver.
>> Follow the class id support by previous patch, this patch introduces device
>> specific ops pointer inside parent ops which points to device specific ops (e.g
>> vfio ops). This allows the future drivers like virtio-mdev to implement its own
>> device specific ops.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> .../driver-api/vfio-mediated-device.rst | 4 +-
>> MAINTAINERS | 1 +
>> drivers/gpu/drm/i915/gvt/kvmgt.c | 15 +++---
>> drivers/s390/cio/vfio_ccw_ops.c | 15 ++++--
>> drivers/s390/crypto/vfio_ap_ops.c | 11 ++--
>> drivers/vfio/mdev/vfio_mdev.c | 31 ++++++-----
>> include/linux/mdev.h | 36 ++-----------
>> include/linux/vfio_mdev.h | 53 +++++++++++++++++++
>> samples/vfio-mdev/mbochs.c | 17 +++---
>> samples/vfio-mdev/mdpy.c | 17 +++---
>> samples/vfio-mdev/mtty.c | 15 ++++--
>> 11 files changed, 138 insertions(+), 77 deletions(-) create mode 100644
>> include/linux/vfio_mdev.h
>>
>> diff --git a/Documentation/driver-api/vfio-mediated-device.rst
>> b/Documentation/driver-api/vfio-mediated-device.rst
>> index 0e052072e1d8..3ab00e48212f 100644
>> --- a/Documentation/driver-api/vfio-mediated-device.rst
>> +++ b/Documentation/driver-api/vfio-mediated-device.rst
>> @@ -152,7 +152,9 @@ callbacks per mdev parent device, per mdev type, or
>> any other categorization.
>> Vendor drivers are expected to be fully asynchronous in this respect or
>> provide their own internal resource protection.)
>>
>> -The callbacks in the mdev_parent_ops structure are as follows:
>> +The device specific callbacks are referred through device_ops pointer
>> +in mdev_parent_ops. For vfio-mdev device, its callbacks in device_ops
>> +are as follows:
>>
>> * open: open callback of mediated device
>> * close: close callback of mediated device diff --git a/MAINTAINERS
>> b/MAINTAINERS index b2326dece28e..89832b316500 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17075,6 +17075,7 @@ S: Maintained
>> F: Documentation/driver-api/vfio-mediated-device.rst
>> F: drivers/vfio/mdev/
>> F: include/linux/mdev.h
>> +F: include/linux/vfio_mdev.h
>> F: samples/vfio-mdev/
>>
>> VFIO PLATFORM DRIVER
>> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> index 19d51a35f019..8ea86b1e69f1 100644
>> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
>> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
>> @@ -42,6 +42,7 @@
>> #include <linux/kvm_host.h>
>> #include <linux/vfio.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>> #include <linux/debugfs.h>
>>
>> #include <linux/nospec.h>
>> @@ -1600,20 +1601,22 @@ static const struct attribute_group
>> *intel_vgpu_groups[] = {
>> NULL,
>> };
>>
>> -static struct mdev_parent_ops intel_vgpu_ops = {
>> - .mdev_attr_groups = intel_vgpu_groups,
>> - .create = intel_vgpu_create,
>> - .remove = intel_vgpu_remove,
>> -
>> +static struct vfio_mdev_parent_ops intel_vfio_vgpu_ops = {
> Naming it with _dev prefix as intel_vfio_vgpu_dev_ops is better to differentiate with parent_ops.
Ok.
>> .open = intel_vgpu_open,
>> .release = intel_vgpu_release,
>> -
>> .read = intel_vgpu_read,
>> .write = intel_vgpu_write,
>> .mmap = intel_vgpu_mmap,
>> .ioctl = intel_vgpu_ioctl,
>> };
>>
>> +static struct mdev_parent_ops intel_vgpu_ops = {
>> + .mdev_attr_groups = intel_vgpu_groups,
>> + .create = intel_vgpu_create,
>> + .remove = intel_vgpu_remove,
>> + .device_ops = &intel_vfio_vgpu_ops,
>> +};
>> +
>> static int kvmgt_host_init(struct device *dev, void *gvt, const void *ops) {
>> struct attribute **kvm_type_attrs;
>> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
>> index 246ff0f80944..02122bbc213e 100644
>> --- a/drivers/s390/cio/vfio_ccw_ops.c
>> +++ b/drivers/s390/cio/vfio_ccw_ops.c
>> @@ -12,6 +12,7 @@
>>
>> #include <linux/vfio.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>> #include <linux/nospec.h>
>> #include <linux/slab.h>
>>
>> @@ -574,11 +575,7 @@ static ssize_t vfio_ccw_mdev_ioctl(struct
>> mdev_device *mdev,
>> }
>> }
>>
>> -static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
>> - .owner = THIS_MODULE,
>> - .supported_type_groups = mdev_type_groups,
>> - .create = vfio_ccw_mdev_create,
>> - .remove = vfio_ccw_mdev_remove,
>> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>> .open = vfio_ccw_mdev_open,
>> .release = vfio_ccw_mdev_release,
>> .read = vfio_ccw_mdev_read,
>> @@ -586,6 +583,14 @@ static const struct mdev_parent_ops
>> vfio_ccw_mdev_ops = {
>> .ioctl = vfio_ccw_mdev_ioctl,
>> };
>>
>> +static const struct mdev_parent_ops vfio_ccw_mdev_ops = {
>> + .owner = THIS_MODULE,
>> + .supported_type_groups = mdev_type_groups,
>> + .create = vfio_ccw_mdev_create,
>> + .remove = vfio_ccw_mdev_remove,
>> + .device_ops = &vfio_mdev_ops,
>> +};
>> +
>> int vfio_ccw_mdev_reg(struct subchannel *sch) {
>> return mdev_register_vfio_device(&sch->dev, &vfio_ccw_mdev_ops);
>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
>> b/drivers/s390/crypto/vfio_ap_ops.c
>> index 7487fc39d2c5..4251becc7a6d 100644
>> --- a/drivers/s390/crypto/vfio_ap_ops.c
>> +++ b/drivers/s390/crypto/vfio_ap_ops.c
>> @@ -16,6 +16,7 @@
>> #include <linux/bitops.h>
>> #include <linux/kvm_host.h>
>> #include <linux/module.h>
>> +#include <linux/vfio_mdev.h>
>> #include <asm/kvm.h>
>> #include <asm/zcrypt.h>
>>
>> @@ -1280,15 +1281,19 @@ static ssize_t vfio_ap_mdev_ioctl(struct
>> mdev_device *mdev,
>> return ret;
>> }
>>
>> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>> + .open = vfio_ap_mdev_open,
>> + .release = vfio_ap_mdev_release,
>> + .ioctl = vfio_ap_mdev_ioctl,
>> +};
>> +
>> static const struct mdev_parent_ops vfio_ap_matrix_ops = {
>> .owner = THIS_MODULE,
>> .supported_type_groups = vfio_ap_mdev_type_groups,
>> .mdev_attr_groups = vfio_ap_mdev_attr_groups,
>> .create = vfio_ap_mdev_create,
>> .remove = vfio_ap_mdev_remove,
>> - .open = vfio_ap_mdev_open,
>> - .release = vfio_ap_mdev_release,
>> - .ioctl = vfio_ap_mdev_ioctl,
>> + .device_ops = &vfio_mdev_ops,
>> };
>>
>> int vfio_ap_mdev_register(void)
>> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
>> index fd2a4d9a3f26..d23c9f58c84f 100644
>> --- a/drivers/vfio/mdev/vfio_mdev.c
>> +++ b/drivers/vfio/mdev/vfio_mdev.c
>> @@ -14,6 +14,7 @@
>> #include <linux/slab.h>
>> #include <linux/vfio.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>>
>> #include "mdev_private.h"
>>
>> @@ -25,15 +26,16 @@ static int vfio_mdev_open(void *device_data) {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>> int ret;
>>
>> - if (unlikely(!parent->ops->open))
>> + if (unlikely(!ops->open))
>> return -EINVAL;
>>
> device_ops is optional and can be NULL for mdev devices which are not required to be mapped via vfio.
> So please change to,
>
> If (!ops || !ops->open)
> return -EINVAL;
>
> and rest of the below places.
My understanding is vfio_mdev can not talk to non vfio mdev device with
the help of class id.
Thanks
>
>> if (!try_module_get(THIS_MODULE))
>> return -ENODEV;
>>
>> - ret = parent->ops->open(mdev);
>> + ret = ops->open(mdev);
>> if (ret)
>> module_put(THIS_MODULE);
>>
>> @@ -44,9 +46,10 @@ static void vfio_mdev_release(void *device_data) {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>>
>> - if (likely(parent->ops->release))
>> - parent->ops->release(mdev);
>> + if (likely(ops->release))
>> + ops->release(mdev);
>>
>> module_put(THIS_MODULE);
>> }
>> @@ -56,11 +59,12 @@ static long vfio_mdev_unlocked_ioctl(void
>> *device_data, {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>>
>> - if (unlikely(!parent->ops->ioctl))
>> + if (unlikely(!ops->ioctl))
>> return -EINVAL;
>>
>> - return parent->ops->ioctl(mdev, cmd, arg);
>> + return ops->ioctl(mdev, cmd, arg);
>> }
>>
>> static ssize_t vfio_mdev_read(void *device_data, char __user *buf, @@ -68,11
>> +72,12 @@ static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
>> {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>>
>> - if (unlikely(!parent->ops->read))
>> + if (unlikely(!ops->read))
>> return -EINVAL;
>>
>> - return parent->ops->read(mdev, buf, count, ppos);
>> + return ops->read(mdev, buf, count, ppos);
>> }
>>
>> static ssize_t vfio_mdev_write(void *device_data, const char __user *buf, @@
>> -80,22 +85,24 @@ static ssize_t vfio_mdev_write(void *device_data, const char
>> __user *buf, {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>>
>> - if (unlikely(!parent->ops->write))
>> + if (unlikely(!ops->write))
>> return -EINVAL;
>>
>> - return parent->ops->write(mdev, buf, count, ppos);
>> + return ops->write(mdev, buf, count, ppos);
>> }
>>
>> static int vfio_mdev_mmap(void *device_data, struct vm_area_struct *vma) {
>> struct mdev_device *mdev = device_data;
>> struct mdev_parent *parent = mdev->parent;
>> + const struct vfio_mdev_parent_ops *ops = parent->ops->device_ops;
>>
>> - if (unlikely(!parent->ops->mmap))
>> + if (unlikely(!ops->mmap))
>> return -EINVAL;
>>
>> - return parent->ops->mmap(mdev, vma);
>> + return ops->mmap(mdev, vma);
>> }
>>
>> static const struct vfio_device_ops vfio_mdev_dev_ops = { diff --git
>> a/include/linux/mdev.h b/include/linux/mdev.h index
>> 3ebae310f599..fa167bcb81e1 100644
>> --- a/include/linux/mdev.h
>> +++ b/include/linux/mdev.h
>> @@ -48,30 +48,8 @@ struct device *mdev_get_iommu_device(struct device
>> *dev);
>> * @mdev: mdev_device device structure which is being
>> * destroyed
>> * Returns integer: success (0) or error (< 0)
>> - * @open: Open mediated device.
>> - * @mdev: mediated device.
>> - * Returns integer: success (0) or error (< 0)
>> - * @release: release mediated device
>> - * @mdev: mediated device.
>> - * @read: Read emulation callback
>> - * @mdev: mediated device structure
>> - * @buf: read buffer
>> - * @count: number of bytes to read
>> - * @ppos: address.
>> - * Retuns number on bytes read on success or error.
>> - * @write: Write emulation callback
>> - * @mdev: mediated device structure
>> - * @buf: write buffer
>> - * @count: number of bytes to be written
>> - * @ppos: address.
>> - * Retuns number on bytes written on success or error.
>> - * @ioctl: IOCTL callback
>> - * @mdev: mediated device structure
>> - * @cmd: ioctl command
>> - * @arg: arguments to ioctl
>> - * @mmap: mmap callback
>> - * @mdev: mediated device structure
>> - * @vma: vma structure
>> + * @device_ops: Device specific emulation callback.
>> + *
>> * Parent device that support mediated device should be registered with mdev
>> * module with mdev_parent_ops structure.
>> **/
>> @@ -83,15 +61,7 @@ struct mdev_parent_ops {
>>
>> int (*create)(struct kobject *kobj, struct mdev_device *mdev);
>> int (*remove)(struct mdev_device *mdev);
>> - int (*open)(struct mdev_device *mdev);
>> - void (*release)(struct mdev_device *mdev);
>> - ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
>> - size_t count, loff_t *ppos);
>> - ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
>> - size_t count, loff_t *ppos);
>> - long (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>> - unsigned long arg);
>> - int (*mmap)(struct mdev_device *mdev, struct vm_area_struct
>> *vma);
>> + const void *device_ops;
>> };
>>
>> /* interface for exporting mdev supported type attributes */ diff --git
>> a/include/linux/vfio_mdev.h b/include/linux/vfio_mdev.h new file mode
>> 100644 index 000000000000..0c1b34f98f5d
>> --- /dev/null
>> +++ b/include/linux/vfio_mdev.h
>> @@ -0,0 +1,53 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * VFIO Mediated device definition
>> + */
>> +
>> +#ifndef VFIO_MDEV_H
>> +#define VFIO_MDEV_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/mdev.h>
>> +
>> +/**
>> + * struct vfio_mdev_parent_ops - Structure to be registered for each
>> + * parent device to register the device to vfio-mdev module.
>> + *
>> + * @open: Open mediated device.
>> + * @mdev: mediated device.
>> + * Returns integer: success (0) or error (< 0)
>> + * @release: release mediated device
>> + * @mdev: mediated device.
>> + * @read: Read emulation callback
>> + * @mdev: mediated device structure
>> + * @buf: read buffer
>> + * @count: number of bytes to read
>> + * @ppos: address.
>> + * Retuns number on bytes read on success or error.
>> + * @write: Write emulation callback
>> + * @mdev: mediated device structure
>> + * @buf: write buffer
>> + * @count: number of bytes to be written
>> + * @ppos: address.
>> + * Retuns number on bytes written on success or error.
>> + * @ioctl: IOCTL callback
>> + * @mdev: mediated device structure
>> + * @cmd: ioctl command
>> + * @arg: arguments to ioctl
>> + * @mmap: mmap callback
>> + * @mdev: mediated device structure
>> + * @vma: vma structure
>> + */
>> +struct vfio_mdev_parent_ops {
>> + int (*open)(struct mdev_device *mdev);
>> + void (*release)(struct mdev_device *mdev);
>> + ssize_t (*read)(struct mdev_device *mdev, char __user *buf,
>> + size_t count, loff_t *ppos);
>> + ssize_t (*write)(struct mdev_device *mdev, const char __user *buf,
>> + size_t count, loff_t *ppos);
>> + long (*ioctl)(struct mdev_device *mdev, unsigned int cmd,
>> + unsigned long arg);
>> + int (*mmap)(struct mdev_device *mdev, struct vm_area_struct
>> *vma);
>> +};
>> +
>> +#endif
>> diff --git a/samples/vfio-mdev/mbochs.c b/samples/vfio-mdev/mbochs.c index
>> 71a4469be85d..107cc30d0f45 100644
>> --- a/samples/vfio-mdev/mbochs.c
>> +++ b/samples/vfio-mdev/mbochs.c
>> @@ -30,6 +30,7 @@
>> #include <linux/iommu.h>
>> #include <linux/sysfs.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>> #include <linux/pci.h>
>> #include <linux/dma-buf.h>
>> #include <linux/highmem.h>
>> @@ -1418,12 +1419,7 @@ static struct attribute_group *mdev_type_groups[]
>> = {
>> NULL,
>> };
>>
>> -static const struct mdev_parent_ops mdev_fops = {
>> - .owner = THIS_MODULE,
>> - .mdev_attr_groups = mdev_dev_groups,
>> - .supported_type_groups = mdev_type_groups,
>> - .create = mbochs_create,
>> - .remove = mbochs_remove,
>> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>> .open = mbochs_open,
>> .release = mbochs_close,
>> .read = mbochs_read,
>> @@ -1432,6 +1428,15 @@ static const struct mdev_parent_ops mdev_fops = {
>> .mmap = mbochs_mmap,
>> };
>>
>> +static const struct mdev_parent_ops mdev_fops = {
>> + .owner = THIS_MODULE,
>> + .mdev_attr_groups = mdev_dev_groups,
>> + .supported_type_groups = mdev_type_groups,
>> + .create = mbochs_create,
>> + .remove = mbochs_remove,
>> + .device_ops = &vfio_mdev_ops,
>> +};
>> +
>> static const struct file_operations vd_fops = {
>> .owner = THIS_MODULE,
>> };
>> diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c index
>> d3029dd27d91..2cd2018a53f9 100644
>> --- a/samples/vfio-mdev/mdpy.c
>> +++ b/samples/vfio-mdev/mdpy.c
>> @@ -26,6 +26,7 @@
>> #include <linux/iommu.h>
>> #include <linux/sysfs.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>> #include <linux/pci.h>
>> #include <drm/drm_fourcc.h>
>> #include "mdpy-defs.h"
>> @@ -725,12 +726,7 @@ static struct attribute_group *mdev_type_groups[] = {
>> NULL,
>> };
>>
>> -static const struct mdev_parent_ops mdev_fops = {
>> - .owner = THIS_MODULE,
>> - .mdev_attr_groups = mdev_dev_groups,
>> - .supported_type_groups = mdev_type_groups,
>> - .create = mdpy_create,
>> - .remove = mdpy_remove,
>> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>> .open = mdpy_open,
>> .release = mdpy_close,
>> .read = mdpy_read,
>> @@ -739,6 +735,15 @@ static const struct mdev_parent_ops mdev_fops = {
>> .mmap = mdpy_mmap,
>> };
>>
>> +static const struct mdev_parent_ops mdev_fops = {
>> + .owner = THIS_MODULE,
>> + .mdev_attr_groups = mdev_dev_groups,
>> + .supported_type_groups = mdev_type_groups,
>> + .create = mdpy_create,
>> + .remove = mdpy_remove,
>> + .device_ops = &vfio_mdev_ops,
>> +};
>> +
>> static const struct file_operations vd_fops = {
>> .owner = THIS_MODULE,
>> };
>> diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index
>> 744c88a6b22c..e427425b5daf 100644
>> --- a/samples/vfio-mdev/mtty.c
>> +++ b/samples/vfio-mdev/mtty.c
>> @@ -27,6 +27,7 @@
>> #include <linux/ctype.h>
>> #include <linux/file.h>
>> #include <linux/mdev.h>
>> +#include <linux/vfio_mdev.h>
>> #include <linux/pci.h>
>> #include <linux/serial.h>
>> #include <uapi/linux/serial_reg.h>
>> @@ -1410,6 +1411,14 @@ static struct attribute_group *mdev_type_groups[]
>> = {
>> NULL,
>> };
>>
>> +static const struct vfio_mdev_parent_ops vfio_mdev_ops = {
>> + .open = mtty_open,
>> + .release = mtty_close,
>> + .read = mtty_read,
>> + .write = mtty_write,
>> + .ioctl = mtty_ioctl,
>> +};
>> +
>> static const struct mdev_parent_ops mdev_fops = {
>> .owner = THIS_MODULE,
>> .dev_attr_groups = mtty_dev_groups,
>> @@ -1417,11 +1426,7 @@ static const struct mdev_parent_ops mdev_fops = {
>> .supported_type_groups = mdev_type_groups,
>> .create = mtty_create,
>> .remove = mtty_remove,
>> - .open = mtty_open,
>> - .release = mtty_close,
>> - .read = mtty_read,
>> - .write = mtty_write,
>> - .ioctl = mtty_ioctl,
>> + .device_ops = &vfio_mdev_ops,
>> };
>>
>> static void mtty_device_release(struct device *dev)
>> --
>> 2.19.1
^ permalink raw reply
* [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym_pause
From: Michal Vokáč @ 2019-09-24 11:27 UTC (permalink / raw)
To: netdev@vger.kernel.org, Andrew Lunn, Vivien Didelot,
Florian Fainelli
Cc: David S. Miller
Hi,
just tried booting latest next-20190920 on our imx6dl-yapp4-hydra platform
with QCA8334 switch and got this:
[ 6.957822] 8<--- cut here ---
[ 6.963550] Unable to handle kernel NULL pointer dereference at virtual address 00000264
[ 6.974342] pgd = (ptrval)
[ 6.979751] [00000264] *pgd=00000000
[ 6.986005] Internal error: Oops: 5 [#1] SMP ARM
[ 6.993318] CPU: 0 PID: 21 Comm: kworker/0:1 Not tainted 5.3.0-rc5-00985-g0394a63acfe2 #7
[ 7.004196] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[ 7.013467] Workqueue: events deferred_probe_work_func
[ 7.021339] PC is at phy_support_asym_pause+0x14/0x44
[ 7.029149] LR is at qca8k_port_enable+0x40/0x48
[ 7.036485] pc : [<806840f4>] lr : [<80686724>] psr: 60000013
[ 7.045489] sp : e821bca0 ip : e821bcb0 fp : e821bcac
[ 7.053456] r10: 00000000 r9 : e8241f00 r8 : e812d040
[ 7.061431] r7 : 00000000 r6 : 00000000 r5 : e8931640 r4 : 00000002
[ 7.070702] r3 : 00000001 r2 : 00000000 r1 : 00000000 r0 : 00000000
[ 7.079947] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 7.089825] Control: 10c5387d Table: 1000404a DAC: 00000051
[ 7.098382] Process kworker/0:1 (pid: 21, stack limit = 0x(ptrval))
[ 7.107431] Stack: (0xe821bca0 to 0xe821c000)
[ 7.114607] bca0: e821bccc e821bcb0 80686724 806840ec e812d090 e812d090 e88a02cc 00000000
[ 7.125637] bcc0: e821bce4 e821bcd0 80a74134 806866f0 e812d090 e812d0cc e821bd7c e821bce8
[ 7.136666] bce0: 80a730bc 80a74104 00000000 e88a02cc 00000004 e821bd00 e88a02e0 80cc95f8
[ 7.147725] bd00: e88a02e0 e88a02c0 e812d090 e812d090 81006548 e88a02e4 e812d040 e88a02c0
[ 7.158791] bd20: 00000003 e812d040 e8830c00 00000000 e821bd5c e821bd40 806243b8 80623abc
[ 7.169887] bd40: e8931640 00000000 00000000 5303fd08 e821bd7c e8931640 e8830c00 e8931680
[ 7.181000] bd60: 00000000 81077adc 00000004 00000000 e821bd9c e821bd80 806864f8 80a72930
[ 7.192107] bd80: 81077adc e8830c00 81123b24 00000000 e821bdb4 e821bda0 806856c8 80686354
[ 7.203216] bda0: 81123a18 e8830c00 e821bde4 e821bdb8 8061f944 80685694 00000000 e8830c00
[ 7.214318] bdc0: 81077adc e8830c00 81006548 00000001 810c07f0 00000000 e821be1c e821bde8
[ 7.225434] bde0: 8061ff68 8061f850 e821be04 e821bdf8 81077adc e821be74 81077adc e821be74
[ 7.236589] be00: e8830c00 81006548 00000001 810c07f0 e821be3c e821be20 80620178 8061ff04
[ 7.247788] be20: 00000000 e821be74 806200dc 81006548 e821be6c e821be40 8061e160 806200e8
[ 7.258982] be40: e821be6c e836d96c e86acdb8 5303fd08 e8830c00 81006548 e8830c44 81071194
[ 7.270217] be60: e821bea4 e821be70 8061fd80 8061e104 8061b58c e8830c00 00000001 5303fd08
[ 7.281470] be80: 00000000 e8830c00 81077998 e8830c00 81071194 00000000 e821beb4 e821bea8
[ 7.292770] bea0: 80620220 8061fcac e821bed4 e821beb8 8061e360 80620210 e8830c00 81071180
[ 7.304105] bec0: 81071180 81071194 e821bef4 e821bed8 8061f144 8061e2d8 810711b8 e81f5080
[ 7.315494] bee0: eada5f40 eada7100 e821bf34 e821bef8 80142e2c 8061f0dc e8200d00 ffffe000
[ 7.326890] bf00: e821bf1c e821bf10 8014234c e81f5080 eada5f40 e81f5094 eada5f58 ffffe000
[ 7.338312] bf20: 00000008 81003d00 e821bf74 e821bf38 801433f4 80142c14 ffffe000 80e1264c
[ 7.349778] bf40: 810c00c7 eada5f40 80148afc e81f76c0 e81f7680 00000000 e821a000 e81f5080
[ 7.361246] bf60: 80143130 e8143e74 e821bfac e821bf78 80148e2c 8014313c e81f76dc e81f76dc
[ 7.372758] bf80: e821bfac e81f7680 80148cc4 00000000 00000000 00000000 00000000 00000000
[ 7.384313] bfa0: 00000000 e821bfb0 801010e8 80148cd0 00000000 00000000 00000000 00000000
[ 7.395858] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 7.407363] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 7.418856] Backtrace:
[ 7.424620] [<806840e0>] (phy_support_asym_pause) from [<80686724>] (qca8k_port_enable+0x40/0x48)
[ 7.436911] [<806866e4>] (qca8k_port_enable) from [<80a74134>] (dsa_port_enable+0x3c/0x6c)
[ 7.448629] r7:00000000 r6:e88a02cc r5:e812d090 r4:e812d090
[ 7.457708] [<80a740f8>] (dsa_port_enable) from [<80a730bc>] (dsa_register_switch+0x798/0xacc)
[ 7.469833] r5:e812d0cc r4:e812d090
[ 7.476909] [<80a72924>] (dsa_register_switch) from [<806864f8>] (qca8k_sw_probe+0x1b0/0x1c4)
[ 7.489008] r10:00000000 r9:00000004 r8:81077adc r7:00000000 r6:e8931680 r5:e8830c00
[ 7.500419] r4:e8931640
[ 7.506528] [<80686348>] (qca8k_sw_probe) from [<806856c8>] (mdio_probe+0x40/0x64)
[ 7.517774] r7:00000000 r6:81123b24 r5:e8830c00 r4:81077adc
[ 7.527048] [<80685688>] (mdio_probe) from [<8061f944>] (really_probe+0x100/0x2d8)
[ 7.538284] r5:e8830c00 r4:81123a18
[ 7.545528] [<8061f844>] (really_probe) from [<8061ff68>] (driver_probe_device+0x70/0x180)
[ 7.557539] r10:00000000 r9:810c07f0 r8:00000001 r7:81006548 r6:e8830c00 r5:81077adc
[ 7.569129] r4:e8830c00 r3:00000000
[ 7.576439] [<8061fef8>] (driver_probe_device) from [<80620178>] (__device_attach_driver+0x9c/0xc8)
[ 7.589338] r9:810c07f0 r8:00000001 r7:81006548 r6:e8830c00 r5:e821be74 r4:81077adc
[ 7.600949] [<806200dc>] (__device_attach_driver) from [<8061e160>] (bus_for_each_drv+0x68/0xc8)
[ 7.613617] r7:81006548 r6:806200dc r5:e821be74 r4:00000000
[ 7.623099] [<8061e0f8>] (bus_for_each_drv) from [<8061fd80>] (__device_attach+0xe0/0x14c)
[ 7.635235] r7:81071194 r6:e8830c44 r5:81006548 r4:e8830c00
[ 7.644681] [<8061fca0>] (__device_attach) from [<80620220>] (device_initial_probe+0x1c/0x20)
[ 7.657036] r8:00000000 r7:81071194 r6:e8830c00 r5:81077998 r4:e8830c00
[ 7.667551] [<80620204>] (device_initial_probe) from [<8061e360>] (bus_probe_device+0x94/0x9c)
[ 7.680054] [<8061e2cc>] (bus_probe_device) from [<8061f144>] (deferred_probe_work_func+0x74/0xa0)
[ 7.692914] r7:81071194 r6:81071180 r5:81071180 r4:e8830c00
[ 7.702440] [<8061f0d0>] (deferred_probe_work_func) from [<80142e2c>] (process_one_work+0x224/0x528)
[ 7.715483] r7:eada7100 r6:eada5f40 r5:e81f5080 r4:810711b8
[ 7.725046] [<80142c08>] (process_one_work) from [<801433f4>] (worker_thread+0x2c4/0x5e4)
[ 7.737175] r10:81003d00 r9:00000008 r8:ffffe000 r7:eada5f58 r6:e81f5094 r5:eada5f40
[ 7.748949] r4:e81f5080
[ 7.755402] [<80143130>] (worker_thread) from [<80148e2c>] (kthread+0x168/0x170)
[ 7.766823] r10:e8143e74 r9:80143130 r8:e81f5080 r7:e821a000 r6:00000000 r5:e81f7680
[ 7.778636] r4:e81f76c0
[ 7.785159] [<80148cc4>] (kthread) from [<801010e8>] (ret_from_fork+0x14/0x2c)
[ 7.796457] Exception stack(0xe821bfb0 to 0xe821bff8)
[ 7.805543] bfa0: 00000000 00000000 00000000 00000000
[ 7.817805] bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 7.830047] bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 7.840688] r10:00000000 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:80148cc4
[ 7.852535] r4:e81f7680
[ 7.859061] Code: e92dd800 e24cb004 e52de004 e8bd4000 (e5902264)
[ 7.869239] ---[ end trace 1b5559d3dc3f80a4 ]---
Bisecting the dsa code changes since v5.3 brought me to this commit:
0394a63acfe2a6e1c08af0eb1a9133ee8650d7bd is the first bad commit
commit 0394a63acfe2a6e1c08af0eb1a9133ee8650d7bd
Author: Vivien Didelot <vivien.didelot@gmail.com>
Date: Mon Aug 19 16:00:50 2019 -0400
net: dsa: enable and disable all ports
Call the .port_enable and .port_disable functions for all ports,
not only the user ports, so that drivers may optimize the power
consumption of all ports after a successful setup.
Unused ports are now disabled on setup. CPU and DSA ports are now
enabled on setup and disabled on teardown. User ports were already
enabled at slave creation and disabled at slave destruction.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
:040000 040000 0462b53f03ece23b4af955c3b8a48dce05d90970 34e5df083585e6cce35600698a757508b539e508 M net
Any ideas what might be wrong?
Thank you!
Michal
^ permalink raw reply
* Re: [PATCH 4/6] virtio: introduce a mdev based transport
From: Jason Wang @ 2019-09-24 11:30 UTC (permalink / raw)
To: Parav Pandit, kvm@vger.kernel.org, linux-s390@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org, kwankhede@nvidia.com,
alex.williamson@redhat.com, mst@redhat.com, tiwei.bie@intel.com
Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
cohuck@redhat.com, maxime.coquelin@redhat.com,
cunming.liang@intel.com, zhihong.wang@intel.com,
rob.miller@broadcom.com, xiao.w.wang@intel.com,
haotian.wang@sifive.com, zhenyuw@linux.intel.com,
zhi.a.wang@intel.com, jani.nikula@linux.intel.com,
joonas.lahtinen@linux.intel.com, rodrigo.vivi@intel.com,
airlied@linux.ie, daniel@ffwll.ch, farman@linux.ibm.com,
pasic@linux.ibm.com, sebott@linux.ibm.com, oberpar@linux.ibm.com,
heiko.carstens@de.ibm.com, gor@linux.ibm.com,
borntraeger@de.ibm.com, akrowiak@linux.ibm.com,
freude@linux.ibm.com, lingshan.zhu@intel.com, Ido Shamay,
eperezma@redhat.com, lulu@redhat.com
In-Reply-To: <AM0PR05MB4866178BECC3C96DA7046590D1850@AM0PR05MB4866.eurprd05.prod.outlook.com>
On 2019/9/24 上午6:28, Parav Pandit wrote:
>
>> -----Original Message-----
>> From: Jason Wang <jasowang@redhat.com>
>> Sent: Monday, September 23, 2019 8:03 AM
>> To: kvm@vger.kernel.org; linux-s390@vger.kernel.org; linux-
>> kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; intel-
>> gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org;
>> kwankhede@nvidia.com; alex.williamson@redhat.com; mst@redhat.com;
>> tiwei.bie@intel.com
>> Cc: virtualization@lists.linux-foundation.org; netdev@vger.kernel.org;
>> cohuck@redhat.com; maxime.coquelin@redhat.com;
>> cunming.liang@intel.com; zhihong.wang@intel.com;
>> rob.miller@broadcom.com; xiao.w.wang@intel.com;
>> haotian.wang@sifive.com; zhenyuw@linux.intel.com; zhi.a.wang@intel.com;
>> jani.nikula@linux.intel.com; joonas.lahtinen@linux.intel.com;
>> rodrigo.vivi@intel.com; airlied@linux.ie; daniel@ffwll.ch;
>> farman@linux.ibm.com; pasic@linux.ibm.com; sebott@linux.ibm.com;
>> oberpar@linux.ibm.com; heiko.carstens@de.ibm.com; gor@linux.ibm.com;
>> borntraeger@de.ibm.com; akrowiak@linux.ibm.com; freude@linux.ibm.com;
>> lingshan.zhu@intel.com; Ido Shamay <idos@mellanox.com>;
>> eperezma@redhat.com; lulu@redhat.com; Parav Pandit
>> <parav@mellanox.com>; Jason Wang <jasowang@redhat.com>
>> Subject: [PATCH 4/6] virtio: introduce a mdev based transport
>>
>> This patch introduces a new mdev transport for virtio. This is used to use kernel
>> virtio driver to drive the mediated device that is capable of populating
>> virtqueue directly.
>>
>> A new virtio-mdev driver will be registered to the mdev bus, when a new virtio-
>> mdev device is probed, it will register the device with mdev based config ops.
>> This means it is a software transport between mdev driver and mdev device.
>> The transport was implemented through device specific opswhich is a part of
>> mdev_parent_ops now.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> MAINTAINERS | 1 +
>> drivers/vfio/mdev/Kconfig | 7 +
>> drivers/vfio/mdev/Makefile | 1 +
>> drivers/vfio/mdev/virtio_mdev.c | 416 ++++++++++++++++++++++++++++++++
>> 4 files changed, 425 insertions(+)
>> create mode 100644 drivers/vfio/mdev/virtio_mdev.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 89832b316500..820ec250cc52 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -17202,6 +17202,7 @@ F: include/linux/virtio*.h
>> F: include/uapi/linux/virtio_*.h
>> F: drivers/crypto/virtio/
>> F: mm/balloon_compaction.c
>> +F: drivers/vfio/mdev/virtio_mdev.c
>>
>> VIRTIO BLOCK AND SCSI DRIVERS
>> M: "Michael S. Tsirkin" <mst@redhat.com>
>> diff --git a/drivers/vfio/mdev/Kconfig b/drivers/vfio/mdev/Kconfig index
>> 5da27f2100f9..c488c31fc137 100644
>> --- a/drivers/vfio/mdev/Kconfig
>> +++ b/drivers/vfio/mdev/Kconfig
>> @@ -16,3 +16,10 @@ config VFIO_MDEV_DEVICE
>> default n
>> help
>> VFIO based driver for Mediated devices.
>> +
>> +config VIRTIO_MDEV_DEVICE
>> + tristate "VIRTIO driver for Mediated devices"
>> + depends on VFIO_MDEV && VIRTIO
>> + default n
>> + help
>> + VIRTIO based driver for Mediated devices.
>> diff --git a/drivers/vfio/mdev/Makefile b/drivers/vfio/mdev/Makefile index
>> 101516fdf375..99d31e29c23e 100644
>> --- a/drivers/vfio/mdev/Makefile
>> +++ b/drivers/vfio/mdev/Makefile
>> @@ -4,3 +4,4 @@ mdev-y := mdev_core.o mdev_sysfs.o mdev_driver.o
>>
>> obj-$(CONFIG_VFIO_MDEV) += mdev.o
>> obj-$(CONFIG_VFIO_MDEV_DEVICE) += vfio_mdev.o
>> +obj-$(CONFIG_VIRTIO_MDEV_DEVICE) += virtio_mdev.o
>> diff --git a/drivers/vfio/mdev/virtio_mdev.c b/drivers/vfio/mdev/virtio_mdev.c
>> new file mode 100644 index 000000000000..919a082adc9c
>> --- /dev/null
>> +++ b/drivers/vfio/mdev/virtio_mdev.c
>> @@ -0,0 +1,416 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * VIRTIO based driver for Mediated device
>> + *
>> + * Copyright (c) 2019, Red Hat. All rights reserved.
>> + * Author: Jason Wang <jasowang@redhat.com>
>> + *
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/uuid.h>
>> +#include <linux/mdev.h>
>> +#include <linux/virtio_mdev.h>
>> +#include <linux/virtio.h>
>> +#include <linux/virtio_config.h>
>> +#include <linux/virtio_ring.h>
>> +#include "mdev_private.h"
>> +
>> +#define DRIVER_VERSION "0.1"
>> +#define DRIVER_AUTHOR "Red Hat Corporation"
>> +#define DRIVER_DESC "VIRTIO based driver for Mediated device"
>> +
>> +#define to_virtio_mdev_device(dev) \
>> + container_of(dev, struct virtio_mdev_device, vdev)
>> +
>> +struct virtio_mdev_device {
>> + struct virtio_device vdev;
>> + struct mdev_device *mdev;
>> + unsigned long version;
>> +
>> + struct virtqueue **vqs;
> Every lock must need a comment to describe what it locks.
> I don't see this lock is used in this patch. Please introduce in the patch that uses it.
Actually, it is used to sync the virtqueue list add and remove, but the
logic is missed in this patch.
Will fix it in next version.
>> + spinlock_t lock;
>> +};
>> +
>> +struct virtio_mdev_vq_info {
>> + /* the actual virtqueue */
>> + struct virtqueue *vq;
>> +
>> + /* the list node for the virtqueues list */
>> + struct list_head node;
>> +};
>> +
>> +static struct mdev_device *vm_get_mdev(struct virtio_device *vdev) {
>> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
>> + struct mdev_device *mdev = vm_dev->mdev;
>> +
>> + return mdev;
>> +}
>> +
>> +static const struct virtio_mdev_parent_ops *mdev_get_parent_ops(struct
>> +mdev_device *mdev) {
>> + struct mdev_parent *parent = mdev->parent;
>> +
>> + return parent->ops->device_ops;
>> +}
>> +
>> +static void virtio_mdev_get(struct virtio_device *vdev, unsigned offset,
>> + void *buf, unsigned len)
>> +{
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + ops->get_config(mdev, offset, buf, len); }
>> +
>> +static void virtio_mdev_set(struct virtio_device *vdev, unsigned offset,
>> + const void *buf, unsigned len)
>> +{
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + ops->set_config(mdev, offset, buf, len); }
>> +
>> +static u32 virtio_mdev_generation(struct virtio_device *vdev) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + return ops->get_generation(mdev);
>> +}
>> +
>> +static u8 virtio_mdev_get_status(struct virtio_device *vdev) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + return ops->get_status(mdev);
>> +}
>> +
>> +static void virtio_mdev_set_status(struct virtio_device *vdev, u8
>> +status) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + return ops->set_status(mdev, status);
>> +}
>> +
>> +static void virtio_mdev_reset(struct virtio_device *vdev) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + return ops->set_status(mdev, 0);
>> +}
>> +
>> +static bool virtio_mdev_notify(struct virtqueue *vq) {
>> + struct mdev_device *mdev = vm_get_mdev(vq->vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + ops->kick_vq(mdev, vq->index);
>> +
>> + return true;
>> +}
>> +
>> +static irqreturn_t virtio_mdev_config_cb(void *private) {
>> + struct virtio_mdev_device *vm_dev = private;
>> +
>> + virtio_config_changed(&vm_dev->vdev);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t virtio_mdev_virtqueue_cb(void *private) {
>> + struct virtio_mdev_vq_info *info = private;
>> +
>> + return vring_interrupt(0, info->vq);
>> +}
>> +
>> +static struct virtqueue *
>> +virtio_mdev_setup_vq(struct virtio_device *vdev, unsigned index,
>> + void (*callback)(struct virtqueue *vq),
>> + const char *name, bool ctx)
>> +{
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> + struct virtio_mdev_vq_info *info;
>> + struct virtio_mdev_callback cb;
>> + struct virtqueue *vq;
>> + u32 align, num;
>> + u64 desc_addr, driver_addr, device_addr;
>> + int err;
>> +
>> + if (!name)
>> + return NULL;
>> +
>> + /* Queue shouldn't already be set up. */
>> + if (ops->get_vq_ready(mdev, index)) {
>> + err = -ENOENT;
>> + goto error_available;
>> + }
>> +
> No need for a goto, to single return done later.
> Just do
> if (ops->get_vq_ready(mdev, index))
> return -ENOENT;
Ok, will fix.
>
>> + /* Allocate and fill out our active queue description */
>> + info = kmalloc(sizeof(*info), GFP_KERNEL);
>> + if (!info) {
>> + err = -ENOMEM;
>> + goto error_kmalloc;
>> + }
>> +
> Similar to above one.
Ok.
>
>> + num = ops->get_queue_max(mdev);
>> + if (num == 0) {
>> + err = -ENOENT;
>> + goto error_new_virtqueue;
>> + }
>> +
>> + /* Create the vring */
>> + align = ops->get_vq_align(mdev);
>> + vq = vring_create_virtqueue(index, num, align, vdev,
>> + true, true, ctx,
>> + virtio_mdev_notify, callback, name);
>> + if (!vq) {
>> + err = -ENOMEM;
>> + goto error_new_virtqueue;
>> + }
>> +
>> + /* Setup virtqueue callback */
>> + cb.callback = virtio_mdev_virtqueue_cb;
>> + cb.private = info;
>> + ops->set_vq_cb(mdev, index, &cb);
>> + ops->set_vq_num(mdev, index, virtqueue_get_vring_size(vq));
>> +
>> + desc_addr = virtqueue_get_desc_addr(vq);
>> + driver_addr = virtqueue_get_avail_addr(vq);
>> + device_addr = virtqueue_get_used_addr(vq);
>> +
>> + if (ops->set_vq_address(mdev, index,
>> + desc_addr, driver_addr,
>> + device_addr)) {
>> + err = -EINVAL;
>> + goto err_vq;
>> + }
>> +
>> + ops->set_vq_ready(mdev, index, 1);
>> +
>> + vq->priv = info;
>> + info->vq = vq;
>> +
>> + return vq;
>> +
>> +err_vq:
>> + vring_del_virtqueue(vq);
>> +error_new_virtqueue:
>> + ops->set_vq_ready(mdev, index, 0);
>> + WARN_ON(ops->get_vq_ready(mdev, index));
>> + kfree(info);
>> +error_kmalloc:
>> +error_available:
>> + return ERR_PTR(err);
>> +
>> +}
>> +
>> +static void virtio_mdev_del_vq(struct virtqueue *vq) {
>> + struct mdev_device *mdev = vm_get_mdev(vq->vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> + struct virtio_mdev_vq_info *info = vq->priv;
>> + unsigned int index = vq->index;
>> +
>> + /* Select and deactivate the queue */
>> + ops->set_vq_ready(mdev, index, 0);
>> + WARN_ON(ops->get_vq_ready(mdev, index));
>> +
>> + vring_del_virtqueue(vq);
>> +
>> + kfree(info);
>> +}
>> +
>> +static void virtio_mdev_del_vqs(struct virtio_device *vdev) {
>> + struct virtqueue *vq, *n;
>> +
>> + list_for_each_entry_safe(vq, n, &vdev->vqs, list)
>> + virtio_mdev_del_vq(vq);
>> +}
>> +
>> +static int virtio_mdev_find_vqs(struct virtio_device *vdev, unsigned nvqs,
>> + struct virtqueue *vqs[],
>> + vq_callback_t *callbacks[],
>> + const char * const names[],
>> + const bool *ctx,
>> + struct irq_affinity *desc)
>> +{
>> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> + struct virtio_mdev_callback cb;
>> + int i, err, queue_idx = 0;
>> +
>> + vm_dev->vqs = kmalloc_array(queue_idx, sizeof(*vm_dev->vqs),
>> + GFP_KERNEL);
>> + if (!vm_dev->vqs)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < nvqs; ++i) {
>> + if (!names[i]) {
>> + vqs[i] = NULL;
>> + continue;
>> + }
>> +
>> + vqs[i] = virtio_mdev_setup_vq(vdev, queue_idx++,
>> + callbacks[i], names[i], ctx ?
>> + ctx[i] : false);
>> + if (IS_ERR(vqs[i])) {
>> + err = PTR_ERR(vqs[i]);
>> + goto err_setup_vq;
>> + }
>> + }
>> +
>> + cb.callback = virtio_mdev_config_cb;
>> + cb.private = vm_dev;
>> + ops->set_config_cb(mdev, &cb);
>> +
>> + return 0;
>> +
>> +err_setup_vq:
>> + kfree(vm_dev->vqs);
>> + virtio_mdev_del_vqs(vdev);
>> + return err;
>> +}
>> +
>> +static u64 virtio_mdev_get_features(struct virtio_device *vdev) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + return ops->get_features(mdev);
>> +}
>> +
>> +static int virtio_mdev_finalize_features(struct virtio_device *vdev) {
>> + struct mdev_device *mdev = vm_get_mdev(vdev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> +
>> + /* Give virtio_ring a chance to accept features. */
>> + vring_transport_features(vdev);
>> +
>> + return ops->set_features(mdev, vdev->features); }
>> +
>> +static const char *virtio_mdev_bus_name(struct virtio_device *vdev) {
>> + struct virtio_mdev_device *vm_dev = to_virtio_mdev_device(vdev);
>> + struct mdev_device *mdev = vm_dev->mdev;
>> +
>> + return dev_name(&mdev->dev);
>> +}
>> +
>> +static const struct virtio_config_ops virtio_mdev_config_ops = {
>> + .get = virtio_mdev_get,
>> + .set = virtio_mdev_set,
>> + .generation = virtio_mdev_generation,
>> + .get_status = virtio_mdev_get_status,
>> + .set_status = virtio_mdev_set_status,
>> + .reset = virtio_mdev_reset,
>> + .find_vqs = virtio_mdev_find_vqs,
>> + .del_vqs = virtio_mdev_del_vqs,
>> + .get_features = virtio_mdev_get_features,
>> + .finalize_features = virtio_mdev_finalize_features,
>> + .bus_name = virtio_mdev_bus_name,
>> +};
>> +
>> +static void virtio_mdev_release_dev(struct device *_d) {
>> + struct virtio_device *vdev =
>> + container_of(_d, struct virtio_device, dev);
>> + struct virtio_mdev_device *vm_dev =
>> + container_of(vdev, struct virtio_mdev_device, vdev);
>> +
>> + devm_kfree(_d, vm_dev);
>> +}
>> +
>> +static int virtio_mdev_probe(struct device *dev) {
>> + struct mdev_device *mdev = to_mdev_device(dev);
>> + const struct virtio_mdev_parent_ops *ops =
>> mdev_get_parent_ops(mdev);
>> + struct virtio_mdev_device *vm_dev;
>> + int rc;
>> +
>> + vm_dev = devm_kzalloc(dev, sizeof(*vm_dev), GFP_KERNEL);
>> + if (!vm_dev)
>> + return -ENOMEM;
>> +
>> + vm_dev->vdev.dev.parent = dev;
>> + vm_dev->vdev.dev.release = virtio_mdev_release_dev;
>> + vm_dev->vdev.config = &virtio_mdev_config_ops;
>> + vm_dev->mdev = mdev;
>> + vm_dev->vqs = NULL;
>> + spin_lock_init(&vm_dev->lock);
>> +
>> + vm_dev->version = ops->get_version(mdev);
>> + if (vm_dev->version != 1) {
>> + dev_err(dev, "Version %ld not supported!\n",
>> + vm_dev->version);
>> + return -ENXIO;
>> + }
>> +
>> + vm_dev->vdev.id.device = ops->get_device_id(mdev);
>> + if (vm_dev->vdev.id.device == 0)
>> + return -ENODEV;
>> +
>> + vm_dev->vdev.id.vendor = ops->get_vendor_id(mdev);
>> + rc = register_virtio_device(&vm_dev->vdev);
>> + if (rc)
>> + put_device(dev);
>> +
>> + dev_set_drvdata(dev, vm_dev);
> No need to set drvdata when there is error returned from register_virtio_device().
>
Fixed in V2.
>> +
>> + return rc;
>> +
> Extra line not needed.
>> +}
Fixed.
>> +
>> +static void virtio_mdev_remove(struct device *dev) {
>> + struct virtio_mdev_device *vm_dev = dev_get_drvdata(dev);
>> +
>> + unregister_virtio_device(&vm_dev->vdev);
>> +}
>> +
>> +static struct mdev_class_id id_table[] = {
>> + { MDEV_ID_VIRTIO },
>> + { 0 },
>> +};
>> +
>> +static struct mdev_driver virtio_mdev_driver = {
>> + .name = "virtio_mdev",
>> + .probe = virtio_mdev_probe,
>> + .remove = virtio_mdev_remove,
> No need for tab, just do single white space.
Yes, fixed.
Thanks
>> + .id_table = id_table,
>> +};
>> +
>> +static int __init virtio_mdev_init(void) {
>> + return mdev_register_driver(&virtio_mdev_driver, THIS_MODULE); }
>> +
>> +static void __exit virtio_mdev_exit(void) {
>> + mdev_unregister_driver(&virtio_mdev_driver);
>> +}
>> +
>> +module_init(virtio_mdev_init)
>> +module_exit(virtio_mdev_exit)
>> +
>> +MODULE_VERSION(DRIVER_VERSION);
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_AUTHOR(DRIVER_AUTHOR);
>> +MODULE_DESCRIPTION(DRIVER_DESC);
>> --
>> 2.19.1
^ permalink raw reply
* Re: CONFIG_NET_TC_SKB_EXT
From: Paul Blakey @ 2019-09-24 11:48 UTC (permalink / raw)
To: Edward Cree, Jakub Kicinski
Cc: Pravin Shelar, Daniel Borkmann, Vlad Buslov, David Miller,
netdev@vger.kernel.org, Jiri Pirko, Cong Wang, Jamal Hadi Salim,
Simon Horman, Or Gerlitz
In-Reply-To: <d6867e6c-2b81-5fcd-1d88-46663bed6e26@solarflare.com>
On 9/23/2019 8:17 PM, Edward Cree wrote:
> On 23/09/2019 17:56, Paul Blakey wrote:
>> Even following this approach in tc only is challenging for some
>> scenarios, consider the following tc rules:
>>
>> tc filter add dev1 ... chain 0 flower <match1> action goto chain 1
>> tc filter add dev1 ... chain 0 flower <match2> action goto chain 1
>> tc filter add dev1 ... chain 0 flower <match3> action goto chain 1
>> ..
>> ..
>> tc filter add dev1 ... chain 0 flower ip_dst <match1000> action goto chain 1
>>
>> tc filter add dev1 ... chain 1 flower ip_proto tcp action tunnel_key set dst_ip 8.8.8.8 action mirred egress redirect dev2
> This one is easy, if a packet gets to the end of a chain without matching any rules then it just gets delivered to the uplink (PF), and software TC starts over from the beginning of chain 0. AFAICT this is the natural hardware behaviour for any design of offload, and it's inherently all-or-nothing.
>> You'd also have to keep track of what fields were originally on the packet, and not a match resulting from a modifications,
>> See the following set of rules:
>> tc filter add dev1 ... chain 0 prio 1 flower src_mac <mac1> action pedit munge ex set dst mac <mac2> pipe action goto chain 1
>> tc filter add dev1 ... chain 0 prio 2 flower dst_mac <mac2> action goto chain 1
>> tc filter add dev1 ... chain 1 prio 1 flower dst_mac <mac3> action <action3>
>> tc filter add dev1 ... chain 1 prio 2 flower src_mac <mac1> action <action4>
> This one is slightly harder in that you have to either essentially 'gather up' actions as you go and only 'commit' them at the end of your processing pipeline (in which case you have to deal with the data hazard you cleverly included in your example), or keep a copy of the original packet around. But I don't see the data-hazard case as having any realistic use-cases (does anyone actually put actions other than ct before a goto chain?) and it's easy enough to detect it in SW at rule-insertion time.
The 'miss' for all or nothing is easy, but the hard part is combining
all the paths a packet can take in software to a single 'all or nothing'
rule in hardware.
And when you do that, any change to any part of the path requires
updating all the resulting rules. In the above case an update to the
route of 8.8.8.8 affects the 1000 rules before it.
I can see two approaches to do the all of nothing approach, one is to
statically analyze the software policy (tc rules in this case), and
carefully offload
all possible paths. For that, you (who? the driver? tc? ovs?) would need
to construct a graph, which even if you could do, there isn't any
infrastructure for it, it would only valid while the configuration stays
the same,
as any change to an edge on the graph requires re-analyzing it and
possible changing tens of thousands of rules. In case of connection
tracking, maybe hundred thousands...
Or the second approach, to save the expensive analysis, and unused
hardware rules in the above, trace the path of processing the packet
(gather and commit), save it somewhere on the SKB or per CPU, and
creating new rules - basically a hardware cache.
But again, any change to the policy, and large chunks of this cache
would be invalid.
Regarding the use-case, For OvS, as far as I know, it uses
re-circulation for ct action, and dp hash actions, since it needs kernel
help.
This extension is first introduced for ovs->tc offloads of connection
tracking, but as we said, we are going to re-use it for tc->hardware in
the same manner.
And that can take on many use-cases. The above is a valid use-case if
the controller isn't OvS.
The controller learn rules and insert the first 1000 rules matching on
some 5-tuple in chain 0, but then when you want to encap the packets,
you jump to the encap chain.
This allows you to update the encap action for all rules, and segment
the logic for your policy.
tc chains are always used this way for a lot of cases which is
segmenting policy logic.
> FWIW I think the only way this infra will ever be used is: chain 0 rules match <blah> action ct zone <n> goto chain <m>; chain <m> rules match ct_state +xyz <blah> action <actually do stuff to packet>. This can be supported with a hardware pipeline with three tables in series in a fairly natural way — and since all the actions that modify the packet (rather than just directing subsequent matching) are at the end, the natural miss behaviour is deliver-unmodified.
>
> I tried to explain this back on your [v3] net: openvswitch: Set OvS recirc_id from tc chain index, but you seemed set on your approach so I didn't persist. Now it looks like maybe I should have...
What you describe with the three tables (chain 0, chain X, and ct table
for each ct zone), follows the software model and requires continuing
from some miss point
and is what we plan on doing.
We offload something like:
chain 0 dst_mac aa:bb:cc:dd:ee:ff ct_state -trk action ct (goto special
CT table), action goto chain X
chain X ct_state +trk+est action forward to port
And a table that replicates the ct zone table, which has something like
<match on tuple 1.... > set metadata ct_state = +est, action continue
<match on tuple 2.... > set metadata ct_state = +est, action continue
...
Lots tuples...
What if you 'miss' on the match for the tuple? You already did some
processing in hardware, so either you revert those, or you continue in
software where you left off (the action ct).
We want to preserve the current software model, just as tc can do part
of the processing and it will continue to the rest of the pipepline, be
it OvS, bridge, or loopback. And in hardware we would do the same.
The all or nothing approach will require changing the software model to
allow
merging the ct zone table matches into the hardware rules and offload
something like:
dst_mac aa:bb:cc:dd:ee:ff < match on tuple 1> fwd port
dst_mac aa:bb:cc:dd:ee:ff < match on tuple 2> fwd port
...
Lots of 'merged' rules.
You delete the rule with action ct, and you have to delete all this
merged, instead of just one rule.
Tracing the packet, or merging rules will require new infra to support this.
^ permalink raw reply
* Re: [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym_pause
From: Andrew Lunn @ 2019-09-24 12:31 UTC (permalink / raw)
To: Michal Vokáč
Cc: netdev@vger.kernel.org, Vivien Didelot, Florian Fainelli,
David S. Miller
In-Reply-To: <573ffa6a-f29a-84d9-5895-b3d6cc389619@ysoft.com>
On Tue, Sep 24, 2019 at 01:27:24PM +0200, Michal Vokáč wrote:
> Hi,
>
> just tried booting latest next-20190920 on our imx6dl-yapp4-hydra platform
> with QCA8334 switch and got this:
>
> [ 7.424620] [<806840e0>] (phy_support_asym_pause) from [<80686724>] (qca8k_port_enable+0x40/0x48)
> [ 7.436911] [<806866e4>] (qca8k_port_enable) from [<80a74134>] (dsa_port_enable+0x3c/0x6c)
> [ 7.448629] r7:00000000 r6:e88a02cc r5:e812d090 r4:e812d090
> [ 7.457708] [<80a740f8>] (dsa_port_enable) from [<80a730bc>] (dsa_register_switch+0x798/0xacc)
> [ 7.469833] r5:e812d0cc r4:e812d090
Hi Michal
Please could you add a printk to verify it is the CPU port, and that
in qca8k_port_enable() phy is a NULL pointer.
I think the fix is going to look something like:
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 16f15c93a102..86c80a873e30 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -939,7 +939,8 @@ qca8k_port_enable(struct dsa_switch *ds, int port,
qca8k_port_set_status(priv, port, 1);
priv->port_sts[port].enabled = 1;
- phy_support_asym_pause(phy);
+ if (phy)
+ phy_support_asym_pause(phy);
return 0;
}
But i want to take a closer look at what priv->port_sts[port].enabled
= 1; does. Also, if there are any other port_enable() functions which
always assume a valid phy device.
Andrew
^ permalink raw reply related
* [PATCH 2/3] clk: let init callback return an error code
From: Jerome Brunet @ 2019-09-24 12:39 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: Jerome Brunet, linux-clk, linux-kernel, Heiko Stuebner,
Tero Kristo, Andrew Lunn, Florian Fainelli, Heiner Kallweit,
David S. Miller, netdev, linux-amlogic, linux-arm-msm,
linux-rockchip, linux-omap
In-Reply-To: <20190924123954.31561-1-jbrunet@baylibre.com>
If the init callback is allowed to request resources, it needs a return
value to report the outcome of such a request.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
Sorry about the spam.
This patch change quite a few files so I have tried to Cc the
relevant people. Stephen, You may notice that I have added a
couple of the network people. You need an Ack from one of them
since the Amlogic G12a mdio mux has a clock which uses the .init()
callback
drivers/clk/clk.c | 17 ++++++++++------
drivers/clk/meson/clk-mpll.c | 4 +++-
drivers/clk/meson/clk-phase.c | 4 +++-
drivers/clk/meson/clk-pll.c | 4 +++-
drivers/clk/meson/sclk-div.c | 4 +++-
drivers/clk/microchip/clk-core.c | 8 ++++++--
drivers/clk/mmp/clk-frac.c | 4 +++-
drivers/clk/mmp/clk-mix.c | 4 +++-
drivers/clk/qcom/clk-hfpll.c | 6 ++++--
drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++++++-----------
drivers/clk/ti/clock.h | 2 +-
drivers/clk/ti/clockdomain.c | 8 +++++---
drivers/net/phy/mdio-mux-meson-g12a.c | 4 +++-
include/linux/clk-provider.h | 10 +++++++---
14 files changed, 72 insertions(+), 35 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 232144cca6cf..df853a98fad2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3298,16 +3298,21 @@ static int __clk_core_init(struct clk_core *core)
* optional platform-specific magic
*
* The .init callback is not used by any of the basic clock types, but
- * exists for weird hardware that must perform initialization magic.
- * Please consider other ways of solving initialization problems before
- * using this callback, as its use is discouraged.
+ * exists for weird hardware that must perform initialization magic for
+ * CCF to get an accurate view of clock for any other callbacks. It may
+ * also be used needs to perform dynamic allocations. Such allocation
+ * must be freed in the terminate() callback.
+ * This callback shall not be used to initialize the parameters state,
+ * such as rate, parent, etc ...
*
* If it exist, this callback should called before any other callback of
* the clock
*/
- if (core->ops->init)
- core->ops->init(core->hw);
-
+ if (core->ops->init) {
+ ret = core->ops->init(core->hw);
+ if (ret)
+ goto out;
+ }
core->parent = __clk_init_parent(core);
diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
index 2d39a8bc367c..fc9df4860872 100644
--- a/drivers/clk/meson/clk-mpll.c
+++ b/drivers/clk/meson/clk-mpll.c
@@ -129,7 +129,7 @@ static int mpll_set_rate(struct clk_hw *hw,
return 0;
}
-static void mpll_init(struct clk_hw *hw)
+static int mpll_init(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk);
@@ -151,6 +151,8 @@ static void mpll_init(struct clk_hw *hw)
/* Set the magic misc bit if required */
if (MESON_PARM_APPLICABLE(&mpll->misc))
meson_parm_write(clk->map, &mpll->misc, 1);
+
+ return 0;
}
const struct clk_ops meson_clk_mpll_ro_ops = {
diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
index 80c3ada193a4..fe22e171121a 100644
--- a/drivers/clk/meson/clk-phase.c
+++ b/drivers/clk/meson/clk-phase.c
@@ -78,7 +78,7 @@ meson_clk_triphase_data(struct clk_regmap *clk)
return (struct meson_clk_triphase_data *)clk->data;
}
-static void meson_clk_triphase_sync(struct clk_hw *hw)
+static int meson_clk_triphase_sync(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk);
@@ -88,6 +88,8 @@ static void meson_clk_triphase_sync(struct clk_hw *hw)
val = meson_parm_read(clk->map, &tph->ph0);
meson_parm_write(clk->map, &tph->ph1, val);
meson_parm_write(clk->map, &tph->ph2, val);
+
+ return 0;
}
static int meson_clk_triphase_get_phase(struct clk_hw *hw)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index ddb1e5634739..489092dde3a6 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -277,7 +277,7 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw)
return -ETIMEDOUT;
}
-static void meson_clk_pll_init(struct clk_hw *hw)
+static int meson_clk_pll_init(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_clk_pll_data *pll = meson_clk_pll_data(clk);
@@ -288,6 +288,8 @@ static void meson_clk_pll_init(struct clk_hw *hw)
pll->init_count);
meson_parm_write(clk->map, &pll->rst, 0);
}
+
+ return 0;
}
static int meson_clk_pll_is_enabled(struct clk_hw *hw)
diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index 3acf03780221..76d31c0a3342 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -216,7 +216,7 @@ static int sclk_div_is_enabled(struct clk_hw *hw)
return 0;
}
-static void sclk_div_init(struct clk_hw *hw)
+static int sclk_div_init(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk);
@@ -231,6 +231,8 @@ static void sclk_div_init(struct clk_hw *hw)
sclk->cached_div = val + 1;
sclk_div_get_duty_cycle(hw, &sclk->cached_duty);
+
+ return 0;
}
const struct clk_ops meson_sclk_div_ops = {
diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index 567755d6f844..1b4f023cdc8b 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -266,10 +266,12 @@ static void roclk_disable(struct clk_hw *hw)
writel(REFO_ON | REFO_OE, PIC32_CLR(refo->ctrl_reg));
}
-static void roclk_init(struct clk_hw *hw)
+static int roclk_init(struct clk_hw *hw)
{
/* initialize clock in disabled state */
roclk_disable(hw);
+
+ return 0;
}
static u8 roclk_get_parent(struct clk_hw *hw)
@@ -880,7 +882,7 @@ static int sclk_set_parent(struct clk_hw *hw, u8 index)
return err;
}
-static void sclk_init(struct clk_hw *hw)
+static int sclk_init(struct clk_hw *hw)
{
struct pic32_sys_clk *sclk = clkhw_to_sys_clk(hw);
unsigned long flags;
@@ -899,6 +901,8 @@ static void sclk_init(struct clk_hw *hw)
writel(v, sclk->slew_reg);
spin_unlock_irqrestore(&sclk->core->reg_lock, flags);
}
+
+ return 0;
}
/* sclk with post-divider */
diff --git a/drivers/clk/mmp/clk-frac.c b/drivers/clk/mmp/clk-frac.c
index 90bf181f191a..fabc09aca6c4 100644
--- a/drivers/clk/mmp/clk-frac.c
+++ b/drivers/clk/mmp/clk-frac.c
@@ -109,7 +109,7 @@ static int clk_factor_set_rate(struct clk_hw *hw, unsigned long drate,
return 0;
}
-static void clk_factor_init(struct clk_hw *hw)
+static int clk_factor_init(struct clk_hw *hw)
{
struct mmp_clk_factor *factor = to_clk_factor(hw);
struct mmp_clk_factor_masks *masks = factor->masks;
@@ -146,6 +146,8 @@ static void clk_factor_init(struct clk_hw *hw)
if (factor->lock)
spin_unlock_irqrestore(factor->lock, flags);
+
+ return 0;
}
static const struct clk_ops clk_factor_ops = {
diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c
index 90814b2613c0..d2cd36c54474 100644
--- a/drivers/clk/mmp/clk-mix.c
+++ b/drivers/clk/mmp/clk-mix.c
@@ -419,12 +419,14 @@ static int mmp_clk_set_rate(struct clk_hw *hw, unsigned long rate,
}
}
-static void mmp_clk_mix_init(struct clk_hw *hw)
+static int mmp_clk_mix_init(struct clk_hw *hw)
{
struct mmp_clk_mix *mix = to_clk_mix(hw);
if (mix->table)
_filter_clk_table(mix, mix->table, mix->table_size);
+
+ return 0;
}
const struct clk_ops mmp_clk_mix_ops = {
diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index 3c04805f2a55..e847d586a73a 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -196,7 +196,7 @@ static unsigned long clk_hfpll_recalc_rate(struct clk_hw *hw,
return l_val * parent_rate;
}
-static void clk_hfpll_init(struct clk_hw *hw)
+static int clk_hfpll_init(struct clk_hw *hw)
{
struct clk_hfpll *h = to_clk_hfpll(hw);
struct hfpll_data const *hd = h->d;
@@ -206,7 +206,7 @@ static void clk_hfpll_init(struct clk_hw *hw)
regmap_read(regmap, hd->mode_reg, &mode);
if (mode != (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)) {
__clk_hfpll_init_once(hw);
- return;
+ return 0;
}
if (hd->status_reg) {
@@ -218,6 +218,8 @@ static void clk_hfpll_init(struct clk_hw *hw)
__clk_hfpll_init_once(hw);
}
}
+
+ return 0;
}
static int hfpll_is_enabled(struct clk_hw *hw)
diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index 198417d56300..10560d963baf 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -282,7 +282,7 @@ static int rockchip_rk3036_pll_is_enabled(struct clk_hw *hw)
return !(pllcon & RK3036_PLLCON1_PWRDOWN);
}
-static void rockchip_rk3036_pll_init(struct clk_hw *hw)
+static int rockchip_rk3036_pll_init(struct clk_hw *hw)
{
struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
const struct rockchip_pll_rate_table *rate;
@@ -290,14 +290,14 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw)
unsigned long drate;
if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
- return;
+ return 0;
drate = clk_hw_get_rate(hw);
rate = rockchip_get_pll_settings(pll, drate);
/* when no rate setting for the current rate, rely on clk_set_rate */
if (!rate)
- return;
+ return 0;
rockchip_rk3036_pll_get_params(pll, &cur);
@@ -319,13 +319,15 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw)
if (!parent) {
pr_warn("%s: parent of %s not available\n",
__func__, __clk_get_name(hw->clk));
- return;
+ return 0;
}
pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n",
__func__, __clk_get_name(hw->clk));
rockchip_rk3036_pll_set_params(pll, rate);
}
+
+ return 0;
}
static const struct clk_ops rockchip_rk3036_pll_clk_norate_ops = {
@@ -515,7 +517,7 @@ static int rockchip_rk3066_pll_is_enabled(struct clk_hw *hw)
return !(pllcon & RK3066_PLLCON3_PWRDOWN);
}
-static void rockchip_rk3066_pll_init(struct clk_hw *hw)
+static int rockchip_rk3066_pll_init(struct clk_hw *hw)
{
struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
const struct rockchip_pll_rate_table *rate;
@@ -523,14 +525,14 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw)
unsigned long drate;
if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
- return;
+ return 0;
drate = clk_hw_get_rate(hw);
rate = rockchip_get_pll_settings(pll, drate);
/* when no rate setting for the current rate, rely on clk_set_rate */
if (!rate)
- return;
+ return 0;
rockchip_rk3066_pll_get_params(pll, &cur);
@@ -543,6 +545,8 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw)
__func__, clk_hw_get_name(hw));
rockchip_rk3066_pll_set_params(pll, rate);
}
+
+ return 0;
}
static const struct clk_ops rockchip_rk3066_pll_clk_norate_ops = {
@@ -761,7 +765,7 @@ static int rockchip_rk3399_pll_is_enabled(struct clk_hw *hw)
return !(pllcon & RK3399_PLLCON3_PWRDOWN);
}
-static void rockchip_rk3399_pll_init(struct clk_hw *hw)
+static int rockchip_rk3399_pll_init(struct clk_hw *hw)
{
struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw);
const struct rockchip_pll_rate_table *rate;
@@ -769,14 +773,14 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw)
unsigned long drate;
if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE))
- return;
+ return 0;
drate = clk_hw_get_rate(hw);
rate = rockchip_get_pll_settings(pll, drate);
/* when no rate setting for the current rate, rely on clk_set_rate */
if (!rate)
- return;
+ return 0;
rockchip_rk3399_pll_get_params(pll, &cur);
@@ -798,13 +802,15 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw)
if (!parent) {
pr_warn("%s: parent of %s not available\n",
__func__, __clk_get_name(hw->clk));
- return;
+ return 0;
}
pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n",
__func__, __clk_get_name(hw->clk));
rockchip_rk3399_pll_set_params(pll, rate);
}
+
+ return 0;
}
static const struct clk_ops rockchip_rk3399_pll_clk_norate_ops = {
diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h
index e4b8392ff63c..adef9c16e43b 100644
--- a/drivers/clk/ti/clock.h
+++ b/drivers/clk/ti/clock.h
@@ -252,7 +252,7 @@ extern const struct clk_ops omap_gate_clk_ops;
extern struct ti_clk_features ti_clk_features;
-void omap2_init_clk_clkdm(struct clk_hw *hw);
+int omap2_init_clk_clkdm(struct clk_hw *hw);
int omap2_clkops_enable_clkdm(struct clk_hw *hw);
void omap2_clkops_disable_clkdm(struct clk_hw *hw);
diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c
index 423a99b9f10c..ee56306f79d5 100644
--- a/drivers/clk/ti/clockdomain.c
+++ b/drivers/clk/ti/clockdomain.c
@@ -101,16 +101,16 @@ void omap2_clkops_disable_clkdm(struct clk_hw *hw)
*
* Convert a clockdomain name stored in a struct clk 'clk' into a
* clockdomain pointer, and save it into the struct clk. Intended to be
- * called during clk_register(). No return value.
+ * called during clk_register(). Returns 0 on success, -EERROR otherwise.
*/
-void omap2_init_clk_clkdm(struct clk_hw *hw)
+int omap2_init_clk_clkdm(struct clk_hw *hw)
{
struct clk_hw_omap *clk = to_clk_hw_omap(hw);
struct clockdomain *clkdm;
const char *clk_name;
if (!clk->clkdm_name)
- return;
+ return 0;
clk_name = __clk_get_name(hw->clk);
@@ -123,6 +123,8 @@ void omap2_init_clk_clkdm(struct clk_hw *hw)
pr_debug("clock: could not associate clk %s to clkdm %s\n",
clk_name, clk->clkdm_name);
}
+
+ return 0;
}
static void __init of_ti_clockdomain_setup(struct device_node *node)
diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c
index 7a9ad54582e1..bf86c9c7a288 100644
--- a/drivers/net/phy/mdio-mux-meson-g12a.c
+++ b/drivers/net/phy/mdio-mux-meson-g12a.c
@@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw)
return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0;
}
-static void g12a_ephy_pll_init(struct clk_hw *hw)
+static int g12a_ephy_pll_init(struct clk_hw *hw)
{
struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw);
@@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw)
writel(0x20200000, pll->base + ETH_PLL_CTL5);
writel(0x0000c002, pll->base + ETH_PLL_CTL6);
writel(0x00000023, pll->base + ETH_PLL_CTL7);
+
+ return 0;
}
static const struct clk_ops g12a_ephy_pll_ops = {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 2fdfe8061363..b82ec4c4ca95 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -190,8 +190,12 @@ struct clk_duty {
*
* @init: Perform platform-specific initialization magic.
* This is not not used by any of the basic clock types.
- * Please consider other ways of solving initialization problems
- * before using this callback, as its use is discouraged.
+ * This callback exist for HW which needs to perform some
+ * initialisation magic for CCF to get an accurate view of the
+ * clock. It may also be used dynamic resource allocation is
+ * required. It shall not used to deal with clock parameters,
+ * such as rate or parents.
+ * Returns 0 on success, -EERROR otherwise.
*
* @debug_init: Set up type-specific debugfs entries for this clock. This
* is called once, after the debugfs directory entry for this
@@ -243,7 +247,7 @@ struct clk_ops {
struct clk_duty *duty);
int (*set_duty_cycle)(struct clk_hw *hw,
struct clk_duty *duty);
- void (*init)(struct clk_hw *hw);
+ int (*init)(struct clk_hw *hw);
void (*debug_init)(struct clk_hw *hw, struct dentry *dentry);
};
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] ipv6: do not free rt if FIB_LOOKUP_NOREF is set on suppress rule
From: David Miller @ 2019-09-24 12:52 UTC (permalink / raw)
To: Jason; +Cc: netdev, linux-kernel, stable
In-Reply-To: <20190924073615.31704-1-Jason@zx2c4.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Tue, 24 Sep 2019 09:36:15 +0200
> Commit 7d9e5f422150 removed references from certain dsts, but accounting
> for this never translated down into the fib6 suppression code. This bug
> was triggered by WireGuard users who use wg-quick(8), which uses the
> "suppress-prefix" directive to ip-rule(8) for routing all of their
> internet traffic without routing loops. The test case in the link of
> this commit reliably triggers various crashes due to the use-after-free
> caused by the reference underflow.
>
> Cc: stable@vger.kernel.org
> Fixes: 7d9e5f422150 ("ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF")
> Test-case: https://git.zx2c4.com/WireGuard/commit/?id=ad66532000f7a20b149e47c5eb3a957362c8e161
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Please make such test cases integratabe into the selftests area for networking
and submit it along with this fix.
Otherwise this code will regress.
Thank you.
^ permalink raw reply
* Re: [PATCH] ipv6: do not free rt if FIB_LOOKUP_NOREF is set on suppress rule
From: Jason A. Donenfeld @ 2019-09-24 12:55 UTC (permalink / raw)
To: David Miller; +Cc: Netdev, LKML, stable
In-Reply-To: <20190924.145257.2013712373872209531.davem@davemloft.net>
On Tue, Sep 24, 2019 at 2:53 PM David Miller <davem@davemloft.net> wrote:
> Please make such test cases integratabe into the selftests area for networking
> and submit it along with this fix.
That link is for a WireGuard test-case. When we get that upstream,
those will all live in selftests/ all the same as you'd like. For now,
it's running for every kernel on https://build.wireguard.com/ which in
turn runs for every new commit.
^ permalink raw reply
* [PATCH 1/3] docs: fix some broken references
From: Mauro Carvalho Chehab @ 2019-09-24 13:01 UTC (permalink / raw)
To: Linux Doc Mailing List, corbet
Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
Rob Herring, Mark Rutland, Daniel Lezcano, Thomas Gleixner,
Linus Walleij, Bartosz Golaszewski, Jean Delvare, Guenter Roeck,
Ralf Baechle, Paul Burton, James Hogan, Saeed Mahameed,
Leon Romanovsky, David S. Miller, Shannon Nelson,
Pensando Drivers, Steve French, Paul Walmsley, Palmer Dabbelt,
Albert Ou, devicetree, linux-gpio, linux-hwmon, linux-mips,
netdev, linux-rdma, linux-cifs, samba-technical, linux-riscv
There are a number of documentation files that got moved or
renamed. update their references.
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
Documentation/devicetree/bindings/cpu/cpu-topology.txt | 2 +-
Documentation/devicetree/bindings/timer/ingenic,tcu.txt | 2 +-
Documentation/driver-api/gpio/driver.rst | 2 +-
Documentation/hwmon/inspur-ipsps1.rst | 2 +-
Documentation/mips/ingenic-tcu.rst | 2 +-
Documentation/networking/device_drivers/mellanox/mlx5.rst | 2 +-
MAINTAINERS | 2 +-
drivers/net/ethernet/faraday/ftgmac100.c | 2 +-
drivers/net/ethernet/pensando/ionic/ionic_if.h | 4 ++--
fs/cifs/cifsfs.c | 2 +-
10 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/cpu/cpu-topology.txt b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
index 99918189403c..9bd530a35d14 100644
--- a/Documentation/devicetree/bindings/cpu/cpu-topology.txt
+++ b/Documentation/devicetree/bindings/cpu/cpu-topology.txt
@@ -549,5 +549,5 @@ Example 3: HiFive Unleashed (RISC-V 64 bit, 4 core system)
[2] Devicetree NUMA binding description
Documentation/devicetree/bindings/numa.txt
[3] RISC-V Linux kernel documentation
- Documentation/devicetree/bindings/riscv/cpus.txt
+ Documentation/devicetree/bindings/riscv/cpus.yaml
[4] https://www.devicetree.org/specifications/
diff --git a/Documentation/devicetree/bindings/timer/ingenic,tcu.txt b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
index 5a4b9ddd9470..7f6fe20503f5 100644
--- a/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
+++ b/Documentation/devicetree/bindings/timer/ingenic,tcu.txt
@@ -2,7 +2,7 @@ Ingenic JZ47xx SoCs Timer/Counter Unit devicetree bindings
==========================================================
For a description of the TCU hardware and drivers, have a look at
-Documentation/mips/ingenic-tcu.txt.
+Documentation/mips/ingenic-tcu.rst.
Required properties:
diff --git a/Documentation/driver-api/gpio/driver.rst b/Documentation/driver-api/gpio/driver.rst
index 3fdb32422f8a..9076cc76d5bf 100644
--- a/Documentation/driver-api/gpio/driver.rst
+++ b/Documentation/driver-api/gpio/driver.rst
@@ -493,7 +493,7 @@ available but we try to move away from this:
gpiochip. It will pass the struct gpio_chip* for the chip to all IRQ
callbacks, so the callbacks need to embed the gpio_chip in its state
container and obtain a pointer to the container using container_of().
- (See Documentation/driver-model/design-patterns.txt)
+ (See Documentation/driver-api/driver-model/design-patterns.rst)
- gpiochip_irqchip_add_nested(): adds a nested cascaded irqchip to a gpiochip,
as discussed above regarding different types of cascaded irqchips. The
diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
index 2b871ae3448f..ed32a65c30e1 100644
--- a/Documentation/hwmon/inspur-ipsps1.rst
+++ b/Documentation/hwmon/inspur-ipsps1.rst
@@ -17,7 +17,7 @@ Usage Notes
-----------
This driver does not auto-detect devices. You will have to instantiate the
-devices explicitly. Please see Documentation/i2c/instantiating-devices for
+devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for
details.
Sysfs entries
diff --git a/Documentation/mips/ingenic-tcu.rst b/Documentation/mips/ingenic-tcu.rst
index c4ef4c45aade..c5a646b14450 100644
--- a/Documentation/mips/ingenic-tcu.rst
+++ b/Documentation/mips/ingenic-tcu.rst
@@ -68,4 +68,4 @@ and frameworks can be controlled from the same registers, all of these
drivers access their registers through the same regmap.
For more information regarding the devicetree bindings of the TCU drivers,
-have a look at Documentation/devicetree/bindings/mfd/ingenic,tcu.txt.
+have a look at Documentation/devicetree/bindings/timer/ingenic,tcu.txt.
diff --git a/Documentation/networking/device_drivers/mellanox/mlx5.rst b/Documentation/networking/device_drivers/mellanox/mlx5.rst
index d071c6b49e1f..a74422058351 100644
--- a/Documentation/networking/device_drivers/mellanox/mlx5.rst
+++ b/Documentation/networking/device_drivers/mellanox/mlx5.rst
@@ -258,7 +258,7 @@ mlx5 tracepoints
================
mlx5 driver provides internal trace points for tracking and debugging using
-kernel tracepoints interfaces (refer to Documentation/trace/ftrase.rst).
+kernel tracepoints interfaces (refer to Documentation/trace/ftrace.rst).
For the list of support mlx5 events check /sys/kernel/debug/tracing/events/mlx5/
diff --git a/MAINTAINERS b/MAINTAINERS
index 54f1286087e9..65b7d9a0a44a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3680,7 +3680,7 @@ M: Oleksij Rempel <o.rempel@pengutronix.de>
R: Pengutronix Kernel Team <kernel@pengutronix.de>
L: linux-can@vger.kernel.org
S: Maintained
-F: Documentation/networking/j1939.txt
+F: Documentation/networking/j1939.rst
F: net/can/j1939/
F: include/uapi/linux/can/j1939.h
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 9b7af94a40bb..8abe5e90d268 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1835,7 +1835,7 @@ static int ftgmac100_probe(struct platform_device *pdev)
}
/* Indicate that we support PAUSE frames (see comment in
- * Documentation/networking/phy.txt)
+ * Documentation/networking/phy.rst)
*/
phy_support_asym_pause(phy);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index 5bfdda19f64d..80028f781c83 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -596,8 +596,8 @@ enum ionic_txq_desc_opcode {
* the @encap is set, the device will
* offload the outer header checksums using
* LCO (local checksum offload) (see
- * Documentation/networking/checksum-
- * offloads.txt for more info).
+ * Documentation/networking/checksum-offloads.rst
+ * for more info).
*
* IONIC_TXQ_DESC_OPCODE_CSUM_HW:
*
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2e9c7f493f99..811f510578cb 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -1529,7 +1529,7 @@ init_cifs(void)
/*
* Consider in future setting limit!=0 maybe to min(num_of_cores - 1, 3)
* so that we don't launch too many worker threads but
- * Documentation/workqueue.txt recommends setting it to 0
+ * Documentation/core-api/workqueue.rst recommends setting it to 0
*/
/* WQ_UNBOUND allows decrypt tasks to run on any CPU */
--
2.21.0
^ permalink raw reply related
* Re: [BUG] Unable to handle kernel NULL pointer dereference in phy_support_asym_pause
From: Michal Vokáč @ 2019-09-24 13:10 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, Vivien Didelot, Florian Fainelli,
David S. Miller
In-Reply-To: <20190924123126.GE14477@lunn.ch>
On 24. 09. 19 14:31, Andrew Lunn wrote:
> On Tue, Sep 24, 2019 at 01:27:24PM +0200, Michal Vokáč wrote:
>> Hi,
>>
>> just tried booting latest next-20190920 on our imx6dl-yapp4-hydra platform
>> with QCA8334 switch and got this:
>>
>> [ 7.424620] [<806840e0>] (phy_support_asym_pause) from [<80686724>] (qca8k_port_enable+0x40/0x48)
>> [ 7.436911] [<806866e4>] (qca8k_port_enable) from [<80a74134>] (dsa_port_enable+0x3c/0x6c)
>> [ 7.448629] r7:00000000 r6:e88a02cc r5:e812d090 r4:e812d090
>> [ 7.457708] [<80a740f8>] (dsa_port_enable) from [<80a730bc>] (dsa_register_switch+0x798/0xacc)
>> [ 7.469833] r5:e812d0cc r4:e812d090
>
> Hi Michal
>
> Please could you add a printk to verify it is the CPU port, and that
> in qca8k_port_enable() phy is a NULL pointer.
>
> I think the fix is going to look something like:
>
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 16f15c93a102..86c80a873e30 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -939,7 +939,8 @@ qca8k_port_enable(struct dsa_switch *ds, int port,
> qca8k_port_set_status(priv, port, 1);
> priv->port_sts[port].enabled = 1;
dev_info(priv->dev, "port: %d, phy: %d", port, (u32)phy);
>
> - phy_support_asym_pause(phy);
> + if (phy)
> + phy_support_asym_pause(phy);
>
> return 0;
> }
>
I added the printk and the above fix and can confirm that it is the CPU
port and the phy is NULL pointer:
[ 6.976366] qca8k 2188000.ethernet-1:0a: Using legacy PHYLIB callbacks. Please migrate to PHYLINK!
[ 6.992021] qca8k 2188000.ethernet-1:0a: port: 0, phy: 0
[ 7.001323] qca8k 2188000.ethernet-1:0a eth2 (uninitialized): PHY [2188000.ethernet-1:01] driver [Generic PHY]
[ 7.014221] qca8k 2188000.ethernet-1:0a eth2 (uninitialized): phy: setting supported 00,00000000,000062ef advertising 00,00000000,000062ef
[ 7.030598] qca8k 2188000.ethernet-1:0a eth1 (uninitialized): PHY [2188000.ethernet-1:02] driver [Generic PHY]
[ 7.043500] qca8k 2188000.ethernet-1:0a eth1 (uninitialized): phy: setting supported 00,00000000,000062ef advertising 00,00000000,000062ef
[ 7.063335] DSA: tree 0 setup
Now the device boots but there is a problem with the CPU port configuration:
root@hydraco:~# ifconfig eth0 up
[ 255.256047] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
[ 255.272449] fec 2188000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
[ 255.286539] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
root@hydraco:~# ifconfig eth1 up
[ 268.350078] qca8k 2188000.ethernet-1:0a: port: 3, phy: -393143296
[ 268.364442] qca8k 2188000.ethernet-1:0a eth1: configuring for phy/ link mode
[ 268.375400] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//Unknown/Unknown adv=00,00000000,000062ef pause=10 link=0 an=1
[ 268.393901] qca8k 2188000.ethernet-1:0a eth1: phy link up /1Gbps/Full
[ 268.404849] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//1Gbps/Full adv=00,00000000,00000000 pause=0e link=1 an=0
[ 268.420740] qca8k 2188000.ethernet-1:0a eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[ 268.432995] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
root@hydraco:~# udhcpc -i eth1
udhcpc (v1.23.2) started
Sending discover...
Sending discover...
Sending discover...
On v5.3.1 it works OK:
root@hydraco:~# ifconfig eth0 up
[ 29.665997] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
[ 29.682075] Generic PHY fixed-0:00: PHY advertising (00,00000200,000022e0) more modes than genphy supports, some modes not advertised.
[ 29.698284] fec 2188000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
[ 29.709568] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
root@hydraco:~# ifconfig eth1 up
[ 33.699923] qca8k 2188000.ethernet-1:0a eth1: configuring for phy/ link mode
[ 33.714466] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//Unknown/Unknown adv=00,00000000,000062ef pause=10 link=0 an=1
[ 33.735356] 8021q: adding VLAN 0 to HW filter on device eth1
[ 33.755671] qca8k 2188000.ethernet-1:0a eth1: phy link up /1Gbps/Full
[ 33.766071] qca8k 2188000.ethernet-1:0a eth1: phylink_mac_config: mode=phy//1Gbps/Full adv=00,00000000,00000000 pause=0e link=1 an=0
[ 33.781987] qca8k 2188000.ethernet-1:0a eth1: Link is Up - 1Gbps/Full - flow control rx/tx
[ 33.794290] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready
oot@hydraco:~# udhcpc -i eth1
udhcpc (v1.23.2) started
Sending discover...
Sending select for 10.1.8.103...
> But i want to take a closer look at what priv->port_sts[port].enabled
> = 1; does. Also, if there are any other port_enable() functions which
> always assume a valid phy device.
Anything else I can do to help with this?
Thank you!
Michal
^ permalink raw reply
* Re: [PATCH] ipv6: do not free rt if FIB_LOOKUP_NOREF is set on suppress rule
From: David Miller @ 2019-09-24 13:30 UTC (permalink / raw)
To: Jason; +Cc: netdev, linux-kernel, stable
In-Reply-To: <CAHmME9oqRg9L+wdhOra=UO3ypuy9N82DHVrbDJDgLpxSmS-rHQ@mail.gmail.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Tue, 24 Sep 2019 14:55:04 +0200
> On Tue, Sep 24, 2019 at 2:53 PM David Miller <davem@davemloft.net> wrote:
>> Please make such test cases integratabe into the selftests area for networking
>> and submit it along with this fix.
>
> That link is for a WireGuard test-case. When we get that upstream,
> those will all live in selftests/ all the same as you'd like. For now,
> it's running for every kernel on https://build.wireguard.com/ which in
> turn runs for every new commit.
I'm asking you to make a non-wireguard test that triggers the problem.
Or would you like a situation you're interested in to break from time
to time.
Jason, please don't be difficult about this and write a proper test
case just like I would ask anyone else fixing bugs like this to write.
Thank you.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox