[PATCH 0/5] vfio/pci: add blocklist and disable qat

[PATCH 0/5] vfio/pci: add blocklist and disable qat

[Giovanni Cabiddu]

This patchset defines a blocklist of devices in the vfio-pci module and adds
the current generation of Intel(R) QuickAssist devices to it as they are
not designed to run in an untrusted environment.

By default, if a device is in the blocklist, the probe of vfio-pci fails.
If a user wants to use a device in the blocklist, he needs to disable the
full blocklist providing the option disable_blocklist=1 at the load of
vfio-pci or specifying that parameter in a config file in /etc/modprobe.d.

This series also moves the device ids definitions present in the qat driver
to linux/pci_ids.h since they will be shared between the vfio-pci and the qat
drivers and replaces the custom ADF_SYSTEM_DEVICE macro with PCI_VDEVICE.

The series is applicable to Herbert's tree but only partially applicable to
Alex's tree due to a merge conflict.

Giovanni Cabiddu (5):
  PCI: add Intel QuickAssist device IDs
  vfio/pci: add device blocklist
  vfio/pci: add qat devices to blocklist
  crypto: qat - replace device ids defines
  crypto: qat - use PCI_VDEVICE

 drivers/crypto/qat/qat_c3xxx/adf_drv.c        | 11 ++---
 drivers/crypto/qat/qat_c3xxxvf/adf_drv.c      | 11 ++---
 drivers/crypto/qat/qat_c62x/adf_drv.c         | 11 ++---
 drivers/crypto/qat/qat_c62xvf/adf_drv.c       | 11 ++---
 .../crypto/qat/qat_common/adf_accel_devices.h |  6 ---
 drivers/crypto/qat/qat_common/qat_hal.c       |  7 +--
 drivers/crypto/qat/qat_common/qat_uclo.c      |  9 ++--
 drivers/crypto/qat/qat_dh895xcc/adf_drv.c     | 11 ++---
 drivers/crypto/qat/qat_dh895xccvf/adf_drv.c   | 11 ++---
 drivers/vfio/pci/vfio_pci.c                   | 48 +++++++++++++++++++
 include/linux/pci_ids.h                       |  6 +++
 11 files changed, 87 insertions(+), 55 deletions(-)

-- 
2.26.2

[PATCH 1/5] PCI: add Intel QuickAssist device IDs

[Giovanni Cabiddu]

Add device IDs for the following Intel QuickAssist devices: DH895XCC,
C3XXX and C62X.

The defines in this patch are going to be referenced in two independent
drivers, qat and vfio-pci.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 include/linux/pci_ids.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 0ad57693f392..f3166b1425ca 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2659,6 +2659,8 @@
 #define PCI_DEVICE_ID_INTEL_80332_1	0x0332
 #define PCI_DEVICE_ID_INTEL_80333_0	0x0370
 #define PCI_DEVICE_ID_INTEL_80333_1	0x0372
+#define PCI_DEVICE_ID_INTEL_QAT_DH895XCC	0x0435
+#define PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF	0x0443
 #define PCI_DEVICE_ID_INTEL_82375	0x0482
 #define PCI_DEVICE_ID_INTEL_82424	0x0483
 #define PCI_DEVICE_ID_INTEL_82378	0x0484
@@ -2708,6 +2710,8 @@
 #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_NHI     0x1577
 #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE  0x1578
 #define PCI_DEVICE_ID_INTEL_80960_RP	0x1960
+#define PCI_DEVICE_ID_INTEL_QAT_C3XXX	0x19e2
+#define PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF	0x19e3
 #define PCI_DEVICE_ID_INTEL_82840_HB	0x1a21
 #define PCI_DEVICE_ID_INTEL_82845_HB	0x1a30
 #define PCI_DEVICE_ID_INTEL_IOAT	0x1a38
@@ -2924,6 +2928,8 @@
 #define PCI_DEVICE_ID_INTEL_IOAT_JSF7	0x3717
 #define PCI_DEVICE_ID_INTEL_IOAT_JSF8	0x3718
 #define PCI_DEVICE_ID_INTEL_IOAT_JSF9	0x3719
+#define PCI_DEVICE_ID_INTEL_QAT_C62X	0x37c8
+#define PCI_DEVICE_ID_INTEL_QAT_C62X_VF	0x37c9
 #define PCI_DEVICE_ID_INTEL_ICH10_0	0x3a14
 #define PCI_DEVICE_ID_INTEL_ICH10_1	0x3a16
 #define PCI_DEVICE_ID_INTEL_ICH10_2	0x3a18
-- 
2.26.2

[PATCH 2/5] vfio/pci: add device blocklist

[Giovanni Cabiddu]

Add blocklist of devices that by default are not probed by vfio-pci.
Devices in this list may be susceptible to untrusted application, even
if the IOMMU is enabled. To be accessed via vfio-pci, the user has to
explicitly disable the blocklist.

The blocklist can be disabled via the module parameter disable_blocklist.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/vfio/pci/vfio_pci.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 7c0779018b1b..ea5904ca6cbf 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -60,6 +60,10 @@ module_param(enable_sriov, bool, 0644);
 MODULE_PARM_DESC(enable_sriov, "Enable support for SR-IOV configuration.  Enabling SR-IOV on a PF typically requires support of the userspace PF driver, enabling VFs without such support may result in non-functional VFs or PF.");
 #endif
 
+static bool disable_blocklist;
+module_param(disable_blocklist, bool, 0444);
+MODULE_PARM_DESC(disable_blocklist, "Disable device blocklist. If set, i.e. blocklist disabled, then blocklisted devices are allowed to be probed by vfio-pci.");
+
 static inline bool vfio_vga_disabled(void)
 {
 #ifdef CONFIG_VFIO_PCI_VGA
@@ -69,6 +73,29 @@ static inline bool vfio_vga_disabled(void)
 #endif
 }
 
+static bool vfio_pci_dev_in_blocklist(struct pci_dev *pdev)
+{
+	return false;
+}
+
+static bool vfio_pci_is_blocklisted(struct pci_dev *pdev)
+{
+	if (!vfio_pci_dev_in_blocklist(pdev))
+		return false;
+
+	if (disable_blocklist) {
+		pci_warn(pdev,
+			 "device blocklist disabled - allowing device %04x:%04x.\n",
+			 pdev->vendor, pdev->device);
+		return false;
+	}
+
+	pci_warn(pdev, "%04x:%04x is blocklisted - probe will fail.\n",
+		 pdev->vendor, pdev->device);
+
+	return true;
+}
+
 /*
  * Our VGA arbiter participation is limited since we don't know anything
  * about the device itself.  However, if the device is the only VGA device
@@ -1847,6 +1874,9 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct iommu_group *group;
 	int ret;
 
+	if (vfio_pci_is_blocklisted(pdev))
+		return -EINVAL;
+
 	if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
 		return -EINVAL;
 
@@ -2336,6 +2366,9 @@ static int __init vfio_pci_init(void)
 
 	vfio_pci_fill_ids();
 
+	if (disable_blocklist)
+		pr_warn("device blocklist disabled.\n");
+
 	return 0;
 
 out_driver:
-- 
2.26.2

[PATCH 3/5] vfio/pci: add qat devices to blocklist

[Giovanni Cabiddu]

The current generation of Intel® QuickAssist Technology devices
are not designed to run in an untrusted environment because of the
following issues reported in the release notes in
https://01.org/intel-quickassist-technology:

QATE-39220 - GEN - Intel® QAT API submissions with bad addresses that
             trigger DMA to invalid or unmapped addresses can cause a
             platform hang
QATE-7495  - GEN - An incorrectly formatted request to Intel® QAT can
             hang the entire Intel® QAT Endpoint

This patch adds the following QAT devices to the blocklist: DH895XCC,
C3XXX and C62X.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/vfio/pci/vfio_pci.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index ea5904ca6cbf..dcac5408c764 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -75,6 +75,21 @@ static inline bool vfio_vga_disabled(void)
 
 static bool vfio_pci_dev_in_blocklist(struct pci_dev *pdev)
 {
+	switch (pdev->vendor) {
+	case PCI_VENDOR_ID_INTEL:
+		switch (pdev->device) {
+		case PCI_DEVICE_ID_INTEL_QAT_C3XXX:
+		case PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF:
+		case PCI_DEVICE_ID_INTEL_QAT_C62X:
+		case PCI_DEVICE_ID_INTEL_QAT_C62X_VF:
+		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC:
+		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF:
+			return true;
+		default:
+			return false;
+		}
+	}
+
 	return false;
 }
 
-- 
2.26.2

[PATCH 4/5] crypto: qat - replace device ids defines

[Giovanni Cabiddu]

Replace device ids defined in the qat drivers with the ones in
include/linux/pci_ids.h.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_c3xxx/adf_drv.c            | 6 +++---
 drivers/crypto/qat/qat_c3xxxvf/adf_drv.c          | 6 +++---
 drivers/crypto/qat/qat_c62x/adf_drv.c             | 6 +++---
 drivers/crypto/qat/qat_c62xvf/adf_drv.c           | 6 +++---
 drivers/crypto/qat/qat_common/adf_accel_devices.h | 6 ------
 drivers/crypto/qat/qat_common/qat_hal.c           | 7 ++++---
 drivers/crypto/qat/qat_common/qat_uclo.c          | 9 +++++----
 drivers/crypto/qat/qat_dh895xcc/adf_drv.c         | 6 +++---
 drivers/crypto/qat/qat_dh895xccvf/adf_drv.c       | 6 +++---
 9 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/crypto/qat/qat_c3xxx/adf_drv.c b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
index 020d099409e5..bba0f142f7f6 100644
--- a/drivers/crypto/qat/qat_c3xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_C3XXX_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C3XXX),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_C3XXX_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_C3XXX:
 			adf_clean_hw_data_c3xxx(accel_dev->hw_device);
 			break;
 		default:
@@ -83,7 +83,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_C3XXX_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C3XXX:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
index 11039fe55f61..b77a58886599 100644
--- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_C3XXXIOV_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_C3XXXIOV_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF:
 			adf_clean_hw_data_c3xxxiov(accel_dev->hw_device);
 			break;
 		default:
@@ -85,7 +85,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_C3XXXIOV_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
diff --git a/drivers/crypto/qat/qat_c62x/adf_drv.c b/drivers/crypto/qat/qat_c62x/adf_drv.c
index 4ba9c14383af..722838ff03be 100644
--- a/drivers/crypto/qat/qat_c62x/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62x/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_C62X_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C62X),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_C62X_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_C62X:
 			adf_clean_hw_data_c62x(accel_dev->hw_device);
 			break;
 		default:
@@ -83,7 +83,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_C62X_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C62X:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
index b8b021d54bb5..a766cc18aae9 100644
--- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_C62XIOV_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C62X_VF),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_C62XIOV_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_C62X_VF:
 			adf_clean_hw_data_c62xiov(accel_dev->hw_device);
 			break;
 		default:
@@ -85,7 +85,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_C62XIOV_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C62X_VF:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
diff --git a/drivers/crypto/qat/qat_common/adf_accel_devices.h b/drivers/crypto/qat/qat_common/adf_accel_devices.h
index c1db8c26afb6..06952ece53d9 100644
--- a/drivers/crypto/qat/qat_common/adf_accel_devices.h
+++ b/drivers/crypto/qat/qat_common/adf_accel_devices.h
@@ -15,12 +15,6 @@
 #define ADF_C62XVF_DEVICE_NAME "c6xxvf"
 #define ADF_C3XXX_DEVICE_NAME "c3xxx"
 #define ADF_C3XXXVF_DEVICE_NAME "c3xxxvf"
-#define ADF_DH895XCC_PCI_DEVICE_ID 0x435
-#define ADF_DH895XCCIOV_PCI_DEVICE_ID 0x443
-#define ADF_C62X_PCI_DEVICE_ID 0x37c8
-#define ADF_C62XIOV_PCI_DEVICE_ID 0x37c9
-#define ADF_C3XXX_PCI_DEVICE_ID 0x19e2
-#define ADF_C3XXXIOV_PCI_DEVICE_ID 0x19e3
 #define ADF_ERRSOU3 (0x3A000 + 0x0C)
 #define ADF_ERRSOU5 (0x3A000 + 0xD8)
 #define ADF_DEVICE_FUSECTL_OFFSET 0x40
diff --git a/drivers/crypto/qat/qat_common/qat_hal.c b/drivers/crypto/qat/qat_common/qat_hal.c
index fa467e0f8285..6b9d47682d04 100644
--- a/drivers/crypto/qat/qat_common/qat_hal.c
+++ b/drivers/crypto/qat/qat_common/qat_hal.c
@@ -2,6 +2,7 @@
 /* Copyright(c) 2014 - 2020 Intel Corporation */
 #include <linux/slab.h>
 #include <linux/delay.h>
+#include <linux/pci_ids.h>
 
 #include "adf_accel_devices.h"
 #include "adf_common_drv.h"
@@ -412,7 +413,7 @@ static int qat_hal_init_esram(struct icp_qat_fw_loader_handle *handle)
 	unsigned int csr_val;
 	int times = 30;
 
-	if (handle->pci_dev->device != ADF_DH895XCC_PCI_DEVICE_ID)
+	if (handle->pci_dev->device != PCI_DEVICE_ID_INTEL_QAT_DH895XCC)
 		return 0;
 
 	csr_val = ADF_CSR_RD(csr_addr, 0);
@@ -672,13 +673,13 @@ int qat_hal_init(struct adf_accel_dev *accel_dev)
 		(void __iomem *)((uintptr_t)handle->hal_cap_ae_xfer_csr_addr_v +
 				 LOCAL_TO_XFER_REG_OFFSET);
 	handle->pci_dev = pci_info->pci_dev;
-	if (handle->pci_dev->device == ADF_DH895XCC_PCI_DEVICE_ID) {
+	if (handle->pci_dev->device == PCI_DEVICE_ID_INTEL_QAT_DH895XCC) {
 		sram_bar =
 			&pci_info->pci_bars[hw_data->get_sram_bar_id(hw_data)];
 		handle->hal_sram_addr_v = sram_bar->virt_addr;
 	}
 	handle->fw_auth = (handle->pci_dev->device ==
-			   ADF_DH895XCC_PCI_DEVICE_ID) ? false : true;
+			   PCI_DEVICE_ID_INTEL_QAT_DH895XCC) ? false : true;
 	handle->hal_handle = kzalloc(sizeof(*handle->hal_handle), GFP_KERNEL);
 	if (!handle->hal_handle)
 		goto out_hal_handle;
diff --git a/drivers/crypto/qat/qat_common/qat_uclo.c b/drivers/crypto/qat/qat_common/qat_uclo.c
index 4cc1f436b075..bfe52e428650 100644
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -4,6 +4,7 @@
 #include <linux/ctype.h>
 #include <linux/kernel.h>
 #include <linux/delay.h>
+#include <linux/pci_ids.h>
 #include "adf_accel_devices.h"
 #include "adf_common_drv.h"
 #include "icp_qat_uclo.h"
@@ -706,11 +707,11 @@ static unsigned int
 qat_uclo_get_dev_type(struct icp_qat_fw_loader_handle *handle)
 {
 	switch (handle->pci_dev->device) {
-	case ADF_DH895XCC_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_DH895XCC:
 		return ICP_QAT_AC_895XCC_DEV_TYPE;
-	case ADF_C62X_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C62X:
 		return ICP_QAT_AC_C62X_DEV_TYPE;
-	case ADF_C3XXX_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_C3XXX:
 		return ICP_QAT_AC_C3XXX_DEV_TYPE;
 	default:
 		pr_err("QAT: unsupported device 0x%x\n",
@@ -1386,7 +1387,7 @@ int qat_uclo_wr_mimage(struct icp_qat_fw_loader_handle *handle,
 			status = qat_uclo_auth_fw(handle, desc);
 		qat_uclo_ummap_auth_fw(handle, &desc);
 	} else {
-		if (handle->pci_dev->device == ADF_C3XXX_PCI_DEVICE_ID) {
+		if (handle->pci_dev->device == PCI_DEVICE_ID_INTEL_QAT_C3XXX) {
 			pr_err("QAT: C3XXX doesn't support unsigned MMP\n");
 			return -EINVAL;
 		}
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index 4e877b75822b..4c3aea07f444 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_DH895XCC_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_DH895XCC),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_DH895XCC_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC:
 			adf_clean_hw_data_dh895xcc(accel_dev->hw_device);
 			break;
 		default:
@@ -83,7 +83,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_DH895XCC_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_DH895XCC:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
index 7d6e1db272c2..673348ca5dea 100644
--- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
@@ -22,7 +22,7 @@
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
 
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(ADF_DH895XCCIOV_PCI_DEVICE_ID),
+	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF),
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
@@ -58,7 +58,7 @@ static void adf_cleanup_accel(struct adf_accel_dev *accel_dev)
 
 	if (accel_dev->hw_device) {
 		switch (accel_pci_dev->pci_dev->device) {
-		case ADF_DH895XCCIOV_PCI_DEVICE_ID:
+		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF:
 			adf_clean_hw_data_dh895xcciov(accel_dev->hw_device);
 			break;
 		default:
@@ -85,7 +85,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	int ret;
 
 	switch (ent->device) {
-	case ADF_DH895XCCIOV_PCI_DEVICE_ID:
+	case PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF:
 		break;
 	default:
 		dev_err(&pdev->dev, "Invalid device 0x%x.\n", ent->device);
-- 
2.26.2

[PATCH 5/5] crypto: qat - use PCI_VDEVICE

[Giovanni Cabiddu]

Build pci_device_id structure using the PCI_VDEVICE macro.
This removes any references to the ADF_SYSTEM_DEVICE macro.

Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
---
 drivers/crypto/qat/qat_c3xxx/adf_drv.c      | 7 ++-----
 drivers/crypto/qat/qat_c3xxxvf/adf_drv.c    | 7 ++-----
 drivers/crypto/qat/qat_c62x/adf_drv.c       | 7 ++-----
 drivers/crypto/qat/qat_c62xvf/adf_drv.c     | 7 ++-----
 drivers/crypto/qat/qat_dh895xcc/adf_drv.c   | 7 ++-----
 drivers/crypto/qat/qat_dh895xccvf/adf_drv.c | 7 ++-----
 6 files changed, 12 insertions(+), 30 deletions(-)

diff --git a/drivers/crypto/qat/qat_c3xxx/adf_drv.c b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
index bba0f142f7f6..43929d70c41d 100644
--- a/drivers/crypto/qat/qat_c3xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_c3xxx_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C3XXX),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_C3XXX), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
diff --git a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
index b77a58886599..dca52de22e8d 100644
--- a/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxxvf/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_c3xxxvf_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
diff --git a/drivers/crypto/qat/qat_c62x/adf_drv.c b/drivers/crypto/qat/qat_c62x/adf_drv.c
index 722838ff03be..f104c9d1195d 100644
--- a/drivers/crypto/qat/qat_c62x/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62x/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_c62x_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C62X),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_C62X), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
diff --git a/drivers/crypto/qat/qat_c62xvf/adf_drv.c b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
index a766cc18aae9..e0b909e70712 100644
--- a/drivers/crypto/qat/qat_c62xvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62xvf/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_c62xvf_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_C62X_VF),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_C62X_VF), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index 4c3aea07f444..857aa4c8595f 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_dh895xcc_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_DH895XCC),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_DH895XCC), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
diff --git a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
index 673348ca5dea..2987855a70dc 100644
--- a/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xccvf/adf_drv.c
@@ -18,12 +18,9 @@
 #include <adf_cfg.h>
 #include "adf_dh895xccvf_hw_data.h"
 
-#define ADF_SYSTEM_DEVICE(device_id) \
-	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, device_id)}
-
 static const struct pci_device_id adf_pci_tbl[] = {
-	ADF_SYSTEM_DEVICE(PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF),
-	{0,}
+	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF), },
+	{ }
 };
 MODULE_DEVICE_TABLE(pci, adf_pci_tbl);
 
-- 
2.26.2

Re: [PATCH 0/5] vfio/pci: add blocklist and disable qat

[Christoph Hellwig]

On Wed, Jul 01, 2020 at 12:02:57PM +0100, Giovanni Cabiddu wrote:
> This patchset defines a blocklist of devices in the vfio-pci module and adds
> the current generation of Intel(R) QuickAssist devices to it as they are
> not designed to run in an untrusted environment.

How can they not be safe?  If any device is not safe to assign the
whole vfio concept has major issues that we need to fix for real instead
of coming up with quirk lists for specific IDs.

Re: [PATCH 1/5] PCI: add Intel QuickAssist device IDs

[Bjorn Helgaas]

Please follow the subject line convention, e.g.,

  PCI: Add Intel QuickAssist Device IDs

On Wed, Jul 01, 2020 at 12:02:58PM +0100, Giovanni Cabiddu wrote:
> Add device IDs for the following Intel QuickAssist devices: DH895XCC,
> C3XXX and C62X.
> 
> The defines in this patch are going to be referenced in two independent
> drivers, qat and vfio-pci.
> 
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
>  include/linux/pci_ids.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 0ad57693f392..f3166b1425ca 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2659,6 +2659,8 @@
>  #define PCI_DEVICE_ID_INTEL_80332_1	0x0332
>  #define PCI_DEVICE_ID_INTEL_80333_0	0x0370
>  #define PCI_DEVICE_ID_INTEL_80333_1	0x0372
> +#define PCI_DEVICE_ID_INTEL_QAT_DH895XCC	0x0435
> +#define PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF	0x0443
>  #define PCI_DEVICE_ID_INTEL_82375	0x0482
>  #define PCI_DEVICE_ID_INTEL_82424	0x0483
>  #define PCI_DEVICE_ID_INTEL_82378	0x0484
> @@ -2708,6 +2710,8 @@
>  #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_NHI     0x1577
>  #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE  0x1578
>  #define PCI_DEVICE_ID_INTEL_80960_RP	0x1960
> +#define PCI_DEVICE_ID_INTEL_QAT_C3XXX	0x19e2
> +#define PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF	0x19e3
>  #define PCI_DEVICE_ID_INTEL_82840_HB	0x1a21
>  #define PCI_DEVICE_ID_INTEL_82845_HB	0x1a30
>  #define PCI_DEVICE_ID_INTEL_IOAT	0x1a38
> @@ -2924,6 +2928,8 @@
>  #define PCI_DEVICE_ID_INTEL_IOAT_JSF7	0x3717
>  #define PCI_DEVICE_ID_INTEL_IOAT_JSF8	0x3718
>  #define PCI_DEVICE_ID_INTEL_IOAT_JSF9	0x3719
> +#define PCI_DEVICE_ID_INTEL_QAT_C62X	0x37c8
> +#define PCI_DEVICE_ID_INTEL_QAT_C62X_VF	0x37c9
>  #define PCI_DEVICE_ID_INTEL_ICH10_0	0x3a14
>  #define PCI_DEVICE_ID_INTEL_ICH10_1	0x3a16
>  #define PCI_DEVICE_ID_INTEL_ICH10_2	0x3a18
> -- 
> 2.26.2
> 

Re: [PATCH 2/5] vfio/pci: add device blocklist

[Bjorn Helgaas]

On Wed, Jul 01, 2020 at 12:02:59PM +0100, Giovanni Cabiddu wrote:
> Add blocklist of devices that by default are not probed by vfio-pci.

> Devices in this list may be susceptible to untrusted application, even
> if the IOMMU is enabled. 

I can't quite parse this sentence.  I think it means something about
these devices being able to bypass an IOMMU or otherwise cause
problems via DMA.  It would be nice to know exactly what problem this
is avoiding.

I assume *all* applications are untrusted, right?  The wording above
makes it sound like there may be a class of trusted applications in
addition to the untrusted ones.  But I don't see anything like that in
the patch.

> To be accessed via vfio-pci, the user has to
> explicitly disable the blocklist.
> 
> The blocklist can be disabled via the module parameter disable_blocklist.
> 
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
>  drivers/vfio/pci/vfio_pci.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 7c0779018b1b..ea5904ca6cbf 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -60,6 +60,10 @@ module_param(enable_sriov, bool, 0644);
>  MODULE_PARM_DESC(enable_sriov, "Enable support for SR-IOV configuration.  Enabling SR-IOV on a PF typically requires support of the userspace PF driver, enabling VFs without such support may result in non-functional VFs or PF.");
>  #endif
>  
> +static bool disable_blocklist;
> +module_param(disable_blocklist, bool, 0444);
> +MODULE_PARM_DESC(disable_blocklist, "Disable device blocklist. If set, i.e. blocklist disabled, then blocklisted devices are allowed to be probed by vfio-pci.");
> +
>  static inline bool vfio_vga_disabled(void)
>  {
>  #ifdef CONFIG_VFIO_PCI_VGA
> @@ -69,6 +73,29 @@ static inline bool vfio_vga_disabled(void)
>  #endif
>  }
>  
> +static bool vfio_pci_dev_in_blocklist(struct pci_dev *pdev)
> +{
> +	return false;
> +}
> +
> +static bool vfio_pci_is_blocklisted(struct pci_dev *pdev)
> +{
> +	if (!vfio_pci_dev_in_blocklist(pdev))
> +		return false;
> +
> +	if (disable_blocklist) {
> +		pci_warn(pdev,
> +			 "device blocklist disabled - allowing device %04x:%04x.\n",
> +			 pdev->vendor, pdev->device);
> +		return false;
> +	}
> +
> +	pci_warn(pdev, "%04x:%04x is blocklisted - probe will fail.\n",
> +		 pdev->vendor, pdev->device);
> +
> +	return true;
> +}
> +
>  /*
>   * Our VGA arbiter participation is limited since we don't know anything
>   * about the device itself.  However, if the device is the only VGA device
> @@ -1847,6 +1874,9 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	struct iommu_group *group;
>  	int ret;
>  
> +	if (vfio_pci_is_blocklisted(pdev))
> +		return -EINVAL;
> +
>  	if (pdev->hdr_type != PCI_HEADER_TYPE_NORMAL)
>  		return -EINVAL;
>  
> @@ -2336,6 +2366,9 @@ static int __init vfio_pci_init(void)
>  
>  	vfio_pci_fill_ids();
>  
> +	if (disable_blocklist)
> +		pr_warn("device blocklist disabled.\n");
> +
>  	return 0;
>  
>  out_driver:
> -- 
> 2.26.2
> 

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Bjorn Helgaas]

On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:
> The current generation of Intel® QuickAssist Technology devices
> are not designed to run in an untrusted environment because of the
> following issues reported in the release notes in
> https://01.org/intel-quickassist-technology:

It would be nice if this link were directly clickable, e.g., if there
were no trailing ":" or something.

And it would be even better if it went to a specific doc that
described these issues.  I assume these are errata, and it's not easy
to figure out which doc mentions them.

> QATE-39220 - GEN - Intel® QAT API submissions with bad addresses that
>              trigger DMA to invalid or unmapped addresses can cause a
>              platform hang
> QATE-7495  - GEN - An incorrectly formatted request to Intel® QAT can
>              hang the entire Intel® QAT Endpoint
> 
> This patch adds the following QAT devices to the blocklist: DH895XCC,
> C3XXX and C62X.
> 
> Signed-off-by: Giovanni Cabiddu <giovanni.cabiddu@intel.com>
> ---
>  drivers/vfio/pci/vfio_pci.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index ea5904ca6cbf..dcac5408c764 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -75,6 +75,21 @@ static inline bool vfio_vga_disabled(void)
>  
>  static bool vfio_pci_dev_in_blocklist(struct pci_dev *pdev)
>  {
> +	switch (pdev->vendor) {
> +	case PCI_VENDOR_ID_INTEL:
> +		switch (pdev->device) {
> +		case PCI_DEVICE_ID_INTEL_QAT_C3XXX:
> +		case PCI_DEVICE_ID_INTEL_QAT_C3XXX_VF:
> +		case PCI_DEVICE_ID_INTEL_QAT_C62X:
> +		case PCI_DEVICE_ID_INTEL_QAT_C62X_VF:
> +		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC:
> +		case PCI_DEVICE_ID_INTEL_QAT_DH895XCC_VF:
> +			return true;
> +		default:
> +			return false;
> +		}
> +	}
> +
>  	return false;
>  }
>  
> -- 
> 2.26.2
> 

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Giovanni Cabiddu]

On Wed, Jul 01, 2020 at 04:28:12PM -0500, Bjorn Helgaas wrote:
> On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:
> > The current generation of Intel® QuickAssist Technology devices
> > are not designed to run in an untrusted environment because of the
> > following issues reported in the release notes in
> > https://01.org/intel-quickassist-technology:
> 
> It would be nice if this link were directly clickable, e.g., if there
> were no trailing ":" or something.
> 
> And it would be even better if it went to a specific doc that
> described these issues.  I assume these are errata, and it's not easy
> to figure out which doc mentions them.
Sure. I will fix the commit message in the next revision and point to the
actual document:
https://01.org/sites/default/files/downloads/336211-015-qatsoftwareforlinux-rn-hwv1.7-final.pdf

Regards,

-- 
Giovanni

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Bjorn Helgaas]

On Fri, Jul 10, 2020 at 04:08:19PM +0100, Giovanni Cabiddu wrote:
> On Wed, Jul 01, 2020 at 04:28:12PM -0500, Bjorn Helgaas wrote:
> > On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:
> > > The current generation of Intel® QuickAssist Technology devices
> > > are not designed to run in an untrusted environment because of the
> > > following issues reported in the release notes in
> > > https://01.org/intel-quickassist-technology:
> > 
> > It would be nice if this link were directly clickable, e.g., if there
> > were no trailing ":" or something.
> > 
> > And it would be even better if it went to a specific doc that
> > described these issues.  I assume these are errata, and it's not easy
> > to figure out which doc mentions them.
> Sure. I will fix the commit message in the next revision and point to the
> actual document:
> https://01.org/sites/default/files/downloads/336211-015-qatsoftwareforlinux-rn-hwv1.7-final.pdf

Since URLs tend to go stale, please also include the Intel document
number and title.

When you update this, you might also update the subject lines.  It
looks like the VFIO convention is "vfio/pci: <Capitalized> ...",
based on "git log --oneline drivers/vfio/pci/".  And "QAT" should be
capitalized also since it's not a word by itself (and to match usage
in the spec).

Bjorn

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Bjorn Helgaas]

On Fri, Jul 10, 2020 at 10:37:45AM -0500, Bjorn Helgaas wrote:
> On Fri, Jul 10, 2020 at 04:08:19PM +0100, Giovanni Cabiddu wrote:
> > On Wed, Jul 01, 2020 at 04:28:12PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:
> > > > The current generation of Intel® QuickAssist Technology devices
> > > > are not designed to run in an untrusted environment because of the
> > > > following issues reported in the release notes in
> > > > https://01.org/intel-quickassist-technology:
> > > 
> > > It would be nice if this link were directly clickable, e.g., if there
> > > were no trailing ":" or something.
> > > 
> > > And it would be even better if it went to a specific doc that
> > > described these issues.  I assume these are errata, and it's not easy
> > > to figure out which doc mentions them.
> > Sure. I will fix the commit message in the next revision and point to the
> > actual document:
> > https://01.org/sites/default/files/downloads/336211-015-qatsoftwareforlinux-rn-hwv1.7-final.pdf
> 
> Since URLs tend to go stale, please also include the Intel document
> number and title.

Oh, and is "01.org" really the right place for that?  It looks like an
Intel document, so I'd expect it to be somewhere on intel.com.

I'm still a little confused.  That doc seems to be about *software*
and Linux software in particular.  But when you said these "devices
are not designed to run in an untrusted environment", I thought you
meant there was some *hardware* design issue that caused a problem.

Bjorn

Re: [PATCH 0/5] vfio/pci: add blocklist and disable qat

[Christoph Hellwig]

On Wed, Jul 01, 2020 at 01:42:09PM +0100, Christoph Hellwig wrote:
> On Wed, Jul 01, 2020 at 12:02:57PM +0100, Giovanni Cabiddu wrote:
> > This patchset defines a blocklist of devices in the vfio-pci module and adds
> > the current generation of Intel(R) QuickAssist devices to it as they are
> > not designed to run in an untrusted environment.
> 
> How can they not be safe?  If any device is not safe to assign the
> whole vfio concept has major issues that we need to fix for real instead
> of coming up with quirk lists for specific IDs.

No answer yet:  how is this device able to bypass the IOMMU?  Don't
we have a fundamental model flaw if a random device can bypass the
IOMMU protection?  Except for an ATS bug I can't really think of a way
how a device could bypass the IOMMU, and in that case we should just
disable ATS.

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Alex Williamson]

On Fri, 10 Jul 2020 10:44:33 -0500
Bjorn Helgaas <helgaas@kernel.org> wrote:

> On Fri, Jul 10, 2020 at 10:37:45AM -0500, Bjorn Helgaas wrote:
> > On Fri, Jul 10, 2020 at 04:08:19PM +0100, Giovanni Cabiddu wrote:  
> > > On Wed, Jul 01, 2020 at 04:28:12PM -0500, Bjorn Helgaas wrote:  
> > > > On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:  
> > > > > The current generation of Intel® QuickAssist Technology devices
> > > > > are not designed to run in an untrusted environment because of the
> > > > > following issues reported in the release notes in
> > > > > https://01.org/intel-quickassist-technology:  
> > > > 
> > > > It would be nice if this link were directly clickable, e.g., if there
> > > > were no trailing ":" or something.
> > > > 
> > > > And it would be even better if it went to a specific doc that
> > > > described these issues.  I assume these are errata, and it's not easy
> > > > to figure out which doc mentions them.  
> > > Sure. I will fix the commit message in the next revision and point to the
> > > actual document:
> > > https://01.org/sites/default/files/downloads/336211-015-qatsoftwareforlinux-rn-hwv1.7-final.pdf  
> > 
> > Since URLs tend to go stale, please also include the Intel document
> > number and title.  
> 
> Oh, and is "01.org" really the right place for that?  It looks like an
> Intel document, so I'd expect it to be somewhere on intel.com.
> 
> I'm still a little confused.  That doc seems to be about *software*
> and Linux software in particular.  But when you said these "devices
> are not designed to run in an untrusted environment", I thought you
> meant there was some *hardware* design issue that caused a problem.

There seems to be a fair bit of hardware errata in the doc too, see:

3.1.2 QATE-7495 - GEN - An incorrectly formatted request to Intel® QAT can
hang the entire Intel® QAT Endpoint

3.1.9 QATE-39220 - GEN - QAT API submissions with bad addresses that
trigger DMA to invalid or unmapped addresses can cause a platform
hang

3.1.17 QATE-52389 - SR-IOV -Huge pages may not be compatible with QAT
VF usage

3.1.19 QATE-60953 - GEN – Intel® QAT API submissions with bad addresses
that trigger DMA to invalid or unmapped addresses can impact QAT
service availability

Thanks,
Alex

Re: [PATCH 0/5] vfio/pci: add blocklist and disable qat

[Giovanni Cabiddu]

On Fri, Jul 10, 2020 at 04:48:07PM +0100, Christoph Hellwig wrote:
> On Wed, Jul 01, 2020 at 01:42:09PM +0100, Christoph Hellwig wrote:
> > On Wed, Jul 01, 2020 at 12:02:57PM +0100, Giovanni Cabiddu wrote:
> > > This patchset defines a blocklist of devices in the vfio-pci module and adds
> > > the current generation of Intel(R) QuickAssist devices to it as they are
> > > not designed to run in an untrusted environment.
> > 
> > How can they not be safe?  If any device is not safe to assign the
> > whole vfio concept has major issues that we need to fix for real instead
> > of coming up with quirk lists for specific IDs.
> 
> No answer yet:  how is this device able to bypass the IOMMU?  Don't
> we have a fundamental model flaw if a random device can bypass the
> IOMMU protection?  Except for an ATS bug I can't really think of a way
> how a device could bypass the IOMMU, and in that case we should just
> disable ATS.
Apologies.
This is specific to the QAT device and described in QATE-39220 in the
QAT release notes:
https://01.org/sites/default/files/downloads/336211-014-qatforlinux-releasenotes-hwv1.7_0.pdf
If a request with an address outside of the IOMMU domain attached to the
device is submitted, the device can lock up or induce a platform hang.

Regards,

-- 
Giovanni

Re: [PATCH 3/5] vfio/pci: add qat devices to blocklist

[Giovanni Cabiddu]

On Fri, Jul 10, 2020 at 10:10:34AM -0600, Alex Williamson wrote:
> On Fri, 10 Jul 2020 10:44:33 -0500
> Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> > On Fri, Jul 10, 2020 at 10:37:45AM -0500, Bjorn Helgaas wrote:
> > > On Fri, Jul 10, 2020 at 04:08:19PM +0100, Giovanni Cabiddu wrote:  
> > > > On Wed, Jul 01, 2020 at 04:28:12PM -0500, Bjorn Helgaas wrote:  
> > > > > On Wed, Jul 01, 2020 at 12:03:00PM +0100, Giovanni Cabiddu wrote:  
> > > > > > The current generation of Intel® QuickAssist Technology devices
> > > > > > are not designed to run in an untrusted environment because of the
> > > > > > following issues reported in the release notes in
> > > > > > https://01.org/intel-quickassist-technology:  
> > > > > 
> > > > > It would be nice if this link were directly clickable, e.g., if there
> > > > > were no trailing ":" or something.
> > > > > 
> > > > > And it would be even better if it went to a specific doc that
> > > > > described these issues.  I assume these are errata, and it's not easy
> > > > > to figure out which doc mentions them.  
> > > > Sure. I will fix the commit message in the next revision and point to the
> > > > actual document:
> > > > https://01.org/sites/default/files/downloads/336211-015-qatsoftwareforlinux-rn-hwv1.7-final.pdf  
> > > 
> > > Since URLs tend to go stale, please also include the Intel document
> > > number and title.  
> > 
> > Oh, and is "01.org" really the right place for that?  It looks like an
> > Intel document, so I'd expect it to be somewhere on intel.com.
> > 
> > I'm still a little confused.  That doc seems to be about *software*
> > and Linux software in particular.  But when you said these "devices
> > are not designed to run in an untrusted environment", I thought you
> > meant there was some *hardware* design issue that caused a problem.
Yes, the problem is in hardware.

> There seems to be a fair bit of hardware errata in the doc too, see:
> 
> 3.1.2 QATE-7495 - GEN - An incorrectly formatted request to Intel® QAT can
> hang the entire Intel® QAT Endpoint
> 
> 3.1.9 QATE-39220 - GEN - QAT API submissions with bad addresses that
> trigger DMA to invalid or unmapped addresses can cause a platform
> hang
> 
> 3.1.17 QATE-52389 - SR-IOV -Huge pages may not be compatible with QAT
> VF usage
> 
> 3.1.19 QATE-60953 - GEN – Intel® QAT API submissions with bad addresses
> that trigger DMA to invalid or unmapped addresses can impact QAT
> service availability
Correct, that document contains errata for both the QAT HW and the
current software.

Regards,

-- 
Giovanni