RE: [PATCH] iommu/amd: return devid as alias for ACPI HID devices
Hi Joerg, > -Original Message- > From: Joerg Roedel > Sent: Tuesday, September 25, 2018 7:02 PM > To: Nath, Arindam > Cc: iommu@lists.linux-foundation.org; Suthikulpanit, Suravee > > Subject: Re: [PATCH] iommu/amd: return devid as alias for ACPI HID devices > > Hi Arindam, > > On Tue, Sep 18, 2018 at 03:40:58PM +0530, Arindam Nath wrote: > > @@ -246,7 +246,13 @@ static u16 get_alias(struct device *dev) > > > > /* The callers make sure that get_device_id() does not fail here */ > > devid = get_device_id(dev); > > + > > + /* For ACPI HID devices, we simply return the devid as such */ > > + if (!dev_is_pci(dev)) > > + return devid; > > + > > Does this fix some bug? If there is no alias defined for the device-id, > it should just return the correct devid even without that change. It does fix an issue where eMMC controller is an ACPI HID device and it fails to work with IOMMU enabled. Since for the ACPI HID, there is no actual pci_dev backing it, there is a NULL pointer de-referencing inside pci_for_each_dma_alias() while trying to access pdev->bus->number. Thanks, Arindam > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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 <arindam.n...@amd.com> > Cc: iommu@lists.linux-foundation.org; Bridgman, John > <john.bridg...@amd.com>; j...@8bytes.org; amd- > g...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; > Suthikulpanit, Suravee <suravee.suthikulpa...@amd.com>; Deucher, > Alexander <alexander.deuc...@amd.com>; Kuehling, Felix > <felix.kuehl...@amd.com>; li...@endlessm.com; mic...@daenzer.net; > 1747...@bugs.launchpad.net; Lendacky, Thomas > <thomas.lenda...@amd.com> > 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 <arindam.n...@amd.com> > > Cc: iommu@lists.linux-foundation.org; Bridgman, John > > <john.bridg...@amd.com>; j...@8bytes.org; > > amd-...@lists.freedesktop.org; dr...@endlessm.com; > stein...@gmail.com; > > Suthikulpanit, Suravee <suravee.suthikulpa...@amd.com>; Deucher, > > Alexander <alexander.deuc...@amd.com>; Kuehling, Felix > > <felix.kuehl...@amd.com>; 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
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 <arindam.n...@amd.com> Cc: iommu@lists.linux-foundation.org; Bridgman, John <john.bridg...@amd.com>; j...@8bytes.org; amd-...@lists.freedesktop.org; dr...@endlessm.com; stein...@gmail.com; Suthikulpanit, Suravee <suravee.suthikulpa...@amd.com>; Deucher, Alexander <alexander.deuc...@amd.com>; Kuehling, Felix <felix.kuehl...@amd.com>; 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only (v3)
Hi Daniel, >-Original Message- >From: Daniel Drake [mailto:dr...@endlessm.com] >Sent: Tuesday, September 12, 2017 12:20 PM >To: Nath, Arindam <arindam.n...@amd.com> >Cc: Joerg Roedel <j...@8bytes.org>; Lendacky, Thomas ><thomas.lenda...@amd.com>; iommu@lists.linux-foundation.org; amd- >g...@lists.freedesktop.org; Deucher, Alexander ><alexander.deuc...@amd.com>; Bridgman, John ><john.bridg...@amd.com>; Suthikulpanit, Suravee ><suravee.suthikulpa...@amd.com>; li...@endlessm.com; Craig Stein ><stein...@gmail.com>; mic...@daenzer.net; Kuehling, Felix ><felix.kuehl...@amd.com>; sta...@vger.kernel.org >Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v3) > >Hi, > >On Tue, May 30, 2017 at 3:38 PM, Nath, Arindam <arindam.n...@amd.com> >wrote: >>>-----Original Message- >>>From: Joerg Roedel [mailto:j...@8bytes.org] >>>Sent: Monday, May 29, 2017 8:09 PM >>>To: Nath, Arindam <arindam.n...@amd.com>; Lendacky, Thomas >>><thomas.lenda...@amd.com> >>>Cc: iommu@lists.linux-foundation.org; amd-...@lists.freedesktop.org; >>>Deucher, Alexander <alexander.deuc...@amd.com>; Bridgman, John >>><john.bridg...@amd.com>; dr...@endlessm.com; Suthikulpanit, >Suravee >>><suravee.suthikulpa...@amd.com>; li...@endlessm.com; Craig Stein >>><stein...@gmail.com>; mic...@daenzer.net; Kuehling, Felix >>><felix.kuehl...@amd.com>; sta...@vger.kernel.org >>>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only >(v3) >>> >>>Hi Arindam, >>> >>>I met Tom Lendacky last week in Nuremberg last week and he told me he is >>>working on the same area of the code that this patch is for. His reason >>>for touching this code was to solve some locking problems. Maybe you two >>>can work together on a joint approach to improve this? >> >> Sure Joerg, I will work with Tom. > >What was the end result here? I see that the code has been reworked in >4.13 so your original patch no longer applies. Is the reworked version >also expected to solve the original issue? Yes, if the reworked patch resolves the issue, my patch won't be required anymore. Thanks, Arindam > >Thanks >Daniel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v1 3/3] iommu/amd: Optimize the IOMMU queue flush
>-Original Message-
>From: Lendacky, Thomas
>Sent: Tuesday, June 06, 2017 1:23 AM
>To: iommu@lists.linux-foundation.org
>Cc: Nath, Arindam <arindam.n...@amd.com>; Joerg Roedel
><j...@8bytes.org>; Duran, Leo <leo.du...@amd.com>; Suthikulpanit,
>Suravee <suravee.suthikulpa...@amd.com>
>Subject: [PATCH v1 3/3] iommu/amd: Optimize the IOMMU queue flush
>
>After reducing the amount of MMIO performed by the IOMMU during
>operation,
>perf data shows that flushing the TLB for all protection domains during
>DMA unmapping is a performance issue. It is not necessary to flush the
>TLBs for all protection domains, only the protection domains associated
>with iova's on the flush queue.
>
>Create a separate queue that tracks the protection domains associated with
>the iova's on the flush queue. This new queue optimizes the flushing of
>TLBs to the required protection domains.
>
>Reviewed-by: Arindam Nath <arindam.n...@amd.com>
>Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
>---
> drivers/iommu/amd_iommu.c | 56
>-
> 1 file changed, 50 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>index 856103b..a5e77f0 100644
>--- a/drivers/iommu/amd_iommu.c
>+++ b/drivers/iommu/amd_iommu.c
>@@ -103,7 +103,18 @@ struct flush_queue {
> struct flush_queue_entry *entries;
> };
>
>+struct flush_pd_queue_entry {
>+ struct protection_domain *pd;
>+};
>+
>+struct flush_pd_queue {
>+ /* No lock needed, protected by flush_queue lock */
>+ unsigned next;
>+ struct flush_pd_queue_entry *entries;
>+};
>+
> static DEFINE_PER_CPU(struct flush_queue, flush_queue);
>+static DEFINE_PER_CPU(struct flush_pd_queue, flush_pd_queue);
>
> static atomic_t queue_timer_on;
> static struct timer_list queue_timer;
>@@ -2227,16 +2238,20 @@ static struct iommu_group
>*amd_iommu_device_group(struct device *dev)
> *
>
>***
>**/
>
>-static void __queue_flush(struct flush_queue *queue)
>+static void __queue_flush(struct flush_queue *queue,
>+struct flush_pd_queue *pd_queue)
> {
>- struct protection_domain *domain;
> unsigned long flags;
> int idx;
>
> /* First flush TLB of all known domains */
> spin_lock_irqsave(_iommu_pd_lock, flags);
>- list_for_each_entry(domain, _iommu_pd_list, list)
>- domain_flush_tlb(domain);
>+ for (idx = 0; idx < pd_queue->next; ++idx) {
>+ struct flush_pd_queue_entry *entry;
>+
>+ entry = pd_queue->entries + idx;
>+ domain_flush_tlb(entry->pd);
>+ }
> spin_unlock_irqrestore(_iommu_pd_lock, flags);
>
> /* Wait until flushes have completed */
>@@ -2255,6 +2270,7 @@ static void __queue_flush(struct flush_queue
>*queue)
> entry->dma_dom = NULL;
> }
>
>+ pd_queue->next = 0;
> queue->next = 0;
> }
>
>@@ -2263,13 +2279,15 @@ static void queue_flush_all(void)
> int cpu;
>
> for_each_possible_cpu(cpu) {
>+ struct flush_pd_queue *pd_queue;
> struct flush_queue *queue;
> unsigned long flags;
>
> queue = per_cpu_ptr(_queue, cpu);
>+ pd_queue = per_cpu_ptr(_pd_queue, cpu);
> spin_lock_irqsave(>lock, flags);
> if (queue->next > 0)
>- __queue_flush(queue);
>+ __queue_flush(queue, pd_queue);
> spin_unlock_irqrestore(>lock, flags);
> }
> }
>@@ -2283,6 +2301,8 @@ static void queue_flush_timeout(unsigned long
>unsused)
> static void queue_add(struct dma_ops_domain *dma_dom,
> unsigned long address, unsigned long pages)
> {
>+ struct flush_pd_queue_entry *pd_entry;
>+ struct flush_pd_queue *pd_queue;
> struct flush_queue_entry *entry;
> struct flush_queue *queue;
> unsigned long flags;
>@@ -2292,10 +2312,22 @@ static void queue_add(struct dma_ops_domain
>*dma_dom,
> address >>= PAGE_SHIFT;
>
> queue = get_cpu_ptr(_queue);
>+ pd_queue = get_cpu_ptr(_pd_queue);
> spin_lock_irqsave(>lock, flags);
>
> if (queue->next == FLUSH_QUEUE_SIZE)
>- __queue_flush(queue);
>+ __queue_flush(queue, pd_queue);
>+
>+ for (idx = 0; idx < pd_queue->next; ++idx) {
>+ pd_entry = pd_queue->entries + idx;
>+ if (pd_entry->
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only (v3)
>-Original Message-
>From: Joerg Roedel [mailto:j...@8bytes.org]
>Sent: Monday, May 29, 2017 8:09 PM
>To: Nath, Arindam <arindam.n...@amd.com>; Lendacky, Thomas
><thomas.lenda...@amd.com>
>Cc: iommu@lists.linux-foundation.org; amd-...@lists.freedesktop.org;
>Deucher, Alexander <alexander.deuc...@amd.com>; Bridgman, John
><john.bridg...@amd.com>; dr...@endlessm.com; Suthikulpanit, Suravee
><suravee.suthikulpa...@amd.com>; li...@endlessm.com; Craig Stein
><stein...@gmail.com>; mic...@daenzer.net; Kuehling, Felix
><felix.kuehl...@amd.com>; sta...@vger.kernel.org
>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only (v3)
>
>Hi Arindam,
>
>I met Tom Lendacky last week in Nuremberg last week and he told me he is
>working on the same area of the code that this patch is for. His reason
>for touching this code was to solve some locking problems. Maybe you two
>can work together on a joint approach to improve this?
Sure Joerg, I will work with Tom.
Thanks.
>
>On Mon, May 22, 2017 at 01:18:01PM +0530, arindam.n...@amd.com wrote:
>> 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(_iommu_pd_lock, flags);
>> -list_for_each_entry(domain, _iommu_pd_list, list)
>> -domain_flush_tlb(domain);
>> -spin_unlock_irqrestore(_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(>dma_dom->domain);
>> +}
>> +}
>
>There is also a race condition here I have to look into. It is not
>introduced by your patch, but needs fixing anyway. I'll look into this
>too.
>
>
>Regards,
>
> Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
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; iommu@lists.linux-foundation.org
>Cc: Bridgman, John; Joerg Roedel; amd-...@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 <arindam.n...@amd.com>
>>
>> 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 <j...@8bytes.org>
>> Signed-off-by: Arindam Nath <arindam.n...@amd.com>
>> ---
>> 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(_iommu_pd_lock, flags);
>> -list_for_each_entry(domain, _iommu_pd_list, list)
>> -domain_flush_tlb(domain);
>> -spin_unlock_irqrestore(_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(>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(_queue);
>> spin_lock_irqsave(>lock, flags);
>>
>> diff --git a/drivers/iommu/amd
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only
>-Original Message- >From: Daniel Drake [mailto:dr...@endlessm.com] >Sent: Thursday, March 30, 2017 7:15 PM >To: Nath, Arindam >Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd- >g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit, >Suravee; Linux Upstreaming Team >Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only > >On Thu, Mar 30, 2017 at 12:23 AM, Nath, Arindam <arindam.n...@amd.com> >wrote: >> Daniel, did you get chance to test this patch? > >Not yet. Should we test it alone or alongside "PCI: Blacklist AMD >Stoney GPU devices for ATS"? Daniel, any luck with this patch? Thanks, Arindam > >Thanks >Daniel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only
>-Original Message- >From: Daniel Drake [mailto:dr...@endlessm.com] >Sent: Thursday, March 30, 2017 7:15 PM >To: Nath, Arindam >Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd- >g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit, >Suravee; Linux Upstreaming Team >Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only > >On Thu, Mar 30, 2017 at 12:23 AM, Nath, Arindam <arindam.n...@amd.com> >wrote: >> Daniel, did you get chance to test this patch? > >Not yet. Should we test it alone or alongside "PCI: Blacklist AMD >Stoney GPU devices for ATS"? You should test this patch alone. Thanks, Arindam > >Thanks >Daniel ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only
>-Original Message-
>From: Nath, Arindam
>Sent: Monday, March 27, 2017 5:57 PM
>To: 'Daniel Drake'
>Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd-
>g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit,
>Suravee; Linux Upstreaming Team
>Subject: RE: [PATCH] iommu/amd: flush IOTLB for specific domains only
>
>>-Original Message-
>>From: Daniel Drake [mailto:dr...@endlessm.com]
>>Sent: Monday, March 27, 2017 5:56 PM
>>To: Nath, Arindam
>>Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd-
>>g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit,
>>Suravee; Linux Upstreaming Team
>>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only
>>
>>Hi Arindam,
>>
>>You CC'd me on this - does this mean that it is a fix for the issue
>>described in the thread "amd-iommu: can't boot with amdgpu, AMD-Vi:
>>Completion-Wait loop timed out" ?
>
>Yes Daniel, please test this patch to confirm if the issue gets resolved.
Daniel, did you get chance to test this patch?
Thanks,
Arindam
>
>Thanks,
>Arindam
>
>>
>>Thanks
>>Daniel
>>
>>
>>On Mon, Mar 27, 2017 at 12:17 AM, <arindam.n...@amd.com> wrote:
>>> From: Arindam Nath <arindam.n...@amd.com>
>>>
>>> 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.
>>>
>>> Signed-off-by: Arindam Nath <arindam.n...@amd.com>
>>> ---
>>> drivers/iommu/amd_iommu.c | 15 ---
>>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>>> index 98940d1..6a9a048 100644
>>> --- a/drivers/iommu/amd_iommu.c
>>> +++ b/drivers/iommu/amd_iommu.c
>>> @@ -2227,15 +2227,16 @@ 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(_iommu_pd_lock, flags);
>>> - list_for_each_entry(domain, _iommu_pd_list, list)
>>> - domain_flush_tlb(domain);
>>> - spin_unlock_irqrestore(_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;
>>> +
>>> + domain_flush_tlb(>dma_dom->domain);
>>> + }
>>>
>>> /* Wait until flushes have completed */
>>> domain_flush_complete(NULL);
>>> --
>>> 1.9.1
>>>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] iommu/amd: flush IOTLB for specific domains only
>-Original Message-
>From: Daniel Drake [mailto:dr...@endlessm.com]
>Sent: Monday, March 27, 2017 5:56 PM
>To: Nath, Arindam
>Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd-
>g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit,
>Suravee; Linux Upstreaming Team
>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only
>
>Hi Arindam,
>
>You CC'd me on this - does this mean that it is a fix for the issue
>described in the thread "amd-iommu: can't boot with amdgpu, AMD-Vi:
>Completion-Wait loop timed out" ?
Yes Daniel, please test this patch to confirm if the issue gets resolved.
Thanks,
Arindam
>
>Thanks
>Daniel
>
>
>On Mon, Mar 27, 2017 at 12:17 AM, <arindam.n...@amd.com> wrote:
>> From: Arindam Nath <arindam.n...@amd.com>
>>
>> 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.
>>
>> Signed-off-by: Arindam Nath <arindam.n...@amd.com>
>> ---
>> drivers/iommu/amd_iommu.c | 15 ---
>> 1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
>> index 98940d1..6a9a048 100644
>> --- a/drivers/iommu/amd_iommu.c
>> +++ b/drivers/iommu/amd_iommu.c
>> @@ -2227,15 +2227,16 @@ 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(_iommu_pd_lock, flags);
>> - list_for_each_entry(domain, _iommu_pd_list, list)
>> - domain_flush_tlb(domain);
>> - spin_unlock_irqrestore(_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;
>> +
>> + domain_flush_tlb(>dma_dom->domain);
>> + }
>>
>> /* Wait until flushes have completed */
>> domain_flush_complete(NULL);
>> --
>> 1.9.1
>>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: amd-iommu: can't boot with amdgpu, AMD-Vi: Completion-Wait loop timed out
>-Original Message- >From: 'j...@8bytes.org' [mailto:j...@8bytes.org] >Sent: Tuesday, March 21, 2017 9:41 PM >To: Deucher, Alexander >Cc: Alex Deucher; Daniel Drake; Chris Chiu; amd-...@lists.freedesktop.org; >Nath, Arindam; iommu@lists.linux-foundation.org; Suthikulpanit, Suravee; >Linux Upstreaming Team >Subject: Re: amd-iommu: can't boot with amdgpu, AMD-Vi: Completion-Wait >loop timed out > >On Tue, Mar 21, 2017 at 04:01:53PM +, Deucher, Alexander wrote: >> It seems to only affect Stoney systems, but not others (Carrizo, >> Bristol, etc.). Maybe we could just disable it on Stoney until we >> root cause it. > >Completion-wait loop timeouts indicate something is seriously wrong. How >can I detect whether I am running on a 'Stoney' system? > >Other question, a shot into the dark, does the GPU on these systems have >ATS? Probably yes, as they are likely HSA compatible. The GPU on Stoney supports ATS. Thanks, Arindam > > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
