[PATCHv2] iommu/arm-smmu-qcom: Add debug support for TLB sync timeouts
TLB sync timeouts can be due to various reasons such as TBU power down
or pending TCU/TBU invalidation/sync and so on. Debugging these often
require dumping of some implementation defined registers to know the
status of TBU/TCU operations and some of these registers are not
accessible in non-secure world such as from kernel and requires SMC
calls to read them in the secure world. So, add this debug support
to dump implementation defined registers for TLB sync timeout issues.
Signed-off-by: Sai Prakash Ranjan
---
Changes in v2:
* Use scm call consistently so that it works on older chipsets where
some of these regs are secure registers.
* Add device specific data to get the implementation defined register
offsets.
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 161 ++---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
3 files changed, 146 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 7820711c4560..bb68aa85b28b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -5,13 +5,27 @@
#include
#include
+#include
#include
#include
#include "arm-smmu.h"
+#define QCOM_DUMMY_VAL -1
+
+enum qcom_smmu_impl_reg_offset {
+ QCOM_SMMU_TBU_PWR_STATUS,
+ QCOM_SMMU_STATS_SYNC_INV_TBU_ACK,
+ QCOM_SMMU_MMU2QSS_AND_SAFE_WAIT_CNTR,
+};
+
+struct qcom_smmu_config {
+ const u32 *reg_offset;
+};
+
struct qcom_smmu {
struct arm_smmu_device smmu;
+ const struct qcom_smmu_config *cfg;
bool bypass_quirk;
u8 bypass_cbndx;
u32 stall_enabled;
@@ -22,6 +36,56 @@ static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device
*smmu)
return container_of(smmu, struct qcom_smmu, smmu);
}
+static void qcom_smmu_tlb_sync(struct arm_smmu_device *smmu, int page,
+ int sync, int status)
+{
+ int ret;
+ unsigned int spin_cnt, delay;
+ u32 reg, tbu_pwr_status, sync_inv_ack, sync_inv_progress;
+ struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+ const struct qcom_smmu_config *cfg;
+
+ arm_smmu_writel(smmu, page, sync, QCOM_DUMMY_VAL);
+ for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) {
+ for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) {
+ reg = arm_smmu_readl(smmu, page, status);
+ if (!(reg & ARM_SMMU_sTLBGSTATUS_GSACTIVE))
+ return;
+ cpu_relax();
+ }
+ udelay(delay);
+ }
+
+ dev_err_ratelimited(smmu->dev,
+ "TLB sync timed out -- SMMU may be deadlocked\n");
+
+ cfg = qsmmu->cfg;
+ if (!cfg)
+ return;
+
+ ret = qcom_scm_io_readl(smmu->ioaddr +
cfg->reg_offset[QCOM_SMMU_TBU_PWR_STATUS],
+ &tbu_pwr_status);
+ if (ret)
+ dev_err_ratelimited(smmu->dev,
+ "Failed to read TBU power status: %d\n",
ret);
+
+ ret = qcom_scm_io_readl(smmu->ioaddr +
cfg->reg_offset[QCOM_SMMU_STATS_SYNC_INV_TBU_ACK],
+ &sync_inv_ack);
+ if (ret)
+ dev_err_ratelimited(smmu->dev,
+ "Failed to read TBU sync/inv ack status:
%d\n", ret);
+
+ ret = qcom_scm_io_readl(smmu->ioaddr +
cfg->reg_offset[QCOM_SMMU_MMU2QSS_AND_SAFE_WAIT_CNTR],
+ &sync_inv_progress);
+ if (ret)
+ dev_err_ratelimited(smmu->dev,
+ "Failed to read TCU syn/inv progress:
%d\n", ret);
+
+ dev_err_ratelimited(smmu->dev,
+ "TBU: power_status %#x sync_inv_ack %#x
sync_inv_progress %#x\n",
+ tbu_pwr_status, sync_inv_ack, sync_inv_progress);
+}
+
static void qcom_adreno_smmu_write_sctlr(struct arm_smmu_device *smmu, int idx,
u32 reg)
{
@@ -374,6 +438,7 @@ static const struct arm_smmu_impl qcom_smmu_impl = {
.def_domain_type = qcom_smmu_def_domain_type,
.reset = qcom_smmu500_reset,
.write_s2cr = qcom_smmu_write_s2cr,
+ .tlb_sync = qcom_smmu_tlb_sync,
};
static const struct arm_smmu_impl qcom_adreno_smmu_impl = {
@@ -382,12 +447,84 @@ static const struct arm_smmu_impl qcom_adreno_smmu_impl =
{
.reset = qcom_smmu500_reset,
.alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
.write_sctlr = qcom_adreno_smmu_write_sctlr,
+ .tlb_sync = qcom_smmu_tlb_sync,
+};
+
+/* Implementation Defined Register Space 0 register offsets */
+static const u32 qcom_smmu_impl0_reg_offset[] = {
+ [QCOM_SMMU_TBU_PWR_STATUS] = 0x2204,
+ [QCOM_SMMU_STATS_SYNC_INV_TBU_ACK] = 0x25dc,
+ [QCOM_SMMU_MMU2QSS_A
Re: [PATCH v2] iommu/amd: Set translation valid bit only when IO page tables are in used
Joerg, On 5/20/22 3:09 PM, Joerg Roedel wrote: Hi Suravee, On Mon, May 16, 2022 at 07:27:51PM +0700, Suravee Suthikulpanit wrote: - Also, it seems that the current iommu v2 page table use case, where GVA->GPA=SPA will no longer be supported on system w/ SNPSup=1. Any thoughts? Support for that is not upstream yet, it should be easy to disallow this configuration and just use the v1 page-tables when SNP is active. This can be handled entirely inside the AMD IOMMU driver. Actually, I am referring to when user uses the IOMMU v2 table for shared virtual address in current iommu_v2 driver (e.g. amd_iommu_init_device(), amd_iommu_bind_pasid). Best Regards, Suravee ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [GIT PULL] dma-mapping updates for Linux 5.19
The pull request you sent on Wed, 25 May 2022 19:57:12 +0200: > git://git.infradead.org/users/hch/dma-mapping.git > tags/dma-mapping-5.19-2022-05-25 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/3f306ea2e18568f693f7763d1c2178f349ae8f31 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver
On Thu, May 26, 2022 at 12:43:25AM +, Tian, Kevin wrote: > iiuc then this suggests there should be only one iommu group per atu, > instead of using generic_device_group() to create one group per > device. Sounds like it > > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > > return that from ops->def_domain_type(). > > BLOCKING should not be used as a default domain type for DMA API > which needs either a DMA or IDENTITY type. New drivers should not have a NULL group->default_domain. IMHO this driver does not support the DMA API so the default_domain should be assigned to blocking and the DMA API disabled. We might need some core changes to accommodate this. The alternative would be to implement the identity domain, assuming the ATU thing can store that kind of translation. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
On 2022/5/25 23:25, Jason Gunthorpe wrote: On Wed, May 25, 2022 at 01:19:08PM +0800, Baolu Lu wrote: Hi Jason, On 2022/5/24 21:44, Jason Gunthorpe wrote: diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c index 106506143896..210c376f6043 100644 +++ b/drivers/iommu/iommu-sva-lib.c @@ -69,3 +69,51 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid) return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero); } EXPORT_SYMBOL_GPL(iommu_sva_find); + +/* + * IOMMU SVA driver-oriented interfaces + */ +struct iommu_domain * +iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm) This should return the proper type Can you please elaborate a bit on "return the proper type"? Did you mean return iommu_sva_domain instead? That's a wrapper of iommu_domain and only for iommu internal usage. If you are exposing special SVA APIs then it is not internal usage only anymore, so expose the type. Ah, got you. Thanks for the explanation. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver
> From: Jason Gunthorpe
> Sent: Thursday, May 26, 2022 2:27 AM
>
> On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote:
> > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote:
> > > +static const struct iommu_ops visconti_atu_ops = {
> > > + .domain_alloc = visconti_atu_domain_alloc,
> > > + .probe_device = visconti_atu_probe_device,
> > > + .release_device = visconti_atu_release_device,
> > > + .device_group = generic_device_group,
> > > + .of_xlate = visconti_atu_of_xlate,
> > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
> > > + .default_domain_ops = &(const struct iommu_domain_ops) {
> > > + .attach_dev = visconti_atu_attach_device,
> > > + .detach_dev = visconti_atu_detach_device,
> >
> > The detach_dev callback is about to be deprecated. The new drivers
> > should implement the default domain and blocking domain instead.
>
> Yes please, new drivers need to use default_domains.
>
> It is very strange that visconti_atu_detach_device() does nothing. It
> is not required that a domain is fully unmapped before being
> destructed, I think detach should set ATU_AT_EN to 0.
Looks the atu is shared by all devices behind and can only serve one
I/O address space. The atu registers only keep information about
iova ranges without any device notation. That is probably the reason
why both attach/detach() don't touch hardware.
iiuc then this suggests there should be only one iommu group per atu,
instead of using generic_device_group() to create one group per
device.
>
> What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is
I guess it's a blocking behavior since that register tracks which iova range
register is valid.
> rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and
> return that from ops->def_domain_type().
BLOCKING should not be used as a default domain type for DMA API
which needs either a DMA or IDENTITY type.
>
> Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0
Agree
>
> Also, if I surmise how this works properly, it is not following the
> iommu API to halt all DMA during map/unmap operations. Should at least
> document this and explain why it is OK..
>
> I'm feeling like these "special" drivers need some kind of handshake
> with their only users because they don't work with things like VFIO..
>
> Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1] driver core: Extend deferred probe timeout on driver registration
On Wed, May 25, 2022 at 12:12 AM Sebastian Andrzej Siewior wrote: > > On 2022-05-24 10:46:49 [-0700], Saravana Kannan wrote: > > > Removing probe_timeout_waitqueue (as suggested) or setting the timeout > > > to 0 avoids the delay. > > > > In your case, I think it might be working as intended? Curious, what > > was the call stack in your case where it was blocked? > > Why is then there 10sec delay during boot? The backtrace is > |[ cut here ] > |WARNING: CPU: 4 PID: 1 at drivers/base/dd.c:742 > wait_for_device_probe+0x30/0x110 > |Modules linked in: > |CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc5+ #154 > |RIP: 0010:wait_for_device_probe+0x30/0x110 > |Call Trace: > | > | prepare_namespace+0x2b/0x160 > | kernel_init_freeable+0x2b3/0x2dd > | kernel_init+0x11/0x110 > | ret_from_fork+0x22/0x30 > | > > Looking closer, it can't access init. This in particular box boots > directly the kernel without an initramfs so the kernel later mounts > /dev/sda1 and everything is good. So that seems to be the reason… Hmmm... that part shouldn't matter. As long as you are hitting the same code path. My guess is one of them has CONFIG_MODULES enabled and the other doesn't. In either case, I think the patch needs to be reverted (I'll send out one soon), but that'll also mean I need to revert part of my patch (sets the timeout back to 0) or I need to fix this case: https://lore.kernel.org/lkml/tyapr01mb45443df63b9ef29054f7c41fd8...@tyapr01mb4544.jpnprd01.prod.outlook.com/ I'll try to do the latter if I can get something reasonable soon. Otherwise, I'll just do the revert + partial revert. -Saravana > My other machine with an initramfs does not show this problem. > > > -Saravana > > Sebastian ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[GIT PULL] dma-mapping updates for Linux 5.19
The following changes since commit b2d229d4ddb17db541098b83524d901257e93845: Linux 5.18-rc3 (2022-04-17 13:57:31 -0700) are available in the Git repository at: git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-5.19-2022-05-25 for you to fetch changes up to 4a37f3dd9a83186cb88d44808ab35b78375082c9: dma-direct: don't over-decrypt memory (2022-05-23 15:25:40 +0200) There is a small merge conflict with the (as of now not merged yet) iommu tree, which removes some code modified in this pull request. The proper merge resolution is to still remove the modified code. dma-mapping updates for Linux 5.19 - don't over-decrypt memory (Robin Murphy) - takes min align mask into account for the swiotlb max mapping size (Tianyu Lan) - use GFP_ATOMIC in dma-debug (Mikulas Patocka) - fix DMA_ATTR_NO_KERNEL_MAPPING on xen/arm (me) - don't fail on highmem CMA pages in dma_direct_alloc_pages (me) - cleanup swiotlb initialization and share more code with swiotlb-xen (me, Stefano Stabellini) Christoph Hellwig (19): dma-direct: use is_swiotlb_active in dma_direct_map_page swiotlb: make swiotlb_exit a no-op if SWIOTLB_FORCE is set swiotlb: simplify swiotlb_max_segment swiotlb: rename swiotlb_late_init_with_default_size MIPS/octeon: use swiotlb_init instead of open coding it x86: remove the IOMMU table infrastructure x86: centralize setting SWIOTLB_FORCE when guest memory encryption is enabled swiotlb: make the swiotlb_init interface more useful swiotlb: add a SWIOTLB_ANY flag to lift the low memory restriction swiotlb: pass a gfp_mask argument to swiotlb_init_late swiotlb: provide swiotlb_init variants that remap the buffer swiotlb: merge swiotlb-xen initialization into swiotlb swiotlb: remove swiotlb_init_with_tbl and swiotlb_init_late_with_tbl x86: remove cruft from swiotlb-xen: fix DMA_ATTR_NO_KERNEL_MAPPING on arm dma-direct: don't fail on highmem CMA pages in dma_direct_alloc_pages swiotlb: don't panic when the swiotlb buffer can't be allocated swiotlb: use the right nslabs value in swiotlb_init_remap swiotlb: use the right nslabs-derived sizes in swiotlb_init_late Mikulas Patocka (1): dma-debug: change allocation mode from GFP_NOWAIT to GFP_ATIOMIC Robin Murphy (1): dma-direct: don't over-decrypt memory Stefano Stabellini (1): arm/xen: don't check for xen_initial_domain() in xen_create_contiguous_region Tianyu Lan (1): swiotlb: max mapping size takes min align mask into account arch/arm/include/asm/xen/page-coherent.h | 2 - arch/arm/mm/init.c | 6 +- arch/arm/xen/mm.c | 38 ++--- arch/arm64/include/asm/xen/page-coherent.h | 2 - arch/arm64/mm/init.c | 6 +- arch/ia64/include/asm/iommu_table.h| 7 - arch/ia64/mm/init.c| 4 +- arch/mips/cavium-octeon/dma-octeon.c | 15 +- arch/mips/loongson64/dma.c | 2 +- arch/mips/pci/pci-octeon.c | 2 +- arch/mips/sibyte/common/dma.c | 2 +- arch/powerpc/include/asm/svm.h | 4 - arch/powerpc/include/asm/swiotlb.h | 1 + arch/powerpc/kernel/dma-swiotlb.c | 1 + arch/powerpc/mm/mem.c | 6 +- arch/powerpc/platforms/pseries/setup.c | 3 - arch/powerpc/platforms/pseries/svm.c | 26 +--- arch/riscv/mm/init.c | 8 +- arch/s390/mm/init.c| 3 +- arch/x86/include/asm/dma-mapping.h | 12 -- arch/x86/include/asm/gart.h| 5 +- arch/x86/include/asm/iommu.h | 8 + arch/x86/include/asm/iommu_table.h | 102 - arch/x86/include/asm/swiotlb.h | 30 arch/x86/include/asm/xen/page-coherent.h | 24 --- arch/x86/include/asm/xen/page.h| 5 - arch/x86/include/asm/xen/swiotlb-xen.h | 8 +- arch/x86/kernel/Makefile | 2 - arch/x86/kernel/amd_gart_64.c | 5 +- arch/x86/kernel/aperture_64.c | 14 +- arch/x86/kernel/cpu/mshyperv.c | 8 - arch/x86/kernel/pci-dma.c | 114 +++--- arch/x86/kernel/pci-iommu_table.c | 77 -- arch/x86/kernel/pci-swiotlb.c | 77 -- arch/x86/kernel/tboot.c| 1 - arch/x86/kernel/vmlinux.lds.S | 12 -- arch/x86/mm/mem_encrypt_amd.c | 3 - arch/x86/pci/sta2x11-fixup.c | 2 +- arch/x86/xen/Makefile | 2 - arch/x86/xen/mmu_pv.c | 1 + arch/x86/xen/pci-swiotlb-xen.c | 96 drivers/iommu/amd/init.c | 6 - drivers/
Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver
On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote:
> On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote:
> > +static const struct iommu_ops visconti_atu_ops = {
> > + .domain_alloc = visconti_atu_domain_alloc,
> > + .probe_device = visconti_atu_probe_device,
> > + .release_device = visconti_atu_release_device,
> > + .device_group = generic_device_group,
> > + .of_xlate = visconti_atu_of_xlate,
> > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
> > + .default_domain_ops = &(const struct iommu_domain_ops) {
> > + .attach_dev = visconti_atu_attach_device,
> > + .detach_dev = visconti_atu_detach_device,
>
> The detach_dev callback is about to be deprecated. The new drivers
> should implement the default domain and blocking domain instead.
Yes please, new drivers need to use default_domains.
It is very strange that visconti_atu_detach_device() does nothing. It
is not required that a domain is fully unmapped before being
destructed, I think detach should set ATU_AT_EN to 0.
What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is
rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and
return that from ops->def_domain_type().
Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0
Also, if I surmise how this works properly, it is not following the
iommu API to halt all DMA during map/unmap operations. Should at least
document this and explain why it is OK..
I'm feeling like these "special" drivers need some kind of handshake
with their only users because they don't work with things like VFIO..
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Use try_cmpxchg64 in alloc_pte and free_clear_pte
Use try_cmpxchg64 instead of cmpxchg64 (*ptr, old, new) != old in
alloc_pte and free_clear_pte. cmpxchg returns success in ZF flag, so this
change saves a compare after cmpxchg (and related move instruction
in front of cmpxchg). Also, remove racy explicit assignment to pteval
when cmpxchg fails, this is what try_cmpxchg does implicitly from
*pte in an atomic way.
Signed-off-by: Uros Bizjak
Cc: Joerg Roedel
Cc: Suravee Suthikulpanit
Cc: Will Deacon
---
drivers/iommu/amd/io_pgtable.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index 6608d1717574..7d4b61e5db47 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -258,7 +258,7 @@ static u64 *alloc_pte(struct protection_domain *domain,
__npte = PM_LEVEL_PDE(level, iommu_virt_to_phys(page));
/* pte could have been changed somewhere. */
- if (cmpxchg64(pte, __pte, __npte) != __pte)
+ if (!try_cmpxchg64(pte, &__pte, __npte))
free_page((unsigned long)page);
else if (IOMMU_PTE_PRESENT(__pte))
*updated = true;
@@ -341,10 +341,8 @@ static void free_clear_pte(u64 *pte, u64 pteval, struct
list_head *freelist)
u64 *pt;
int mode;
- while (cmpxchg64(pte, pteval, 0) != pteval) {
+ while (!try_cmpxchg64(pte, &pteval, 0))
pr_warn("AMD-Vi: IOMMU pte changed since we read it\n");
- pteval = *pte;
- }
if (!IOMMU_PTE_PRESENT(pteval))
return;
--
2.35.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
On Wed, May 25, 2022 at 01:19:08PM +0800, Baolu Lu wrote: > Hi Jason, > > On 2022/5/24 21:44, Jason Gunthorpe wrote: > > > diff --git a/drivers/iommu/iommu-sva-lib.c b/drivers/iommu/iommu-sva-lib.c > > > index 106506143896..210c376f6043 100644 > > > +++ b/drivers/iommu/iommu-sva-lib.c > > > @@ -69,3 +69,51 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid) > > > return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero); > > > } > > > EXPORT_SYMBOL_GPL(iommu_sva_find); > > > + > > > +/* > > > + * IOMMU SVA driver-oriented interfaces > > > + */ > > > +struct iommu_domain * > > > +iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm) > > This should return the proper type > > > > Can you please elaborate a bit on "return the proper type"? Did you mean > return iommu_sva_domain instead? That's a wrapper of iommu_domain and > only for iommu internal usage. If you are exposing special SVA APIs then it is not internal usage only anymore, so expose the type. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
On 2022/5/25 19:06, Jean-Philippe Brucker wrote:
On Wed, May 25, 2022 at 11:07:49AM +0100, Robin Murphy wrote:
Did you mean @handler and @handler_token staffs below?
struct iommu_domain {
unsigned type;
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
void *handler_token;
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
};
Is it only for DMA domains? From the point view of IOMMU faults, it
seems to be generic.
Yes, it's the old common iommu_set_fault_handler() stuff (which arguably is
more of a "notifier" than a "handler"), but I assume that that's irrelevant
if SVA is using IOPF instead?
Yes IOMMU drivers call either the newer iommu_report_device_fault() or the
old report_iommu_fault(), and only the former can support IOPF/SVA. I've
tried to merge them before but never completed it. I think the main issue
was with finding the endpoint that caused the fault from the fault
handler. Some IOMMU drivers just pass the IOMMU device to
report_iommu_fault(). I'll probably pick that up at some point.
Thank you all for the comments and suggestions. Below is the refreshed
patch. Hope that I didn't miss anything.
From 463c04cada8e8640598f981d8d16157781b9de6f Mon Sep 17 00:00:00 2001
From: Lu Baolu
Date: Wed, 11 May 2022 20:59:24 +0800
Subject: [PATCH 04/11] iommu: Add sva iommu_domain support
The sva iommu_domain represents a hardware pagetable that the IOMMU
hardware could use for SVA translation. This adds some infrastructure
to support SVA domain in the iommu common layer. It includes:
- Extend the iommu_domain to support a new IOMMU_DOMAIN_SVA domain
type. The IOMMU drivers that support SVA should provide the sva
domain specific iommu_domain_ops.
- Add a helper to allocate an SVA domain. The iommu_domain_free()
is still used to free an SVA domain.
- Add helpers to attach an SVA domain to a device and the reverse
operation.
Some buses, like PCI, route packets without considering the PASID value.
Thus a DMA target address with PASID might be treated as P2P if the
address falls into the MMIO BAR of other devices in the group. To make
things simple, the attach/detach interfaces only apply to devices
belonging to the singleton groups, and the singleton is immutable in
fabric i.e. not affected by hotplug.
The iommu_attach/detach_device_pasid() can be used for other purposes,
such as kernel DMA with pasid, mediation device, etc.
Suggested-by: Jean-Philippe Brucker
Suggested-by: Jason Gunthorpe
Signed-off-by: Lu Baolu
Reviewed-by: Jean-Philippe Brucker
---
drivers/iommu/iommu.c | 93 +++
include/linux/iommu.h | 45 -
2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 63b64b4e8a38..b1a2ad64a413 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
@@ -39,6 +40,7 @@ struct iommu_group {
struct kobject kobj;
struct kobject *devices_kobj;
struct list_head devices;
+ struct xarray pasid_array;
struct mutex mutex;
void *iommu_data;
void (*iommu_data_release)(void *iommu_data);
@@ -666,6 +668,7 @@ struct iommu_group *iommu_group_alloc(void)
mutex_init(&group->mutex);
INIT_LIST_HEAD(&group->devices);
INIT_LIST_HEAD(&group->entry);
+ xa_init(&group->pasid_array);
ret = ida_simple_get(&iommu_group_ida, 0, 0, GFP_KERNEL);
if (ret < 0) {
@@ -1961,6 +1964,8 @@ EXPORT_SYMBOL_GPL(iommu_domain_alloc);
void iommu_domain_free(struct iommu_domain *domain)
{
+ if (domain->type == IOMMU_DOMAIN_SVA)
+ mmdrop(domain->mm);
iommu_put_dma_cookie(domain);
domain->ops->free(domain);
}
@@ -3277,3 +3282,91 @@ bool iommu_group_dma_owner_claimed(struct
iommu_group *group)
return user;
}
EXPORT_SYMBOL_GPL(iommu_group_dma_owner_claimed);
+
+struct iommu_domain *iommu_sva_domain_alloc(struct device *dev,
+ struct mm_struct *mm)
+{
+ const struct iommu_ops *ops = dev_iommu_ops(dev);
+ struct iommu_domain *domain;
+
+ domain = ops->domain_alloc(IOMMU_DOMAIN_SVA);
+ if (!domain)
+ return NULL;
+
+ domain->type = IOMMU_DOMAIN_SVA;
+ mmgrab(mm);
+ domain->mm = mm;
+
+ return domain;
+}
+
+static bool iommu_group_immutable_singleton(struct iommu_group *group,
+ struct device *dev)
+{
+ int count;
+
+ mutex_lock(&group->mutex);
+ count = iommu_group_device_count(group);
+ mutex_unlock(&group->mutex);
+
+ if (count != 1)
+ return false;
+
+ /*
+
Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
On Wed, May 25, 2022 at 11:07:49AM +0100, Robin Murphy wrote:
> > Did you mean @handler and @handler_token staffs below?
> >
> > struct iommu_domain {
> > unsigned type;
> > const struct iommu_domain_ops *ops;
> > unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
> > iommu_fault_handler_t handler;
> > void *handler_token;
> > struct iommu_domain_geometry geometry;
> > struct iommu_dma_cookie *iova_cookie;
> > };
> >
> > Is it only for DMA domains? From the point view of IOMMU faults, it
> > seems to be generic.
>
> Yes, it's the old common iommu_set_fault_handler() stuff (which arguably is
> more of a "notifier" than a "handler"), but I assume that that's irrelevant
> if SVA is using IOPF instead?
Yes IOMMU drivers call either the newer iommu_report_device_fault() or the
old report_iommu_fault(), and only the former can support IOPF/SVA. I've
tried to merge them before but never completed it. I think the main issue
was with finding the endpoint that caused the fault from the fault
handler. Some IOMMU drivers just pass the IOMMU device to
report_iommu_fault(). I'll probably pick that up at some point.
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 03/10] iommu/sva: Add iommu_sva_domain support
On 2022-05-25 07:20, Baolu Lu wrote:
Hi Robin,
On 2022/5/24 22:36, Robin Murphy wrote:
On 2022-05-19 08:20, Lu Baolu wrote:
[...]
diff --git a/drivers/iommu/iommu-sva-lib.c
b/drivers/iommu/iommu-sva-lib.c
index 106506143896..210c376f6043 100644
--- a/drivers/iommu/iommu-sva-lib.c
+++ b/drivers/iommu/iommu-sva-lib.c
@@ -69,3 +69,51 @@ struct mm_struct *iommu_sva_find(ioasid_t pasid)
return ioasid_find(&iommu_sva_pasid, pasid, __mmget_not_zero);
}
EXPORT_SYMBOL_GPL(iommu_sva_find);
+
+/*
+ * IOMMU SVA driver-oriented interfaces
+ */
+struct iommu_domain *
+iommu_sva_alloc_domain(struct bus_type *bus, struct mm_struct *mm)
Argh, please no new bus-based external interfaces! Domain allocation
needs to resolve to the right IOMMU instance to solve a number of
issues, and cleaning up existing users of iommu_domain_alloc() to
prepare for that is already hard enough. This is arguably even more
relevant here than for other domain types, since SVA support is more
likely to depend on specific features that can vary between IOMMU
instances even with the same driver. Please make the external
interface take a struct device, then resolve the ops through dev->iommu.
Further nit: the naming inconsistency bugs me a bit -
iommu_sva_domain_alloc() seems more natural. Also I'd question the
symmetry vs. usability dichotomy of whether we *really* want two
different free functions for a struct iommu_domain pointer, where any
caller which might mix SVA and non-SVA usage then has to remember how
they allocated any particular domain :/
+{
+ struct iommu_sva_domain *sva_domain;
+ struct iommu_domain *domain;
+
+ if (!bus->iommu_ops || !bus->iommu_ops->sva_domain_ops)
+ return ERR_PTR(-ENODEV);
+
+ sva_domain = kzalloc(sizeof(*sva_domain), GFP_KERNEL);
+ if (!sva_domain)
+ return ERR_PTR(-ENOMEM);
+
+ mmgrab(mm);
+ sva_domain->mm = mm;
+
+ domain = &sva_domain->domain;
+ domain->type = IOMMU_DOMAIN_SVA;
+ domain->ops = bus->iommu_ops->sva_domain_ops;
I'd have thought it would be logical to pass IOMMU_DOMAIN_SVA to the
normal domain_alloc call, so that driver-internal stuff like context
descriptors can be still be hung off the domain as usual (rather than
all drivers having to implement some extra internal lookup mechanism
to handle all the SVA domain ops), but that's something we're free to
come
Agreed with above comments. Thanks! I will post an additional patch
for review later.
back and change later. FWIW I'd just stick the mm pointer in struct
iommu_domain, in a union with the fault handler stuff and/or
iova_cookie - those are mutually exclusive with SVA, right?
"iova_cookie" is mutually exclusive with SVA, but I am not sure about
the fault handler stuff.
To correct myself, the IOVA cookie should be irrelevant to *current*
SVA, but if we ever get as far as whole-device-SVA without PASIDs then
we might need an MSI cookie (modulo the additional problem of stealing
some procvess address space for it).
Did you mean @handler and @handler_token staffs below?
struct iommu_domain {
unsigned type;
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
void *handler_token;
struct iommu_domain_geometry geometry;
struct iommu_dma_cookie *iova_cookie;
};
Is it only for DMA domains? From the point view of IOMMU faults, it
seems to be generic.
Yes, it's the old common iommu_set_fault_handler() stuff (which arguably
is more of a "notifier" than a "handler"), but I assume that that's
irrelevant if SVA is using IOPF instead?
Thanks,
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver
Hi Nobuhiro, I love your patch! Yet something to improve: [auto build test ERROR on joro-iommu/next] [also build test ERROR on arm-perf/for-next/perf soc/for-next linus/master v5.18 next-20220524] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/intel-lab-lkp/linux/commits/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326 base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220525/202205251708.q7cwjpf8-...@intel.com/config) compiler: alpha-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/69bb4f3c2ef0bb1f65922bc72bb31109897a6393 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326 git checkout 69bb4f3c2ef0bb1f65922bc72bb31109897a6393 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot Note: the linux-review/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326 HEAD 07739c72b066c0781c371eec7614ed876441e8dd builds fine. It only hurts bisectability. All errors (new ones prefixed by >>): >> drivers/iommu/visconti-atu.c:47:29: error: field 'iommu' has incomplete type 47 | struct iommu_device iommu; | ^ >> drivers/iommu/visconti-atu.c:62:29: error: field 'io_domain' has incomplete >> type 62 | struct iommu_domain io_domain; | ^ In file included from include/linux/bits.h:22, from include/linux/ratelimit_types.h:5, from include/linux/ratelimit.h:5, from include/linux/dev_printk.h:16, from include/linux/device.h:15, from include/linux/dma-mapping.h:7, from drivers/iommu/visconti-atu.c:12: drivers/iommu/visconti-atu.c: In function 'to_atu_domain': include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer 293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^~~~ include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert' 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) |^~~~ include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert' 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^ include/linux/container_of.h:19:23: note: in expansion of macro '__same_type' 19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \ | ^~~ drivers/iommu/visconti-atu.c:70:16: note: in expansion of macro 'container_of' 70 | return container_of(domain, struct visconti_atu_domain, io_domain); |^~~~ drivers/iommu/visconti-atu.c: In function 'visconti_atu_attach_device': >> drivers/iommu/visconti-atu.c:121:43: error: implicit declaration of function >> 'dev_iommu_priv_get' [-Werror=implicit-function-declaration] 121 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev); | ^~ drivers/iommu/visconti-atu.c:121:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion] drivers/iommu/visconti-atu.c: In function 'visconti_atu_detach_device': drivers/iommu/visconti-atu.c:150:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion] 150 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev); | ^~ drivers/iommu/visconti-atu.c: At top level: drivers/iommu/visconti-atu.c:196:41: warning: 's
RE: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node
> -Original Message- > From: Shameerali Kolothum Thodi > Sent: 17 May 2022 08:18 > To: 'Lorenzo Pieralisi' ; Robin Murphy > ; raf...@kernel.org; j...@8bytes.org > Cc: Guohanjun (Hanjun Guo) ; > linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; > iommu@lists.linux-foundation.org; Linuxarm ; > w...@kernel.org; wanghuiqiang ; > steven.pr...@arm.com; sami.muja...@arm.com; j...@solid-run.com; > eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org > Subject: RE: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node > > > > -Original Message- > > From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com] > > Sent: 13 May 2022 10:50 > > To: Robin Murphy ; Shameerali Kolothum Thodi > > ; raf...@kernel.org; > > j...@8bytes.org > > Cc: Shameerali Kolothum Thodi ; > > Guohanjun (Hanjun Guo) ; > > linux-arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org; > > iommu@lists.linux-foundation.org; Linuxarm ; > > w...@kernel.org; wanghuiqiang ; > > steven.pr...@arm.com; sami.muja...@arm.com; j...@solid-run.com; > > eric.au...@redhat.com; laurentiu.tu...@nxp.com; h...@infradead.org > > Subject: Re: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node > > > > [with Christoph's correct email address] > > > > On Tue, May 10, 2022 at 09:07:00AM +0100, Robin Murphy wrote: > > > On 2022-05-10 08:23, Shameerali Kolothum Thodi wrote: > > > > Hi Joerg/Robin, > > > > > > > > I think this series is now ready to be merged. Could you please let > > > > me know if there is anything missing. > > > > > > Fine by me - these patches have had enough review and testing now that > > > even if anything else did come up, I think it would be better done as > > > follow-up work on the merged code. > > > > Given the ACPICA dependency I believe it is best for this series > > to go via the ACPI tree, right ? > > > > I assume there are all the required ACKs for that to happen. > > The SMMUv3/SMMU related changes (patches 6 - 9) still doesn't have > explicit ACK from maintainers other than the go ahead above from Robin. > > Just thought of highlighting it as not sure that will be an issue or not. > All, Just a gentle ping on this series again. Any chance this can make into 5.19? Please consider. Thanks, Shameer ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 06/10] iommu/sva: Refactoring iommu_sva_bind/unbind_device()
On Wed, May 25, 2022 at 02:04:49AM +, Tian, Kevin wrote:
> > From: Jean-Philippe Brucker
> > Sent: Tuesday, May 24, 2022 6:58 PM
> >
> > On Tue, May 24, 2022 at 10:22:28AM +, Tian, Kevin wrote:
> > > > From: Lu Baolu
> > > > Sent: Thursday, May 19, 2022 3:21 PM
> > > >
> > > > The existing iommu SVA interfaces are implemented by calling the SVA
> > > > specific iommu ops provided by the IOMMU drivers. There's no need for
> > > > any SVA specific ops in iommu_ops vector anymore as we can achieve
> > > > this through the generic attach/detach_dev_pasid domain ops.
> > >
> > > set/block_pasid_dev, to be consistent.
> > >
> > > > +
> > > > + mutex_lock(&iommu_sva_lock);
> > > > + /* Search for an existing domain. */
> > > > + domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid);
> > > > + if (domain) {
> > > > + sva_domain = to_sva_domain(domain);
> > > > + refcount_inc(&sva_domain->bond.users);
> > > > + goto out_success;
> > > > + }
> > > > +
> > >
> > > why would one device/pasid be bound to a mm more than once?
> >
> > Device drivers can call bind() multiple times for the same device and mm,
> > for example if one process wants to open multiple accelerator queues.
> >
>
> Is it clearer to have a sva_bond_get/put() pair instead of calling
> bind() multiple times here?
I don't think it's clearer, and it would force device drivers to keep
track of {dev, mm} pairs, when the IOMMU subsystem already does that.
At the moment a device driver calls
bond = iommu_sva_bind_device(dev, mm)
for each ADI that it wants to assign to userspace. If a process happens to
want multiple ADIs on one device, then the {dev, mm} parameters are the
same and bind() returns the same bond. Since the IOMMU driver needs to
track these anyway, it might as well refcount them.
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1] driver core: Extend deferred probe timeout on driver registration
On 2022-05-24 10:46:49 [-0700], Saravana Kannan wrote: > > Removing probe_timeout_waitqueue (as suggested) or setting the timeout > > to 0 avoids the delay. > > In your case, I think it might be working as intended? Curious, what > was the call stack in your case where it was blocked? Why is then there 10sec delay during boot? The backtrace is |[ cut here ] |WARNING: CPU: 4 PID: 1 at drivers/base/dd.c:742 wait_for_device_probe+0x30/0x110 |Modules linked in: |CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.18.0-rc5+ #154 |RIP: 0010:wait_for_device_probe+0x30/0x110 |Call Trace: | | prepare_namespace+0x2b/0x160 | kernel_init_freeable+0x2b3/0x2dd | kernel_init+0x11/0x110 | ret_from_fork+0x22/0x30 | Looking closer, it can't access init. This in particular box boots directly the kernel without an initramfs so the kernel later mounts /dev/sda1 and everything is good. So that seems to be the reason… My other machine with an initramfs does not show this problem. > -Saravana Sebastian ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
