[RFCv2 PATCH 14/36] iommu/arm-smmu-v3: Add support for Substream IDs
Shameerali Kolothum Thodi
shameerali.kolothum.thodi at huawei.com
Fri Nov 3 02:39:11 PDT 2017
> -----Original Message-----
> From: Jean-Philippe Brucker [mailto:jean-philippe.brucker at arm.com]
> Sent: Friday, November 03, 2017 9:37 AM
> To: xieyisheng (A) <xieyisheng1 at huawei.com>; Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi at huawei.com>
> 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 <Mark.Rutland at arm.com>; Gabriele Paoloni
> <gabriele.paoloni at huawei.com>; Catalin Marinas
> <Catalin.Marinas at arm.com>; Will Deacon <Will.Deacon at arm.com>;
> okaya at codeaurora.org; yi.l.liu at intel.com; Lorenzo Pieralisi
> <Lorenzo.Pieralisi at arm.com>; 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) <thunder.leizhen at huawei.com>;
> bhelgaas at google.com; dwmw2 at infradead.org; liubo (CU)
> <liubo95 at huawei.com>; rjw at rjwysocki.net; robdclark at gmail.com;
> hanjun.guo at linaro.org; Sudeep Holla <Sudeep.Holla at arm.com>; Robin
> Murphy <Robin.Murphy at arm.com>; nwatters at codeaurora.org; Linuxarm
> <linuxarm at huawei.com>
> Subject: Re: [RFCv2 PATCH 14/36] iommu/arm-smmu-v3: Add support for
> Substream IDs
>
> 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
> <shameerali.kolothum.thodi at huawei.com>
> >>> 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 <Mark.Rutland at arm.com>; xieyisheng (A)
> >>> <xieyisheng1 at huawei.com>; Gabriele Paoloni
> >>> <gabriele.paoloni at huawei.com>; Catalin Marinas
> >>> <Catalin.Marinas at arm.com>; Will Deacon <Will.Deacon at arm.com>;
> >>> okaya at codeaurora.org; yi.l.liu at intel.com; Lorenzo Pieralisi
> >>> <Lorenzo.Pieralisi at arm.com>; 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)
> <thunder.leizhen at huawei.com>;
> >>> bhelgaas at google.com; dwmw2 at infradead.org; liubo (CU)
> >>> <liubo95 at huawei.com>; rjw at rjwysocki.net; robdclark at gmail.com;
> >>> hanjun.guo at linaro.org; Sudeep Holla <Sudeep.Holla at arm.com>; Robin
> >>> Murphy <Robin.Murphy at arm.com>; nwatters at codeaurora.org; Linuxarm
> >>> <linuxarm at huawei.com>
> >>> 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 ?
>
> 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?
+1 for that. As ssid can be zero as per the spec, num_ssids=1 will be slightly misleading.
Thanks,
Shameer
More information about the linux-arm-kernel
mailing list