* [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
@ 2022-02-21 4:59 ` Vasant Hegde via iommu
2022-02-28 12:47 ` Joerg Roedel
2022-02-21 4:59 ` [PATCH 2/5] iommu/amd: Call memunmap in error path Vasant Hegde via iommu
` (4 subsequent siblings)
5 siblings, 1 reply; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-02-21 4:59 UTC (permalink / raw)
To: joro, iommu; +Cc: Vasant Hegde
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Add error messages to prevent silent failure.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/init.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 1eacd43cb436..770ac679b682 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1942,9 +1942,10 @@ static int __init amd_iommu_init_pci(void)
for_each_iommu(iommu) {
ret = iommu_init_pci(iommu);
- if (ret)
- break;
-
+ if (ret) {
+ pr_err("IOMMU:%d Failed to initialize!\n", iommu->index);
+ goto out;
+ }
/* Need to setup range after PCI init */
iommu_set_cwwb_range(iommu);
}
@@ -1960,6 +1961,10 @@ static int __init amd_iommu_init_pci(void)
* active.
*/
ret = amd_iommu_init_api();
+ if (ret) {
+ pr_err("IOMMU: Failed to initialize api!\n");
+ goto out;
+ }
init_device_table_dma();
@@ -1969,6 +1974,7 @@ static int __init amd_iommu_init_pci(void)
if (!ret)
print_iommu_info();
+out:
return ret;
}
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci
2022-02-21 4:59 ` [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci Vasant Hegde via iommu
@ 2022-02-28 12:47 ` Joerg Roedel
2022-03-01 8:58 ` Vasant Hegde via iommu
0 siblings, 1 reply; 9+ messages in thread
From: Joerg Roedel @ 2022-02-28 12:47 UTC (permalink / raw)
To: Vasant Hegde; +Cc: iommu
On Mon, Feb 21, 2022 at 10:29:12AM +0530, Vasant Hegde wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>
> Add error messages to prevent silent failure.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
> drivers/iommu/amd/init.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 1eacd43cb436..770ac679b682 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -1942,9 +1942,10 @@ static int __init amd_iommu_init_pci(void)
>
> for_each_iommu(iommu) {
> ret = iommu_init_pci(iommu);
> - if (ret)
> - break;
> -
> + if (ret) {
> + pr_err("IOMMU:%d Failed to initialize!\n", iommu->index);
Please make that message "IOMMU%d: Failed to initialize IOMMU Hardware (error=%d)!\n".
> + goto out;
> + }
> /* Need to setup range after PCI init */
> iommu_set_cwwb_range(iommu);
> }
> @@ -1960,6 +1961,10 @@ static int __init amd_iommu_init_pci(void)
> * active.
> */
> ret = amd_iommu_init_api();
> + if (ret) {
> + pr_err("IOMMU: Failed to initialize api!\n");
And that "IOMMU: Failed to initialize IOMMU-API interface (error=%d)!\n"
> + goto out;
> + }
>
> init_device_table_dma();
>
> @@ -1969,6 +1974,7 @@ static int __init amd_iommu_init_pci(void)
> if (!ret)
> print_iommu_info();
>
> +out:
> return ret;
> }
>
> --
> 2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci
2022-02-28 12:47 ` Joerg Roedel
@ 2022-03-01 8:58 ` Vasant Hegde via iommu
0 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-03-01 8:58 UTC (permalink / raw)
To: Joerg Roedel; +Cc: iommu
On 2/28/2022 6:17 PM, Joerg Roedel wrote:
> On Mon, Feb 21, 2022 at 10:29:12AM +0530, Vasant Hegde wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> Add error messages to prevent silent failure.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>> drivers/iommu/amd/init.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
>> index 1eacd43cb436..770ac679b682 100644
>> --- a/drivers/iommu/amd/init.c
>> +++ b/drivers/iommu/amd/init.c
>> @@ -1942,9 +1942,10 @@ static int __init amd_iommu_init_pci(void)
>>
>> for_each_iommu(iommu) {
>> ret = iommu_init_pci(iommu);
>> - if (ret)
>> - break;
>> -
>> + if (ret) {
>> + pr_err("IOMMU:%d Failed to initialize!\n", iommu->index);
>
> Please make that message "IOMMU%d: Failed to initialize IOMMU Hardware (error=%d)!\n".
Thanks Joerg for the review. I have fixed it in v2.
>
>> + goto out;
>> + }
>> /* Need to setup range after PCI init */
>> iommu_set_cwwb_range(iommu);
>> }
>> @@ -1960,6 +1961,10 @@ static int __init amd_iommu_init_pci(void)
>> * active.
>> */
>> ret = amd_iommu_init_api();
>> + if (ret) {
>> + pr_err("IOMMU: Failed to initialize api!\n");
>
> And that "IOMMU: Failed to initialize IOMMU-API interface (error=%d)!\n"
Fixed in v2.
-Vasant
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/5] iommu/amd: Call memunmap in error path
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci Vasant Hegde via iommu
@ 2022-02-21 4:59 ` Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 3/5] iommu/amd: Clean up function declarations Vasant Hegde via iommu
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-02-21 4:59 UTC (permalink / raw)
To: joro, iommu; +Cc: Vasant Hegde
From: Vasant Hegde <Vasant.Hegde@amd.com>
Unmap old_devtb in error path.
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/init.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 770ac679b682..92c489c1467d 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -979,6 +979,7 @@ static bool copy_device_table(void)
get_order(dev_table_size));
if (old_dev_tbl_cpy == NULL) {
pr_err("Failed to allocate memory for copying old device table!\n");
+ memunmap(old_devtb);
return false;
}
@@ -1009,6 +1010,7 @@ static bool copy_device_table(void)
if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
(int_tab_len != DTE_INTTABLEN)) {
pr_err("Wrong old irq remapping flag: %#x\n", devid);
+ memunmap(old_devtb);
return false;
}
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/5] iommu/amd: Clean up function declarations
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 1/5] iommu/amd: Improve error handling for amd_iommu_init_pci Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 2/5] iommu/amd: Call memunmap in error path Vasant Hegde via iommu
@ 2022-02-21 4:59 ` Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 4/5] iommu/amd: Remove unused struct fault.devid Vasant Hegde via iommu
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-02-21 4:59 UTC (permalink / raw)
To: joro, iommu; +Cc: Vasant Hegde
Remove unused declarations and add static keyword as needed.
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/amd_iommu.h | 4 ----
drivers/iommu/amd/init.c | 2 +-
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
index 416815a525d6..94d33626d692 100644
--- a/drivers/iommu/amd/amd_iommu.h
+++ b/drivers/iommu/amd/amd_iommu.h
@@ -14,10 +14,6 @@
extern irqreturn_t amd_iommu_int_thread(int irq, void *data);
extern irqreturn_t amd_iommu_int_handler(int irq, void *data);
extern void amd_iommu_apply_erratum_63(u16 devid);
-extern void amd_iommu_reset_cmd_buffer(struct amd_iommu *iommu);
-extern int amd_iommu_init_devices(void);
-extern void amd_iommu_uninit_devices(void);
-extern void amd_iommu_init_notifier(void);
extern int amd_iommu_init_api(void);
#ifdef CONFIG_AMD_IOMMU_DEBUGFS
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 92c489c1467d..df11c50deb74 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -660,7 +660,7 @@ static int __init alloc_command_buffer(struct amd_iommu *iommu)
* This function resets the command buffer if the IOMMU stopped fetching
* commands from it.
*/
-void amd_iommu_reset_cmd_buffer(struct amd_iommu *iommu)
+static void amd_iommu_reset_cmd_buffer(struct amd_iommu *iommu)
{
iommu_feature_disable(iommu, CONTROL_CMDBUF_EN);
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/5] iommu/amd: Remove unused struct fault.devid
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
` (2 preceding siblings ...)
2022-02-21 4:59 ` [PATCH 3/5] iommu/amd: Clean up function declarations Vasant Hegde via iommu
@ 2022-02-21 4:59 ` Vasant Hegde via iommu
2022-02-21 4:59 ` [PATCH 5/5] iommu/amd: Improve amd_iommu_v2_exit() Vasant Hegde via iommu
2022-02-28 12:48 ` [PATCH 0/5] iommu/amd: Cleanup and fixes Joerg Roedel
5 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-02-21 4:59 UTC (permalink / raw)
To: joro, iommu; +Cc: Vasant Hegde
This variable has not been used since it was introduced.
Co-developed-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/iommu_v2.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 58da08cc3d01..2daf37c21b85 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -71,7 +71,6 @@ struct fault {
struct pasid_state *state;
struct mm_struct *mm;
u64 address;
- u16 devid;
u32 pasid;
u16 tag;
u16 finish;
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/5] iommu/amd: Improve amd_iommu_v2_exit()
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
` (3 preceding siblings ...)
2022-02-21 4:59 ` [PATCH 4/5] iommu/amd: Remove unused struct fault.devid Vasant Hegde via iommu
@ 2022-02-21 4:59 ` Vasant Hegde via iommu
2022-02-28 12:48 ` [PATCH 0/5] iommu/amd: Cleanup and fixes Joerg Roedel
5 siblings, 0 replies; 9+ messages in thread
From: Vasant Hegde via iommu @ 2022-02-21 4:59 UTC (permalink / raw)
To: joro, iommu; +Cc: Vasant Hegde
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
During module exit, the current logic loops through all possible
16-bit device ID space to search for existing devices and clean up
device state structures. This can be simplified by looping through
the device state list.
Also, refactor various clean up logic into free_device_state()
for better reusability.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
---
drivers/iommu/amd/iommu_v2.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/amd/iommu_v2.c b/drivers/iommu/amd/iommu_v2.c
index 2daf37c21b85..c72969ac4956 100644
--- a/drivers/iommu/amd/iommu_v2.c
+++ b/drivers/iommu/amd/iommu_v2.c
@@ -24,7 +24,6 @@
MODULE_LICENSE("GPL v2");
MODULE_AUTHOR("Joerg Roedel <jroedel@suse.de>");
-#define MAX_DEVICES 0x10000
#define PRI_QUEUE_SIZE 512
struct pri_queue {
@@ -124,6 +123,15 @@ static void free_device_state(struct device_state *dev_state)
{
struct iommu_group *group;
+ /* Get rid of any remaining pasid states */
+ free_pasid_states(dev_state);
+
+ /*
+ * Wait until the last reference is dropped before freeing
+ * the device state.
+ */
+ wait_event(dev_state->wq, !atomic_read(&dev_state->count));
+
/*
* First detach device from domain - No more PRI requests will arrive
* from that device after it is unbound from the IOMMUv2 domain.
@@ -849,15 +857,7 @@ void amd_iommu_free_device(struct pci_dev *pdev)
spin_unlock_irqrestore(&state_lock, flags);
- /* Get rid of any remaining pasid states */
- free_pasid_states(dev_state);
-
put_device_state(dev_state);
- /*
- * Wait until the last reference is dropped before freeing
- * the device state.
- */
- wait_event(dev_state->wq, !atomic_read(&dev_state->count));
free_device_state(dev_state);
}
EXPORT_SYMBOL(amd_iommu_free_device);
@@ -954,8 +954,8 @@ static int __init amd_iommu_v2_init(void)
static void __exit amd_iommu_v2_exit(void)
{
- struct device_state *dev_state;
- int i;
+ struct device_state *dev_state, *next;
+ unsigned long flags;
if (!amd_iommu_v2_supported())
return;
@@ -968,18 +968,18 @@ static void __exit amd_iommu_v2_exit(void)
* The loop below might call flush_workqueue(), so call
* destroy_workqueue() after it
*/
- for (i = 0; i < MAX_DEVICES; ++i) {
- dev_state = get_device_state(i);
-
- if (dev_state == NULL)
- continue;
+ spin_lock_irqsave(&state_lock, flags);
+ list_for_each_entry_safe(dev_state, next, &state_list, list) {
WARN_ON_ONCE(1);
put_device_state(dev_state);
- amd_iommu_free_device(dev_state->pdev);
+ list_del(&dev_state->list);
+ free_device_state(dev_state);
}
+ spin_unlock_irqrestore(&state_lock, flags);
+
destroy_workqueue(iommu_wq);
}
--
2.27.0
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/5] iommu/amd: Cleanup and fixes
2022-02-21 4:59 [PATCH 0/5] iommu/amd: Cleanup and fixes Vasant Hegde via iommu
` (4 preceding siblings ...)
2022-02-21 4:59 ` [PATCH 5/5] iommu/amd: Improve amd_iommu_v2_exit() Vasant Hegde via iommu
@ 2022-02-28 12:48 ` Joerg Roedel
5 siblings, 0 replies; 9+ messages in thread
From: Joerg Roedel @ 2022-02-28 12:48 UTC (permalink / raw)
To: Vasant Hegde; +Cc: iommu
On Mon, Feb 21, 2022 at 10:29:11AM +0530, Vasant Hegde wrote:
> This series contains various cleanup and trivial fixes.
>
> Suravee Suthikulpanit (2):
> iommu/amd: Improve error handling for amd_iommu_init_pci
> iommu/amd: Improve amd_iommu_v2_exit()
>
> Vasant Hegde (3):
> iommu/amd: Call memunmap in error path
> iommu/amd: Clean up function declarations
> iommu/amd: Remove unused struct fault.devid
>
> drivers/iommu/amd/amd_iommu.h | 4 ----
> drivers/iommu/amd/init.c | 16 ++++++++++++----
> drivers/iommu/amd/iommu_v2.c | 35 +++++++++++++++++------------------
> 3 files changed, 29 insertions(+), 26 deletions(-)
Besides the error messages in the first patch this looks good to me.
Please re-send with the updates and I consider it for v5.18.
Thanks,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 9+ messages in thread