Messages in this thread | | | Date | Tue, 21 Apr 2020 11:13:52 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net-next 1/3] hinic: add mailbox function support |
| |
On Tue, 21 Apr 2020 04:56:33 +0000 Luo bin wrote: > virtual function and physical function can communicate with each > other through mailbox channel supported by hw > > Signed-off-by: Luo bin <luobin9@huawei.com>
> +static int recv_vf_mbox_handler(struct hinic_mbox_func_to_func *func_to_func, > + struct hinic_recv_mbox *recv_mbox, > + void *buf_out, u16 *out_size) > +{ > + hinic_vf_mbox_cb cb; > + int ret = 0; > + > + if (recv_mbox->mod >= HINIC_MOD_MAX) { > + dev_err(&func_to_func->hwif->pdev->dev, "Receive illegal mbox message, mod = %d\n", > + recv_mbox->mod);
You may want to rate limit these, otherwise VF may spam PFs logs.
> + return -EINVAL; > + }
> +static int mbox_func_params_valid(struct hinic_mbox_func_to_func *func_to_func, > + void *buf_in, u16 in_size) > +{ > + if (!buf_in || !in_size) > + return -EINVAL;
This is defensive programming, we don't do that in the kernel, callers should not pass NULL buffer and 0 size.
Also you probably want the size to be size_t, otherwise it may get truncated before the check below.
> + if (in_size > HINIC_MBOX_DATA_SIZE) { > + dev_err(&func_to_func->hwif->pdev->dev, > + "Mbox msg len(%d) exceed limit(%d)\n", > + in_size, HINIC_MBOX_DATA_SIZE); > + return -EINVAL; > + } > + > + return 0; > +} > + > +int hinic_mbox_to_pf(struct hinic_hwdev *hwdev, > + enum hinic_mod_type mod, u8 cmd, void *buf_in, > + u16 in_size, void *buf_out, u16 *out_size, u32 timeout) > +{ > + struct hinic_mbox_func_to_func *func_to_func = hwdev->func_to_func; > + int err = mbox_func_params_valid(func_to_func, buf_in, in_size); > + > + if (err) > + return err; > + > + if (!HINIC_IS_VF(hwdev->hwif)) { > + dev_err(&hwdev->hwif->pdev->dev, "Params error, func_type: %d\n", > + HINIC_FUNC_TYPE(hwdev->hwif)); > + return -EINVAL; > + } > + > + err = hinic_mbox_to_func(func_to_func, mod, cmd, > + hinic_pf_id_of_vf_hw(hwdev->hwif), buf_in, > + in_size, buf_out, out_size, timeout); > + return err;
return hinic_mbox_to...
directly
> +}
> +static int comm_pf_mbox_handler(void *handle, u16 vf_id, u8 cmd, void *buf_in, > + u16 in_size, void *buf_out, u16 *out_size) > +{ > + struct hinic_hwdev *hwdev = handle; > + struct hinic_pfhwdev *pfhwdev; > + int err = 0; > + > + pfhwdev = container_of(hwdev, struct hinic_pfhwdev, hwdev); > + > + if (cmd == HINIC_COMM_CMD_START_FLR) { > + *out_size = 0; > + } else { > + err = hinic_msg_to_mgmt(&pfhwdev->pf_to_mgmt, HINIC_MOD_COMM, > + cmd, buf_in, in_size, buf_out, out_size, > + HINIC_MGMT_MSG_SYNC); > + if (err && err != HINIC_MBOX_PF_BUSY_ACTIVE_FW)
Double space, please run checkpatch --strict on the patches
> + dev_err(&hwdev->hwif->pdev->dev, > + "PF mbox common callback handler err: %d\n", > + err); > + } > + > + return err; > +}
| |