* [bug report] net/mlx4_core: Support more than 64 VFs
@ 2022-04-22 15:41 Dan Carpenter
2022-05-23 12:18 ` Leon Romanovsky
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-04-22 15:41 UTC (permalink / raw)
To: matanb; +Cc: linux-rdma
Hello Matan Barak,
The patch de966c592802: "net/mlx4_core: Support more than 64 VFs"
from Nov 13, 2014, leads to the following Smatch static checker
warning:
drivers/net/ethernet/mellanox/mlx4/main.c:3455 mlx4_load_one()
warn: missing error code 'err'
drivers/net/ethernet/mellanox/mlx4/main.c
3438 if (mlx4_is_master(dev)) {
3439 /* when we hit the goto slave_start below, dev_cap already initialized */
3440 if (!dev_cap) {
3441 dev_cap = kzalloc(sizeof(*dev_cap), GFP_KERNEL);
3442
3443 if (!dev_cap) {
3444 err = -ENOMEM;
3445 goto err_fw;
3446 }
3447
3448 err = mlx4_QUERY_DEV_CAP(dev, dev_cap);
3449 if (err) {
3450 mlx4_err(dev, "QUERY_DEV_CAP command failed, aborting.\n");
3451 goto err_fw;
3452 }
3453
3454 if (mlx4_check_dev_cap(dev, dev_cap, nvfs))
--> 3455 goto err_fw;
^^^^^^^^^^^^
Should this have an error code?
3456
3457 if (!(dev_cap->flags2 & MLX4_DEV_CAP_FLAG2_SYS_EQS)) {
3458 u64 dev_flags = mlx4_enable_sriov(dev, pdev,
3459 total_vfs,
3460 existing_vfs,
3461 reset_flow);
3462
3463 mlx4_close_fw(dev);
3464 mlx4_cmd_cleanup(dev, MLX4_CMD_CLEANUP_ALL);
3465 dev->flags = dev_flags;
3466 if (!SRIOV_VALID_STATE(dev->flags)) {
3467 mlx4_err(dev, "Invalid SRIOV state\n");
3468 goto err_sriov;
Same
3469 }
3470 err = mlx4_reset(dev);
3471 if (err) {
3472 mlx4_err(dev, "Failed to reset HCA, aborting.\n");
3473 goto err_sriov;
3474 }
3475 goto slave_start;
3476 }
3477 } else {
3478 /* Legacy mode FW requires SRIOV to be enabled before
3479 * doing QUERY_DEV_CAP, since max_eq's value is different if
3480 * SRIOV is enabled.
3481 */
3482 memset(dev_cap, 0, sizeof(*dev_cap));
3483 err = mlx4_QUERY_DEV_CAP(dev, dev_cap);
3484 if (err) {
3485 mlx4_err(dev, "QUERY_DEV_CAP command failed, aborting.\n");
3486 goto err_fw;
3487 }
3488
3489 if (mlx4_check_dev_cap(dev, dev_cap, nvfs))
3490 goto err_fw;
Same
3491 }
3492 }
3493
3494 err = mlx4_init_hca(dev);
3495 if (err) {
3496 if (err == -EACCES) {
3497 /* Not primary Physical function
3498 * Running in slave mode */
3499 mlx4_cmd_cleanup(dev, MLX4_CMD_CLEANUP_ALL);
3500 /* We're not a PF */
3501 if (dev->flags & MLX4_FLAG_SRIOV) {
3502 if (!existing_vfs)
3503 pci_disable_sriov(pdev);
3504 if (mlx4_is_master(dev) && !reset_flow)
3505 atomic_dec(&pf_loading);
3506 dev->flags &= ~MLX4_FLAG_SRIOV;
3507 }
3508 if (!mlx4_is_slave(dev))
3509 mlx4_free_ownership(dev);
3510 dev->flags |= MLX4_FLAG_SLAVE;
3511 dev->flags &= ~MLX4_FLAG_MASTER;
3512 goto slave_start;
3513 } else
3514 goto err_fw;
3515 }
3516
3517 if (mlx4_is_master(dev) && (dev_cap->flags2 & MLX4_DEV_CAP_FLAG2_SYS_EQS)) {
3518 u64 dev_flags = mlx4_enable_sriov(dev, pdev, total_vfs,
3519 existing_vfs, reset_flow);
3520
3521 if ((dev->flags ^ dev_flags) & (MLX4_FLAG_MASTER | MLX4_FLAG_SLAVE)) {
3522 mlx4_cmd_cleanup(dev, MLX4_CMD_CLEANUP_VHCR);
3523 dev->flags = dev_flags;
3524 err = mlx4_cmd_init(dev);
3525 if (err) {
3526 /* Only VHCR is cleaned up, so could still
3527 * send FW commands
3528 */
3529 mlx4_err(dev, "Failed to init VHCR command interface, aborting\n");
3530 goto err_close;
3531 }
3532 } else {
3533 dev->flags = dev_flags;
3534 }
3535
3536 if (!SRIOV_VALID_STATE(dev->flags)) {
3537 mlx4_err(dev, "Invalid SRIOV state\n");
3538 err = -EINVAL;
3539 goto err_close;
3540 }
3541 }
3542
3543 /* check if the device is functioning at its maximum possible speed.
3544 * No return code for this call, just warn the user in case of PCI
3545 * express device capabilities are under-satisfied by the bus.
3546 */
3547 if (!mlx4_is_slave(dev))
3548 pcie_print_link_status(dev->persist->pdev);
3549
3550 /* In master functions, the communication channel must be initialized
3551 * after obtaining its address from fw */
3552 if (mlx4_is_master(dev)) {
3553 if (dev->caps.num_ports < 2 &&
3554 num_vfs_argc > 1) {
3555 err = -EINVAL;
3556 mlx4_err(dev,
3557 "Error: Trying to configure VFs on port 2, but HCA has only %d physical ports\n",
3558 dev->caps.num_ports);
3559 goto err_close;
3560 }
3561 memcpy(dev->persist->nvfs, nvfs, sizeof(dev->persist->nvfs));
3562
3563 for (i = 0;
3564 i < sizeof(dev->persist->nvfs)/
3565 sizeof(dev->persist->nvfs[0]); i++) {
3566 unsigned j;
3567
3568 for (j = 0; j < dev->persist->nvfs[i]; ++sum, ++j) {
3569 dev->dev_vfs[sum].min_port = i < 2 ? i + 1 : 1;
3570 dev->dev_vfs[sum].n_ports = i < 2 ? 1 :
3571 dev->caps.num_ports;
3572 }
3573 }
3574
3575 /* In master functions, the communication channel
3576 * must be initialized after obtaining its address from fw
3577 */
3578 err = mlx4_multi_func_init(dev);
3579 if (err) {
3580 mlx4_err(dev, "Failed to init master mfunc interface, aborting.\n");
3581 goto err_close;
3582 }
3583 }
3584
3585 err = mlx4_alloc_eq_table(dev);
3586 if (err)
3587 goto err_master_mfunc;
3588
3589 bitmap_zero(priv->msix_ctl.pool_bm, MAX_MSIX);
3590 mutex_init(&priv->msix_ctl.pool_lock);
3591
3592 mlx4_enable_msi_x(dev);
3593 if ((mlx4_is_mfunc(dev)) &&
3594 !(dev->flags & MLX4_FLAG_MSI_X)) {
3595 err = -EOPNOTSUPP;
3596 mlx4_err(dev, "INTx is not supported in multi-function mode, aborting\n");
3597 goto err_free_eq;
3598 }
3599
3600 if (!mlx4_is_slave(dev)) {
3601 err = mlx4_init_steering(dev);
3602 if (err)
3603 goto err_disable_msix;
3604 }
3605
3606 mlx4_init_quotas(dev);
3607
3608 err = mlx4_setup_hca(dev);
3609 if (err == -EBUSY && (dev->flags & MLX4_FLAG_MSI_X) &&
3610 !mlx4_is_mfunc(dev)) {
3611 dev->flags &= ~MLX4_FLAG_MSI_X;
3612 dev->caps.num_comp_vectors = 1;
3613 pci_disable_msix(pdev);
3614 err = mlx4_setup_hca(dev);
3615 }
3616
3617 if (err)
3618 goto err_steer;
3619
3620 /* When PF resources are ready arm its comm channel to enable
3621 * getting commands
3622 */
3623 if (mlx4_is_master(dev)) {
3624 err = mlx4_ARM_COMM_CHANNEL(dev);
3625 if (err) {
3626 mlx4_err(dev, " Failed to arm comm channel eq: %x\n",
3627 err);
3628 goto err_steer;
3629 }
3630 }
3631
3632 for (port = 1; port <= dev->caps.num_ports; port++) {
3633 err = mlx4_init_port_info(dev, port);
3634 if (err)
3635 goto err_port;
3636 }
3637
3638 priv->v2p.port1 = 1;
3639 priv->v2p.port2 = 2;
3640
3641 err = mlx4_register_device(dev);
3642 if (err)
3643 goto err_port;
3644
3645 mlx4_request_modules(dev);
3646
3647 mlx4_sense_init(dev);
3648 mlx4_start_sense(dev);
3649
3650 priv->removed = 0;
3651
3652 if (mlx4_is_master(dev) && dev->persist->num_vfs && !reset_flow)
3653 atomic_dec(&pf_loading);
3654
3655 kfree(dev_cap);
3656 return 0;
3657
3658 err_port:
3659 for (--port; port >= 1; --port)
3660 mlx4_cleanup_port_info(&priv->port[port]);
3661
3662 mlx4_cleanup_default_counters(dev);
3663 if (!mlx4_is_slave(dev))
3664 mlx4_cleanup_counters_table(dev);
3665 mlx4_cleanup_qp_table(dev);
3666 mlx4_cleanup_srq_table(dev);
3667 mlx4_cleanup_cq_table(dev);
3668 mlx4_cmd_use_polling(dev);
3669 mlx4_cleanup_eq_table(dev);
3670 mlx4_cleanup_mcg_table(dev);
3671 mlx4_cleanup_mr_table(dev);
3672 mlx4_cleanup_xrcd_table(dev);
3673 mlx4_cleanup_pd_table(dev);
3674 mlx4_cleanup_uar_table(dev);
3675
3676 err_steer:
3677 if (!mlx4_is_slave(dev))
3678 mlx4_clear_steering(dev);
3679
3680 err_disable_msix:
3681 if (dev->flags & MLX4_FLAG_MSI_X)
3682 pci_disable_msix(pdev);
3683
3684 err_free_eq:
3685 mlx4_free_eq_table(dev);
3686
3687 err_master_mfunc:
3688 if (mlx4_is_master(dev)) {
3689 mlx4_free_resource_tracker(dev, RES_TR_FREE_STRUCTS_ONLY);
3690 mlx4_multi_func_cleanup(dev);
3691 }
3692
3693 if (mlx4_is_slave(dev))
3694 mlx4_slave_destroy_special_qp_cap(dev);
3695
3696 err_close:
3697 mlx4_close_hca(dev);
3698
3699 err_fw:
3700 mlx4_close_fw(dev);
3701
3702 err_mfunc:
3703 if (mlx4_is_slave(dev))
3704 mlx4_multi_func_cleanup(dev);
3705
3706 err_cmd:
3707 mlx4_cmd_cleanup(dev, MLX4_CMD_CLEANUP_ALL);
3708
3709 err_sriov:
3710 if (dev->flags & MLX4_FLAG_SRIOV && !existing_vfs) {
3711 pci_disable_sriov(pdev);
3712 dev->flags &= ~MLX4_FLAG_SRIOV;
3713 }
3714
3715 if (mlx4_is_master(dev) && dev->persist->num_vfs && !reset_flow)
3716 atomic_dec(&pf_loading);
3717
3718 kfree(priv->dev.dev_vfs);
3719
3720 if (!mlx4_is_slave(dev))
3721 mlx4_free_ownership(dev);
3722
3723 kfree(dev_cap);
3724 return err;
3725 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] net/mlx4_core: Support more than 64 VFs
2022-04-22 15:41 [bug report] net/mlx4_core: Support more than 64 VFs Dan Carpenter
@ 2022-05-23 12:18 ` Leon Romanovsky
0 siblings, 0 replies; 2+ messages in thread
From: Leon Romanovsky @ 2022-05-23 12:18 UTC (permalink / raw)
To: Dan Carpenter; +Cc: matanb, linux-rdma
On Fri, Apr 22, 2022 at 06:41:12PM +0300, Dan Carpenter wrote:
> Hello Matan Barak,
>
> The patch de966c592802: "net/mlx4_core: Support more than 64 VFs"
> from Nov 13, 2014, leads to the following Smatch static checker
> warning:
>
> drivers/net/ethernet/mellanox/mlx4/main.c:3455 mlx4_load_one()
> warn: missing error code 'err'
>
> drivers/net/ethernet/mellanox/mlx4/main.c
> 3438 if (mlx4_is_master(dev)) {
> 3439 /* when we hit the goto slave_start below, dev_cap already initialized */
> 3440 if (!dev_cap) {
> 3441 dev_cap = kzalloc(sizeof(*dev_cap), GFP_KERNEL);
> 3442
> 3443 if (!dev_cap) {
> 3444 err = -ENOMEM;
> 3445 goto err_fw;
> 3446 }
> 3447
> 3448 err = mlx4_QUERY_DEV_CAP(dev, dev_cap);
> 3449 if (err) {
> 3450 mlx4_err(dev, "QUERY_DEV_CAP command failed, aborting.\n");
> 3451 goto err_fw;
> 3452 }
> 3453
> 3454 if (mlx4_check_dev_cap(dev, dev_cap, nvfs))
> --> 3455 goto err_fw;
> ^^^^^^^^^^^^
> Should this have an error code?
I don't think so. Failure here means that more than 64 VFs were
requested for unsupported device. In such case, we will perform same
flow as for failure in SR-IOV a couple of lines below - graceful exit.
It is hard to say if it is correct behaviour, but I won't change
anything for such old driver.
Thanks
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-05-23 12:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-22 15:41 [bug report] net/mlx4_core: Support more than 64 VFs Dan Carpenter
2022-05-23 12:18 ` Leon Romanovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.