Date:   Thu, 2 Feb 2023 17:09:01 +0000
From:   "Dr. David Alan Gilbert" <dave@treblig.org>
To:     Christoph Hellwig <hch@lst.de>
Cc:     iommu@lists.linux.dev, mchehab@kernel.org,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Regression: v4l/bttv vbi vs iommu
Message-ID: <Y9vuLV+dQM/MpyWv@gallifrey>
References: <Y9qSwkLxeMpffZK/@gallifrey>
 <20230202100926.GA14931@lst.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <20230202100926.GA14931@lst.de>
User-Agent: Mutt/2.0.5 (2021-01-21)
Precedence: bulk

* Christoph Hellwig (hch@lst.de) wrote:
> On Wed, Feb 01, 2023 at 04:26:42PM +0000, Dr. David Alan Gilbert wrote:
> > f5ff79fddf0efecca538046b5cc20fb3ded2ec4f is the first bad commit
> > commit f5ff79fddf0efecca538046b5cc20fb3ded2ec4f
> > Author: Christoph Hellwig <hch@lst.de>
> > Date:   Sat Feb 26 16:40:21 2022 +0100
> > 
> >     dma-mapping: remove CONFIG_DMA_REMAP

Hi Christoph,
  Thanks for having a look.

> Which just enabled some code on common x86 configs that was already
> used on many other platforms.  In other words:  the code already
> was buggy, but got away with it on x86 so far as no one tested on
> e.g. arm or arm64.

I guess most of these are older PCI devices (that are on the older
v4l api), and there were never a huge number of arm boxes with
parallel PCI, and the ones that there were became even rarer
by 2014 when this code was added.

> The bug is in videobuf_dma_init_kernel:
> 
> 	for (i = 0; i < nr_pages; i++) {
> 		void *addr;
> 
> 		addr = dma_alloc_coherent(dma->dev, PAGE_SIZE,
> 					  &(dma->dma_addr[i]), GFP_KERNEL);
> 		if (addr == NULL)
> 			goto out_free_pages;
> 
> 		dma->vaddr_pages[i] = virt_to_page(addr);
> 	}
> 	dma->vaddr = vmap(dma->vaddr_pages, nr_pages, VM_MAP | VM_IOREMAP,
> 			  PAGE_KERNEL);
> 
> The address by dma_alloc_coherent is just a kernel virtual address,
> and virt_to_page must not be used on it as it could be vmalloc (as in
> this case) or various other really odd forms of memory.

OK, so at least why we know it's broken, the temporary work around
for most is turning the iommu off, and the long term fix is the new
API.

Thanks,

Dave

-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/