iommu/amd: flush IOTLB for specific domains only (v2)
Hello Arindam, There is a bug report[0] that you created a patch[1] for a while back. However, the patch never landed in mainline. There is a bug reporter in Ubuntu[2] that is affected by this bug and is willing to test the patch. I attempted to build a test kernel with the patch, but it does not apply to currently mainline cleanly. Do you still think this patch may resolve this bug? If so, is there a version of your patch available that will apply to current mainline? Thanks, Joe [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 [1] https://patchwork.freedesktop.org/patch/157327/ [2] http://pad.lv/1747463 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
From: Arindam Nath
Change History
--
v2: changes suggested by Joerg
- add flush flag to improve efficiency of flush operation
v1:
- The idea behind flush queues is to defer the IOTLB flushing
for domains for which the mappings are no longer valid. We
add such domains in queue_add(), and when the queue size
reaches FLUSH_QUEUE_SIZE, we perform __queue_flush().
Since we have already taken lock before __queue_flush()
is called, we need to make sure the IOTLB flushing is
performed as quickly as possible.
In the current implementation, we perform IOTLB flushing
for all domains irrespective of which ones were actually
added in the flush queue initially. This can be quite
expensive especially for domains for which unmapping is
not required at this point of time.
This patch makes use of domain information in
'struct flush_queue_entry' to make sure we only flush
IOTLBs for domains who need it, skipping others.
Suggested-by: Joerg Roedel
Signed-off-by: Arindam Nath
---
drivers/iommu/amd_iommu.c | 27 ---
drivers/iommu/amd_iommu_types.h | 2 ++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 63cacf5..1edeebec 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2227,15 +2227,26 @@ static struct iommu_group
*amd_iommu_device_group(struct device *dev)
static void __queue_flush(struct flush_queue *queue)
{
- struct protection_domain *domain;
- unsigned long flags;
int idx;
- /* First flush TLB of all known domains */
- spin_lock_irqsave(&amd_iommu_pd_lock, flags);
- list_for_each_entry(domain, &amd_iommu_pd_list, list)
- domain_flush_tlb(domain);
- spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
+ /* First flush TLB of all domains which were added to flush queue */
+ for (idx = 0; idx < queue->next; ++idx) {
+ struct flush_queue_entry *entry;
+
+ entry = queue->entries + idx;
+
+ /*
+* There might be cases where multiple IOVA entries for the
+* same domain are queued in the flush queue. To avoid
+* flushing the same domain again, we check whether the
+* flag is set or not. This improves the efficiency of
+* flush operation.
+*/
+ if (!entry->dma_dom->domain.already_flushed) {
+ entry->dma_dom->domain.already_flushed = true;
+ domain_flush_tlb(&entry->dma_dom->domain);
+ }
+ }
/* Wait until flushes have completed */
domain_flush_complete(NULL);
@@ -2289,6 +2300,8 @@ static void queue_add(struct dma_ops_domain *dma_dom,
pages = __roundup_pow_of_two(pages);
address >>= PAGE_SHIFT;
+ dma_dom->domain.already_flushed = false;
+
queue = get_cpu_ptr(&flush_queue);
spin_lock_irqsave(&queue->lock, flags);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 4de8f41..4f5519d 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -454,6 +454,8 @@ struct protection_domain {
bool updated; /* complete domain flush required */
unsigned dev_cnt; /* devices assigned to this domain */
unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
+ bool already_flushed; /* flag to avoid flushing the same domain again
+ in a single invocation of __queue_flush() */
};
/*
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: iommu/amd: flush IOTLB for specific domains only (v2)
Adding Tom. Hi Joe, My original patch was never accepted. Tom and Joerg worked on another patch series which was supposed to fix the issue in question in addition to do some code cleanups. I believe their patches are already in the mainline. If I remember correctly, one of the patches disabled PCI ATS for the graphics card which was causing the issue. Do you still see the issue with latest mainline kernel? BR, Arindam -Original Message- From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] Sent: Tuesday, May 15, 2018 1:17 AM To: Nath, Arindam Cc: io...@lists.linux-foundation.org; Bridgman, John ; j...@8bytes.org; amd-gfx@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; Suthikulpanit, Suravee ; Deucher, Alexander ; Kuehling, Felix ; li...@endlessm.com; mic...@daenzer.net; 1747...@bugs.launchpad.net Subject: iommu/amd: flush IOTLB for specific domains only (v2) Hello Arindam, There is a bug report[0] that you created a patch[1] for a while back. However, the patch never landed in mainline. There is a bug reporter in Ubuntu[2] that is affected by this bug and is willing to test the patch. I attempted to build a test kernel with the patch, but it does not apply to currently mainline cleanly. Do you still think this patch may resolve this bug? If so, is there a version of your patch available that will apply to current mainline? Thanks, Joe [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 [1] https://patchwork.freedesktop.org/patch/157327/ [2] http://pad.lv/1747463 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: iommu/amd: flush IOTLB for specific domains only (v2)
> -Original Message- > From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] > Sent: Tuesday, May 15, 2018 5:40 PM > To: Nath, Arindam > Cc: io...@lists.linux-foundation.org; Bridgman, John > ; j...@8bytes.org; amd- > g...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; > Suthikulpanit, Suravee ; Deucher, > Alexander ; Kuehling, Felix > ; li...@endlessm.com; mic...@daenzer.net; > 1747...@bugs.launchpad.net; Lendacky, Thomas > > Subject: Re: iommu/amd: flush IOTLB for specific domains only (v2) > > On 05/15/2018 04:03 AM, Nath, Arindam wrote: > > Adding Tom. > > > > Hi Joe, > > > > My original patch was never accepted. Tom and Joerg worked on another > patch series which was supposed to fix the issue in question in addition to do > some code cleanups. I believe their patches are already in the mainline. If I > remember correctly, one of the patches disabled PCI ATS for the graphics > card which was causing the issue. > > > > Do you still see the issue with latest mainline kernel? > > > > BR, > > Arindam > > > > -Original Message- > > From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] > > Sent: Tuesday, May 15, 2018 1:17 AM > > To: Nath, Arindam > > Cc: io...@lists.linux-foundation.org; Bridgman, John > > ; j...@8bytes.org; > > amd-gfx@lists.freedesktop.org; dr...@endlessm.com; > stein...@gmail.com; > > Suthikulpanit, Suravee ; Deucher, > > Alexander ; Kuehling, Felix > > ; li...@endlessm.com; mic...@daenzer.net; > > 1747...@bugs.launchpad.net > > Subject: iommu/amd: flush IOTLB for specific domains only (v2) > > > > Hello Arindam, > > > > There is a bug report[0] that you created a patch[1] for a while back. > However, the patch never landed in mainline. There is a bug reporter in > Ubuntu[2] that is affected by this bug and is willing to test the patch. I > attempted to build a test kernel with the patch, but it does not apply to > currently mainline cleanly. Do you still think this patch may resolve this > bug? If so, is there a version of your patch available that will apply to > current > mainline? > > > > Thanks, > > > > Joe > > > > [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 > > [1] https://patchwork.freedesktop.org/patch/157327/ > > [2] http://pad.lv/1747463 > > > Hi Arindam, > > Thanks for the feedback. Yes, the latest mainline kernel was tested, and it > is > reported the bug still happens in the Ubuntu kernel bug[0]. Is there any > specific diagnostic info we can collect that might help? Joe, I believe all the information needed is already provided in [2]. Let us wait for inputs from Tom and Joerg. I could take a look at the issue locally, but it will take me some really long time since I am occupied with other assignments right now. BR, Arindam > > Thanks, > > Joe > > [0] http://pad.lv/1747463 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: iommu/amd: flush IOTLB for specific domains only (v2)
On 5/15/2018 7:34 AM, Nath, Arindam wrote: > > >> -Original Message- >> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >> Sent: Tuesday, May 15, 2018 5:40 PM >> To: Nath, Arindam >> Cc: io...@lists.linux-foundation.org; Bridgman, John >> ; j...@8bytes.org; amd- >> g...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; >> Suthikulpanit, Suravee ; Deucher, >> Alexander ; Kuehling, Felix >> ; li...@endlessm.com; mic...@daenzer.net; >> 1747...@bugs.launchpad.net; Lendacky, Thomas >> >> Subject: Re: iommu/amd: flush IOTLB for specific domains only (v2) >> >> On 05/15/2018 04:03 AM, Nath, Arindam wrote: >>> Adding Tom. >>> >>> Hi Joe, >>> >>> My original patch was never accepted. Tom and Joerg worked on another >> patch series which was supposed to fix the issue in question in addition to >> do >> some code cleanups. I believe their patches are already in the mainline. If I >> remember correctly, one of the patches disabled PCI ATS for the graphics >> card which was causing the issue. >>> >>> Do you still see the issue with latest mainline kernel? >>> >>> BR, >>> Arindam >>> >>> -Original Message- >>> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >>> Sent: Tuesday, May 15, 2018 1:17 AM >>> To: Nath, Arindam >>> Cc: io...@lists.linux-foundation.org; Bridgman, John >>> ; j...@8bytes.org; >>> amd-gfx@lists.freedesktop.org; dr...@endlessm.com; >> stein...@gmail.com; >>> Suthikulpanit, Suravee ; Deucher, >>> Alexander ; Kuehling, Felix >>> ; li...@endlessm.com; mic...@daenzer.net; >>> 1747...@bugs.launchpad.net >>> Subject: iommu/amd: flush IOTLB for specific domains only (v2) >>> >>> Hello Arindam, >>> >>> There is a bug report[0] that you created a patch[1] for a while back. >> However, the patch never landed in mainline. There is a bug reporter in >> Ubuntu[2] that is affected by this bug and is willing to test the patch. I >> attempted to build a test kernel with the patch, but it does not apply to >> currently mainline cleanly. Do you still think this patch may resolve this >> bug? If so, is there a version of your patch available that will apply to >> current >> mainline? >>> >>> Thanks, >>> >>> Joe >>> >>> [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 >>> [1] https://patchwork.freedesktop.org/patch/157327/ >>> [2] http://pad.lv/1747463 >>> >> Hi Arindam, >> >> Thanks for the feedback. Yes, the latest mainline kernel was tested, and it >> is >> reported the bug still happens in the Ubuntu kernel bug[0]. Is there any >> specific diagnostic info we can collect that might help? > > Joe, I believe all the information needed is already provided in [2]. Let us > wait for inputs from Tom and Joerg. > > I could take a look at the issue locally, but it will take me some really > long time since I am occupied with other assignments right now. I don't see anything in the bug that indicates the latest mainline kernel was tested. The patches/fixes in question are part of the 4.13 kernel, I only see references to 4.10 kernels so I wouldn't expect the issue to be resolved unless the patches from 4.13 were backported to the Ubuntu 4.10 kernel. Thanks, Tom > > BR, > Arindam > >> >> Thanks, >> >> Joe >> >> [0] http://pad.lv/1747463 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: iommu/amd: flush IOTLB for specific domains only (v2)
On 05/15/2018 04:03 AM, Nath, Arindam wrote: > Adding Tom. > > Hi Joe, > > My original patch was never accepted. Tom and Joerg worked on another patch > series which was supposed to fix the issue in question in addition to do some > code cleanups. I believe their patches are already in the mainline. If I > remember correctly, one of the patches disabled PCI ATS for the graphics card > which was causing the issue. > > Do you still see the issue with latest mainline kernel? > > BR, > Arindam > > -Original Message- > From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] > Sent: Tuesday, May 15, 2018 1:17 AM > To: Nath, Arindam > Cc: io...@lists.linux-foundation.org; Bridgman, John ; > j...@8bytes.org; amd-gfx@lists.freedesktop.org; dr...@endlessm.com; > stein...@gmail.com; Suthikulpanit, Suravee ; > Deucher, Alexander ; Kuehling, Felix > ; li...@endlessm.com; mic...@daenzer.net; > 1747...@bugs.launchpad.net > Subject: iommu/amd: flush IOTLB for specific domains only (v2) > > Hello Arindam, > > There is a bug report[0] that you created a patch[1] for a while back. > However, the patch never landed in mainline. There is a bug reporter in > Ubuntu[2] that is affected by this bug and is willing to test the patch. I > attempted to build a test kernel with the patch, but it does not apply to > currently mainline cleanly. Do you still think this patch may resolve this > bug? If so, is there a version of your patch available that will apply to > current mainline? > > Thanks, > > Joe > > [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 > [1] https://patchwork.freedesktop.org/patch/157327/ > [2] http://pad.lv/1747463 > Hi Arindam, Thanks for the feedback. Yes, the latest mainline kernel was tested, and it is reported the bug still happens in the Ubuntu kernel bug[0]. Is there any specific diagnostic info we can collect that might help? Thanks, Joe [0] http://pad.lv/1747463 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: iommu/amd: flush IOTLB for specific domains only (v2)
On 05/15/2018 09:08 AM, Tom Lendacky wrote: > On 5/15/2018 7:34 AM, Nath, Arindam wrote: >> >>> -Original Message- >>> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >>> Sent: Tuesday, May 15, 2018 5:40 PM >>> To: Nath, Arindam >>> Cc: io...@lists.linux-foundation.org; Bridgman, John >>> ; j...@8bytes.org; amd- >>> g...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; >>> Suthikulpanit, Suravee ; Deucher, >>> Alexander ; Kuehling, Felix >>> ; li...@endlessm.com; mic...@daenzer.net; >>> 1747...@bugs.launchpad.net; Lendacky, Thomas >>> >>> Subject: Re: iommu/amd: flush IOTLB for specific domains only (v2) >>> >>> On 05/15/2018 04:03 AM, Nath, Arindam wrote: >>>> Adding Tom. >>>> >>>> Hi Joe, >>>> >>>> My original patch was never accepted. Tom and Joerg worked on another >>> patch series which was supposed to fix the issue in question in addition to >>> do >>> some code cleanups. I believe their patches are already in the mainline. If >>> I >>> remember correctly, one of the patches disabled PCI ATS for the graphics >>> card which was causing the issue. >>>> Do you still see the issue with latest mainline kernel? >>>> >>>> BR, >>>> Arindam >>>> >>>> -Original Message- >>>> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >>>> Sent: Tuesday, May 15, 2018 1:17 AM >>>> To: Nath, Arindam >>>> Cc: io...@lists.linux-foundation.org; Bridgman, John >>>> ; j...@8bytes.org; >>>> amd-gfx@lists.freedesktop.org; dr...@endlessm.com; >>> stein...@gmail.com; >>>> Suthikulpanit, Suravee ; Deucher, >>>> Alexander ; Kuehling, Felix >>>> ; li...@endlessm.com; mic...@daenzer.net; >>>> 1747...@bugs.launchpad.net >>>> Subject: iommu/amd: flush IOTLB for specific domains only (v2) >>>> >>>> Hello Arindam, >>>> >>>> There is a bug report[0] that you created a patch[1] for a while back. >>> However, the patch never landed in mainline. There is a bug reporter in >>> Ubuntu[2] that is affected by this bug and is willing to test the patch. I >>> attempted to build a test kernel with the patch, but it does not apply to >>> currently mainline cleanly. Do you still think this patch may resolve this >>> bug? If so, is there a version of your patch available that will apply to >>> current >>> mainline? >>>> Thanks, >>>> >>>> Joe >>>> >>>> [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 >>>> [1] https://patchwork.freedesktop.org/patch/157327/ >>>> [2] http://pad.lv/1747463 >>>> >>> Hi Arindam, >>> >>> Thanks for the feedback. Yes, the latest mainline kernel was tested, and >>> it is >>> reported the bug still happens in the Ubuntu kernel bug[0]. Is there any >>> specific diagnostic info we can collect that might help? >> Joe, I believe all the information needed is already provided in [2]. Let us >> wait for inputs from Tom and Joerg. >> >> I could take a look at the issue locally, but it will take me some really >> long time since I am occupied with other assignments right now. > I don't see anything in the bug that indicates the latest mainline kernel > was tested. The patches/fixes in question are part of the 4.13 kernel, I > only see references to 4.10 kernels so I wouldn't expect the issue to be > resolved unless the patches from 4.13 were backported to the Ubuntu 4.10 > kernel. > > Thanks, > Tom > >> BR, >> Arindam >> >>> Thanks, >>> >>> Joe >>> >>> [0] http://pad.lv/1747463 Hi Tom, The request to test mainline was in comment #30[0]. However, the bug reporter stated the bug still existed on IRC and not in the bug report. I'll request he adds the test results to the bug. Thanks, Joe [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1747463/comments/30 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: iommu/amd: flush IOTLB for specific domains only (v2)
On 5/15/2018 9:47 AM, Joseph Salisbury wrote: > On 05/15/2018 09:08 AM, Tom Lendacky wrote: >> On 5/15/2018 7:34 AM, Nath, Arindam wrote: >>> >>>> -Original Message- >>>> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >>>> Sent: Tuesday, May 15, 2018 5:40 PM >>>> To: Nath, Arindam >>>> Cc: io...@lists.linux-foundation.org; Bridgman, John >>>> ; j...@8bytes.org; amd- >>>> g...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; >>>> Suthikulpanit, Suravee ; Deucher, >>>> Alexander ; Kuehling, Felix >>>> ; li...@endlessm.com; mic...@daenzer.net; >>>> 1747...@bugs.launchpad.net; Lendacky, Thomas >>>> >>>> Subject: Re: iommu/amd: flush IOTLB for specific domains only (v2) >>>> >>>> On 05/15/2018 04:03 AM, Nath, Arindam wrote: >>>>> Adding Tom. >>>>> >>>>> Hi Joe, >>>>> >>>>> My original patch was never accepted. Tom and Joerg worked on another >>>> patch series which was supposed to fix the issue in question in addition >>>> to do >>>> some code cleanups. I believe their patches are already in the mainline. >>>> If I >>>> remember correctly, one of the patches disabled PCI ATS for the graphics >>>> card which was causing the issue. >>>>> Do you still see the issue with latest mainline kernel? >>>>> >>>>> BR, >>>>> Arindam >>>>> >>>>> -Original Message- >>>>> From: Joseph Salisbury [mailto:joseph.salisb...@canonical.com] >>>>> Sent: Tuesday, May 15, 2018 1:17 AM >>>>> To: Nath, Arindam >>>>> Cc: io...@lists.linux-foundation.org; Bridgman, John >>>>> ; j...@8bytes.org; >>>>> amd-gfx@lists.freedesktop.org; dr...@endlessm.com; >>>> stein...@gmail.com; >>>>> Suthikulpanit, Suravee ; Deucher, >>>>> Alexander ; Kuehling, Felix >>>>> ; li...@endlessm.com; mic...@daenzer.net; >>>>> 1747...@bugs.launchpad.net >>>>> Subject: iommu/amd: flush IOTLB for specific domains only (v2) >>>>> >>>>> Hello Arindam, >>>>> >>>>> There is a bug report[0] that you created a patch[1] for a while back. >>>> However, the patch never landed in mainline. There is a bug reporter in >>>> Ubuntu[2] that is affected by this bug and is willing to test the patch. I >>>> attempted to build a test kernel with the patch, but it does not apply to >>>> currently mainline cleanly. Do you still think this patch may resolve this >>>> bug? If so, is there a version of your patch available that will apply to >>>> current >>>> mainline? >>>>> Thanks, >>>>> >>>>> Joe >>>>> >>>>> [0] https://bugs.freedesktop.org/show_bug.cgi?id=101029 >>>>> [1] https://patchwork.freedesktop.org/patch/157327/ >>>>> [2] http://pad.lv/1747463 >>>>> >>>> Hi Arindam, >>>> >>>> Thanks for the feedback. Yes, the latest mainline kernel was tested, and >>>> it is >>>> reported the bug still happens in the Ubuntu kernel bug[0]. Is there any >>>> specific diagnostic info we can collect that might help? >>> Joe, I believe all the information needed is already provided in [2]. Let >>> us wait for inputs from Tom and Joerg. >>> >>> I could take a look at the issue locally, but it will take me some really >>> long time since I am occupied with other assignments right now. >> I don't see anything in the bug that indicates the latest mainline kernel >> was tested. The patches/fixes in question are part of the 4.13 kernel, I >> only see references to 4.10 kernels so I wouldn't expect the issue to be >> resolved unless the patches from 4.13 were backported to the Ubuntu 4.10 >> kernel. >> >> Thanks, >> Tom >> >>> BR, >>> Arindam >>> >>>> Thanks, >>>> >>>> Joe >>>> >>>> [0] http://pad.lv/1747463 > Hi Tom, > > The request to test mainline was in comment #30[0]. However, the bug > reporter stated the bug still existed on IRC and not in the bug report. > I'll request he adds the test results to the bug. > Ok, I was looking at the wrong bug. For the original 4.13 kernel, I don't see a
Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
On Fri, 2017-05-19 at 15:32 +0530, arindam.n...@amd.com wrote:
> From: Arindam Nath
>
> Change History
> --
>
> v2: changes suggested by Joerg
> - add flush flag to improve efficiency of flush operation
>
> v1:
> - The idea behind flush queues is to defer the IOTLB flushing
> for domains for which the mappings are no longer valid. We
> add such domains in queue_add(), and when the queue size
> reaches FLUSH_QUEUE_SIZE, we perform __queue_flush().
>
> Since we have already taken lock before __queue_flush()
> is called, we need to make sure the IOTLB flushing is
> performed as quickly as possible.
>
> In the current implementation, we perform IOTLB flushing
> for all domains irrespective of which ones were actually
> added in the flush queue initially. This can be quite
> expensive especially for domains for which unmapping is
> not required at this point of time.
>
> This patch makes use of domain information in
> 'struct flush_queue_entry' to make sure we only flush
> IOTLBs for domains who need it, skipping others.
Hi,
just a note, the patch also fixes "AMD-Vi: Completion-Wait loop timed
out" boot hang on my system (carrizo + iceland) [0,1,2]. Presumably the
old loop also tried to flush domains that included powered-off devices.
regards,
Jan
[0] https://github.com/RadeonOpenCompute/ROCK-Kernel-Driver/issues/20
[1] https://bugs.freedesktop.org/show_bug.cgi?id=101029
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1448121
>
> Suggested-by: Joerg Roedel
> Signed-off-by: Arindam Nath
> ---
> drivers/iommu/amd_iommu.c | 27 ---
> drivers/iommu/amd_iommu_types.h | 2 ++
> 2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 63cacf5..1edeebec 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -2227,15 +2227,26 @@ static struct iommu_group
> *amd_iommu_device_group(struct device *dev)
>
> static void __queue_flush(struct flush_queue *queue)
> {
> - struct protection_domain *domain;
> - unsigned long flags;
> int idx;
>
> - /* First flush TLB of all known domains */
> - spin_lock_irqsave(&amd_iommu_pd_lock, flags);
> - list_for_each_entry(domain, &amd_iommu_pd_list, list)
> - domain_flush_tlb(domain);
> - spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
> + /* First flush TLB of all domains which were added to flush queue */
> + for (idx = 0; idx < queue->next; ++idx) {
> + struct flush_queue_entry *entry;
> +
> + entry = queue->entries + idx;
> +
> + /*
> + * There might be cases where multiple IOVA entries for the
> + * same domain are queued in the flush queue. To avoid
> + * flushing the same domain again, we check whether the
> + * flag is set or not. This improves the efficiency of
> + * flush operation.
> + */
> + if (!entry->dma_dom->domain.already_flushed) {
> + entry->dma_dom->domain.already_flushed = true;
> + domain_flush_tlb(&entry->dma_dom->domain);
> + }
> + }
>
> /* Wait until flushes have completed */
> domain_flush_complete(NULL);
> @@ -2289,6 +2300,8 @@ static void queue_add(struct dma_ops_domain *dma_dom,
> pages = __roundup_pow_of_two(pages);
> address >>= PAGE_SHIFT;
>
> + dma_dom->domain.already_flushed = false;
> +
> queue = get_cpu_ptr(&flush_queue);
> spin_lock_irqsave(&queue->lock, flags);
>
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 4de8f41..4f5519d 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -454,6 +454,8 @@ struct protection_domain {
> bool updated; /* complete domain flush required */
> unsigned dev_cnt; /* devices assigned to this domain */
> unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
> + bool already_flushed; /* flag to avoid flushing the same domain again
> +in a single invocation of __queue_flush() */
> };
>
> /*
--
Jan Vesely
signature.asc
Description: This is a digitally signed message part
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
>-Original Message-
>From: Jan Vesely [mailto:jv...@scarletmail.rutgers.edu] On Behalf Of Jan
>Vesely
>Sent: Friday, May 19, 2017 7:06 PM
>To: Nath, Arindam; io...@lists.linux-foundation.org
>Cc: Bridgman, John; Joerg Roedel; amd-gfx@lists.freedesktop.org;
>dr...@endlessm.com; Craig Stein; Suthikulpanit, Suravee; Deucher,
>Alexander; Kuehling, Felix; li...@endlessm.com; mic...@daenzer.net
>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
>
>On Fri, 2017-05-19 at 15:32 +0530, arindam.n...@amd.com wrote:
>> From: Arindam Nath
>>
>> Change History
>> --
>>
>> v2: changes suggested by Joerg
>> - add flush flag to improve efficiency of flush operation
Joerg, do you have any comments on the patch?
Thanks,
Arindam
>>
>> v1:
>> - The idea behind flush queues is to defer the IOTLB flushing
>> for domains for which the mappings are no longer valid. We
>> add such domains in queue_add(), and when the queue size
>> reaches FLUSH_QUEUE_SIZE, we perform __queue_flush().
>>
>> Since we have already taken lock before __queue_flush()
>> is called, we need to make sure the IOTLB flushing is
>> performed as quickly as possible.
>>
>> In the current implementation, we perform IOTLB flushing
>> for all domains irrespective of which ones were actually
>> added in the flush queue initially. This can be quite
>> expensive especially for domains for which unmapping is
>> not required at this point of time.
>>
>> This patch makes use of domain information in
>> 'struct flush_queue_entry' to make sure we only flush
>> IOTLBs for domains who need it, skipping others.
>
>Hi,
>
>just a note, the patch also fixes "AMD-Vi: Completion-Wait loop timed
>out" boot hang on my system (carrizo + iceland) [0,1,2]. Presumably the
>old loop also tried to flush domains that included powered-off devices.
>
>regards,
>Jan
>
>[0] https://github.com/RadeonOpenCompute/ROCK-Kernel-Driver/issues/20
>[1] https://bugs.freedesktop.org/show_bug.cgi?id=101029
>[2] https://bugzilla.redhat.com/show_bug.cgi?id=1448121
>
>>
>> Suggested-by: Joerg Roedel
>> Signed-off-by: Arindam Nath
>> ---
>> drivers/iommu/amd_iommu.c | 27 ---
>> drivers/iommu/amd_iommu_types.h | 2 ++
>> 2 files changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>> index 63cacf5..1edeebec 100644
>> --- a/drivers/iommu/amd_iommu.c
>> +++ b/drivers/iommu/amd_iommu.c
>> @@ -2227,15 +2227,26 @@ static struct iommu_group
>*amd_iommu_device_group(struct device *dev)
>>
>> static void __queue_flush(struct flush_queue *queue)
>> {
>> -struct protection_domain *domain;
>> -unsigned long flags;
>> int idx;
>>
>> -/* First flush TLB of all known domains */
>> -spin_lock_irqsave(&amd_iommu_pd_lock, flags);
>> -list_for_each_entry(domain, &amd_iommu_pd_list, list)
>> -domain_flush_tlb(domain);
>> -spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
>> +/* First flush TLB of all domains which were added to flush queue */
>> +for (idx = 0; idx < queue->next; ++idx) {
>> +struct flush_queue_entry *entry;
>> +
>> +entry = queue->entries + idx;
>> +
>> +/*
>> + * There might be cases where multiple IOVA entries for the
>> + * same domain are queued in the flush queue. To avoid
>> + * flushing the same domain again, we check whether the
>> + * flag is set or not. This improves the efficiency of
>> + * flush operation.
>> + */
>> +if (!entry->dma_dom->domain.already_flushed) {
>> +entry->dma_dom->domain.already_flushed = true;
>> +domain_flush_tlb(&entry->dma_dom->domain);
>> +}
>> +}
>>
>> /* Wait until flushes have completed */
>> domain_flush_complete(NULL);
>> @@ -2289,6 +2300,8 @@ static void queue_add(struct dma_ops_domain
>*dma_dom,
>> pages = __roundup_pow_of_two(pages);
>> address >>= PAGE_SHIFT;
>>
>> +dma_dom->domain.already_flushed = false;
>> +
>> queue = get_cpu_ptr(&flush_queue);
>> spin_lock_irqsave(&queue->lock, flags);
>>
>> diff --git a/drivers/iommu/amd_iommu_types.h
>b/drivers/iommu/amd_iommu_types.h
>>
Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
On 19/05/17 07:02 PM, arindam.n...@amd.com wrote:
> From: Arindam Nath
>
> Change History
> --
>
> v2: changes suggested by Joerg
> - add flush flag to improve efficiency of flush operation
>
> v1:
> - The idea behind flush queues is to defer the IOTLB flushing
> for domains for which the mappings are no longer valid. We
> add such domains in queue_add(), and when the queue size
> reaches FLUSH_QUEUE_SIZE, we perform __queue_flush().
>
> Since we have already taken lock before __queue_flush()
> is called, we need to make sure the IOTLB flushing is
> performed as quickly as possible.
>
> In the current implementation, we perform IOTLB flushing
> for all domains irrespective of which ones were actually
> added in the flush queue initially. This can be quite
> expensive especially for domains for which unmapping is
> not required at this point of time.
>
> This patch makes use of domain information in
> 'struct flush_queue_entry' to make sure we only flush
> IOTLBs for domains who need it, skipping others.
>
> Suggested-by: Joerg Roedel
> Signed-off-by: Arindam Nath
Please add these tags:
Fixes: b1516a14657a ("iommu/amd: Implement flush queue")
Cc: sta...@vger.kernel.org
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v2)
On 22/05/17 10:08 AM, Michel Dänzer wrote:
> On 19/05/17 07:02 PM, arindam.n...@amd.com wrote:
>> From: Arindam Nath
>>
>> Change History
>> --
>>
>> v2: changes suggested by Joerg
>> - add flush flag to improve efficiency of flush operation
>>
>> v1:
>> - The idea behind flush queues is to defer the IOTLB flushing
>> for domains for which the mappings are no longer valid. We
>> add such domains in queue_add(), and when the queue size
>> reaches FLUSH_QUEUE_SIZE, we perform __queue_flush().
>>
>> Since we have already taken lock before __queue_flush()
>> is called, we need to make sure the IOTLB flushing is
>> performed as quickly as possible.
>>
>> In the current implementation, we perform IOTLB flushing
>> for all domains irrespective of which ones were actually
>> added in the flush queue initially. This can be quite
>> expensive especially for domains for which unmapping is
>> not required at this point of time.
>>
>> This patch makes use of domain information in
>> 'struct flush_queue_entry' to make sure we only flush
>> IOTLBs for domains who need it, skipping others.
>>
>> Suggested-by: Joerg Roedel
>> Signed-off-by: Arindam Nath
>
> Please add these tags:
>
> Fixes: b1516a14657a ("iommu/amd: Implement flush queue")
> Cc: sta...@vger.kernel.org
Also
Bugzilla: https://bugs.freedesktop.org/101029
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
