Re: [PATCH v5 10/17] iommu: Do iommu_group_create_direct_mappings() before attach

Re: [PATCH v5 10/17] iommu: Do iommu_group_create_direct_mappings() before attach

[Jason Gunthorpe]

On Mon, Jun 05, 2023 at 09:09:59AM +0200, Ricardo Cañuelo wrote:
> [   16.267594] Call trace:
> [   16.267597]  mtk_iommu_flush_iotlb_all+0x18/0x70
> [   16.267603]  iommu_create_device_direct_mappings.part.0+0x13c/0x21c
> [   16.267608]  iommu_setup_default_domain+0x27c/0x430
> [   16.267611]  iommu_probe_device+0x7c/0x144
> [   16.267615]  of_iommu_configure+0x114/0x200
> [   16.267619]  of_dma_configure_id+0x1e0/0x3b4

This is definitely some problem in the mtk driver.. But I can't guess
what is wrong:

static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
{
	struct mtk_iommu_domain *dom = to_mtk_domain(domain);

	if (dom->bank)
		mtk_iommu_tlb_flush_all(dom->bank->parent_data);
}

We know domain != NULL since the caller checked (and de-ref'd)
it.

dom->bank should either be NULL (if pre-finalize) or a devm tracked
pointer.

parent_data is always valid if bank is valid.

So I'm at a loss.. Can you debug a bit more and find out where mtk is
crashing?

Thanks,
Jason

Re: [PATCH v5 10/17] iommu: Do iommu_group_create_direct_mappings() before attach

[Robin Murphy]

On 2023-06-05 14:47, Jason Gunthorpe wrote:
> On Mon, Jun 05, 2023 at 09:09:59AM +0200, Ricardo Cañuelo wrote:
>> [   16.267594] Call trace:
>> [   16.267597]  mtk_iommu_flush_iotlb_all+0x18/0x70
>> [   16.267603]  iommu_create_device_direct_mappings.part.0+0x13c/0x21c
>> [   16.267608]  iommu_setup_default_domain+0x27c/0x430
>> [   16.267611]  iommu_probe_device+0x7c/0x144
>> [   16.267615]  of_iommu_configure+0x114/0x200
>> [   16.267619]  of_dma_configure_id+0x1e0/0x3b4
> 
> This is definitely some problem in the mtk driver.. But I can't guess
> what is wrong:
> 
> static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
> {
> 	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> 
> 	if (dom->bank)
> 		mtk_iommu_tlb_flush_all(dom->bank->parent_data);
> }
> 
> We know domain != NULL since the caller checked (and de-ref'd)
> it.
> 
> dom->bank should either be NULL (if pre-finalize) or a devm tracked
> pointer.
> 
> parent_data is always valid if bank is valid.
> 
> So I'm at a loss.. Can you debug a bit more and find out where mtk is
> crashing?

The log says it's next-20230530 - the fix you're seeing there from 
commit b3fc95709c54 landed 2 days later :)

Robin.

Re: [PATCH v5 10/17] iommu: Do iommu_group_create_direct_mappings() before attach

[Jason Gunthorpe]

On Mon, Jun 05, 2023 at 03:00:24PM +0100, Robin Murphy wrote:
> On 2023-06-05 14:47, Jason Gunthorpe wrote:
> > On Mon, Jun 05, 2023 at 09:09:59AM +0200, Ricardo Cañuelo wrote:
> > > [   16.267594] Call trace:
> > > [   16.267597]  mtk_iommu_flush_iotlb_all+0x18/0x70
> > > [   16.267603]  iommu_create_device_direct_mappings.part.0+0x13c/0x21c
> > > [   16.267608]  iommu_setup_default_domain+0x27c/0x430
> > > [   16.267611]  iommu_probe_device+0x7c/0x144
> > > [   16.267615]  of_iommu_configure+0x114/0x200
> > > [   16.267619]  of_dma_configure_id+0x1e0/0x3b4
> > 
> > This is definitely some problem in the mtk driver.. But I can't guess
> > what is wrong:
> > 
> > static void mtk_iommu_flush_iotlb_all(struct iommu_domain *domain)
> > {
> > 	struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> > 
> > 	if (dom->bank)
> > 		mtk_iommu_tlb_flush_all(dom->bank->parent_data);
> > }
> > 
> > We know domain != NULL since the caller checked (and de-ref'd)
> > it.
> > 
> > dom->bank should either be NULL (if pre-finalize) or a devm tracked
> > pointer.
> > 
> > parent_data is always valid if bank is valid.
> > 
> > So I'm at a loss.. Can you debug a bit more and find out where mtk is
> > crashing?
> 
> The log says it's next-20230530 - the fix you're seeing there from commit
> b3fc95709c54 landed 2 days later :)

Ah, I was looking at Joerg's tree, mystery solved then :)

Thanks,
Jason

Re: [PATCH v5 10/17] iommu: Do iommu_group_create_direct_mappings() before attach

[Ricardo Cañuelo]

Hi all,

On 5/6/23 16:00, Robin Murphy wrote:
> The log says it's next-20230530 - the fix you're seeing there from commit b3fc95709c54 landed 2 days later :)

Thanks for checking that out Robin, KernelCI still hasn't run
that build but I tested it manually and the issue is not there
anymore.

#regzbot fix: b3fc95709c54

Cheers,
Ricardo