All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kernel-janitors@vger.kernel.org
Subject: [bug report] hinic: add mailbox function support
Date: Fri, 19 Jun 2020 10:03:27 +0000	[thread overview]
Message-ID: <20200619100327.GA245452@mwanda> (raw)

Hello Luo bin,

The patch a425b6e1c69b: "hinic: add mailbox function support" from
Apr 25, 2020, leads to the following static checker warning:

	drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c:817 hinic_init_hwdev()
	error: passing non negative 65280 to ERR_PTR

drivers/net/ethernet/huawei/hinic/hinic_hw_dev.c
   716  struct hinic_hwdev *hinic_init_hwdev(struct pci_dev *pdev)
   717  {
   718          struct hinic_pfhwdev *pfhwdev;
   719          struct hinic_hwdev *hwdev;
   720          struct hinic_hwif *hwif;
   721          int err, num_aeqs;
   722  
   723          hwif = devm_kzalloc(&pdev->dev, sizeof(*hwif), GFP_KERNEL);
   724          if (!hwif)
   725                  return ERR_PTR(-ENOMEM);
   726  
   727          err = hinic_init_hwif(hwif, pdev);
   728          if (err) {
   729                  dev_err(&pdev->dev, "Failed to init HW interface\n");
   730                  return ERR_PTR(err);
   731          }
   732  
   733          pfhwdev = devm_kzalloc(&pdev->dev, sizeof(*pfhwdev), GFP_KERNEL);
   734          if (!pfhwdev) {
   735                  err = -ENOMEM;
   736                  goto err_pfhwdev_alloc;
   737          }
   738  
   739          hwdev = &pfhwdev->hwdev;
   740          hwdev->hwif = hwif;
   741  
   742          err = init_msix(hwdev);
   743          if (err) {
   744                  dev_err(&pdev->dev, "Failed to init msix\n");
   745                  goto err_init_msix;
   746          }
   747  
   748          err = wait_for_outbound_state(hwdev);
   749          if (err) {
   750                  dev_warn(&pdev->dev, "outbound - disabled, try again\n");
   751                  hinic_outbound_state_set(hwif, HINIC_OUTBOUND_ENABLE);
   752          }
   753  
   754          num_aeqs = HINIC_HWIF_NUM_AEQS(hwif);
   755  
   756          err = hinic_aeqs_init(&hwdev->aeqs, hwif, num_aeqs,
   757                                HINIC_DEFAULT_AEQ_LEN, HINIC_EQ_PAGE_SIZE,
   758                                hwdev->msix_entries);
   759          if (err) {
   760                  dev_err(&pdev->dev, "Failed to init async event queues\n");
   761                  goto err_aeqs_init;
   762          }
   763  
   764          err = init_pfhwdev(pfhwdev);
   765          if (err) {
   766                  dev_err(&pdev->dev, "Failed to init PF HW device\n");
   767                  goto err_init_pfhwdev;
   768          }
   769  
   770          err = hinic_l2nic_reset(hwdev);
   771          if (err)
   772                  goto err_l2nic_reset;
   773  
   774          err = get_dev_cap(hwdev);
                ^^^^^^^^^^^^^^^^^^^^^^^^
This function sometimes returns positive values on error which will
lead to an Oops eventually.

   775          if (err) {
   776                  dev_err(&pdev->dev, "Failed to get device capabilities\n");
   777                  goto err_dev_cap;
   778          }
   779  
   780          err = hinic_vf_func_init(hwdev);
   781          if (err) {
   782                  dev_err(&pdev->dev, "Failed to init nic mbox\n");
   783                  goto err_vf_func_init;
   784          }
   785  
   786          err = init_fw_ctxt(hwdev);
   787          if (err) {
   788                  dev_err(&pdev->dev, "Failed to init function table\n");
   789                  goto err_init_fw_ctxt;
   790          }
   791  
   792          err = set_resources_state(hwdev, HINIC_RES_ACTIVE);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Same here according to Smatch.

   793          if (err) {
   794                  dev_err(&pdev->dev, "Failed to set resources state\n");
   795                  goto err_resources_state;
   796          }
   797  
   798          return hwdev;
   799  
   800  err_resources_state:
   801  err_init_fw_ctxt:
   802          hinic_vf_func_free(hwdev);
   803  err_vf_func_init:
   804  err_l2nic_reset:
   805  err_dev_cap:
   806          free_pfhwdev(pfhwdev);
   807  
   808  err_init_pfhwdev:
   809          hinic_aeqs_free(&hwdev->aeqs);
   810  
   811  err_aeqs_init:
   812          disable_msix(hwdev);
   813  
   814  err_init_msix:
   815  err_pfhwdev_alloc:
   816          hinic_free_hwif(hwif);
   817          return ERR_PTR(err);
                               ^^^
If "err" isn't a negative error code it will lead to an Oops in the
caller.

   818  }

One function which will cause an Oops is mbox_resp_info_handler():

drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c
   832  static int mbox_resp_info_handler(struct hinic_mbox_func_to_func *func_to_func,
   833                                    struct hinic_recv_mbox *mbox_for_resp,
   834                                    enum hinic_mod_type mod, u16 cmd,
   835                                    void *buf_out, u16 *out_size)
   836  {
   837          int err;
   838  
   839          if (mbox_for_resp->msg_info.status) {
   840                  err = mbox_for_resp->msg_info.status;
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

   841                  if (err != HINIC_MBOX_PF_BUSY_ACTIVE_FW)
   842                          dev_err(&func_to_func->hwif->pdev->dev, "Mbox response error(0x%x)\n",
   843                                  mbox_for_resp->msg_info.status);
   844                  return err;
                               ^^^
Here err is a u8 so it can't be a negative error code.

   845          }
   846  
   847          if (buf_out && out_size) {
   848                  if (*out_size < mbox_for_resp->mbox_len) {
   849                          dev_err(&func_to_func->hwif->pdev->dev,
   850                                  "Invalid response mbox message length: %d for mod %d cmd %d, should less than: %d\n",
   851                                  mbox_for_resp->mbox_len, mod, cmd, *out_size);
   852                          return -EFAULT;
   853                  }
   854  
   855                  if (mbox_for_resp->mbox_len)
   856                          memcpy(buf_out, mbox_for_resp->mbox,
   857                                 mbox_for_resp->mbox_len);
   858  
   859                  *out_size = mbox_for_resp->mbox_len;
   860          }
   861  
   862          return 0;
   863  }

I guess the main problem is functions like send_mbox_seg() which do:

drivers/net/ethernet/huawei/hinic/hinic_hw_mbox.c
   662  static int send_mbox_seg(struct hinic_mbox_func_to_func *func_to_func,
   663                           u64 header, u16 dst_func, void *seg, u16 seg_len,
   664                           int poll, void *msg_info)
   665  {
   666          struct hinic_send_mbox *send_mbox = &func_to_func->send_mbox;
   667          u16 seq_dir = HINIC_MBOX_HEADER_GET(header, DIRECTION);
   668          struct hinic_hwdev *hwdev = func_to_func->hwdev;
   669          struct completion *done = &send_mbox->send_done;
   670          u8 num_aeqs = hwdev->hwif->attr.num_aeqs;
   671          u16 dst_aeqn, wb_status = 0, errcode;
   672  
   673          if (num_aeqs >= 4)
   674                  dst_aeqn = (seq_dir = HINIC_HWIF_DIRECT_SEND) ?
   675                             HINIC_MBOX_RECV_AEQN : HINIC_MBOX_RSP_AEQN;
   676          else
   677                  dst_aeqn = 0;
   678  
   679          if (!poll)
   680                  init_completion(done);
   681  
   682          clear_mbox_status(send_mbox);
   683  
   684          mbox_copy_header(hwdev, send_mbox, &header);
   685  
   686          mbox_copy_send_data(hwdev, send_mbox, seg, seg_len);
   687  
   688          write_mbox_msg_attr(func_to_func, dst_func, dst_aeqn, seg_len, poll);
   689  
   690          wmb(); /* writing the mbox msg attributes */
   691  
   692          if (wait_for_mbox_seg_completion(func_to_func, poll, &wb_status))
   693                  return -ETIMEDOUT;
   694  
   695          if (!MBOX_STATUS_SUCCESS(wb_status)) {
   696                  dev_err(&hwdev->hwif->pdev->dev, "Send mailbox segment to function %d error, wb status: 0x%x\n",
   697                          dst_func, wb_status);
   698                  errcode = MBOX_STATUS_ERRCODE(wb_status);
   699                  return errcode ? errcode : -EFAULT;
                                         ^^^^^^^
Positive error code leads to Oops.

   700          }
   701  
   702          return 0;
   703  }

I'm not sure if that's every problem, but it's a start.  ;)

regards,
dan carpenter

             reply	other threads:[~2020-06-19 10:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-19 10:03 Dan Carpenter [this message]
2020-06-20  1:26 ` [bug report] hinic: add mailbox function support luobin (L)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200619100327.GA245452@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.