From mboxrd@z Thu Jan 1 00:00:00 1970 From: jean-philippe.brucker@arm.com (Jean-Philippe Brucker) Date: Fri, 3 Nov 2017 09:37:02 +0000 Subject: [RFCv2 PATCH 14/36] iommu/arm-smmu-v3: Add support for Substream IDs In-Reply-To: <37a82eaa-dc9d-af98-6cb8-941ca925b838@huawei.com> References: <20171006133203.22803-1-jean-philippe.brucker@arm.com> <20171006133203.22803-15-jean-philippe.brucker@arm.com> <5FC3163CFD30C246ABAA99954A238FA838454887@FRAEML521-MBX.china.huawei.com> <20171102155152.GA11899@e106794-lin.localdomain> <5FC3163CFD30C246ABAA99954A238FA838454A9B@FRAEML521-MBX.china.huawei.com> <37a82eaa-dc9d-af98-6cb8-941ca925b838@huawei.com> Message-ID: <0d607058-9dd7-6564-316d-ebada8bff960@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 03/11/17 05:45, Yisheng Xie wrote: > Hi Jean, > > On 2017/11/3 1:02, Shameerali Kolothum Thodi wrote: >> >> >>> -----Original Message----- >>> From: Jean-Philippe Brucker [mailto:Jean-Philippe.Brucker at arm.com] >>> Sent: Thursday, November 02, 2017 3:52 PM >>> To: Shameerali Kolothum Thodi >>> Cc: linux-arm-kernel at lists.infradead.org; linux-pci at vger.kernel.org; linux- >>> acpi at vger.kernel.org; devicetree at vger.kernel.org; iommu at lists.linux- >>> foundation.org; Mark Rutland ; xieyisheng (A) >>> ; Gabriele Paoloni >>> ; Catalin Marinas >>> ; Will Deacon ; >>> okaya at codeaurora.org; yi.l.liu at intel.com; Lorenzo Pieralisi >>> ; ashok.raj at intel.com; tn at semihalf.com; >>> joro at 8bytes.org; rfranz at cavium.com; lenb at kernel.org; >>> jacob.jun.pan at linux.intel.com; alex.williamson at redhat.com; >>> robh+dt at kernel.org; Leizhen (ThunderTown) ; >>> bhelgaas at google.com; dwmw2 at infradead.org; liubo (CU) >>> ; rjw at rjwysocki.net; robdclark at gmail.com; >>> hanjun.guo at linaro.org; Sudeep Holla ; Robin >>> Murphy ; nwatters at codeaurora.org; Linuxarm >>> >>> 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] [] __alloc_pages_nodemask+0x19c/0xc48 >>>> [ 16.481854] [] alloc_pages_current+0x80/0xcc >>>> [ 16.493607] [] __get_free_pages+0xc/0x38 >>>> [ 16.504661] [] swiotlb_alloc_coherent+0x64/0x190 >>>> [ 16.517117] [] __dma_alloc+0x110/0x204 >>>> [ 16.527820] [] dmam_alloc_coherent+0x88/0xf0 >>>> [ 16.539575] [] >>> arm_smmu_domain_finalise_s1+0x60/0x248 >>>> [ 16.552909] [] arm_smmu_attach_dev+0x264/0x300 >>>> [ 16.565013] [] __iommu_attach_device+0x48/0x5c >>>> [ 16.577117] [] iommu_group_add_device+0x144/0x3a4 >>>> [ 16.589746] [] iommu_group_get_for_dev+0x70/0xf8 >>>> [ 16.602201] [] arm_smmu_add_device+0x1a4/0x418 >>>> [ 16.614308] [] iort_iommu_configure+0xf0/0x16c >>>> [ 16.626416] [] acpi_dma_configure+0x30/0x70 >>>> [ 16.637994] [] dma_configure+0xa8/0xd4 >>>> [ 16.648695] [] driver_probe_device+0x1a4/0x2dc >>>> [ 16.673081] [] bus_for_each_drv+0x54/0x94 >>>> [ 16.684307] [] __device_attach+0xc4/0x12c >>>> [ 16.695533] [] device_initial_probe+0x10/0x18 >>>> [ 16.707462] [] 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 ? Yes, the context table allocator always needs to allocate at least one entry, even if the master or SMMU doesn't support SSID. I think an earlier version called this field "num_contexts", maybe we should got back to that name for clarity? Thanks, Jean