Re: Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu
On 7/22/20 11:03 AM, Jun Miao wrote: On 7/22/20 10:40 AM, Lu Baolu wrote: Hi Jun, On 7/22/20 10:26 AM, Miao, Jun wrote: Kernel panic - not syncing: DMAR hardware is malfunctioning CPU: 0 PID: 347 Comm: rtcwake Not tainted 5.4.0-yocto-standard #124 Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3162.A00.1904162000 04/16/2019 Call Trace: dump_stack+0x59/0x75 panic+0xff/0x2d4 iommu_disable_translation+0x88/0x90 iommu_suspend+0x12f/0x1b0 syscore_suspend+0x6c/0x220 suspend_devices_and_enter+0x313/0x840 pm_suspend+0x30d/0x390 state_store+0x82/0xf0 kobj_attr_store+0x12/0x20 sysfs_kf_write+0x3c/0x50 kernfs_fop_write+0x11d/0x190 __vfs_write+0x1b/0x40 vfs_write+0xc6/0x1d0 ksys_write+0x5e/0xe0 __x64_sys_write+0x1a/0x20 do_syscall_64+0x4d/0x150 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f97b8080113 Code: 8b 15 81 bd 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 48 89 54 24 18 RSP: 002b:7ffcfa6f48b8 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0004 RCX: 7f97b8080113 RDX: 0004 RSI: 55e7db03b700 RDI: 0004 RBP: 55e7db03b700 R08: 55e7db03b700 R09: 0004 R10: 0004 R11: 0246 R12: 0004 R13: 55e7db039380 R14: 0004 R15: 7f97b814d700 Kernel Offset: 0x38a0 from 0x8100 (relocation range: 0x8000-0xbfff) ---[ end Kernel panic - not syncing: DMAR hardware is malfunctioning ]--- Do you mean that system hangs in iommu_disable_translation() without this fix. Yes ,From the call trace and i also read the DMARD_GCMD_RGS is wrong without this patch. Okay! Thanks a lot for confirming this. Best regards, baolu [S3 successfully with the patch] And, this failure disappeared after you applied this fix? Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: Rename intel-pasid.h to pasid.h
As Intel VT-d files have been move to its own subdirectory, the prefix
makes no sense.
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/debugfs.c | 2 +-
drivers/iommu/intel/iommu.c| 2 +-
drivers/iommu/intel/pasid.c| 2 +-
drivers/iommu/intel/{intel-pasid.h => pasid.h} | 2 +-
drivers/iommu/intel/svm.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
rename drivers/iommu/intel/{intel-pasid.h => pasid.h} (98%)
diff --git a/drivers/iommu/intel/debugfs.c b/drivers/iommu/intel/debugfs.c
index cf1ebb98e418..efea7f02abd9 100644
--- a/drivers/iommu/intel/debugfs.c
+++ b/drivers/iommu/intel/debugfs.c
@@ -15,7 +15,7 @@
#include
-#include "intel-pasid.h"
+#include "pasid.h"
struct tbl_walk {
u16 bus;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 5228f2c18a90..60de9daea62e 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -48,7 +48,7 @@
#include
#include "../irq_remapping.h"
-#include "intel-pasid.h"
+#include "pasid.h"
#define ROOT_SIZE VTD_PAGE_SIZE
#define CONTEXT_SIZE VTD_PAGE_SIZE
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index fa0154cce537..e6faedf42fd4 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -19,7 +19,7 @@
#include
#include
-#include "intel-pasid.h"
+#include "pasid.h"
/*
* Intel IOMMU system wide PASID name space:
diff --git a/drivers/iommu/intel/intel-pasid.h b/drivers/iommu/intel/pasid.h
similarity index 98%
rename from drivers/iommu/intel/intel-pasid.h
rename to drivers/iommu/intel/pasid.h
index c5318d40e0fa..c9850766c3a9 100644
--- a/drivers/iommu/intel/intel-pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * intel-pasid.h - PASID idr, table and entry header
+ * pasid.h - PASID idr, table and entry header
*
* Copyright (C) 2018 Intel Corporation
*
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 85ce8daa3177..442623ac4b47 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -20,7 +20,7 @@
#include
#include
-#include "intel-pasid.h"
+#include "pasid.h"
static irqreturn_t prq_event_thread(int irq, void *d);
static void intel_svm_drain_prq(struct device *dev, int pasid);
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu
Hi Jun, On 7/22/20 10:26 AM, Miao, Jun wrote: Kernel panic - not syncing: DMAR hardware is malfunctioning CPU: 0 PID: 347 Comm: rtcwake Not tainted 5.4.0-yocto-standard #124 Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3162.A00.1904162000 04/16/2019 Call Trace: dump_stack+0x59/0x75 panic+0xff/0x2d4 iommu_disable_translation+0x88/0x90 iommu_suspend+0x12f/0x1b0 syscore_suspend+0x6c/0x220 suspend_devices_and_enter+0x313/0x840 pm_suspend+0x30d/0x390 state_store+0x82/0xf0 kobj_attr_store+0x12/0x20 sysfs_kf_write+0x3c/0x50 kernfs_fop_write+0x11d/0x190 __vfs_write+0x1b/0x40 vfs_write+0xc6/0x1d0 ksys_write+0x5e/0xe0 __x64_sys_write+0x1a/0x20 do_syscall_64+0x4d/0x150 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f97b8080113 Code: 8b 15 81 bd 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 55 c3 0f 1f 40 00 48 83 ec 28 48 89 54 24 18 RSP: 002b:7ffcfa6f48b8 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0004 RCX: 7f97b8080113 RDX: 0004 RSI: 55e7db03b700 RDI: 0004 RBP: 55e7db03b700 R08: 55e7db03b700 R09: 0004 R10: 0004 R11: 0246 R12: 0004 R13: 55e7db039380 R14: 0004 R15: 7f97b814d700 Kernel Offset: 0x38a0 from 0x8100 (relocation range: 0x8000-0xbfff) ---[ end Kernel panic - not syncing: DMAR hardware is malfunctioning ]--- Do you mean that system hangs in iommu_disable_translation() without this fix. [S3 successfully with the patch] And, this failure disappeared after you applied this fix? Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 4/7] iommu/vt-d: Handle non-page aligned address
Hi Jacob,
On 7/22/20 12:50 AM, Jacob Pan wrote:
Hi Baolu,
Not sure what state is this patch in, there is a bug in this patch
(see below), shall I send out an updated version of this one only? or
another incremental patch.
Please send an updated version. I hope Joerg could pick these as 5.8
fix.
Best regards,
baolu
Thanks,
Jacob
On Mon, 6 Jul 2020 17:12:51 -0700
Jacob Pan wrote:
From: Liu Yi L
Address information for device TLB invalidation comes from userspace
when device is directly assigned to a guest with vIOMMU support.
VT-d requires page aligned address. This patch checks and enforce
address to be page aligned, otherwise reserved bits can be set in the
invalidation descriptor. Unrecoverable fault will be reported due to
non-zero value in the reserved bits.
Fixes: 61a06a16e36d8 ("iommu/vt-d: Support flushing more translation
cache types")
Acked-by: Lu Baolu
Signed-off-by: Liu Yi L
Signed-off-by: Jacob Pan
---
drivers/iommu/intel/dmar.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index d9f973fa1190..b2c53bada905 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1455,9 +1455,25 @@ void qi_flush_dev_iotlb_pasid(struct
intel_iommu *iommu, u16 sid, u16 pfsid,
* Max Invs Pending (MIP) is set to 0 for now until we have
DIT in
* ECAP.
*/
- desc.qw1 |= addr & ~mask;
- if (size_order)
+ if (addr & GENMASK_ULL(size_order + VTD_PAGE_SHIFT, 0))
+ pr_warn_ratelimited("Invalidate non-aligned address
%llx, order %d\n", addr, size_order); +
+ /* Take page address */
+ desc.qw1 = QI_DEV_EIOTLB_ADDR(addr);
+
+ if (size_order) {
+ /*
+* Existing 0s in address below size_order may be
the least
+* significant bit, we must set them to 1s to avoid
having
+* smaller size than desired.
+*/
+ desc.qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT,
+ VTD_PAGE_SHIFT);
Yi reported the issue, it should be:
desc.qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT - 1,
VTD_PAGE_SHIFT);
+ /* Clear size_order bit to indicate size */
+ desc.qw1 &= ~mask;
+ /* Set the S bit to indicate flushing more than 1
page */ desc.qw1 |= QI_DEV_EIOTLB_SIZE;
+ }
qi_submit_sync(iommu, , 1, 0);
}
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/arm-smmu: Implement qcom,skip-init
On Tue 21 Jul 09:20 PDT 2020, Konrad Dybcio wrote: > >The current > >focus has been on moving more of the SMMU specific bits into the > >arm-smmu-qcom > >implementation [1] and I think that is the right way to go. > > Pardon if I overlooked something obvious, but I can't seem to find a > clean way for implementing qcom,skip-init in arm-smmu-qcom, as neither > the arm_smmu_test_smr_masks nor the probe function seem to be > alterable with arm_smmu_impl. I'm open to your ideas guys. > Is the problem on SDM630 that when you write to SMR/S2CR the device reboots? Or that when you start writing out the context bank configuration that trips the display and the device reboots? Regards, Bjorn ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu
> -Original Message- > From: Lu Baolu > Sent: Tuesday, July 21, 2020 6:07 PM > To: Limonciello, Mario; Joerg Roedel > Cc: baolu...@linux.intel.com; Ashok Raj; linux-ker...@vger.kernel.org; > sta...@vger.kernel.org; Koba Ko; iommu@lists.linux-foundation.org > Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated > iommu > > > [EXTERNAL EMAIL] > > Hi Limonciello, > > On 7/21/20 10:44 PM, Limonciello, Mario wrote: > >> -Original Message- > >> From: iommu On Behalf Of Lu > >> Baolu > >> Sent: Monday, July 20, 2020 7:17 PM > >> To: Joerg Roedel > >> Cc: Ashok Raj;linux-ker...@vger.kernel.org;sta...@vger.kernel.org; Koba > >> Ko;iommu@lists.linux-foundation.org > >> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated > >> iommu > >> > >> The VT-d spec requires (10.4.4 Global Command Register, TE field) that: > >> > >> Hardware implementations supporting DMA draining must drain any in-flight > >> DMA read/write requests queued within the Root-Complex before completing > >> the translation enable command and reflecting the status of the command > >> through the TES field in the Global Status register. > >> > >> Unfortunately, some integrated graphic devices fail to do so after some > >> kind of power state transition. As the result, the system might stuck in > >> iommu_disable_translation(), waiting for the completion of TE transition. > >> > >> This provides a quirk list for those devices and skips TE disabling if > >> the qurik hits. > >> > >> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=208363 > > That one is for TGL. > > > > I think you also want to add this one for ICL: > > Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=206571 > > > > Do you mean someone have tested that this patch also fixes the problem > described in 206571? > Yes, confusingly https://bugzilla.kernel.org/show_bug.cgi?id=208363#c31 actually is the XPS 9300 ICL system and issue. I also have a private confirmation from another person that it resolves it for them on another ICL platform. Christian, maybe you can add a tested by clause for the ICL testing. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu
Hi Limonciello, On 7/21/20 10:44 PM, Limonciello, Mario wrote: -Original Message- From: iommu On Behalf Of Lu Baolu Sent: Monday, July 20, 2020 7:17 PM To: Joerg Roedel Cc: Ashok Raj;linux-ker...@vger.kernel.org;sta...@vger.kernel.org; Koba Ko;iommu@lists.linux-foundation.org Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu The VT-d spec requires (10.4.4 Global Command Register, TE field) that: Hardware implementations supporting DMA draining must drain any in-flight DMA read/write requests queued within the Root-Complex before completing the translation enable command and reflecting the status of the command through the TES field in the Global Status register. Unfortunately, some integrated graphic devices fail to do so after some kind of power state transition. As the result, the system might stuck in iommu_disable_translation(), waiting for the completion of TE transition. This provides a quirk list for those devices and skips TE disabling if the qurik hits. Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=208363 That one is for TGL. I think you also want to add this one for ICL: Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=206571 Do you mean someone have tested that this patch also fixes the problem described in 206571? Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 4/5] iommu/uapi: Handle data and argsz filled by users
On Fri, 17 Jul 2020 13:59:54 -0600
Alex Williamson wrote:
> On Thu, 16 Jul 2020 11:45:16 -0700
> Jacob Pan wrote:
>
> > IOMMU UAPI data has a user filled argsz field which indicates the
> > data length comes with the API call. User data is not trusted,
> > argsz must be validated based on the current kernel data size,
> > mandatory data size, and feature flags.
> >
> > User data may also be extended, results in possible argsz increase.
> > Backward compatibility is ensured based on size and flags checking.
> >
> > This patch adds sanity checks in the IOMMU layer. In addition to
> > argsz, reserved/unused fields in padding, flags, and version are
> > also checked. Details are documented in
> > Documentation/userspace-api/iommu.rst
> >
> > Signed-off-by: Liu Yi L
> > Signed-off-by: Jacob Pan
> > ---
> > drivers/iommu/iommu.c | 192
> > --
> > include/linux/iommu.h | 20 -- 2 files changed, 200
> > insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index d43120eb1dc5..fe30a940d19e 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1950,36 +1950,214 @@ int iommu_attach_device(struct
> > iommu_domain *domain, struct device *dev) }
> > EXPORT_SYMBOL_GPL(iommu_attach_device);
> >
> > +/*
> > + * Check flags and other user privided data for valid
> > combinations. We also
> > + * make sure no reserved fields or unused flags are not set. This
> > is to ensure
> > + * not breaking userspace in the future when these fields or flags
> > are used.
> > + */
> > +static int iommu_check_cache_invl_data(struct
> > iommu_cache_invalidate_info *info) +{
> > + u32 mask;
> > +
> > + if (info->version != IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
> > + return -EINVAL;
> > +
> > + mask = (1 << IOMMU_CACHE_INV_TYPE_NR) - 1;
> > + if (info->cache & ~mask)
> > + return -EINVAL;
> > +
> > + if (info->granularity >= IOMMU_INV_GRANU_NR)
> > + return -EINVAL;
> > +
> > + switch (info->granularity) {
> > + case IOMMU_INV_GRANU_ADDR:
> > + mask = IOMMU_INV_ADDR_FLAGS_PASID |
> > + IOMMU_INV_ADDR_FLAGS_ARCHID |
> > + IOMMU_INV_ADDR_FLAGS_LEAF;
> > +
> > + if (info->granu.addr_info.flags & ~mask)
> > + return -EINVAL;
> > + break;
> > + case IOMMU_INV_GRANU_PASID:
> > + mask = IOMMU_INV_PASID_FLAGS_PASID |
> > + IOMMU_INV_PASID_FLAGS_ARCHID;
> > + if (info->granu.pasid_info.flags & ~mask)
> > + return -EINVAL;
> > +
> > + break;
> > + case IOMMU_INV_GRANU_DOMAIN:
> > + /* No flags to check */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (info->padding[0] || info->padding[1])
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > int iommu_cache_invalidate(struct iommu_domain *domain, struct
> > device *dev,
> > - struct iommu_cache_invalidate_info
> > *inv_info)
> > + void __user *uinfo)
> > {
> > + struct iommu_cache_invalidate_info inv_info = { 0 };
> > + u32 minsz, maxsz;
> > + int ret = 0;
> > +
> > if (unlikely(!domain->ops->cache_invalidate))
> > return -ENODEV;
> >
> > - return domain->ops->cache_invalidate(domain, dev,
> > inv_info);
> > + /* Current kernel data size is the max to be copied from
> > user */
> > + maxsz = sizeof(struct iommu_cache_invalidate_info);
> > +
> > + /*
> > +* No new spaces can be added before the variable sized
> > union, the
> > +* minimum size is the offset to the union.
> > +*/
> > + minsz = offsetof(struct iommu_cache_invalidate_info,
> > granu); +
> > + /* Copy minsz from user to get flags and argsz */
> > + if (copy_from_user(_info, uinfo, minsz))
> > + return -EFAULT;
> > +
> > + /* Fields before variable size union is mandatory */
> > + if (inv_info.argsz < minsz)
> > + return -EINVAL;
> > +
> > + /* PASID and address granu requires additional info beyond
> > minsz */
> > + if (inv_info.argsz == minsz &&
> > + ((inv_info.granularity == IOMMU_INV_GRANU_PASID) ||
> > + (inv_info.granularity ==
> > IOMMU_INV_GRANU_ADDR)))
> > + return -EINVAL;
> > + /*
> > +* User might be using a newer UAPI header which has a
> > larger data
> > +* size, we shall support the existing flags within the
> > current
> > +* size. Copy the remaining user data _after_ minsz but
> > not more
> > +* than the current kernel supported size.
> > +*/
> > + if (copy_from_user((void *)_info + minsz, uinfo +
> > minsz,
> > + min(inv_info.argsz, maxsz) -
> > minsz))
> > + return -EFAULT;
>
> To further clarify previous comments about maxsz usage:
>
> s/maxsz/sizeof(inv_info)/
>
Yes, will remove
Re: [PATCH v5 4/5] iommu/uapi: Handle data and argsz filled by users
On Fri, 17 Jul 2020 17:58:04 +0200
Auger Eric wrote:
> Hi Jacob,
> On 7/16/20 8:45 PM, Jacob Pan wrote:
>
> Could you share a branch? I was not able to apply this on either
> iommu/next or master?
>
Will push it to github for the next version. This set is based on
v5.8-rc1 and my fixes for devTLB.
> > IOMMU UAPI data has a user filled argsz field which indicates the
> > data length comes with the API call.
> s/ comes with the API call/ of the structure
> User data is not trusted, argsz must be
Sounds good.
> > validated based on the current kernel data size, mandatory data
> > size, and feature flags.
> >
> > User data may also be extended, results in possible argsz
> > increase.
> s/results/resulting
got it.
> > Backward compatibility is ensured based on size and flags
> > checking.
> flags is missing in iommu_cache_invalidate_info.
Right, I will rephrase as "flags or the functional equivalent fields"
> >
> > This patch adds sanity checks in the IOMMU layer. In addition to
> > argsz, reserved/unused fields in padding, flags, and version are
> > also checked. Details are documented in
> > Documentation/userspace-api/iommu.rst
> >
> > Signed-off-by: Liu Yi L
> > Signed-off-by: Jacob Pan
> > ---
> > drivers/iommu/iommu.c | 192
> > --
> > include/linux/iommu.h | 20 -- 2 files changed, 200
> > insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index d43120eb1dc5..fe30a940d19e 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1950,36 +1950,214 @@ int iommu_attach_device(struct
> > iommu_domain *domain, struct device *dev) }
> > EXPORT_SYMBOL_GPL(iommu_attach_device);
> >
> > +/*
> > + * Check flags and other user privided data for valid
> > combinations. We also
> s/privided/provided
will fix
> > + * make sure no reserved fields or unused flags are not set. This
> > is to ensure
> s/not// ?
right,
> > + * not breaking userspace in the future when these fields or flags
> > are used.
> > + */
> > +static int iommu_check_cache_invl_data(struct
> > iommu_cache_invalidate_info *info) +{
> > + u32 mask;
> > +
> > + if (info->version !=
> > IOMMU_CACHE_INVALIDATE_INFO_VERSION_1)
> increased version number?
Right now we just support version 1. if we increase the version number
in the future, we need to check the current kernel version is newer
than the user provided version.
Does it sounds right to you?
> > + return -EINVAL;
> > +
> > + mask = (1 << IOMMU_CACHE_INV_TYPE_NR) - 1;
> > + if (info->cache & ~mask)
> > + return -EINVAL;
> > +
> > + if (info->granularity >= IOMMU_INV_GRANU_NR)
> > + return -EINVAL;
> > +
> > + switch (info->granularity) {
> > + case IOMMU_INV_GRANU_ADDR:
> > + mask = IOMMU_INV_ADDR_FLAGS_PASID |
> > + IOMMU_INV_ADDR_FLAGS_ARCHID |
> > + IOMMU_INV_ADDR_FLAGS_LEAF;
> > +
> > + if (info->granu.addr_info.flags & ~mask)
> > + return -EINVAL;
> > + break;
> > + case IOMMU_INV_GRANU_PASID:
> > + mask = IOMMU_INV_PASID_FLAGS_PASID |
> > + IOMMU_INV_PASID_FLAGS_ARCHID;
> > + if (info->granu.pasid_info.flags & ~mask)
> > + return -EINVAL;
> > +
> > + break;
> > + case IOMMU_INV_GRANU_DOMAIN:
> > + /* No flags to check */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (info->padding[0] || info->padding[1])
> padding has become "__u8 padding[6];" in 2/5
Good point, will fix
>
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > int iommu_cache_invalidate(struct iommu_domain *domain, struct
> > device *dev,
> > - struct iommu_cache_invalidate_info
> > *inv_info)
> > + void __user *uinfo)
> > {
> > + struct iommu_cache_invalidate_info inv_info = { 0 };
> > + u32 minsz, maxsz;
> > + int ret = 0;
> > +
> > if (unlikely(!domain->ops->cache_invalidate))
> > return -ENODEV;
> >
> > - return domain->ops->cache_invalidate(domain, dev,
> > inv_info);
> > + /* Current kernel data size is the max to be copied from
> > user */
> > + maxsz = sizeof(struct iommu_cache_invalidate_info);
> > +
> > + /*
> > +* No new spaces can be added before the variable sized
> > union, the
> > +* minimum size is the offset to the union.
> > +*/
> > + minsz = offsetof(struct iommu_cache_invalidate_info,
> > granu); +
> > + /* Copy minsz from user to get flags and argsz */
> > + if (copy_from_user(_info, uinfo, minsz))
> > + return -EFAULT;
> > +
> > + /* Fields before variable size union is mandatory */
> > + if (inv_info.argsz < minsz)
> > + return -EINVAL;
> > +
> > + /* PASID and address granu requires additional info beyond
> > minsz */
> s/requires/require
got
Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
On 21/07/2020 13:24, Yong Wu wrote:
On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
On 21/07/2020 04:16, Miles Chen wrote:
In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.
Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.
This is a step towards building mtk_iommu as a kernel module.
Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode
[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147
Cc: Mike Rapoport
Cc: David Hildenbrand
Cc: Yong Wu
Cc: Yingjoe Chen
Cc: Christoph Hellwig
Cc: Yong Wu
Cc: Chao Hao
Cc: Rob Herring
Cc: Matthias Brugger
Signed-off-by: Miles Chen
---
drivers/iommu/mtk_iommu.c | 26 +-
include/linux/soc/mediatek/infracfg.h | 3 +++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
* Copyright (c) 2015-2016 MediaTek Inc.
* Author: Yong Wu
*/
-#include
#include
#include
#include
@@ -15,13 +14,16 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
struct resource *res;
resource_size_t ioaddr;
struct component_match *match = NULL;
+ struct regmap *infracfg;
void*protect;
int i, larb_nr, ret;
+ u32 val;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
return -ENOMEM;
data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
- /* Whether the current dram is over 4GB */
- data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
- if (!data->plat_data->has_4gb_mode)
- data->enable_4GB = false;
+ data->enable_4GB = false;
+ if (data->plat_data->has_4gb_mode) {
+ infracfg = syscon_regmap_lookup_by_compatible(
+ "mediatek,mt8173-infracfg");
+ if (IS_ERR(infracfg)) {
+ infracfg = syscon_regmap_lookup_by_compatible(
+ "mediatek,mt2712-infracfg");
+ if (IS_ERR(infracfg))
+ return PTR_ERR(infracfg);
I think we should check m4u_plat instead to decide which compatible we have to
look for.
Another option would be to add a general compatible something like
"mtk-infracfg" and search for that. That would need an update of all DTS having
a infracfg compatible right now. After thinking twice, this would break newer
kernel with older device tree, so maybe it's better to go with m4u_plat switch
statement.
Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".
is this OK?
It's an option, but I personally find that a bit hacky.
Regards,
Matthias
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] PCI/ATS: PASID and PRI are only enumerated in PF devices.
PASID and PRI capabilities are only enumerated in PF devices. VF devices
do not enumerate these capabilites. IOMMU drivers also need to enumerate
them before enabling features in the IOMMU. Extending the same support as
PASID feature discovery (pci_pasid_features) for PRI.
Signed-off-by: Ashok Raj
v2: Fixed build failure from lkp when CONFIG_PRI=n
Almost all the PRI functions were called only when CONFIG_PASID is
set. Except the new pci_pri_supported().
previous version sent in error because i missed dry-run before recompile
:-(
To: Bjorn Helgaas
To: Joerg Roedel
To: Lu Baolu
Cc: sta...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Ashok Raj
Cc: iommu@lists.linux-foundation.org
---
drivers/iommu/intel/iommu.c | 2 +-
drivers/pci/ats.c | 13 +
include/linux/pci-ats.h | 4
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index d759e7234e98..276452f5e6a7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2560,7 +2560,7 @@ static struct dmar_domain
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
}
if (info->ats_supported && ecap_prs(iommu->ecap) &&
- pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI))
+ pci_pri_supported(pdev))
info->pri_supported = 1;
}
}
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index b761c1f72f67..2e6cf0c700f7 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -325,6 +325,19 @@ int pci_prg_resp_pasid_required(struct pci_dev *pdev)
return pdev->pasid_required;
}
+
+/**
+ * pci_pri_supported - Check if PRI is supported.
+ * @pdev: PCI device structure
+ *
+ * Returns true if PRI capability is present, false otherwise.
+ */
+bool pci_pri_supported(struct pci_dev *pdev)
+{
+ /* VFs share the PF PRI configuration */
+ return !!(pci_physfn(pdev)->pri_cap);
+}
+EXPORT_SYMBOL_GPL(pci_pri_supported);
#endif /* CONFIG_PCI_PRI */
#ifdef CONFIG_PCI_PASID
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
index f75c307f346d..df54cd5b15db 100644
--- a/include/linux/pci-ats.h
+++ b/include/linux/pci-ats.h
@@ -28,6 +28,10 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs);
void pci_disable_pri(struct pci_dev *pdev);
int pci_reset_pri(struct pci_dev *pdev);
int pci_prg_resp_pasid_required(struct pci_dev *pdev);
+bool pci_pri_supported(struct pci_dev *pdev);
+#else
+static inline bool pci_pri_supported(struct pci_dev *pdev)
+{ return false; }
#endif /* CONFIG_PCI_PRI */
#ifdef CONFIG_PCI_PASID
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] PCI/ATS: PASID and PRI are only enumerated in PF devices.
PASID and PRI capabilities are only enumerated in PF devices. VF devices
do not enumerate these capabilites. IOMMU drivers also need to enumerate
them before enabling features in the IOMMU. Extending the same support as
PASID feature discovery (pci_pasid_features) for PRI.
Signed-off-by: Ashok Raj
v2: Fixed build failure from lkp when CONFIG_PRI=n
Almost all the PRI functions were called only when CONFIG_PASID is
set. Except the new pci_pri_supported().
To: Bjorn Helgaas
To: Joerg Roedel
To: Lu Baolu
Cc: sta...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Ashok Raj
Cc: iommu@lists.linux-foundation.org
---
drivers/iommu/intel/iommu.c | 2 +-
drivers/pci/ats.c | 14 ++
include/linux/pci-ats.h | 4
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index d759e7234e98..276452f5e6a7 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2560,7 +2560,7 @@ static struct dmar_domain
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
}
if (info->ats_supported && ecap_prs(iommu->ecap) &&
- pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI))
+ pci_pri_supported(pdev))
info->pri_supported = 1;
}
}
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index b761c1f72f67..ffb4de8c5a77 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -461,6 +461,20 @@ int pci_pasid_features(struct pci_dev *pdev)
}
EXPORT_SYMBOL_GPL(pci_pasid_features);
+/**
+ * pci_pri_supported - Check if PRI is supported.
+ * @pdev: PCI device structure
+ *
+ * Returns false when no PRI capability is present.
+ * Returns true if PRI feature is supported and enabled
+ */
+bool pci_pri_supported(struct pci_dev *pdev)
+{
+ /* VFs share the PF PRI configuration */
+ return !!(pci_physfn(pdev)->pri_cap);
+}
+EXPORT_SYMBOL_GPL(pci_pri_supported);
+
#define PASID_NUMBER_SHIFT 8
#define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT)
/**
diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
index f75c307f346d..fc989295daf3 100644
--- a/include/linux/pci-ats.h
+++ b/include/linux/pci-ats.h
@@ -28,6 +28,10 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs);
void pci_disable_pri(struct pci_dev *pdev);
int pci_reset_pri(struct pci_dev *pdev);
int pci_prg_resp_pasid_required(struct pci_dev *pdev);
+bool pci_pri_supported(struct pci_dev *pdev);
+#else
+bool pci_pri_supported(struct pci_dev *pdev)
+{ return false; }
#endif /* CONFIG_PCI_PRI */
#ifdef CONFIG_PCI_PASID
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 4/7] iommu/vt-d: Handle non-page aligned address
Hi Baolu,
Not sure what state is this patch in, there is a bug in this patch
(see below), shall I send out an updated version of this one only? or
another incremental patch.
Thanks,
Jacob
On Mon, 6 Jul 2020 17:12:51 -0700
Jacob Pan wrote:
> From: Liu Yi L
>
> Address information for device TLB invalidation comes from userspace
> when device is directly assigned to a guest with vIOMMU support.
> VT-d requires page aligned address. This patch checks and enforce
> address to be page aligned, otherwise reserved bits can be set in the
> invalidation descriptor. Unrecoverable fault will be reported due to
> non-zero value in the reserved bits.
>
> Fixes: 61a06a16e36d8 ("iommu/vt-d: Support flushing more translation
> cache types")
> Acked-by: Lu Baolu
> Signed-off-by: Liu Yi L
> Signed-off-by: Jacob Pan
>
> ---
> drivers/iommu/intel/dmar.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index d9f973fa1190..b2c53bada905 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1455,9 +1455,25 @@ void qi_flush_dev_iotlb_pasid(struct
> intel_iommu *iommu, u16 sid, u16 pfsid,
>* Max Invs Pending (MIP) is set to 0 for now until we have
> DIT in
>* ECAP.
>*/
> - desc.qw1 |= addr & ~mask;
> - if (size_order)
> + if (addr & GENMASK_ULL(size_order + VTD_PAGE_SHIFT, 0))
> + pr_warn_ratelimited("Invalidate non-aligned address
> %llx, order %d\n", addr, size_order); +
> + /* Take page address */
> + desc.qw1 = QI_DEV_EIOTLB_ADDR(addr);
> +
> + if (size_order) {
> + /*
> + * Existing 0s in address below size_order may be
> the least
> + * significant bit, we must set them to 1s to avoid
> having
> + * smaller size than desired.
> + */
> + desc.qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT,
> + VTD_PAGE_SHIFT);
Yi reported the issue, it should be:
desc.qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT - 1,
VTD_PAGE_SHIFT);
> + /* Clear size_order bit to indicate size */
> + desc.qw1 &= ~mask;
> + /* Set the S bit to indicate flushing more than 1
> page */ desc.qw1 |= QI_DEV_EIOTLB_SIZE;
> + }
>
> qi_submit_sync(iommu, , 1, 0);
> }
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 2020-07-21 at 20:52 +0530, Amit Pundir wrote:
[...]
> > > > Can you try booting *without* my patch and this in the kernel
> > > > command
> > > > line: "cma=16M@0x1-0x2".
> > >
> > > It doesn't boot with this added kernel command line.
> >
> > For the record, this placed the CMA in the [4GB, 8GB] address space
> > instead of you setup's default: [3GB, 4GB]. All atomic pools fall
> > in
> > that memory area without my patch, which makes me think some of the
> > devices on your board might not like higher addresses.
> >
>
> Thank you Nicolas for the details. Though we don't set the CMA
> alloc-ranges explicitly in upstream sdm845 dts, but I dug around and
> found that CMA alloc-ranges in the downstream kernel are indeed in
> lower address space.
> https://github.com/MiCode/Xiaomi_Kernel_OpenSource/blob/dipper-q-oss/arch/arm64/boot/dts/qcom/sdm845.dtsi#L662
>
> /* global autoconfigured region for contiguous allocations */
> linux,cma {
> compatible = "shared-dma-pool";
> alloc-ranges = <0 0x 0 0x>;
> reusable;
> alignment = <0 0x40>;
> size = <0 0x200>;
> linux,cma-default;
> };
Pretty standard, and similar to what it's being used upstream by
default.
>
> > What happens if you boot with my troublesome patch with this in
> > your
> > device tree? (insert it at the bottom of sdm845-beryllium.dts)
> >
> > {
> > dma-ranges = <0 0 0 0 0x1 0>;
> > };
> >
>
> Device still doesn't boot up to adb shell.
Let's get a bigger hammer, I'm just looking for clues here. Can you
apply this and provide the dmesg output.
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 6bc74a2d5127..2160676bf488 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -268,6 +268,8 @@ void *dma_alloc_from_pool(struct device *dev, size_t size,
schedule_work(_pool_work);
}
+ dev_info(dev, "%s: size %lx, phys addr %llx, flags 0x%x\n", __func__,
size, phys, flags);
+
return ptr;
}
Regards,
Nicolas
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/arm-smmu: Implement qcom,skip-init
>The current >focus has been on moving more of the SMMU specific bits into the arm-smmu-qcom >implementation [1] and I think that is the right way to go. Pardon if I overlooked something obvious, but I can't seem to find a clean way for implementing qcom,skip-init in arm-smmu-qcom, as neither the arm_smmu_test_smr_masks nor the probe function seem to be alterable with arm_smmu_impl. I'm open to your ideas guys. Konrad ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/arm-smmu: Implement qcom,skip-init
On Tue, Jul 21, 2020 at 05:04:11PM +0200, Konrad Dybcio wrote: > So.. is this a no-no? > > I of course would like to omit this entirely, but SMMUs on sdm630 and > friends are REALLY picky.. What seems to happen is that when the > driver tries to do things the "standard" way, hypervisor decides to > hang the platform or force a reboot. Not very usable. > > > This thing is needed for the platform to even boot properly and one > more [1] is required to make mdss work with video mode panels (the > fact that CMD-mode panels work is kinda hilarious to me). > > To be honest, there are even more qcom quirks (of which at least > qcom,dynamic and qcom-use-3-lvl-tables are used on 630).. [2] > > Looking forward to your answers and possibly better solutions. Nobody is disputing that the qcom SMMUs don't have their share of quirks but it seems that the community has mostly settled on the agreement that there are better ways to solve this than a handful of device tree properties. The current focus has been on moving more of the SMMU specific bits into the arm-smmu-qcom implementation [1] and I think that is the right way to go. As for the other quirks we can probably discuss those on a case by case basis. I doubt you will find much enthusiasm for qcom,use-3-lvl-tables and I've been working on replacing qcom,dynamic with something much better [2]. [1] https://lists.linuxfoundation.org/pipermail/iommu/2020-July/046304.html [2] https://lists.linuxfoundation.org/pipermail/iommu/2020-July/046756.html Jordan > [1] > https://github.com/konradybcio/linux/commit/83ac38af259968f92b6a8b7eab90096c78469f87 > [2] > https://github.com/sonyxperiadev/kernel/blob/aosp/LA.UM.7.1.r1/drivers/iommu/arm-smmu.c#L404-L415 > > Regards > Konrad -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 21 Jul 2020 at 18:15, Nicolas Saenz Julienne
wrote:
>
> On Tue, 2020-07-21 at 17:45 +0530, Amit Pundir wrote:
> > On Tue, 21 Jul 2020 at 16:45, Nicolas Saenz Julienne
> > wrote:
> > > On Tue, 2020-07-21 at 14:24 +0530, Amit Pundir wrote:
> > > > On Tue, 21 Jul 2020 at 14:09, Nicolas Saenz Julienne
> > > > wrote:
> > > > > Hi Amit,
> > > > > > Hi Nicolas,
> > > > > >
> > > > > > I see a boot regression with this commit d9765e41d8e9 "dma-
> > > > > > pool:
> > > > > > Do not allocate pool memory from CMA" on my Xiaomi Poco F1
> > > > > > (Qcom sdm845) phone running v5.8-rc6. I can't boot past the
> > > > > > bootloader splash screen with this patch.
> > > > > >
> > > > > > Phone boots fine if I revert this patch. I carry only one out
> > > > > > of
> > > > > > tree
> > > > > > dts patch https://lkml.org/lkml/2020/6/25/52. And since this
> > > > > > is a
> > > > > > stock
> > > > > > phone, I don't have access to serial/dmesg logs until I boot
> > > > > > to
> > > > > > AOSP
> > > > > > (adb) shell.
> > > > > >
> > > > > > Any thoughts as to what might be going wrong here? I'd be
> > > > > > happy
> > > > > > to
> > > > > > help debug things. For what it's worth, I don't see this
> > > > > > regression
> > > > > > on
> > > > > > other two sdm845 devices (db845c and Pixel 3) I tested on.
> > > > >
> > > > > Can you provide a boot log (even if without my patch) and the
> > > > > device-
> > > > > tree files? It'd help a lot figuring things out.
> > > >
> > > > Thank you for the prompt reply Nicolas.
> > > >
> > > > Here is the boot log with the reverted patch
> > > > https://pastebin.ubuntu.com/p/BrhPf83nKF/
> > > >
> > > > Here is my phone's dts
> > > > https://github.com/pundiramit/linux/commit/2a394c199deeaf4c91e0e008e8fba2a72f494d8c
> > >
> > > I'm at loss at what could be failing here. Your device should be
> > > able
> > > to address the whole 8GB memory space, which AFAIK is the max
> > > available
> > > on that smartphone family. But maybe the device-tree is lying, who
> > > knows...
> >
> > If it helps, my phone has 6GB memory space.
> >
> > > Can you try booting *without* my patch and this in the kernel
> > > command
> > > line: "cma=16M@0x1-0x2".
> >
> > It doesn't boot with this added kernel command line.
>
>
> For the record, this placed the CMA in the [4GB, 8GB] address space
> instead of you setup's default: [3GB, 4GB]. All atomic pools fall in
> that memory area without my patch, which makes me think some of the
> devices on your board might not like higher addresses.
>
Thank you Nicolas for the details. Though we don't set the CMA
alloc-ranges explicitly in upstream sdm845 dts, but I dug around and
found that CMA alloc-ranges in the downstream kernel are indeed in
lower address space.
https://github.com/MiCode/Xiaomi_Kernel_OpenSource/blob/dipper-q-oss/arch/arm64/boot/dts/qcom/sdm845.dtsi#L662
/* global autoconfigured region for contiguous allocations */
linux,cma {
compatible = "shared-dma-pool";
alloc-ranges = <0 0x 0 0x>;
reusable;
alignment = <0 0x40>;
size = <0 0x200>;
linux,cma-default;
};
> What happens if you boot with my troublesome patch with this in your
> device tree? (insert it at the bottom of sdm845-beryllium.dts)
>
> {
> dma-ranges = <0 0 0 0 0x1 0>;
> };
>
Device still doesn't boot up to adb shell.
Regards,
Amit Pundir
> Regards,
> Nicolas
>
> > Regards,
> > Amit Pundir
> >
> > > Regards,
> > > Nicolas
> > >
> > > And here is my kernel tree just in case
> > > > https://github.com/pundiramit/linux/commits/beryllium-mainline
> > > >
> > > > Regards,
> > > > Amit Pundir
> > > >
> > > >
> > > > > Regards,
> > > > > Nicolas
> > > > >
> > > > > > Regards,
> > > > > > Amit Pundir
> > > > > >
> > > > > > > Reported-by: Jeremy Linton
> > > > > > > Signed-off-by: Nicolas Saenz Julienne <
> > > > > > > nsaenzjulie...@suse.de>
> > > > > > > ---
> > > > > > >
> > > > > > > An more costly alternative would be adding an option to
> > > > > > > dma_alloc_from_contiguous() so it fails when the allocation
> > > > > > > doesn't
> > > > > > > fall
> > > > > > > in a specific zone.
> > > > > > >
> > > > > > > kernel/dma/pool.c | 11 ++-
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu
> -Original Message-
> From: iommu On Behalf Of Lu
> Baolu
> Sent: Monday, July 20, 2020 7:17 PM
> To: Joerg Roedel
> Cc: Ashok Raj; linux-ker...@vger.kernel.org; sta...@vger.kernel.org; Koba
> Ko; iommu@lists.linux-foundation.org
> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
> iommu
>
> The VT-d spec requires (10.4.4 Global Command Register, TE field) that:
>
> Hardware implementations supporting DMA draining must drain any in-flight
> DMA read/write requests queued within the Root-Complex before completing
> the translation enable command and reflecting the status of the command
> through the TES field in the Global Status register.
>
> Unfortunately, some integrated graphic devices fail to do so after some
> kind of power state transition. As the result, the system might stuck in
> iommu_disable_translation(), waiting for the completion of TE transition.
>
> This provides a quirk list for those devices and skips TE disabling if
> the qurik hits.
>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=208363
That one is for TGL.
I think you also want to add this one for ICL:
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=206571
> Tested-by: Koba Ko
> Cc: Ashok Raj
> Cc: sta...@vger.kernel.org
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/dmar.c | 1 +
> drivers/iommu/intel/iommu.c | 27 +++
> include/linux/dmar.h| 1 +
> include/linux/intel-iommu.h | 2 ++
> 4 files changed, 31 insertions(+)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 683b812c5c47..16f47041f1bf 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1102,6 +1102,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
> }
>
> drhd->iommu = iommu;
> + iommu->drhd = drhd;
>
> return 0;
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 98390a6d8113..11418b14cc3f 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -356,6 +356,7 @@ static int intel_iommu_strict;
> static int intel_iommu_superpage = 1;
> static int iommu_identity_mapping;
> static int intel_no_bounce;
> +static int iommu_skip_te_disable;
>
> #define IDENTMAP_GFX 2
> #define IDENTMAP_AZALIA 4
> @@ -1633,6 +1634,10 @@ static void iommu_disable_translation(struct
> intel_iommu *iommu)
> u32 sts;
> unsigned long flag;
>
> + if (iommu_skip_te_disable && iommu->drhd->gfx_dedicated &&
> + (cap_read_drain(iommu->cap) || cap_write_drain(iommu->cap)))
> + return;
> +
> raw_spin_lock_irqsave(>register_lock, flag);
> iommu->gcmd &= ~DMA_GCMD_TE;
> writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
> @@ -4043,6 +4048,7 @@ static void __init init_no_remapping_devices(void)
>
> /* This IOMMU has *only* gfx devices. Either bypass it or
> set the gfx_mapped flag, as appropriate */
> + drhd->gfx_dedicated = 1;
> if (!dmar_map_gfx) {
> drhd->ignored = 1;
> for_each_active_dev_scope(drhd->devices,
> @@ -6160,6 +6166,27 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,
> 0x0044, quirk_calpella_no_shadow_g
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0062,
> quirk_calpella_no_shadow_gtt);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x006a,
> quirk_calpella_no_shadow_gtt);
>
> +static void quirk_igfx_skip_te_disable(struct pci_dev *dev)
> +{
> + unsigned short ver;
> +
> + if (!IS_GFX_DEVICE(dev))
> + return;
> +
> + ver = (dev->device >> 8) & 0xff;
> + if (ver != 0x45 && ver != 0x46 && ver != 0x4c &&
> + ver != 0x4e && ver != 0x8a && ver != 0x98 &&
> + ver != 0x9a)
> + return;
> +
> + if (risky_device(dev))
> + return;
> +
> + pci_info(dev, "Skip IOMMU disabling for graphics\n");
> + iommu_skip_te_disable = 1;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> quirk_igfx_skip_te_disable);
> +
> /* On Tylersburg chipsets, some BIOSes have been known to enable the
> ISOCH DMAR unit for the Azalia sound device, but not give it any
> TLB entries, which causes it to deadlock. Check for that. We do
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index d7bf029df737..65565820328a 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -48,6 +48,7 @@ struct dmar_drhd_unit {
> u16 segment;/* PCI domain */
> u8 ignored:1; /* ignore drhd */
> u8 include_all:1;
> + u8 gfx_dedicated:1;/* graphic dedicated*/
> struct intel_iommu *iommu;
> };
>
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index bf6009a344f5..329629e1e9de 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@
Re: [PATCH 1/1] iommu/arm-smmu: Implement qcom,skip-init
So.. is this a no-no? I of course would like to omit this entirely, but SMMUs on sdm630 and friends are REALLY picky.. What seems to happen is that when the driver tries to do things the "standard" way, hypervisor decides to hang the platform or force a reboot. Not very usable. This thing is needed for the platform to even boot properly and one more [1] is required to make mdss work with video mode panels (the fact that CMD-mode panels work is kinda hilarious to me). To be honest, there are even more qcom quirks (of which at least qcom,dynamic and qcom-use-3-lvl-tables are used on 630).. [2] Looking forward to your answers and possibly better solutions. [1] https://github.com/konradybcio/linux/commit/83ac38af259968f92b6a8b7eab90096c78469f87 [2] https://github.com/sonyxperiadev/kernel/blob/aosp/LA.UM.7.1.r1/drivers/iommu/arm-smmu.c#L404-L415 Regards Konrad ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 03/12] ACPI/IORT: Make iort_msi_map_rid() PCI agnostic
On Fri, Jun 19, 2020 at 09:20:04AM +0100, Lorenzo Pieralisi wrote:
> There is nothing PCI specific in iort_msi_map_rid().
>
> Rename the function using a bus protocol agnostic name,
> iort_msi_map_id(), and convert current callers to it.
>
> Signed-off-by: Lorenzo Pieralisi
> Cc: Will Deacon
> Cc: Hanjun Guo
> Cc: Bjorn Helgaas
> Cc: Sudeep Holla
> Cc: Catalin Marinas
> Cc: Robin Murphy
> Cc: "Rafael J. Wysocki"
Acked-by: Bjorn Helgaas
Sorry I missed this!
> ---
> drivers/acpi/arm64/iort.c | 12 ++--
> drivers/pci/msi.c | 2 +-
> include/linux/acpi_iort.h | 6 +++---
> 3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 902e2aaca946..53f9ef515089 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -568,22 +568,22 @@ static struct acpi_iort_node *iort_find_dev_node(struct
> device *dev)
> }
>
> /**
> - * iort_msi_map_rid() - Map a MSI requester ID for a device
> + * iort_msi_map_id() - Map a MSI input ID for a device
> * @dev: The device for which the mapping is to be done.
> - * @req_id: The device requester ID.
> + * @input_id: The device input ID.
> *
> - * Returns: mapped MSI RID on success, input requester ID otherwise
> + * Returns: mapped MSI ID on success, input ID otherwise
> */
> -u32 iort_msi_map_rid(struct device *dev, u32 req_id)
> +u32 iort_msi_map_id(struct device *dev, u32 input_id)
> {
> struct acpi_iort_node *node;
> u32 dev_id;
>
> node = iort_find_dev_node(dev);
> if (!node)
> - return req_id;
> + return input_id;
>
> - iort_node_map_id(node, req_id, _id, IORT_MSI_TYPE);
> + iort_node_map_id(node, input_id, _id, IORT_MSI_TYPE);
> return dev_id;
> }
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 74a91f52ecc0..77f48b95e277 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1536,7 +1536,7 @@ u32 pci_msi_domain_get_msi_rid(struct irq_domain
> *domain, struct pci_dev *pdev)
>
> of_node = irq_domain_get_of_node(domain);
> rid = of_node ? of_msi_map_rid(>dev, of_node, rid) :
> - iort_msi_map_rid(>dev, rid);
> + iort_msi_map_id(>dev, rid);
>
> return rid;
> }
> diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h
> index 08ec6bd2297f..e51425e083da 100644
> --- a/include/linux/acpi_iort.h
> +++ b/include/linux/acpi_iort.h
> @@ -28,7 +28,7 @@ void iort_deregister_domain_token(int trans_id);
> struct fwnode_handle *iort_find_domain_token(int trans_id);
> #ifdef CONFIG_ACPI_IORT
> void acpi_iort_init(void);
> -u32 iort_msi_map_rid(struct device *dev, u32 req_id);
> +u32 iort_msi_map_id(struct device *dev, u32 id);
> struct irq_domain *iort_get_device_domain(struct device *dev, u32 id,
> enum irq_domain_bus_token bus_token);
> void acpi_configure_pmsi_domain(struct device *dev);
> @@ -39,8 +39,8 @@ const struct iommu_ops *iort_iommu_configure(struct device
> *dev);
> int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head
> *head);
> #else
> static inline void acpi_iort_init(void) { }
> -static inline u32 iort_msi_map_rid(struct device *dev, u32 req_id)
> -{ return req_id; }
> +static inline u32 iort_msi_map_id(struct device *dev, u32 id)
> +{ return id; }
> static inline struct irq_domain *iort_get_device_domain(
> struct device *dev, u32 id, enum irq_domain_bus_token bus_token)
> { return NULL; }
> --
> 2.26.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] PCI/ATS: PASID and PRI are only enumerated in PF devices.
On Mon, Jul 20, 2020 at 09:43:00AM -0700, Ashok Raj wrote:
> PASID and PRI capabilities are only enumerated in PF devices. VF devices
> do not enumerate these capabilites. IOMMU drivers also need to enumerate
> them before enabling features in the IOMMU. Extending the same support as
> PASID feature discovery (pci_pasid_features) for PRI.
>
> Signed-off-by: Ashok Raj
Hi Ashok,
When you update this for the 0-day implicit declaration thing, can you
update the subject to say what the patch *does*, as opposed to what it
is solving? Also, no need for a period at the end.
Does this fix a regression? Is it associated with a commit that we
could add as a "Fixes:" tag so we know how far back to try to apply
to stable kernels?
> To: Bjorn Helgaas
> To: Joerg Roedel
> To: Lu Baolu
> Cc: sta...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: Ashok Raj
> Cc: iommu@lists.linux-foundation.org
> ---
> drivers/iommu/intel/iommu.c | 2 +-
> drivers/pci/ats.c | 14 ++
> include/linux/pci-ats.h | 1 +
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index d759e7234e98..276452f5e6a7 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2560,7 +2560,7 @@ static struct dmar_domain
> *dmar_insert_one_dev_info(struct intel_iommu *iommu,
> }
>
> if (info->ats_supported && ecap_prs(iommu->ecap) &&
> - pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI))
> + pci_pri_supported(pdev))
> info->pri_supported = 1;
> }
> }
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index b761c1f72f67..ffb4de8c5a77 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -461,6 +461,20 @@ int pci_pasid_features(struct pci_dev *pdev)
> }
> EXPORT_SYMBOL_GPL(pci_pasid_features);
>
> +/**
> + * pci_pri_supported - Check if PRI is supported.
> + * @pdev: PCI device structure
> + *
> + * Returns false when no PRI capability is present.
> + * Returns true if PRI feature is supported and enabled
> + */
> +bool pci_pri_supported(struct pci_dev *pdev)
> +{
> + /* VFs share the PF PRI configuration */
> + return !!(pci_physfn(pdev)->pri_cap);
> +}
> +EXPORT_SYMBOL_GPL(pci_pri_supported);
> +
> #define PASID_NUMBER_SHIFT 8
> #define PASID_NUMBER_MASK(0x1f << PASID_NUMBER_SHIFT)
> /**
> diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h
> index f75c307f346d..073d57292445 100644
> --- a/include/linux/pci-ats.h
> +++ b/include/linux/pci-ats.h
> @@ -28,6 +28,7 @@ int pci_enable_pri(struct pci_dev *pdev, u32 reqs);
> void pci_disable_pri(struct pci_dev *pdev);
> int pci_reset_pri(struct pci_dev *pdev);
> int pci_prg_resp_pasid_required(struct pci_dev *pdev);
> +bool pci_pri_supported(struct pci_dev *pdev);
> #endif /* CONFIG_PCI_PRI */
>
> #ifdef CONFIG_PCI_PASID
> --
> 2.7.4
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v8 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset
On Wed, Jul 15, 2020 at 10:35:11AM -0400, Jim Quinlan wrote:
> The new field 'dma_range_map' in struct device is used to facilitate the
> use of single or multiple offsets between mapping regions of cpu addrs and
> dma addrs. It subsumes the role of "dev->dma_pfn_offset" which was only
> capable of holding a single uniform offset and had no region bounds
> checking.
>
> The function of_dma_get_range() has been modified so that it takes a single
> argument -- the device node -- and returns a map, NULL, or an error code.
> The map is an array that holds the information regarding the DMA regions.
> Each range entry contains the address offset, the cpu_start address, the
> dma_start address, and the size of the region.
>
> of_dma_configure() is the typical manner to set range offsets but there are
> a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
> driver code. These cases now invoke the function
> dma_attach_offset_range(dev, cpu_addr, dma_addr, size).
So my main higher level issue here is the dma_attach_offset_range
function. I think it should keep the old functionality and just
set a global range from 0 to (phys_addr_t)-1, and bail out if there
are DMA ranges already:
int dma_set_global_offset(struct device *dev, u64 offset);
otherwise there is all kinds of minor nitpicks that aren't too
substantial, let me know what you think of something like this
hacked up version:
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca3451..2405afeb79573a 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev)
- pfn -= dev->dma_pfn_offset;
+ if (dev) {
+ phys_addr_t paddr = PFN_PHYS(pfn);
+
+ pfn -= (dma_offset_from_phys_addr(dev, paddr) >> PAGE_SHIFT);
+ }
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -45,8 +48,7 @@ static inline unsigned long dma_to_pfn(struct device *dev,
dma_addr_t addr)
unsigned long pfn = __bus_to_pfn(addr);
if (dev)
- pfn += dev->dma_pfn_offset;
-
+ pfn += (dma_offset_from_dma_addr(dev, addr) >> PAGE_SHIFT);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e12247..7539679205fbf7 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -8,6 +8,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -24,8 +25,6 @@
#include "keystone.h"
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
static int keystone_platform_notifier(struct notifier_block *nb,
unsigned long event, void *data)
{
@@ -38,9 +37,12 @@ static int keystone_platform_notifier(struct notifier_block
*nb,
return NOTIFY_BAD;
if (!dev->of_node) {
- dev->dma_pfn_offset = keystone_dma_pfn_offset;
- dev_err(dev, "set dma_pfn_offset%08lx\n",
- dev->dma_pfn_offset);
+ int ret = dma_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+ KEYSTONE_LOW_PHYS_START,
+ KEYSTONE_HIGH_PHYS_SIZE);
+ dev_err(dev, "set dma_offset%08llx%s\n",
+ KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+ ret ? " failed" : "");
}
return NOTIFY_OK;
}
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
static void __init keystone_init(void)
{
- if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
- keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
- KEYSTONE_LOW_PHYS_START);
+ if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START)
bus_register_notifier(_bus_type, _nb);
- }
keystone_pm_runtime_init();
}
diff --git a/arch/sh/drivers/pci/pcie-sh7786.c
b/arch/sh/drivers/pci/pcie-sh7786.c
index e0b568aaa7014c..e929f85c503852 100644
--- a/arch/sh/drivers/pci/pcie-sh7786.c
+++ b/arch/sh/drivers/pci/pcie-sh7786.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -31,6 +32,8 @@ struct sh7786_pcie_port {
static struct sh7786_pcie_port *sh7786_pcie_ports;
static unsigned int nr_ports;
static unsigned long dma_pfn_offset;
+size_t memsize;
+u64 memstart;
static struct sh7786_pcie_hwops {
int (*core_init)(void);
@@ -301,7 +304,6 @@ static int __init pcie_init(struct sh7786_pcie_port *port)
struct pci_channel *chan = port->hose;
unsigned int data;
phys_addr_t memstart, memend;
- size_t
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 2020-07-21 at 17:45 +0530, Amit Pundir wrote:
> On Tue, 21 Jul 2020 at 16:45, Nicolas Saenz Julienne
> wrote:
> > On Tue, 2020-07-21 at 14:24 +0530, Amit Pundir wrote:
> > > On Tue, 21 Jul 2020 at 14:09, Nicolas Saenz Julienne
> > > wrote:
> > > > Hi Amit,
> > > > > Hi Nicolas,
> > > > >
> > > > > I see a boot regression with this commit d9765e41d8e9 "dma-
> > > > > pool:
> > > > > Do not allocate pool memory from CMA" on my Xiaomi Poco F1
> > > > > (Qcom sdm845) phone running v5.8-rc6. I can't boot past the
> > > > > bootloader splash screen with this patch.
> > > > >
> > > > > Phone boots fine if I revert this patch. I carry only one out
> > > > > of
> > > > > tree
> > > > > dts patch https://lkml.org/lkml/2020/6/25/52. And since this
> > > > > is a
> > > > > stock
> > > > > phone, I don't have access to serial/dmesg logs until I boot
> > > > > to
> > > > > AOSP
> > > > > (adb) shell.
> > > > >
> > > > > Any thoughts as to what might be going wrong here? I'd be
> > > > > happy
> > > > > to
> > > > > help debug things. For what it's worth, I don't see this
> > > > > regression
> > > > > on
> > > > > other two sdm845 devices (db845c and Pixel 3) I tested on.
> > > >
> > > > Can you provide a boot log (even if without my patch) and the
> > > > device-
> > > > tree files? It'd help a lot figuring things out.
> > >
> > > Thank you for the prompt reply Nicolas.
> > >
> > > Here is the boot log with the reverted patch
> > > https://pastebin.ubuntu.com/p/BrhPf83nKF/
> > >
> > > Here is my phone's dts
> > > https://github.com/pundiramit/linux/commit/2a394c199deeaf4c91e0e008e8fba2a72f494d8c
> >
> > I'm at loss at what could be failing here. Your device should be
> > able
> > to address the whole 8GB memory space, which AFAIK is the max
> > available
> > on that smartphone family. But maybe the device-tree is lying, who
> > knows...
>
> If it helps, my phone has 6GB memory space.
>
> > Can you try booting *without* my patch and this in the kernel
> > command
> > line: "cma=16M@0x1-0x2".
>
> It doesn't boot with this added kernel command line.
For the record, this placed the CMA in the [4GB, 8GB] address space
instead of you setup's default: [3GB, 4GB]. All atomic pools fall in
that memory area without my patch, which makes me think some of the
devices on your board might not like higher addresses.
What happens if you boot with my troublesome patch with this in your
device tree? (insert it at the bottom of sdm845-beryllium.dts)
{
dma-ranges = <0 0 0 0 0x1 0>;
};
Regards,
Nicolas
> Regards,
> Amit Pundir
>
> > Regards,
> > Nicolas
> >
> > And here is my kernel tree just in case
> > > https://github.com/pundiramit/linux/commits/beryllium-mainline
> > >
> > > Regards,
> > > Amit Pundir
> > >
> > >
> > > > Regards,
> > > > Nicolas
> > > >
> > > > > Regards,
> > > > > Amit Pundir
> > > > >
> > > > > > Reported-by: Jeremy Linton
> > > > > > Signed-off-by: Nicolas Saenz Julienne <
> > > > > > nsaenzjulie...@suse.de>
> > > > > > ---
> > > > > >
> > > > > > An more costly alternative would be adding an option to
> > > > > > dma_alloc_from_contiguous() so it fails when the allocation
> > > > > > doesn't
> > > > > > fall
> > > > > > in a specific zone.
> > > > > >
> > > > > > kernel/dma/pool.c | 11 ++-
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 21 Jul 2020 at 17:07, Nicolas Saenz Julienne wrote: > > On Tue, 2020-07-21 at 13:28 +0200, Christoph Hellwig wrote: > > On Tue, Jul 21, 2020 at 01:15:23PM +0200, Nicolas Saenz Julienne > > wrote: > > > I'm at loss at what could be failing here. Your device should be > > > able > > > to address the whole 8GB memory space, which AFAIK is the max > > > available > > > on that smartphone family. But maybe the device-tree is lying, who > > > knows... > > > > Maybe we should give your patch to allocate from CMA but check the > > address a try? (just because we can..) > > Yes, good idea! > > Amir, could you also test this patch[1] (having reverted the one that > casues trouble) and report on whether it boots or not? Can't boot with that patch either. Regards, Amit Pundir > > Regards, > Nicolas > > [1] https://lore.kernel.org/linux-iomhttps://lore.kernel.org/linux-iom > mu/fe14037b02fd887a73cd91c115dccc4485f8446e.ca...@suse.de/T/#t > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 21 Jul 2020 at 16:45, Nicolas Saenz Julienne wrote: > > On Tue, 2020-07-21 at 14:24 +0530, Amit Pundir wrote: > > On Tue, 21 Jul 2020 at 14:09, Nicolas Saenz Julienne > > wrote: > > > Hi Amit, > > > > Hi Nicolas, > > > > > > > > I see a boot regression with this commit d9765e41d8e9 "dma-pool: > > > > Do not allocate pool memory from CMA" on my Xiaomi Poco F1 > > > > (Qcom sdm845) phone running v5.8-rc6. I can't boot past the > > > > bootloader splash screen with this patch. > > > > > > > > Phone boots fine if I revert this patch. I carry only one out of > > > > tree > > > > dts patch https://lkml.org/lkml/2020/6/25/52. And since this is a > > > > stock > > > > phone, I don't have access to serial/dmesg logs until I boot to > > > > AOSP > > > > (adb) shell. > > > > > > > > Any thoughts as to what might be going wrong here? I'd be happy > > > > to > > > > help debug things. For what it's worth, I don't see this > > > > regression > > > > on > > > > other two sdm845 devices (db845c and Pixel 3) I tested on. > > > > > > Can you provide a boot log (even if without my patch) and the > > > device- > > > tree files? It'd help a lot figuring things out. > > > > Thank you for the prompt reply Nicolas. > > > > Here is the boot log with the reverted patch > > https://pastebin.ubuntu.com/p/BrhPf83nKF/ > > > > Here is my phone's dts > > https://github.com/pundiramit/linux/commit/2a394c199deeaf4c91e0e008e8fba2a72f494d8c > > I'm at loss at what could be failing here. Your device should be able > to address the whole 8GB memory space, which AFAIK is the max available > on that smartphone family. But maybe the device-tree is lying, who > knows... If it helps, my phone has 6GB memory space. > > Can you try booting *without* my patch and this in the kernel command > line: "cma=16M@0x1-0x2". It doesn't boot with this added kernel command line. Regards, Amit Pundir > > Regards, > Nicolas > > And here is my kernel tree just in case > > https://github.com/pundiramit/linux/commits/beryllium-mainline > > > > Regards, > > Amit Pundir > > > > > > > Regards, > > > Nicolas > > > > > > > Regards, > > > > Amit Pundir > > > > > > > > > Reported-by: Jeremy Linton > > > > > Signed-off-by: Nicolas Saenz Julienne > > > > > --- > > > > > > > > > > An more costly alternative would be adding an option to > > > > > dma_alloc_from_contiguous() so it fails when the allocation > > > > > doesn't > > > > > fall > > > > > in a specific zone. > > > > > > > > > > kernel/dma/pool.c | 11 ++- > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 2020-07-21 at 13:28 +0200, Christoph Hellwig wrote: > On Tue, Jul 21, 2020 at 01:15:23PM +0200, Nicolas Saenz Julienne > wrote: > > I'm at loss at what could be failing here. Your device should be > > able > > to address the whole 8GB memory space, which AFAIK is the max > > available > > on that smartphone family. But maybe the device-tree is lying, who > > knows... > > Maybe we should give your patch to allocate from CMA but check the > address a try? (just because we can..) Yes, good idea! Amir, could you also test this patch[1] (having reverted the one that casues trouble) and report on whether it boots or not? Regards, Nicolas [1] https://lore.kernel.org/linux-iomhttps://lore.kernel.org/linux-iom mu/fe14037b02fd887a73cd91c115dccc4485f8446e.ca...@suse.de/T/#t ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, Jul 21, 2020 at 01:15:23PM +0200, Nicolas Saenz Julienne wrote: > I'm at loss at what could be failing here. Your device should be able > to address the whole 8GB memory space, which AFAIK is the max available > on that smartphone family. But maybe the device-tree is lying, who > knows... Maybe we should give your patch to allocate from CMA but check the address a try? (just because we can..) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
On Tue, 2020-07-21 at 11:40 +0200, Matthias Brugger wrote:
>
> On 21/07/2020 04:16, Miles Chen wrote:
> > In previous discussion [1] and [2], we found that it is risky to
> > use max_pfn or totalram_pages to tell if 4GB mode is enabled.
> >
> > Check 4GB mode by reading infracfg register, remove the usage
> > of the un-exported symbol max_pfn.
> >
> > This is a step towards building mtk_iommu as a kernel module.
> >
> > Change since v1:
> > 1. remove the phandle usage, search for infracfg instead [3]
> > 2. use infracfg instead of infracfg_regmap
> > 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> > 4. update enable_4GB only when has_4gb_mode
> >
> > [1] https://lkml.org/lkml/2020/6/3/733
> > [2] https://lkml.org/lkml/2020/6/4/136
> > [3] https://lkml.org/lkml/2020/7/15/1147
> >
> > Cc: Mike Rapoport
> > Cc: David Hildenbrand
> > Cc: Yong Wu
> > Cc: Yingjoe Chen
> > Cc: Christoph Hellwig
> > Cc: Yong Wu
> > Cc: Chao Hao
> > Cc: Rob Herring
> > Cc: Matthias Brugger
> > Signed-off-by: Miles Chen
> > ---
> > drivers/iommu/mtk_iommu.c | 26 +-
> > include/linux/soc/mediatek/infracfg.h | 3 +++
> > 2 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 2be96f1cdbd2..16765f532853 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -3,7 +3,6 @@
> >* Copyright (c) 2015-2016 MediaTek Inc.
> >* Author: Yong Wu
> >*/
> > -#include
> > #include
> > #include
> > #include
> > @@ -15,13 +14,16 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device
> > *pdev)
> > struct resource *res;
> > resource_size_t ioaddr;
> > struct component_match *match = NULL;
> > + struct regmap *infracfg;
> > void*protect;
> > int i, larb_nr, ret;
> > + u32 val;
> >
> > data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > if (!data)
> > @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device
> > *pdev)
> > return -ENOMEM;
> > data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
> >
> > - /* Whether the current dram is over 4GB */
> > - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> > - if (!data->plat_data->has_4gb_mode)
> > - data->enable_4GB = false;
> > + data->enable_4GB = false;
> > + if (data->plat_data->has_4gb_mode) {
> > + infracfg = syscon_regmap_lookup_by_compatible(
> > + "mediatek,mt8173-infracfg");
> > + if (IS_ERR(infracfg)) {
> > + infracfg = syscon_regmap_lookup_by_compatible(
> > + "mediatek,mt2712-infracfg");
> > + if (IS_ERR(infracfg))
> > + return PTR_ERR(infracfg);
>
> I think we should check m4u_plat instead to decide which compatible we have
> to
> look for.
> Another option would be to add a general compatible something like
> "mtk-infracfg" and search for that. That would need an update of all DTS
> having
> a infracfg compatible right now. After thinking twice, this would break newer
> kernel with older device tree, so maybe it's better to go with m4u_plat
> switch
> statement.
Add a "char *infracfg" in the plat_data, Use the mt2712, mt8173
corresponding string in it. If it is NULL, It means the "enable_4GB"
always is false. Then we also can remove the flag "has_4gb_mode".
is this OK?
>
> Regards,
> Matthias
>
> > +
> > + }
> > + ret = regmap_read(infracfg, REG_INFRA_MISC, );
> > + if (ret)
> > + return ret;
> > + data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
> > + }
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > data->base = devm_ioremap_resource(dev, res);
> > diff --git a/include/linux/soc/mediatek/infracfg.h
> > b/include/linux/soc/mediatek/infracfg.h
> > index fd25f0148566..233463d789c6 100644
> > --- a/include/linux/soc/mediatek/infracfg.h
> > +++ b/include/linux/soc/mediatek/infracfg.h
> > @@ -32,6 +32,9 @@
> > #define MT7622_TOP_AXI_PROT_EN_WB (BIT(2) | BIT(6) | \
> > BIT(7) | BIT(8))
> >
> > +#define REG_INFRA_MISC 0xf00
> > +#define F_DDR_4GB_SUPPORT_EN BIT(13)
> > +
> > int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
> > bool reg_update);
> > int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
> >
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 2020-07-21 at 14:24 +0530, Amit Pundir wrote: > On Tue, 21 Jul 2020 at 14:09, Nicolas Saenz Julienne > wrote: > > Hi Amit, > > > Hi Nicolas, > > > > > > I see a boot regression with this commit d9765e41d8e9 "dma-pool: > > > Do not allocate pool memory from CMA" on my Xiaomi Poco F1 > > > (Qcom sdm845) phone running v5.8-rc6. I can't boot past the > > > bootloader splash screen with this patch. > > > > > > Phone boots fine if I revert this patch. I carry only one out of > > > tree > > > dts patch https://lkml.org/lkml/2020/6/25/52. And since this is a > > > stock > > > phone, I don't have access to serial/dmesg logs until I boot to > > > AOSP > > > (adb) shell. > > > > > > Any thoughts as to what might be going wrong here? I'd be happy > > > to > > > help debug things. For what it's worth, I don't see this > > > regression > > > on > > > other two sdm845 devices (db845c and Pixel 3) I tested on. > > > > Can you provide a boot log (even if without my patch) and the > > device- > > tree files? It'd help a lot figuring things out. > > Thank you for the prompt reply Nicolas. > > Here is the boot log with the reverted patch > https://pastebin.ubuntu.com/p/BrhPf83nKF/ > > Here is my phone's dts > https://github.com/pundiramit/linux/commit/2a394c199deeaf4c91e0e008e8fba2a72f494d8c I'm at loss at what could be failing here. Your device should be able to address the whole 8GB memory space, which AFAIK is the max available on that smartphone family. But maybe the device-tree is lying, who knows... Can you try booting *without* my patch and this in the kernel command line: "cma=16M@0x1-0x2". Regards, Nicolas And here is my kernel tree just in case > https://github.com/pundiramit/linux/commits/beryllium-mainline > > Regards, > Amit Pundir > > > > Regards, > > Nicolas > > > > > Regards, > > > Amit Pundir > > > > > > > Reported-by: Jeremy Linton > > > > Signed-off-by: Nicolas Saenz Julienne > > > > --- > > > > > > > > An more costly alternative would be adding an option to > > > > dma_alloc_from_contiguous() so it fails when the allocation > > > > doesn't > > > > fall > > > > in a specific zone. > > > > > > > > kernel/dma/pool.c | 11 ++- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
On 21/07/2020 04:16, Miles Chen wrote:
In previous discussion [1] and [2], we found that it is risky to
use max_pfn or totalram_pages to tell if 4GB mode is enabled.
Check 4GB mode by reading infracfg register, remove the usage
of the un-exported symbol max_pfn.
This is a step towards building mtk_iommu as a kernel module.
Change since v1:
1. remove the phandle usage, search for infracfg instead [3]
2. use infracfg instead of infracfg_regmap
3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
4. update enable_4GB only when has_4gb_mode
[1] https://lkml.org/lkml/2020/6/3/733
[2] https://lkml.org/lkml/2020/6/4/136
[3] https://lkml.org/lkml/2020/7/15/1147
Cc: Mike Rapoport
Cc: David Hildenbrand
Cc: Yong Wu
Cc: Yingjoe Chen
Cc: Christoph Hellwig
Cc: Yong Wu
Cc: Chao Hao
Cc: Rob Herring
Cc: Matthias Brugger
Signed-off-by: Miles Chen
---
drivers/iommu/mtk_iommu.c | 26 +-
include/linux/soc/mediatek/infracfg.h | 3 +++
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 2be96f1cdbd2..16765f532853 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -3,7 +3,6 @@
* Copyright (c) 2015-2016 MediaTek Inc.
* Author: Yong Wu
*/
-#include
#include
#include
#include
@@ -15,13 +14,16 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
@@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
struct resource *res;
resource_size_t ioaddr;
struct component_match *match = NULL;
+ struct regmap *infracfg;
void*protect;
int i, larb_nr, ret;
+ u32 val;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
return -ENOMEM;
data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
- /* Whether the current dram is over 4GB */
- data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
- if (!data->plat_data->has_4gb_mode)
- data->enable_4GB = false;
+ data->enable_4GB = false;
+ if (data->plat_data->has_4gb_mode) {
+ infracfg = syscon_regmap_lookup_by_compatible(
+ "mediatek,mt8173-infracfg");
+ if (IS_ERR(infracfg)) {
+ infracfg = syscon_regmap_lookup_by_compatible(
+ "mediatek,mt2712-infracfg");
+ if (IS_ERR(infracfg))
+ return PTR_ERR(infracfg);
I think we should check m4u_plat instead to decide which compatible we have to
look for.
Another option would be to add a general compatible something like
"mtk-infracfg" and search for that. That would need an update of all DTS having
a infracfg compatible right now. After thinking twice, this would break newer
kernel with older device tree, so maybe it's better to go with m4u_plat switch
statement.
Regards,
Matthias
+
+ }
+ ret = regmap_read(infracfg, REG_INFRA_MISC, );
+ if (ret)
+ return ret;
+ data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
data->base = devm_ioremap_resource(dev, res);
diff --git a/include/linux/soc/mediatek/infracfg.h
b/include/linux/soc/mediatek/infracfg.h
index fd25f0148566..233463d789c6 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -32,6 +32,9 @@
#define MT7622_TOP_AXI_PROT_EN_WB (BIT(2) | BIT(6) | \
BIT(7) | BIT(8))
+#define REG_INFRA_MISC0xf00
+#define F_DDR_4GB_SUPPORT_EN BIT(13)
+
int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
bool reg_update);
int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: check 4GB mode by reading infracfg
On 21.07.20 04:16, Miles Chen wrote:
> In previous discussion [1] and [2], we found that it is risky to
> use max_pfn or totalram_pages to tell if 4GB mode is enabled.
>
> Check 4GB mode by reading infracfg register, remove the usage
> of the un-exported symbol max_pfn.
>
> This is a step towards building mtk_iommu as a kernel module.
>
> Change since v1:
> 1. remove the phandle usage, search for infracfg instead [3]
> 2. use infracfg instead of infracfg_regmap
> 3. move infracfg definitaions to linux/soc/mediatek/infracfg.h
> 4. update enable_4GB only when has_4gb_mode
Nit: We tend to place such information below the "---" (adding a second
one) such that this information is discarded when the patch is picked up.
>
> [1] https://lkml.org/lkml/2020/6/3/733
> [2] https://lkml.org/lkml/2020/6/4/136
> [3] https://lkml.org/lkml/2020/7/15/1147
>
> Cc: Mike Rapoport
> Cc: David Hildenbrand
> Cc: Yong Wu
> Cc: Yingjoe Chen
> Cc: Christoph Hellwig
> Cc: Yong Wu
> Cc: Chao Hao
> Cc: Rob Herring
> Cc: Matthias Brugger
> Signed-off-by: Miles Chen
> ---
> drivers/iommu/mtk_iommu.c | 26 +-
> include/linux/soc/mediatek/infracfg.h | 3 +++
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index 2be96f1cdbd2..16765f532853 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -3,7 +3,6 @@
> * Copyright (c) 2015-2016 MediaTek Inc.
> * Author: Yong Wu
> */
> -#include
> #include
> #include
> #include
> @@ -15,13 +14,16 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -599,8 +601,10 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> struct resource *res;
> resource_size_t ioaddr;
> struct component_match *match = NULL;
> + struct regmap *infracfg;
> void*protect;
> int i, larb_nr, ret;
> + u32 val;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> @@ -614,10 +618,22 @@ static int mtk_iommu_probe(struct platform_device *pdev)
> return -ENOMEM;
> data->protect_base = ALIGN(virt_to_phys(protect), MTK_PROTECT_PA_ALIGN);
>
> - /* Whether the current dram is over 4GB */
> - data->enable_4GB = !!(max_pfn > (BIT_ULL(32) >> PAGE_SHIFT));
> - if (!data->plat_data->has_4gb_mode)
> - data->enable_4GB = false;
> + data->enable_4GB = false;
> + if (data->plat_data->has_4gb_mode) {
> + infracfg = syscon_regmap_lookup_by_compatible(
> + "mediatek,mt8173-infracfg");
> + if (IS_ERR(infracfg)) {
> + infracfg = syscon_regmap_lookup_by_compatible(
> + "mediatek,mt2712-infracfg");
> + if (IS_ERR(infracfg))
> + return PTR_ERR(infracfg);
> +
> + }
> + ret = regmap_read(infracfg, REG_INFRA_MISC, );
> + if (ret)
> + return ret;
> + data->enable_4GB = !!(val & F_DDR_4GB_SUPPORT_EN);
(I am absolutely not familiar with syscon_regmap_lookup_by ..., I am
missing some context, so sorry for the stupid questions)
Who sets the regmap value and based on what? Who decides that 4GB mode
is supported or not? And who decides if 4GB mode is *required* or not?
--
Thanks,
David / dhildenb
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Tue, 21 Jul 2020 at 14:09, Nicolas Saenz Julienne
wrote:
>
> Hi Amit,
>
> On Tue, 2020-07-21 at 12:51 +0530, Amit Pundir wrote:
> > On Wed, 8 Jul 2020 at 22:43, Nicolas Saenz Julienne
> > wrote:
> > > There is no guarantee to CMA's placement, so allocating a zone
> > > specific
> > > atomic pool from CMA might return memory from a completely
> > > different
> > > memory zone. So stop using it.
> > >
> > > Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to
> > > map to gfp mask")
> >
> > Hi Nicolas,
> >
> > I see a boot regression with this commit d9765e41d8e9 "dma-pool:
> > Do not allocate pool memory from CMA" on my Xiaomi Poco F1
> > (Qcom sdm845) phone running v5.8-rc6. I can't boot past the
> > bootloader splash screen with this patch.
> >
> > Phone boots fine if I revert this patch. I carry only one out of tree
> > dts patch https://lkml.org/lkml/2020/6/25/52. And since this is a
> > stock
> > phone, I don't have access to serial/dmesg logs until I boot to AOSP
> > (adb) shell.
> >
> > Any thoughts as to what might be going wrong here? I'd be happy to
> > help debug things. For what it's worth, I don't see this regression
> > on
> > other two sdm845 devices (db845c and Pixel 3) I tested on.
>
> Can you provide a boot log (even if without my patch) and the device-
> tree files? It'd help a lot figuring things out.
Thank you for the prompt reply Nicolas.
Here is the boot log with the reverted patch
https://pastebin.ubuntu.com/p/BrhPf83nKF/
Here is my phone's dts
https://github.com/pundiramit/linux/commit/2a394c199deeaf4c91e0e008e8fba2a72f494d8c
And here is my kernel tree just in case
https://github.com/pundiramit/linux/commits/beryllium-mainline
Regards,
Amit Pundir
>
> Regards,
> Nicolas
>
> > Regards,
> > Amit Pundir
> >
> > > Reported-by: Jeremy Linton
> > > Signed-off-by: Nicolas Saenz Julienne
> > > ---
> > >
> > > An more costly alternative would be adding an option to
> > > dma_alloc_from_contiguous() so it fails when the allocation doesn't
> > > fall
> > > in a specific zone.
> > >
> > > kernel/dma/pool.c | 11 ++-
> > > 1 file changed, 2 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> > > index 8cfa01243ed2..4bc1c46ae6ef 100644
> > > --- a/kernel/dma/pool.c
> > > +++ b/kernel/dma/pool.c
> > > @@ -6,7 +6,6 @@
> > > #include
> > > #include
> > > #include
> > > -#include
> > > #include
> > > #include
> > > #include
> > > @@ -69,12 +68,7 @@ static int atomic_pool_expand(struct gen_pool
> > > *pool, size_t pool_size,
> > >
> > > do {
> > > pool_size = 1 << (PAGE_SHIFT + order);
> > > -
> > > - if (dev_get_cma_area(NULL))
> > > - page = dma_alloc_from_contiguous(NULL, 1 <<
> > > order,
> > > -order,
> > > false);
> > > - else
> > > - page = alloc_pages(gfp, order);
> > > + page = alloc_pages(gfp, order);
> > > } while (!page && order-- > 0);
> > > if (!page)
> > > goto out;
> > > @@ -118,8 +112,7 @@ static int atomic_pool_expand(struct gen_pool
> > > *pool, size_t pool_size,
> > > dma_common_free_remap(addr, pool_size);
> > > #endif
> > > free_page: __maybe_unused
> > > - if (!dma_release_from_contiguous(NULL, page, 1 << order))
> > > - __free_pages(page, order);
> > > + __free_pages(page, order);
> > > out:
> > > return ret;
> > > }
> > > --
> > > 2.27.0
> > >
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
Hi Amit,
On Tue, 2020-07-21 at 12:51 +0530, Amit Pundir wrote:
> On Wed, 8 Jul 2020 at 22:43, Nicolas Saenz Julienne
> wrote:
> > There is no guarantee to CMA's placement, so allocating a zone
> > specific
> > atomic pool from CMA might return memory from a completely
> > different
> > memory zone. So stop using it.
> >
> > Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to
> > map to gfp mask")
>
> Hi Nicolas,
>
> I see a boot regression with this commit d9765e41d8e9 "dma-pool:
> Do not allocate pool memory from CMA" on my Xiaomi Poco F1
> (Qcom sdm845) phone running v5.8-rc6. I can't boot past the
> bootloader splash screen with this patch.
>
> Phone boots fine if I revert this patch. I carry only one out of tree
> dts patch https://lkml.org/lkml/2020/6/25/52. And since this is a
> stock
> phone, I don't have access to serial/dmesg logs until I boot to AOSP
> (adb) shell.
>
> Any thoughts as to what might be going wrong here? I'd be happy to
> help debug things. For what it's worth, I don't see this regression
> on
> other two sdm845 devices (db845c and Pixel 3) I tested on.
Can you provide a boot log (even if without my patch) and the device-
tree files? It'd help a lot figuring things out.
Regards,
Nicolas
> Regards,
> Amit Pundir
>
> > Reported-by: Jeremy Linton
> > Signed-off-by: Nicolas Saenz Julienne
> > ---
> >
> > An more costly alternative would be adding an option to
> > dma_alloc_from_contiguous() so it fails when the allocation doesn't
> > fall
> > in a specific zone.
> >
> > kernel/dma/pool.c | 11 ++-
> > 1 file changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> > index 8cfa01243ed2..4bc1c46ae6ef 100644
> > --- a/kernel/dma/pool.c
> > +++ b/kernel/dma/pool.c
> > @@ -6,7 +6,6 @@
> > #include
> > #include
> > #include
> > -#include
> > #include
> > #include
> > #include
> > @@ -69,12 +68,7 @@ static int atomic_pool_expand(struct gen_pool
> > *pool, size_t pool_size,
> >
> > do {
> > pool_size = 1 << (PAGE_SHIFT + order);
> > -
> > - if (dev_get_cma_area(NULL))
> > - page = dma_alloc_from_contiguous(NULL, 1 <<
> > order,
> > -order,
> > false);
> > - else
> > - page = alloc_pages(gfp, order);
> > + page = alloc_pages(gfp, order);
> > } while (!page && order-- > 0);
> > if (!page)
> > goto out;
> > @@ -118,8 +112,7 @@ static int atomic_pool_expand(struct gen_pool
> > *pool, size_t pool_size,
> > dma_common_free_remap(addr, pool_size);
> > #endif
> > free_page: __maybe_unused
> > - if (!dma_release_from_contiguous(NULL, page, 1 << order))
> > - __free_pages(page, order);
> > + __free_pages(page, order);
> > out:
> > return ret;
> > }
> > --
> > 2.27.0
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-pool: Do not allocate pool memory from CMA
On Wed, 8 Jul 2020 at 22:43, Nicolas Saenz Julienne
wrote:
>
> There is no guarantee to CMA's placement, so allocating a zone specific
> atomic pool from CMA might return memory from a completely different
> memory zone. So stop using it.
>
> Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to map to gfp
> mask")
Hi Nicolas,
I see a boot regression with this commit d9765e41d8e9 "dma-pool:
Do not allocate pool memory from CMA" on my Xiaomi Poco F1
(Qcom sdm845) phone running v5.8-rc6. I can't boot past the
bootloader splash screen with this patch.
Phone boots fine if I revert this patch. I carry only one out of tree
dts patch https://lkml.org/lkml/2020/6/25/52. And since this is a stock
phone, I don't have access to serial/dmesg logs until I boot to AOSP
(adb) shell.
Any thoughts as to what might be going wrong here? I'd be happy to
help debug things. For what it's worth, I don't see this regression on
other two sdm845 devices (db845c and Pixel 3) I tested on.
Regards,
Amit Pundir
> Reported-by: Jeremy Linton
> Signed-off-by: Nicolas Saenz Julienne
> ---
>
> An more costly alternative would be adding an option to
> dma_alloc_from_contiguous() so it fails when the allocation doesn't fall
> in a specific zone.
>
> kernel/dma/pool.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 8cfa01243ed2..4bc1c46ae6ef 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -6,7 +6,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -69,12 +68,7 @@ static int atomic_pool_expand(struct gen_pool *pool,
> size_t pool_size,
>
> do {
> pool_size = 1 << (PAGE_SHIFT + order);
> -
> - if (dev_get_cma_area(NULL))
> - page = dma_alloc_from_contiguous(NULL, 1 << order,
> -order, false);
> - else
> - page = alloc_pages(gfp, order);
> + page = alloc_pages(gfp, order);
> } while (!page && order-- > 0);
> if (!page)
> goto out;
> @@ -118,8 +112,7 @@ static int atomic_pool_expand(struct gen_pool *pool,
> size_t pool_size,
> dma_common_free_remap(addr, pool_size);
> #endif
> free_page: __maybe_unused
> - if (!dma_release_from_contiguous(NULL, page, 1 << order))
> - __free_pages(page, order);
> + __free_pages(page, order);
> out:
> return ret;
> }
> --
> 2.27.0
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[GIT PULL] iommu/arm-smmu: Updates for 5.9
Hi Joerg, Please pull these Arm SMMU driver updates for 5.9. Summary is in the tag, but the main thing is support for two new SoC integrations, one of which is considerably more brain-dead than the other (determining which one is left as an exercise to the reader although the diffstat is fairly revealing). Cheers, Will --->8 The following changes since commit 9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68: Linux 5.8-rc3 (2020-06-28 15:00:24 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git tags/arm-smmu-updates for you to fetch changes up to aa7ec73297df57a86308fee78d2bf86e22ea0bae: iommu/arm-smmu: Add global/context fault implementation hooks (2020-07-20 09:30:51 +0100) Arm SMMU updates for 5.9 - Support for SMMU-500 implementation in Marvell Armada-AP806 SoC - Support for SMMU-500 implementation in NVIDIA Tegra194 SoC - DT compatible string updates - Remove unused IOMMU_SYS_CACHE_ONLY flag Hanna Hawa (1): iommu/arm-smmu: Workaround for Marvell Armada-AP806 SoC erratum #582743 John Garry (1): iommu/arm-smmu-v3: Fix trivial typo Jonathan Marek (2): dt-bindings: arm-smmu: Add sm8150 and sm8250 compatible strings iommu: arm-smmu-impl: Use qcom impl for sm8150 and sm8250 compatibles Krishna Reddy (5): iommu/arm-smmu: move TLB timeout and spin count macros iommu/arm-smmu: ioremap smmu mmio region before implementation init iommu/arm-smmu: add NVIDIA implementation for ARM MMU-500 usage dt-bindings: arm-smmu: add binding for Tegra194 SMMU iommu/arm-smmu: Add global/context fault implementation hooks Robin Murphy (1): iommu/arm-smmu: Update impl quirks comment Tomasz Nowicki (2): iommu/arm-smmu: Call configuration impl hook before consuming features dt-bindings: arm-smmu: add compatible string for Marvell Armada-AP806 SMMU-500 Will Deacon (1): iommu: Remove unused IOMMU_SYS_CACHE_ONLY flag Documentation/arm64/silicon-errata.rst | 3 + .../devicetree/bindings/iommu/arm,smmu.yaml| 31 ++- MAINTAINERS| 2 + drivers/iommu/Makefile | 2 +- drivers/iommu/arm-smmu-impl.c | 60 - drivers/iommu/arm-smmu-nvidia.c| 278 + drivers/iommu/arm-smmu-v3.c| 2 +- drivers/iommu/arm-smmu.c | 40 ++- drivers/iommu/arm-smmu.h | 6 + drivers/iommu/io-pgtable-arm.c | 3 - include/linux/iommu.h | 6 - 11 files changed, 403 insertions(+), 30 deletions(-) create mode 100644 drivers/iommu/arm-smmu-nvidia.c ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
