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: iommu@lists.linux-foundation.org; Bridgman, John ; j...@8bytes.org; amd-...@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 ___ 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)
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: iommu@lists.linux-foundation.org; Bridgman, John ; > j...@8bytes.org; amd-...@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 ___ 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 > Cc: iommu@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: iommu@lists.linux-foundation.org; Bridgman, John > > ; j...@8bytes.org; > > amd-...@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 ___ 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)
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: iommu@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: iommu@lists.linux-foundation.org; Bridgman, John >>> ; j...@8bytes.org; >>> amd-...@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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 2/2] iommu/amd: Add basic debugfs infrastructure for AMD IOMMU
On Mon, May 14, 2018 at 03:00:50PM -0500, Gary R Hook wrote: > This was brought up a few weeks ago in, I believe, version 3 of this patch. > That question was discussed (because that's what I did the first time out), > and _someone_ _else_ asked about why I didn't just do it the way I've done > it here. You don't have this problem if you put the code in amd_iommu.c in an IOMMU_DEBUGFS ifdef. Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote: > PATCH 4~9 implement per domain PASID table. Current per IOMMU > PASID table implementation is insecure in the cases where > multiple devices under one single IOMMU unit support PASID > feature. With per domain PASID table, we can achieve finer > protection and isolation granularity. Hold on, we hat discussions in the past about doing a system-wide pasid space, so that every mm_struct with devices attached gets the same pasid across all devices it is talking to. Reason was that some devices (will) require this to work correctly. This goes into the opposite direction, so I am a bit confused here. Please explain, is this not longer necessary? Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 16/40] arm64: mm: Pin down ASIDs for sharing mm with devices
Hi Jean-Philippe,
On Fri, May 11, 2018 at 08:06:17PM +0100, Jean-Philippe Brucker wrote:
> +unsigned long mm_context_get(struct mm_struct *mm)
> +{
> + unsigned long flags;
> + u64 asid;
> +
> + raw_spin_lock_irqsave(&cpu_asid_lock, flags);
> +
> + asid = atomic64_read(&mm->context.id);
> +
> + if (mm->context.pinned) {
> + mm->context.pinned++;
> + asid &= ~ASID_MASK;
> + goto out_unlock;
> + }
> +
> + if (nr_pinned_asids >= max_pinned_asids) {
> + asid = 0;
> + goto out_unlock;
> + }
> +
> + if (!asid_gen_match(asid)) {
> + /*
> + * We went through one or more rollover since that ASID was
> + * used. Ensure that it is still valid, or generate a new one.
> + * The cpu argument isn't used by new_context.
> + */
> + asid = new_context(mm, 0);
> + atomic64_set(&mm->context.id, asid);
> + }
> +
> + asid &= ~ASID_MASK;
> +
> + nr_pinned_asids++;
> + __set_bit(asid2idx(asid), pinned_asid_map);
> + mm->context.pinned++;
> +
> +out_unlock:
> + raw_spin_unlock_irqrestore(&cpu_asid_lock, flags);
> +
> + return asid;
> +}
With CONFIG_UNMAP_KERNEL_AT_EL0 (a.k.a. KPTI), the hardware ASID has bit
0 set automatically when entering user space (and cleared when getting
back to the kernel). If the returned asid value here is going to be used
as is in the calling code, you should probably set bit 0 when KPTI is
enabled.
--
Catalin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1] iommu: Remove extra NULL check when call strtobool()
On Mon, May 14, 2018 at 07:22:25PM +0300, Andy Shevchenko wrote: > strtobool() does check for NULL parameter already. No need to repeat. > > While here, switch to kstrtobool() and unshadow actual error code > (which is still -EINVAL). > > No functional change intended. > > Signed-off-by: Andy Shevchenko Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/2] iommu/vt-d: Fix mapping PSI missing for iommu_map()
On Fri, May 04, 2018 at 10:34:51AM +0800, Peter Xu wrote: > Peter Xu (2): > iommu/vt-d: Introduce __mapping_notify_one() > iommu/vt-d: Fix iotlb psi missing for mappings > > drivers/iommu/intel-iommu.c | 65 ++--- > 1 file changed, 46 insertions(+), 19 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/4] iommu/vt-d: Several cleanup patches
On Fri, May 04, 2018 at 01:08:15PM +0800, Lu Baolu wrote: > Lu Baolu (4): > iommu: Clean up the comments for iommu_group_alloc > iommu/vt-d: Clean up unused variable in find_or_alloc_domain > iommu/vt-d: Clean up pasid quirk for pre-production devices > iommu/vt-d: Remove unnecessary parentheses > > drivers/iommu/intel-iommu.c | 36 +++- > drivers/iommu/intel-svm.c | 2 +- > drivers/iommu/iommu.c | 1 - > include/linux/intel-iommu.h | 1 - > 4 files changed, 4 insertions(+), 36 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/4] iommu/amd: Cleanup locking in __attach/detach_device()
On Mon, May 07, 2018 at 02:53:28PM +0200, Anna-Maria Gleixner wrote:
> Since introduction of the pd_bitmap_lock in commit 2bc001808904
> ("iommu/amd: Split domain id out of amd_iommu_devtable_lock")
> amd_iommu_devtable_lock is only taken around __detach_device() and
> __attach_device() calls.
>
> The lock is not protecting anything as all operations are domain specific
> and protected by domain->lock in __detach_device() and __attach_device(),
> so amd_iommu_devtable_lock has no real purpose anymore.
There is a subtle problem with this, as the lock you remove here
protects changes to the device table. Now one could argue that we never
check what we actually overwrite there, so it doesn't matter and the
lock can be removed. That is true, but it leaves an existing
race-condition in the code when a two cores try to assign the same
device to different domains.
So while at it, can you look into fixing that too before removing the
devtable_lock?
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)
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: iommu@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: iommu@lists.linux-foundation.org; Bridgman, John ; j...@8bytes.org; amd-...@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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] iommu/amd: Fix grammar of comments
On Mon, May 07, 2018 at 02:53:26PM +0200, Anna-Maria Gleixner wrote: > Suggested-by: Gary R Hook > Signed-off-by: Anna-Maria Gleixner > --- > drivers/iommu/amd_iommu.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Applied, thanks. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/4] iommu/amd: Prevent possible null pointer dereference and infinite loop
On Mon, May 07, 2018 at 02:53:27PM +0200, Anna-Maria Gleixner wrote:
> @@ -2793,6 +2790,7 @@ static void cleanup_domain(struct protection_domain
> *domain)
> while (!list_empty(&domain->dev_list)) {
> entry = list_first_entry(&domain->dev_list,
>struct iommu_dev_data, list);
> + BUG_ON(!entry->domain);
I am not happy with that BUG_ON, and it is unnecessary because we would
run into a NULL-ptr deref later if we don't BUG_ON here. So this makes
it easier to debug if it ever happens, but changing the code so that
this can be turned into a WARN_ON would be nicer.
Anyway, applied for now.
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)
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: iommu@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: iommu@lists.linux-foundation.org; Bridgman, John > ; j...@8bytes.org; > amd-...@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 any attachments that have the AMD-Vi messages in question. Were they completion timeouts (like in the later mainline kernel test, which I'll get to in a bit) or I/O page fault messages? Without that information it is hard to determine what the issue really is. (Just as an FYI, if the IOMMU is disabled in BIOS, then iommu=soft is not necessary on the kernel command line). For the upstream kernel test, since this is a Ryzen system, it's possible that the BIOS does not have a requisite fix for SME and IOMMU (see [1]). On the upstream kernel, if memory encryption is active by default without this BIOS fix, then the result is AMD-Vi completion-wait timeout messages. Try booting with mem_encrypt=off on the kernel command line or build a kernel with CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=n and see if that allows the kernel to boot. Thanks, Tom [1] https://bugzilla.kernel.org/show_bug.cgi?id=199513 > Thanks, > > Joe > > > > > [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1747463/comments/30 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listin
[PATCH 1/6] swiotlb: remove a pointless comment
This comments describes an aspect of the map_sg interface that isn't
even exploited by swiotlb.
Signed-off-by: Christoph Hellwig
---
lib/swiotlb.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 16ace0e25d52..721f93677eee 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -927,12 +927,6 @@ swiotlb_sync_single_for_device(struct device *hwdev,
dma_addr_t dev_addr,
* appropriate dma address and length. They are obtained via
* sg_dma_{address,length}(SG).
*
- * NOTE: An implementation may be able to use a smaller number of
- * DMA address/length pairs than there are SG table elements.
- * (for example via virtual mapping capabilities)
- * The routine returns the number of addr/length pairs actually
- * used, at most nents.
- *
* Device ownership issues as mentioned above for swiotlb_map_page are the
* same here.
*/
--
2.17.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
swiotlb cleanup
Hi Konrad, below are a few swiotlb patches. Mostly just cleanups, but the removal of the panic option is an actual change in (rarely used) functionality. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/6] swiotlb: do not panic on mapping failures
We now have error handling in map_single/map_page callers (most of them
anyway). As swiotlb_tbl_map_single already prints a useful warning
when running out of swiotlb pool swace we can also remove swiotlb_full
entirely as it serves no purpose now.
Signed-off-by: Christoph Hellwig
---
lib/swiotlb.c | 33 +
1 file changed, 1 insertion(+), 32 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 721f93677eee..4d36340bc4f9 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -763,34 +763,6 @@ static bool swiotlb_free_buffer(struct device *dev, size_t
size,
return true;
}
-static void
-swiotlb_full(struct device *dev, size_t size, enum dma_data_direction dir,
-int do_panic)
-{
- if (swiotlb_force == SWIOTLB_NO_FORCE)
- return;
-
- /*
-* Ran out of IOMMU space for this operation. This is very bad.
-* Unfortunately the drivers cannot handle this operation properly.
-* unless they check for dma_mapping_error (most don't)
-* When the mapping is small enough return a static buffer to limit
-* the damage, or panic when the transfer is too big.
-*/
- dev_err_ratelimited(dev, "DMA: Out of SW-IOMMU space for %zu bytes\n",
- size);
-
- if (size <= io_tlb_overflow || !do_panic)
- return;
-
- if (dir == DMA_BIDIRECTIONAL)
- panic("DMA: Random memory could be DMA accessed\n");
- if (dir == DMA_FROM_DEVICE)
- panic("DMA: Random memory could be DMA written\n");
- if (dir == DMA_TO_DEVICE)
- panic("DMA: Random memory could be DMA read\n");
-}
-
/*
* Map a single buffer of the indicated size for DMA in streaming mode. The
* physical address to use is returned.
@@ -819,10 +791,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct
page *page,
/* Oh well, have to allocate and map a bounce buffer. */
map = map_single(dev, phys, size, dir, attrs);
- if (map == SWIOTLB_MAP_ERROR) {
- swiotlb_full(dev, size, dir, 1);
+ if (map == SWIOTLB_MAP_ERROR)
return __phys_to_dma(dev, io_tlb_overflow_buffer);
- }
dev_addr = __phys_to_dma(dev, map);
@@ -950,7 +920,6 @@ swiotlb_map_sg_attrs(struct device *hwdev, struct
scatterlist *sgl, int nelems,
if (map == SWIOTLB_MAP_ERROR) {
/* Don't panic here, we expect map_sg users
to do proper error handling. */
- swiotlb_full(hwdev, sg->length, dir, 0);
attrs |= DMA_ATTR_SKIP_CPU_SYNC;
swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
attrs);
--
2.17.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/6] swiotlb: merge swiotlb_unmap_page and unmap_single
Signed-off-by: Christoph Hellwig
---
lib/swiotlb.c | 15 ---
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 4d36340bc4f9..2ebbc7204061 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -814,9 +814,9 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page
*page,
* After this call, reads by the cpu to the buffer are guaranteed to see
* whatever the device wrote there.
*/
-static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
-size_t size, enum dma_data_direction dir,
-unsigned long attrs)
+void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
+ size_t size, enum dma_data_direction dir,
+ unsigned long attrs)
{
phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
@@ -839,13 +839,6 @@ static void unmap_single(struct device *hwdev, dma_addr_t
dev_addr,
dma_mark_clean(phys_to_virt(paddr), size);
}
-void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
- size_t size, enum dma_data_direction dir,
- unsigned long attrs)
-{
- unmap_single(hwdev, dev_addr, size, dir, attrs);
-}
-
/*
* Make physical memory consistent for a single streaming mode DMA translation
* after a transfer.
@@ -949,7 +942,7 @@ swiotlb_unmap_sg_attrs(struct device *hwdev, struct
scatterlist *sgl,
BUG_ON(dir == DMA_NONE);
for_each_sg(sgl, sg, nelems, i)
- unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir,
+ swiotlb_unmap_page(hwdev, sg->dma_address, sg_dma_len(sg), dir,
attrs);
}
--
2.17.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/6] swiotlb: mark is_swiotlb_buffer static
Signed-off-by: Christoph Hellwig
---
include/linux/swiotlb.h | 1 -
lib/swiotlb.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 965be92c33b5..7ef541ce8f34 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -121,7 +121,6 @@ static inline unsigned int swiotlb_max_segment(void) {
return 0; }
#endif
extern void swiotlb_print_info(void);
-extern int is_swiotlb_buffer(phys_addr_t paddr);
extern void swiotlb_set_max_segment(unsigned int);
extern const struct dma_map_ops swiotlb_dma_ops;
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 2ebbc7204061..8333277d1cd1 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -431,7 +431,7 @@ void __init swiotlb_exit(void)
max_segment = 0;
}
-int is_swiotlb_buffer(phys_addr_t paddr)
+static int is_swiotlb_buffer(phys_addr_t paddr)
{
return paddr >= io_tlb_start && paddr < io_tlb_end;
}
--
2.17.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/6] swiotlb: share more code between map_page and map_sg
Refactor all the common code into what previously was map_single, which
is now renamed to __swiotlb_map_page. This also improves the map_sg
error handling and diagnostics to match the map_page ones.
Signed-off-by: Christoph Hellwig
---
lib/swiotlb.c | 114 +++---
1 file changed, 53 insertions(+), 61 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 8333277d1cd1..5becc2fc680a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -595,21 +595,47 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
/*
* Allocates bounce buffer and returns its physical address.
*/
-static phys_addr_t
-map_single(struct device *hwdev, phys_addr_t phys, size_t size,
- enum dma_data_direction dir, unsigned long attrs)
+static int
+__swiotlb_map_page(struct device *dev, phys_addr_t phys, size_t size,
+ enum dma_data_direction dir, unsigned long attrs,
+ dma_addr_t *dma_addr)
{
- dma_addr_t start_dma_addr;
-
- if (swiotlb_force == SWIOTLB_NO_FORCE) {
- dev_warn_ratelimited(hwdev, "Cannot do DMA to address %pa\n",
-&phys);
- return SWIOTLB_MAP_ERROR;
+ phys_addr_t map_addr;
+
+ if (WARN_ON_ONCE(dir == DMA_NONE))
+ return -EINVAL;
+ *dma_addr = phys_to_dma(dev, phys);
+
+ switch (swiotlb_force) {
+ case SWIOTLB_NO_FORCE:
+ dev_warn_ratelimited(dev,
+ "swiotlb: force disabled for address %pa\n", &phys);
+ return -EOPNOTSUPP;
+ case SWIOTLB_NORMAL:
+ /* can we address the memory directly? */
+ if (dma_capable(dev, *dma_addr, size))
+ return 0;
+ break;
+ case SWIOTLB_FORCE:
+ break;
}
- start_dma_addr = __phys_to_dma(hwdev, io_tlb_start);
- return swiotlb_tbl_map_single(hwdev, start_dma_addr, phys, size,
- dir, attrs);
+ trace_swiotlb_bounced(dev, *dma_addr, size, swiotlb_force);
+ map_addr = swiotlb_tbl_map_single(dev, __phys_to_dma(dev, io_tlb_start),
+ phys, size, dir, attrs);
+ if (unlikely(map_addr == SWIOTLB_MAP_ERROR))
+ return -ENOMEM;
+
+ /* Ensure that the address returned is DMA'ble */
+ *dma_addr = __phys_to_dma(dev, map_addr);
+ if (unlikely(!dma_capable(dev, *dma_addr, size))) {
+ dev_err_ratelimited(dev,
+ "DMA: swiotlb buffer not addressable.\n");
+ swiotlb_tbl_unmap_single(dev, map_addr, size, dir,
+ attrs | DMA_ATTR_SKIP_CPU_SYNC);
+ return -EINVAL;
+ }
+ return 0;
}
/*
@@ -775,35 +801,12 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct
page *page,
enum dma_data_direction dir,
unsigned long attrs)
{
- phys_addr_t map, phys = page_to_phys(page) + offset;
- dma_addr_t dev_addr = phys_to_dma(dev, phys);
-
- BUG_ON(dir == DMA_NONE);
- /*
-* If the address happens to be in the device's DMA window,
-* we can safely return the device addr and not worry about bounce
-* buffering it.
-*/
- if (dma_capable(dev, dev_addr, size) && swiotlb_force != SWIOTLB_FORCE)
- return dev_addr;
+ dma_addr_t dma_addr;
- trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
-
- /* Oh well, have to allocate and map a bounce buffer. */
- map = map_single(dev, phys, size, dir, attrs);
- if (map == SWIOTLB_MAP_ERROR)
+ if (unlikely(__swiotlb_map_page(dev, page_to_phys(page) + offset, size,
+ dir, attrs, &dma_addr) < 0))
return __phys_to_dma(dev, io_tlb_overflow_buffer);
-
- dev_addr = __phys_to_dma(dev, map);
-
- /* Ensure that the address returned is DMA'ble */
- if (dma_capable(dev, dev_addr, size))
- return dev_addr;
-
- attrs |= DMA_ATTR_SKIP_CPU_SYNC;
- swiotlb_tbl_unmap_single(dev, map, size, dir, attrs);
-
- return __phys_to_dma(dev, io_tlb_overflow_buffer);
+ return dma_addr;
}
/*
@@ -894,37 +897,26 @@ swiotlb_sync_single_for_device(struct device *hwdev,
dma_addr_t dev_addr,
* same here.
*/
int
-swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
+swiotlb_map_sg_attrs(struct device *dev, struct scatterlist *sgl, int nelems,
enum dma_data_direction dir, unsigned long attrs)
{
struct scatterlist *sg;
int i;
- BUG_ON(dir == DMA_NONE);
-
for_each_sg(sgl, sg, nelems, i) {
- phys_addr_t paddr = sg_phys(sg);
- dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);
-
- if (swiotlb_force == SWIOTLB_FORCE ||
- !dma_capable(hwdev, dev_addr, sg->l
[PATCH 6/6] swiotlb: respect DMA_ATTR_NO_WARN in __swiotlb_map_page
Signed-off-by: Christoph Hellwig
---
lib/swiotlb.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 5becc2fc680a..5cf88e090cb6 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -608,8 +608,11 @@ __swiotlb_map_page(struct device *dev, phys_addr_t phys,
size_t size,
switch (swiotlb_force) {
case SWIOTLB_NO_FORCE:
- dev_warn_ratelimited(dev,
- "swiotlb: force disabled for address %pa\n", &phys);
+ if (!(attrs & DMA_ATTR_NO_WARN)) {
+ dev_warn_ratelimited(dev,
+ "swiotlb: force disabled for address %pa\n",
+ &phys);
+ }
return -EOPNOTSUPP;
case SWIOTLB_NORMAL:
/* can we address the memory directly? */
@@ -629,10 +632,12 @@ __swiotlb_map_page(struct device *dev, phys_addr_t phys,
size_t size,
/* Ensure that the address returned is DMA'ble */
*dma_addr = __phys_to_dma(dev, map_addr);
if (unlikely(!dma_capable(dev, *dma_addr, size))) {
- dev_err_ratelimited(dev,
- "DMA: swiotlb buffer not addressable.\n");
+ if (!(attrs & DMA_ATTR_NO_WARN)) {
+ dev_err_ratelimited(dev,
+ "DMA: swiotlb buffer not addressable.\n");
+ }
swiotlb_tbl_unmap_single(dev, map_addr, size, dir,
- attrs | DMA_ATTR_SKIP_CPU_SYNC);
+ attrs | DMA_ATTR_SKIP_CPU_SYNC);
return -EINVAL;
}
return 0;
--
2.17.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] dt-bindings: mediatek: Add binding for mt2712 IOMMU and SMI
This patch adds decriptions for mt2712 IOMMU and SMI.
In order to balance the bandwidth, mt2712 has two M4Us, two
smi-commons, 10 smi-larbs. and mt2712 is also MTK IOMMU gen2 which
uses ARM Short-Descriptor translation table format.
The mt2712 M4U-SMI HW diagram is as below:
EMI
|
| |
M4U0 M4U1
| |
smi-common0smi-common1
| |
-
| | | | | | || | |
| | | | | | || | |
larb0 larb1 larb2 larb3 larb6larb4larb5larb7 larb8 larb9
disp0 vdec cam venc jpg mdp1/disp1 mdp2/disp2 mdp3 vdo/nr tvd
All the connections are HW fixed, SW can NOT adjust it.
Signed-off-by: Yong Wu
Acked-by: Rob Herring
---
The mt2712 iommu code has already merged in v4.14, but the dt-binding
patch looks lost. Resend it as v3.
v3:
Add a new ECO port(DISP_RDMA2) in larb0/port7.
v2:
https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023848.html
v1:
https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023665.html
---
.../devicetree/bindings/iommu/mediatek,iommu.txt | 6 +-
.../memory-controllers/mediatek,smi-common.txt | 6 +-
.../memory-controllers/mediatek,smi-larb.txt | 5 +-
include/dt-bindings/memory/mt2712-larb-port.h | 103 +
4 files changed, 114 insertions(+), 6 deletions(-)
create mode 100644 include/dt-bindings/memory/mt2712-larb-port.h
diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
index 53c20ca..df5db73 100644
--- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.txt
@@ -40,6 +40,7 @@ video decode local arbiter, all these ports are according to
the video HW.
Required properties:
- compatible : must be one of the following string:
"mediatek,mt2701-m4u" for mt2701 which uses generation one m4u HW.
+ "mediatek,mt2712-m4u" for mt2712 which uses generation two m4u HW.
"mediatek,mt8173-m4u" for mt8173 which uses generation two m4u HW.
- reg : m4u register base and size.
- interrupts : the interrupt of m4u.
@@ -50,8 +51,9 @@ Required properties:
according to the local arbiter index, like larb0, larb1, larb2...
- iommu-cells : must be 1. This is the mtk_m4u_id according to the HW.
Specifies the mtk_m4u_id as defined in
- dt-binding/memory/mt2701-larb-port.h for mt2701 and
- dt-binding/memory/mt8173-larb-port.h for mt8173
+ dt-binding/memory/mt2701-larb-port.h for mt2701,
+ dt-binding/memory/mt2712-larb-port.h for mt2712, and
+ dt-binding/memory/mt8173-larb-port.h for mt8173.
Example:
iommu: iommu@10205000 {
diff --git
a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
index aa614b2..615abdd 100644
---
a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
+++
b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.txt
@@ -2,8 +2,9 @@ SMI (Smart Multimedia Interface) Common
The hardware block diagram please check bindings/iommu/mediatek,iommu.txt
-Mediatek SMI have two generations of HW architecture, mt8173 uses the second
-generation of SMI HW while mt2701 uses the first generation HW of SMI.
+Mediatek SMI have two generations of HW architecture, mt2712 and mt8173 use
+the second generation of SMI HW while mt2701 uses the first generation HW of
+SMI.
There's slight differences between the two SMI, for generation 2, the
register which control the iommu port is at each larb's register base. But
@@ -15,6 +16,7 @@ not needed for SMI generation 2.
Required properties:
- compatible : must be one of :
"mediatek,mt2701-smi-common"
+ "mediatek,mt2712-smi-common"
"mediatek,mt8173-smi-common"
- reg : the register and size of the SMI block.
- power-domains : a phandle to the power domain of this local arbiter.
diff --git
a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
index ddf46b8..083155c 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.txt
@@ -4,8 +4,9 @@ The hardware block diagram please check
bindings/iommu/mediatek,iommu.txt
Required properties:
- compatible : must be one of :
- "mediatek,mt8173-smi-larb"
