Re: [REGRESSION] EMEM address decode error when using Tegra210 media engines

[Jason Gunthorpe <jgg@ziepe.ca>]

On Tue, Jun 03, 2025 at 06:43:49PM +0100, Robin Murphy wrote:
> On 2025-06-03 5:52 pm, Diogo Ivo wrote:
> > 
> > On 6/3/25 4:32 PM, Jason Gunthorpe wrote:
> > > On Tue, Jun 03, 2025 at 04:06:47PM +0100, Diogo Ivo wrote:
> > > > Hello,
> > > > 
> > > > Commit 50568f87d1e233e introduced a regression when trying to
> > > > use the media
> > > > accelerators present on the Tegra X1 SoC.
> > > > 
> > > > I came across this regression when testing the branch [1] that leverages
> > > > the NVJPG engine in the Tegra X1 for decoding a JPEG file. After commit
> > > > 50568f87d1e233e we see the following error messages after
> > > > submitting a job
> > > > through the TEGRA_CHANNEL_SUBMIT IOCTL:
> > > 
> > > Maybe this?
> > > 
> > > @@ -567,7 +567,7 @@ static void tegra_smmu_set_pde(struct
> > > tegra_smmu_as *as, unsigned long iova,
> > >          /* The flush the page directory entry from caches */
> > >          dma_sync_single_range_for_device(smmu->dev, as->pd_dma, offset,
> > > -                                        sizeof(*pd), DMA_TO_DEVICE);
> > > +                                        sizeof(pd->val[0]),
> > > DMA_TO_DEVICE);
> > >          /* And flush the iommu */
> > >          smmu_flush_ptc(smmu, as->pd_dma, offset);
> > > 
> > > It is the only mistake I was able to notice.
> > > 
> > > But I'd be puzzled - I'd expect bigger sizeof would make it slower not
> > > broken.. Though your crash sure looks like either missing cache
> > > coherency or a bad PTE construction.
> 
> I reckon the "unsigned long offset = pd_index * sizeof(*pd);" a few lines
> above is probably more impactful ;)

Oh yes, almost certainly. Very good of you to notice it!

Diogo how about this:

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 61897d50162dd7..72a400f7ae0c20 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -560,14 +560,14 @@ static void tegra_smmu_set_pde(struct tegra_smmu_as *as, unsigned long iova,
        unsigned int pd_index = iova_pd_index(iova);
        struct tegra_smmu *smmu = as->smmu;
        struct tegra_pd *pd = as->pd;
-       unsigned long offset = pd_index * sizeof(*pd);
+       unsigned long offset = pd_index * sizeof(pd->val[0]);
 
        /* Set the page directory entry first */
        pd->val[pd_index] = value;
 
        /* The flush the page directory entry from caches */
        dma_sync_single_range_for_device(smmu->dev, as->pd_dma, offset,
-                                        sizeof(*pd), DMA_TO_DEVICE);
+                                        sizeof(pd->val[0]), DMA_TO_DEVICE);
 
        /* And flush the iommu */
        smmu_flush_ptc(smmu, as->pd_dma, offset);