On 11/30/2023 1:12 AM, Simon Horman wrote:
On Tue, Nov 21, 2023 at 02:51:04AM +0000, Yahui Cao wrote:
From: Lingyu Liu <lingyu.liu@xxxxxxxxx>
Save the virtual channel messages sent by VF on the source side during
runtime. The logged virtchnl messages will be transferred and loaded
into the device on the destination side during the device resume stage.
For the feature which can not be migrated yet, it must be disabled or
blocked to prevent from being abused by VF. Otherwise, it may introduce
functional and security issue. Mask unsupported VF capability flags in
the VF-PF negotiaion stage.
Signed-off-by: Lingyu Liu <lingyu.liu@xxxxxxxxx>
Signed-off-by: Yahui Cao <yahui.cao@xxxxxxxxx>
Hi Lingyu Liu and Yahui Cao,
some minor feedback from my side.
...
diff --git a/drivers/net/ethernet/intel/ice/ice_migration.c b/drivers/net/ethernet/intel/ice/ice_migration.c
...
+/**
+ * ice_migration_log_vf_msg - Log request message from VF
+ * @vf: pointer to the VF structure
+ * @event: pointer to the AQ event
+ *
+ * Log VF message for later device state loading during live migration
+ *
+ * Return 0 for success, negative for error
+ */
+int ice_migration_log_vf_msg(struct ice_vf *vf,
+ struct ice_rq_event_info *event)
+{
+ struct ice_migration_virtchnl_msg_listnode *msg_listnode;
+ u32 v_opcode = le32_to_cpu(event->desc.cookie_high);
+ struct device *dev = ice_pf_to_dev(vf->pf);
+ u16 msglen = event->msg_len;
+ u8 *msg = event->msg_buf;
+
+ if (!ice_migration_is_loggable_msg(v_opcode))
+ return 0;
+
+ if (vf->virtchnl_msg_num >= VIRTCHNL_MSG_MAX) {
+ dev_warn(dev, "VF %d has maximum number virtual channel commands\n",
+ vf->vf_id);
+ return -ENOMEM;
+ }
+
+ msg_listnode = (struct ice_migration_virtchnl_msg_listnode *)
+ kzalloc(struct_size(msg_listnode,
+ msg_slot.msg_buffer,
+ msglen),
+ GFP_KERNEL);
nit: there is no need to cast the void * pointer returned by kzalloc().
Flagged by Coccinelle.
Sure. Will fix in next version.
+ if (!msg_listnode) {
+ dev_err(dev, "VF %d failed to allocate memory for msg listnode\n",
+ vf->vf_id);
+ return -ENOMEM;
+ }
+ dev_dbg(dev, "VF %d save virtual channel command, op code: %d, len: %d\n",
+ vf->vf_id, v_opcode, msglen);
+ msg_listnode->msg_slot.opcode = v_opcode;
+ msg_listnode->msg_slot.msg_len = msglen;
+ memcpy(msg_listnode->msg_slot.msg_buffer, msg, msglen);
+ list_add_tail(&msg_listnode->node, &vf->virtchnl_msg_list);
+ vf->virtchnl_msg_num++;
+ vf->virtchnl_msg_size += struct_size(&msg_listnode->msg_slot,
+ msg_buffer,
+ msglen);
+ return 0;
+}
+
+/**
+ * ice_migration_unlog_vf_msg - revert logged message
+ * @vf: pointer to the VF structure
+ * @v_opcode: virtchnl message operation code
+ *
+ * Remove the last virtual channel message logged before.
+ */
+void ice_migration_unlog_vf_msg(struct ice_vf *vf, u32 v_opcode)
+{
+ struct ice_migration_virtchnl_msg_listnode *msg_listnode;
+
+ if (!ice_migration_is_loggable_msg(v_opcode))
+ return;
+
+ if (WARN_ON_ONCE(list_empty(&vf->virtchnl_msg_list)))
+ return;
+
+ msg_listnode =
+ list_last_entry(&vf->virtchnl_msg_list,
+ struct ice_migration_virtchnl_msg_listnode,
+ node);
+ if (WARN_ON_ONCE(msg_listnode->msg_slot.opcode != v_opcode))
+ return;
+
+ list_del(&msg_listnode->node);
+ kfree(msg_listnode);
msg_listnode is freed on the line above,
but dereferenced in the usage of struct_size() below.
As flagged by Smatch and Coccinelle.
>> + vf->virtchnl_msg_num--;
+ vf->virtchnl_msg_size -= struct_size(&msg_listnode->msg_slot,
+ msg_buffer,
+ msg_listnode->msg_slot.msg_len);
+}
...
Good catch :) Will fix in next version
Thanks.
Yahui.
