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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed,  1 Sep 2021 18:36:07 +0100
From:   Wojciech Ziemba <wojciech.ziemba@...el.com>
To:     herbert@...dor.apana.org.au
Cc:     linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
        qat-linux@...el.com, Maksim Lukoshkov <maksim.lukoshkov@...el.com>,
        Wojciech Ziemba <wojciech.ziemba@...el.com>,
        Giovanni Cabiddu <giovanni.cabiddu@...el.com>
Subject: [PATCH 3/4] crypto: qat - free irqs only if allocated

From: Maksim Lukoshkov <maksim.lukoshkov@...el.com>

Change the irq allocation logic so that it is possible to free only the
allocated irqs in case of error.
A new flag is introduced for every PF/VF interrupt. This flag is set to
"true" only when the interrupt is requested.
During clean up, devm_free_irq() is only called if this flag is set.

Signed-off-by: Maksim Lukoshkov <maksim.lukoshkov@...el.com>
Co-developed-by: Wojciech Ziemba <wojciech.ziemba@...el.com>
Signed-off-by: Wojciech Ziemba <wojciech.ziemba@...el.com>
Reviewed-by: Giovanni Cabiddu <giovanni.cabiddu@...el.com>
---
 .../crypto/qat/qat_common/adf_accel_devices.h | 10 +++-
 drivers/crypto/qat/qat_common/adf_isr.c       | 50 ++++++++-----------
 drivers/crypto/qat/qat_common/adf_vf_isr.c    | 12 ++---
 3 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_accel_devices.h b/drivers/crypto/qat/qat_common/adf_accel_devices.h
index 87de40d6c9a5..e391ca0662bc 100644
--- a/drivers/crypto/qat/qat_common/adf_accel_devices.h
+++ b/drivers/crypto/qat/qat_common/adf_accel_devices.h
@@ -44,8 +44,13 @@ struct adf_bar {
 	resource_size_t size;
 } __packed;
 
+struct adf_irq {
+	bool enabled;
+	char name[ADF_MAX_MSIX_VECTOR_NAME];
+};
+
 struct adf_accel_msix {
-	char **names;
+	struct adf_irq *irqs;
 	u32 num_entries;
 } __packed;
 
@@ -250,7 +255,8 @@ struct adf_accel_dev {
 			struct adf_accel_vf_info *vf_info;
 		} pf;
 		struct {
-			char *irq_name;
+			bool irq_enabled;
+			char irq_name[ADF_MAX_MSIX_VECTOR_NAME];
 			struct tasklet_struct pf2vf_bh_tasklet;
 			struct mutex vf2pf_lock; /* protect CSR access */
 			struct completion iov_msg_completion;
diff --git a/drivers/crypto/qat/qat_common/adf_isr.c b/drivers/crypto/qat/qat_common/adf_isr.c
index 26b546d3f821..861a9368b9db 100644
--- a/drivers/crypto/qat/qat_common/adf_isr.c
+++ b/drivers/crypto/qat/qat_common/adf_isr.c
@@ -130,6 +130,7 @@ static int adf_request_irqs(struct adf_accel_dev *accel_dev)
 {
 	struct adf_accel_pci *pci_dev_info = &accel_dev->accel_pci_dev;
 	struct adf_hw_device_data *hw_data = accel_dev->hw_device;
+	struct adf_irq *irqs = pci_dev_info->msix_entries.irqs;
 	struct adf_etr_data *etr_data = accel_dev->transport;
 	int clust_irq = hw_data->num_banks;
 	int ret, irq, i = 0;
@@ -141,7 +142,7 @@ static int adf_request_irqs(struct adf_accel_dev *accel_dev)
 			struct adf_etr_bank_data *bank = &etr_data->banks[i];
 			unsigned int cpu, cpus = num_online_cpus();
 
-			name = *(pci_dev_info->msix_entries.names + i);
+			name = irqs[i].name;
 			snprintf(name, ADF_MAX_MSIX_VECTOR_NAME,
 				 "qat%d-bundle%d", accel_dev->accel_id, i);
 			irq = pci_irq_vector(pci_dev_info->pci_dev, i);
@@ -163,11 +164,12 @@ static int adf_request_irqs(struct adf_accel_dev *accel_dev)
 			cpu = ((accel_dev->accel_id * hw_data->num_banks) +
 			       i) % cpus;
 			irq_set_affinity_hint(irq, get_cpu_mask(cpu));
+			irqs[i].enabled = true;
 		}
 	}
 
 	/* Request msix irq for AE */
-	name = *(pci_dev_info->msix_entries.names + i);
+	name = irqs[i].name;
 	snprintf(name, ADF_MAX_MSIX_VECTOR_NAME,
 		 "qat%d-ae-cluster", accel_dev->accel_id);
 	irq = pci_irq_vector(pci_dev_info->pci_dev, clust_irq);
@@ -183,6 +185,7 @@ static int adf_request_irqs(struct adf_accel_dev *accel_dev)
 			"Failed to allocate IRQ %d for %s\n", irq, name);
 		return ret;
 	}
+	irqs[i].enabled = true;
 	return ret;
 }
 
@@ -190,60 +193,51 @@ static void adf_free_irqs(struct adf_accel_dev *accel_dev)
 {
 	struct adf_accel_pci *pci_dev_info = &accel_dev->accel_pci_dev;
 	struct adf_hw_device_data *hw_data = accel_dev->hw_device;
+	struct adf_irq *irqs = pci_dev_info->msix_entries.irqs;
 	struct adf_etr_data *etr_data = accel_dev->transport;
 	int clust_irq = hw_data->num_banks;
 	int irq, i = 0;
 
 	if (pci_dev_info->msix_entries.num_entries > 1) {
 		for (i = 0; i < hw_data->num_banks; i++) {
-			irq = pci_irq_vector(pci_dev_info->pci_dev, i);
-			irq_set_affinity_hint(irq, NULL);
-			free_irq(irq, &etr_data->banks[i]);
+			if (irqs[i].enabled) {
+				irq = pci_irq_vector(pci_dev_info->pci_dev, i);
+				irq_set_affinity_hint(irq, NULL);
+				free_irq(irq, &etr_data->banks[i]);
+			}
 		}
 	}
 
-	irq = pci_irq_vector(pci_dev_info->pci_dev, clust_irq);
-	free_irq(irq, accel_dev);
+	if (irqs[i].enabled) {
+		irq = pci_irq_vector(pci_dev_info->pci_dev, clust_irq);
+		free_irq(irq, accel_dev);
+	}
 }
 
 static int adf_isr_alloc_msix_vectors_data(struct adf_accel_dev *accel_dev)
 {
-	int i;
-	char **names;
 	struct adf_hw_device_data *hw_data = accel_dev->hw_device;
 	u32 msix_num_entries = 1;
+	struct adf_irq *irqs;
 
 	/* If SR-IOV is disabled (vf_info is NULL), add entries for each bank */
 	if (!accel_dev->pf.vf_info)
 		msix_num_entries += hw_data->num_banks;
 
-	names = kcalloc(msix_num_entries, sizeof(char *), GFP_KERNEL);
-	if (!names)
+	irqs = kzalloc_node(msix_num_entries * sizeof(*irqs),
+			    GFP_KERNEL, dev_to_node(&GET_DEV(accel_dev)));
+	if (!irqs)
 		return -ENOMEM;
 
-	for (i = 0; i < msix_num_entries; i++) {
-		*(names + i) = kzalloc(ADF_MAX_MSIX_VECTOR_NAME, GFP_KERNEL);
-		if (!(*(names + i)))
-			goto err;
-	}
 	accel_dev->accel_pci_dev.msix_entries.num_entries = msix_num_entries;
-	accel_dev->accel_pci_dev.msix_entries.names = names;
+	accel_dev->accel_pci_dev.msix_entries.irqs = irqs;
 	return 0;
-err:
-	for (i = 0; i < msix_num_entries; i++)
-		kfree(*(names + i));
-	kfree(names);
-	return -ENOMEM;
 }
 
 static void adf_isr_free_msix_vectors_data(struct adf_accel_dev *accel_dev)
 {
-	char **names = accel_dev->accel_pci_dev.msix_entries.names;
-	int i;
-
-	for (i = 0; i < accel_dev->accel_pci_dev.msix_entries.num_entries; i++)
-		kfree(*(names + i));
-	kfree(names);
+	kfree(accel_dev->accel_pci_dev.msix_entries.irqs);
+	accel_dev->accel_pci_dev.msix_entries.irqs = NULL;
 }
 
 static int adf_setup_bh(struct adf_accel_dev *accel_dev)
diff --git a/drivers/crypto/qat/qat_common/adf_vf_isr.c b/drivers/crypto/qat/qat_common/adf_vf_isr.c
index 695c5050b6f3..07f81682c19b 100644
--- a/drivers/crypto/qat/qat_common/adf_vf_isr.c
+++ b/drivers/crypto/qat/qat_common/adf_vf_isr.c
@@ -61,10 +61,6 @@ static int adf_enable_msi(struct adf_accel_dev *accel_dev)
 		return stat;
 	}
 
-	accel_dev->vf.irq_name = kzalloc(ADF_MAX_MSIX_VECTOR_NAME, GFP_KERNEL);
-	if (!accel_dev->vf.irq_name)
-		return -ENOMEM;
-
 	return 0;
 }
 
@@ -72,7 +68,6 @@ static void adf_disable_msi(struct adf_accel_dev *accel_dev)
 {
 	struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
 
-	kfree(accel_dev->vf.irq_name);
 	pci_free_irq_vectors(pdev);
 }
 
@@ -240,6 +235,7 @@ static int adf_request_msi_irq(struct adf_accel_dev *accel_dev)
 	}
 	cpu = accel_dev->accel_id % num_online_cpus();
 	irq_set_affinity_hint(pdev->irq, get_cpu_mask(cpu));
+	accel_dev->vf.irq_enabled = true;
 
 	return ret;
 }
@@ -271,8 +267,10 @@ void adf_vf_isr_resource_free(struct adf_accel_dev *accel_dev)
 {
 	struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
 
-	irq_set_affinity_hint(pdev->irq, NULL);
-	free_irq(pdev->irq, (void *)accel_dev);
+	if (accel_dev->vf.irq_enabled) {
+		irq_set_affinity_hint(pdev->irq, NULL);
+		free_irq(pdev->irq, accel_dev);
+	}
 	adf_cleanup_bh(accel_dev);
 	adf_cleanup_pf2vf_bh(accel_dev);
 	adf_disable_msi(accel_dev);
-- 
2.29.2

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ