Re: [PATCH v10 7/9] misc: bcm-vk: add Broadcom VK driver

2020-07-07 Thread Scott Branden




On 2020-07-07 5:03 p.m., Kees Cook wrote:

On Mon, Jul 06, 2020 at 04:23:07PM -0700, Scott Branden wrote:

Add Broadcom VK driver offload engine.
This driver interfaces to the VK PCIe offload engine to perform
should offload functions as video transcoding on multiple streams
in parallel.  VK device is booted from files loaded using
request_firmware_into_buf mechanism.  After booted card status is updated
and messages can then be sent to the card.
Such messages contain scatter gather list of addresses
to pull data from the host to perform operations on.

Signed-off-by: Scott Branden 
Signed-off-by: Desmond Yan 

nit: your S-o-b chain doesn't make sense (I would expect you at the end
since you're sending it and showing as the Author). Is it Co-developed-by?
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Yes, Co-developed-by.  Will adjust.



[...]
+
+   max_buf = SZ_4M;
+   bufp = dma_alloc_coherent(dev,
+ max_buf,
+ _dma_addr, GFP_KERNEL);
+   if (!bufp) {
+   dev_err(dev, "Error allocating 0x%zx\n", max_buf);
+   ret = -ENOMEM;
+   goto err_buf_out;
+   }
+
+   bcm_vk_buf_notify(vk, bufp, boot_dma_addr, max_buf);
+   } else {
+   dev_err(dev, "Error invalid image type 0x%x\n", load_type);
+   ret = -EINVAL;
+   goto err_buf_out;
+   }
+
+   ret = request_partial_firmware_into_buf(, filename, dev,
+   bufp, max_buf, 0);

Unless I don't understand what's happening here, this needs to be
reordered if you're going to keep Mimi happy and disallow the device
being able to see the firmware before it has been verified. (i.e. please
load the firmware before mapping DMA across the buffer.)
I don't understand your concern here.  We request partial firmware into 
a buffer that we allocated.
After loading it we signal the card the firmware has been loaded into 
that memory region.
The card then pulls the data into its internal memory.  And, 
authenticates it.


Even if the card randomly read and writes to that buffer it shouldn't 
matter to the linux kernel security subsystem.

It passed the security check already when placed in the buffer.
If there is a concern could we add an "nosecurity" 
request_partial_firmware_into_buf instead as there is no need for any 
security on this particular request?




Re: [PATCH v10 7/9] misc: bcm-vk: add Broadcom VK driver

2020-07-07 Thread Kees Cook
On Mon, Jul 06, 2020 at 04:23:07PM -0700, Scott Branden wrote:
> Add Broadcom VK driver offload engine.
> This driver interfaces to the VK PCIe offload engine to perform
> should offload functions as video transcoding on multiple streams
> in parallel.  VK device is booted from files loaded using
> request_firmware_into_buf mechanism.  After booted card status is updated
> and messages can then be sent to the card.
> Such messages contain scatter gather list of addresses
> to pull data from the host to perform operations on.
> 
> Signed-off-by: Scott Branden 
> Signed-off-by: Desmond Yan 

nit: your S-o-b chain doesn't make sense (I would expect you at the end
since you're sending it and showing as the Author). Is it Co-developed-by?
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> [...]
> +
> + max_buf = SZ_4M;
> + bufp = dma_alloc_coherent(dev,
> +   max_buf,
> +   _dma_addr, GFP_KERNEL);
> + if (!bufp) {
> + dev_err(dev, "Error allocating 0x%zx\n", max_buf);
> + ret = -ENOMEM;
> + goto err_buf_out;
> + }
> +
> + bcm_vk_buf_notify(vk, bufp, boot_dma_addr, max_buf);
> + } else {
> + dev_err(dev, "Error invalid image type 0x%x\n", load_type);
> + ret = -EINVAL;
> + goto err_buf_out;
> + }
> +
> + ret = request_partial_firmware_into_buf(, filename, dev,
> + bufp, max_buf, 0);

Unless I don't understand what's happening here, this needs to be
reordered if you're going to keep Mimi happy and disallow the device
being able to see the firmware before it has been verified. (i.e. please
load the firmware before mapping DMA across the buffer.)

-- 
Kees Cook


[PATCH v10 7/9] misc: bcm-vk: add Broadcom VK driver

2020-07-06 Thread Scott Branden
Add Broadcom VK driver offload engine.
This driver interfaces to the VK PCIe offload engine to perform
should offload functions as video transcoding on multiple streams
in parallel.  VK device is booted from files loaded using
request_firmware_into_buf mechanism.  After booted card status is updated
and messages can then be sent to the card.
Such messages contain scatter gather list of addresses
to pull data from the host to perform operations on.

Signed-off-by: Scott Branden 
Signed-off-by: Desmond Yan 
Signed-off-by: James Hu 
---
 drivers/misc/Kconfig |1 +
 drivers/misc/Makefile|1 +
 drivers/misc/bcm-vk/Kconfig  |   29 +
 drivers/misc/bcm-vk/Makefile |   11 +
 drivers/misc/bcm-vk/bcm_vk.h |  419 +
 drivers/misc/bcm-vk/bcm_vk_dev.c | 1357 +++
 drivers/misc/bcm-vk/bcm_vk_msg.c | 1504 ++
 drivers/misc/bcm-vk/bcm_vk_msg.h |  211 +
 drivers/misc/bcm-vk/bcm_vk_sg.c  |  275 ++
 drivers/misc/bcm-vk/bcm_vk_sg.h  |   61 ++
 drivers/misc/bcm-vk/bcm_vk_tty.c |  352 +++
 11 files changed, 4221 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
 create mode 100644 drivers/misc/bcm-vk/bcm_vk_msg.c
 create mode 100644 drivers/misc/bcm-vk/bcm_vk_msg.h
 create mode 100644 drivers/misc/bcm-vk/bcm_vk_sg.c
 create mode 100644 drivers/misc/bcm-vk/bcm_vk_sg.h
 create mode 100644 drivers/misc/bcm-vk/bcm_vk_tty.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 3ca4325cc191..13ed2daf7b5e 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 ..a3a020b19e3b
--- /dev/null
+++ b/drivers/misc/bcm-vk/Kconfig
@@ -0,0 +1,29 @@
+# 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.
+
+if BCM_VK
+
+config BCM_VK_QSTATS
+   bool "VK Queue Statistics"
+   help
+ Turn on to enable Queue Statistics.
+ These are useful for debugging purposes.
+ Some performance loss by enabling this debug config.
+ For properly operating PCIe hardware no need to enable this.
+
+ If unsure, say N.
+
+endif
diff --git a/drivers/misc/bcm-vk/Makefile b/drivers/misc/bcm-vk/Makefile
new file mode 100644
index ..05cb213ee826
--- /dev/null
+++ b/drivers/misc/bcm-vk/Makefile
@@ -0,0 +1,11 @@
+# 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 \
+   bcm_vk_msg.o \
+   bcm_vk_sg.o \
+   bcm_vk_tty.o
diff --git a/drivers/misc/bcm-vk/bcm_vk.h b/drivers/misc/bcm-vk/bcm_vk.h
new file mode 100644
index ..2ecddcc5636f
--- /dev/null
+++ b/drivers/misc/bcm-vk/bcm_vk.h
@@ -0,0 +1,419 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2018-2020 Broadcom.
+ */
+
+#ifndef BCM_VK_H
+#define BCM_VK_H
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "bcm_vk_msg.h"
+
+#define DRV_MODULE_NAME"bcm-vk"
+
+/*
+ * Load Image is completed in two stages:
+ *
+ * 1) When the VK device boot-up, M7 CPU runs and executes the BootROM.
+ * The Secure Boot Loader (SBL) as part of the BootROM will run
+ * to open up ITCM for host to push BOOT1 image.
+ * SBL will authenticate the image before jumping to BOOT1 image.
+ *
+ * 2) Because BOOT1 image is a secured image, we also called it the
+ * Secure Boot Image (SBI). At second stage, SBI