| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2c0bd222-a0d1-377e-766b-e44afc90d5d3@broadcom.com>
Date: Tue, 29 Sep 2020 16:51:43 -0700
From: Scott Branden <scott.branden@...adcom.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Arnd Bergmann <arnd@...db.de>, Kees Cook <keescook@...omium.org>,
linux-kernel@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com
Subject: Re: [PATCH v4 02/16] misc: bcm-vk: add Broadcom VK driver
Hi Greg,
On 2020-09-29 3:28 a.m., Greg Kroah-Hartman wrote:
> On Mon, Sep 28, 2020 at 05:11:55PM -0700, Scott Branden wrote:
>> Add initial version of Broadcom VK driver to enumerate PCI device IDs
>> of Valkyrie and Viper device IDs.
>>
>> VK based cards provide real-time high performance, high throughput,
>> low latency offload compute engine operations.
>> They are used for multiple parallel offload tasks as:
>> audio, video and image processing and crypto operations.
> As we have had this come up for other offload engine devices, we need a
> pointer to the userspace code for this as well, so that we can properly
> review the whole thing together. Otherwise this is just one half, the
> other is a black-box that is pretty impossible to understand :)
We've been meaning to publish the userspace code.
Here is the vk interface library, vkil, and associated unittest and
card utilities.
https://github.com/Broadcom/vkil/
>
>
>> Further commits add additional features to driver beyond probe/remove.
>>
>> Signed-off-by: Scott Branden <scott.branden@...adcom.com>
>> ---
>> drivers/misc/Kconfig | 1 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/bcm-vk/Kconfig | 15 ++++
>> drivers/misc/bcm-vk/Makefile | 8 ++
>> drivers/misc/bcm-vk/bcm_vk.h | 29 +++++++
>> drivers/misc/bcm-vk/bcm_vk_dev.c | 137 +++++++++++++++++++++++++++++++
>> 6 files changed, 191 insertions(+)
>> create mode 100644 drivers/misc/bcm-vk/Kconfig
>> create mode 100644 drivers/misc/bcm-vk/Makefile
>> create mode 100644 drivers/misc/bcm-vk/bcm_vk.h
>> create mode 100644 drivers/misc/bcm-vk/bcm_vk_dev.c
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index ce136d685d14..9d42b5def81b 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -469,6 +469,7 @@ source "drivers/misc/genwqe/Kconfig"
>> source "drivers/misc/echo/Kconfig"
>> source "drivers/misc/cxl/Kconfig"
>> source "drivers/misc/ocxl/Kconfig"
>> +source "drivers/misc/bcm-vk/Kconfig"
>> source "drivers/misc/cardreader/Kconfig"
>> source "drivers/misc/habanalabs/Kconfig"
>> source "drivers/misc/uacce/Kconfig"
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index c7bd01ac6291..766837e4b961 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -52,6 +52,7 @@ obj-$(CONFIG_ECHO) += echo/
>> obj-$(CONFIG_CXL_BASE) += cxl/
>> obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
>> obj-$(CONFIG_OCXL) += ocxl/
>> +obj-$(CONFIG_BCM_VK) += bcm-vk/
>> obj-y += cardreader/
>> obj-$(CONFIG_PVPANIC) += pvpanic.o
>> obj-$(CONFIG_HABANA_AI) += habanalabs/
>> diff --git a/drivers/misc/bcm-vk/Kconfig b/drivers/misc/bcm-vk/Kconfig
>> new file mode 100644
>> index 000000000000..2272e47655ed
>> --- /dev/null
>> +++ b/drivers/misc/bcm-vk/Kconfig
>> @@ -0,0 +1,15 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Broadcom VK device
>> +#
>> +config BCM_VK
>> + tristate "Support for Broadcom VK Accelerators"
>> + depends on PCI_MSI
>> + help
>> + Select this option to enable support for Broadcom
>> + VK Accelerators. VK is used for performing
>> + specific offload processing.
>> + This driver enables userspace programs to access these
>> + accelerators via /dev/bcm-vk.N devices.
>> +
>> + If unsure, say N.
>> diff --git a/drivers/misc/bcm-vk/Makefile b/drivers/misc/bcm-vk/Makefile
>> new file mode 100644
>> index 000000000000..f8a7ac4c242f
>> --- /dev/null
>> +++ b/drivers/misc/bcm-vk/Makefile
>> @@ -0,0 +1,8 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Makefile for Broadcom VK driver
>> +#
>> +
>> +obj-$(CONFIG_BCM_VK) += bcm_vk.o
>> +bcm_vk-objs := \
>> + bcm_vk_dev.o
>> diff --git a/drivers/misc/bcm-vk/bcm_vk.h b/drivers/misc/bcm-vk/bcm_vk.h
>> new file mode 100644
>> index 000000000000..9152785199ab
>> --- /dev/null
>> +++ b/drivers/misc/bcm-vk/bcm_vk.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright 2018-2020 Broadcom.
>> + */
>> +
>> +#ifndef BCM_VK_H
>> +#define BCM_VK_H
>> +
>> +#include <linux/pci.h>
>> +
>> +#define DRV_MODULE_NAME "bcm-vk"
>> +
>> +/* VK device supports a maximum of 3 bars */
>> +#define MAX_BAR 3
>> +
>> +enum pci_barno {
>> + BAR_0 = 0,
>> + BAR_1,
>> + BAR_2
>> +};
>> +
>> +#define BCM_VK_NUM_TTY 2
>> +
>> +struct bcm_vk {
>> + struct pci_dev *pdev;
>> + void __iomem *bar[MAX_BAR];
>> +};
>> +
>> +#endif
>> diff --git a/drivers/misc/bcm-vk/bcm_vk_dev.c b/drivers/misc/bcm-vk/bcm_vk_dev.c
>> new file mode 100644
>> index 000000000000..bb24efb1b9fb
>> --- /dev/null
>> +++ b/drivers/misc/bcm-vk/bcm_vk_dev.c
>> @@ -0,0 +1,137 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright 2018-2020 Broadcom.
>> + */
>> +
>> +#include <linux/dma-mapping.h>
>> +#include <linux/module.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci_regs.h>
>> +
>> +#include "bcm_vk.h"
>> +
>> +#define PCI_DEVICE_ID_VALKYRIE 0x5e87
>> +#define PCI_DEVICE_ID_VIPER 0x5e88
>> +
>> +/* MSIX usages */
>> +#define VK_MSIX_MSGQ_MAX 3
>> +#define VK_MSIX_NOTF_MAX 1
>> +#define VK_MSIX_TTY_MAX BCM_VK_NUM_TTY
>> +#define VK_MSIX_IRQ_MAX (VK_MSIX_MSGQ_MAX + VK_MSIX_NOTF_MAX + \
>> + VK_MSIX_TTY_MAX)
>> +#define VK_MSIX_IRQ_MIN_REQ (VK_MSIX_MSGQ_MAX + VK_MSIX_NOTF_MAX)
>> +
>> +/* Number of bits set in DMA mask*/
>> +#define BCM_VK_DMA_BITS 64
>> +
>> +static int bcm_vk_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> +{
>> + int err;
>> + int i;
>> + int irq;
>> + struct bcm_vk *vk;
>> + struct device *dev = &pdev->dev;
>> +
>> + vk = kzalloc(sizeof(*vk), GFP_KERNEL);
>> + if (!vk)
>> + return -ENOMEM;
>> +
>> + err = pci_enable_device(pdev);
>> + if (err) {
>> + dev_err(dev, "Cannot enable PCI device\n");
>> + return err;
> You just leaked memory :(
Yes, missing kfree(vk).
>
> That was a short review, I didn't get all that far...
>
>> + }
>> + vk->pdev = pci_dev_get(pdev);
>> +
>> + err = pci_request_regions(pdev, DRV_MODULE_NAME);
>> + if (err) {
>> + dev_err(dev, "Cannot obtain PCI resources\n");
>> + goto err_disable_pdev;
>> + }
>> +
>> + /* make sure DMA is good */
>> + err = dma_set_mask_and_coherent(&pdev->dev,
>> + DMA_BIT_MASK(BCM_VK_DMA_BITS));
>> + if (err) {
>> + dev_err(dev, "failed to set DMA mask\n");
>> + goto err_disable_pdev;
>> + }
>> +
>> + pci_set_master(pdev);
>> + pci_set_drvdata(pdev, vk);
>> +
>> + irq = pci_alloc_irq_vectors(pdev,
>> + 1,
>> + VK_MSIX_IRQ_MAX,
>> + PCI_IRQ_MSI | PCI_IRQ_MSIX);
>> +
>> + if (irq < VK_MSIX_IRQ_MIN_REQ) {
>> + dev_err(dev, "failed to get min %d MSIX interrupts, irq(%d)\n",
>> + VK_MSIX_IRQ_MIN_REQ, irq);
>> + err = (irq >= 0) ? -EINVAL : irq;
>> + goto err_disable_pdev;
>> + }
>> +
>> + dev_info(dev, "Number of IRQs %d allocated - requested(%d).\n",
>> + irq, VK_MSIX_IRQ_MAX);
> Drivers are quiet when all works properly.
Will change to dev_warn when number allocated != number requested
and dev_dbg otherwise.
>
>> +
>> + for (i = 0; i < MAX_BAR; i++) {
>> + /* multiple by 2 for 64 bit BAR mapping */
>> + vk->bar[i] = pci_ioremap_bar(pdev, i * 2);
>> + if (!vk->bar[i]) {
>> + dev_err(dev, "failed to remap BAR%d\n", i);
>> + goto err_iounmap;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_iounmap:
>> + for (i = 0; i < MAX_BAR; i++) {
>> + if (vk->bar[i])
>> + pci_iounmap(pdev, vk->bar[i]);
>> + }
>> + pci_release_regions(pdev);
>> +
>> +err_disable_pdev:
>> + pci_free_irq_vectors(pdev);
>> + pci_disable_device(pdev);
>> + pci_dev_put(pdev);
>> +
>> + return err;
> Again, memory is leaked :(
Yes, will kfree.
>
> thanks,
>
> greg k-h
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4169 bytes)
Powered by blists - more mailing lists