Hi Jean, On 2017/11/3 1:02, Shameerali Kolothum Thodi wrote: > > >> -----Original Message----- >> From: Jean-Philippe Brucker [mailto:Jean-Philippe.Brucker@xxxxxxx] >> Sent: Thursday, November 02, 2017 3:52 PM >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@xxxxxxxxxx> >> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux- >> acpi@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; iommu@lists.linux- >> foundation.org; Mark Rutland <Mark.Rutland@xxxxxxx>; xieyisheng (A) >> <xieyisheng1@xxxxxxxxxx>; Gabriele Paoloni >> <gabriele.paoloni@xxxxxxxxxx>; Catalin Marinas >> <Catalin.Marinas@xxxxxxx>; Will Deacon <Will.Deacon@xxxxxxx>; >> okaya@xxxxxxxxxxxxxx; yi.l.liu@xxxxxxxxx; Lorenzo Pieralisi >> <Lorenzo.Pieralisi@xxxxxxx>; ashok.raj@xxxxxxxxx; tn@xxxxxxxxxxxx; >> joro@xxxxxxxxxx; rfranz@xxxxxxxxxx; lenb@xxxxxxxxxx; >> jacob.jun.pan@xxxxxxxxxxxxxxx; alex.williamson@xxxxxxxxxx; >> robh+dt@xxxxxxxxxx; Leizhen (ThunderTown) <thunder.leizhen@xxxxxxxxxx>; >> bhelgaas@xxxxxxxxxx; dwmw2@xxxxxxxxxxxxx; liubo (CU) >> <liubo95@xxxxxxxxxx>; rjw@xxxxxxxxxxxxx; robdclark@xxxxxxxxx; >> hanjun.guo@xxxxxxxxxx; Sudeep Holla <Sudeep.Holla@xxxxxxx>; Robin >> Murphy <Robin.Murphy@xxxxxxx>; nwatters@xxxxxxxxxxxxxx; Linuxarm >> <linuxarm@xxxxxxxxxx> >> Subject: Re: [RFCv2 PATCH 14/36] iommu/arm-smmu-v3: Add support for >> Substream IDs >> >> Hi Shameer, >> >> On Thu, Nov 02, 2017 at 12:49:32PM +0000, Shameerali Kolothum Thodi wrote: >>> We had a go with this series on HiSIlicon D05 platform which doesn't have >>> support for ssids/ATS/PRI, to make sure it generally works. >>> >>> But observed the below crash on boot, >>> >>> [ 16.009084] WARNING: CPU: 59 PID: 391 at mm/page_alloc.c:3883 >> __alloc_pages_nodemask+0x19c/0xc48 >>> [ 16.026797] Modules linked in: >>> [ 16.032944] CPU: 59 PID: 391 Comm: kworker/59:1 Not tainted 4.14.0-rc1- >> 159539-ge42aca3 #236 >>> [...] >>> [ 16.068206] Workqueue: events deferred_probe_work_func >>> [ 16.078557] task: ffff8017d38a0000 task.stack: ffff00000b198000 >>> [ 16.090486] PC is at __alloc_pages_nodemask+0x19c/0xc48 >>> [ 16.101013] LR is at __alloc_pages_nodemask+0xe0/0xc48 >>> [ 16.469220] [<ffff000008186b94>] __alloc_pages_nodemask+0x19c/0xc48 >>> [ 16.481854] [<ffff0000081d65b0>] alloc_pages_current+0x80/0xcc >>> [ 16.493607] [<ffff000008182be8>] __get_free_pages+0xc/0x38 >>> [ 16.504661] [<ffff0000083c4d58>] swiotlb_alloc_coherent+0x64/0x190 >>> [ 16.517117] [<ffff00000809824c>] __dma_alloc+0x110/0x204 >>> [ 16.527820] [<ffff00000858e850>] dmam_alloc_coherent+0x88/0xf0 >>> [ 16.539575] [<ffff000008568884>] >> arm_smmu_domain_finalise_s1+0x60/0x248 >>> [ 16.552909] [<ffff00000856c104>] arm_smmu_attach_dev+0x264/0x300 >>> [ 16.565013] [<ffff00000855d40c>] __iommu_attach_device+0x48/0x5c >>> [ 16.577117] [<ffff00000855e698>] iommu_group_add_device+0x144/0x3a4 >>> [ 16.589746] [<ffff00000855ed18>] iommu_group_get_for_dev+0x70/0xf8 >>> [ 16.602201] [<ffff00000856a314>] arm_smmu_add_device+0x1a4/0x418 >>> [ 16.614308] [<ffff00000849dfcc>] iort_iommu_configure+0xf0/0x16c >>> [ 16.626416] [<ffff000008468c50>] acpi_dma_configure+0x30/0x70 >>> [ 16.637994] [<ffff00000858f00c>] dma_configure+0xa8/0xd4 >>> [ 16.648695] [<ffff00000857706c>] driver_probe_device+0x1a4/0x2dc >>> [ 16.673081] [<ffff0000085752c8>] bus_for_each_drv+0x54/0x94 >>> [ 16.684307] [<ffff000008576db0>] __device_attach+0xc4/0x12c >>> [ 16.695533] [<ffff000008577350>] device_initial_probe+0x10/0x18 >>> [ 16.707462] [<ffff0000085762b4>] bus_probe_device+0x90/0x98 >>> >>> After a bit of debug it looks like on platforms where ssid is not supported, >>> s1_cfg.num_contexts is set to zero and it eventually results in this crash >>> in, >>> arm_smmu_domain_finalise_s1() -->arm_smmu_alloc_cd_tables()--> >>> arm_smmu_alloc_cd_leaf_table() as num_leaf_entries is zero. >>> >>> With the below fix, it works on D05 now, >>> >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >>> index 8ad90e2..51f5821 100644 >>> --- a/drivers/iommu/arm-smmu-v3.c >>> +++ b/drivers/iommu/arm-smmu-v3.c >>> @@ -2433,7 +2433,10 @@ static int arm_smmu_domain_finalise(struct >> iommu_domain *domain, >>> domain->min_pasid = 1; >>> domain->max_pasid = master->num_ssids - 1; >>> smmu_domain->s1_cfg.num_contexts = master->num_ssids; >>> + } else { >>> + smmu_domain->s1_cfg.num_contexts = 1; >>> } >>> + >>> smmu_domain->s1_cfg.can_stall = master->ste.can_stall; >>> break; >>> case ARM_SMMU_DOMAIN_NESTED: >>> >>> >>> I am not sure this is right place do this. Please take a look. >> >> Thanks for testing the series and reporting the bug. I added the >> following patch to branch svm/current, does it work for you? > > Yes, it does. > > Thanks, > Shameer > >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index 42c8378624ed..edda466adc81 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -3169,9 +3169,7 @@ static int arm_smmu_add_device(struct device *dev) >> } >> } >> >> - if (smmu->ssid_bits) >> - master->num_ssids = 1 << min(smmu->ssid_bits, >> - fwspec->num_pasid_bits); >> + master->num_ssids = 1 << min(smmu->ssid_bits, fwspec- >>> num_pasid_bits); If fwspec->num_pasid_bits = 0, then master have _one_ num_ssids ? It seems Shameerali's fix is better ? >> >> if (fwspec->can_stall && smmu->features & ARM_SMMU_FEAT_STALLS) { >> master->can_fault = true; > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
