Hi Jean,
On 2017/11/3 1:02, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com]
>> Sent: Thursday, November 02, 2017 3:52 PM
>> To: Shameerali Kolothum Thodi <shameerali.kolothum.th...@huawei.com>
>> Cc: linux-arm-ker...@lists.infradead.org; linux-...@vger.kernel.org; linux-
>> a...@vger.kernel.org; devicet...@vger.kernel.org; iommu@lists.linux-
>> foundation.org; Mark Rutland <mark.rutl...@arm.com>; xieyisheng (A)
>> <xieyishe...@huawei.com>; Gabriele Paoloni
>> <gabriele.paol...@huawei.com>; Catalin Marinas
>> <catalin.mari...@arm.com>; Will Deacon <will.dea...@arm.com>;
>> ok...@codeaurora.org; yi.l....@intel.com; Lorenzo Pieralisi
>> <lorenzo.pieral...@arm.com>; ashok....@intel.com; t...@semihalf.com;
>> j...@8bytes.org; rfr...@cavium.com; l...@kernel.org;
>> jacob.jun....@linux.intel.com; alex.william...@redhat.com;
>> robh...@kernel.org; Leizhen (ThunderTown) <thunder.leiz...@huawei.com>;
>> bhelg...@google.com; dw...@infradead.org; liubo (CU)
>> <liub...@huawei.com>; r...@rjwysocki.net; robdcl...@gmail.com;
>> hanjun....@linaro.org; Sudeep Holla <sudeep.ho...@arm.com>; Robin
>> Murphy <robin.mur...@arm.com>; nwatt...@codeaurora.org; Linuxarm
>> <linux...@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 ?
It seems Shameerali's fix is better ?
>>
>> if (fwspec->can_stall && smmu->features & ARM_SMMU_FEAT_STALLS) {
>> master->can_fault = true;
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
