lkml.org 
[lkml]   [2006]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: PCI Device Driver / remap_pfn_range()
Brian D. McGrew wrote:
> A few weeks back I posted a cry for help with a PCI device driver which
> maps hardware memory into user/kernel space using what used to be called
> remap_page_range in the 2.6.9 kernel and now I'm trying to use
> remap_pfn_range.
>
> I'm still struggling with this and I'm hoping that there is an expert
> out there who can point out what I'm doing wrong!!! My source code is
> attached and I know I'm probably not doing this in the best way; but
> it's the only way I don't know how, since I'm tasked with a job where I
> have no idea what I'm doing!
>
> When I try and access my device, which should be /dev/ibb[0-3], I get a
> segmentation fault then a core dump and within a few seconds after that,
> the system becomes way unstable and has to be rebooted (if it doesn't
> lock up hard first).
>

Well, posting source code is a good first step.

What kernel are you attempting to port this to? 2.6.16 / head, I presume?

> static void
> free_ibb_usr_shared(IbbSoftDev *ibb_sp)
> {
> struct page *page;
>
> u_long virt_addr;
> u_long ushared_addr = (u_long)ibb_sp->ushared;
>
> for (virt_addr = ushared_addr;
> virt_addr < ushared_addr + PAGE_ALIGN(IBB_SHARED_SIZE);
> virt_addr += PAGE_SIZE)
> {
> page = virt_to_page(virt_addr);
> ClearPageReserved(page);

PageReserved is deprecated, however it makes sense to leave these in
for the moment so you get a little more sanity checking and easier
backwards compatibility.

>
> atomic_set(&page->_count, 1);

You should never access _count directly. There should be no reason to
do this (except for introducing use-after-free), so just delete it.

> vfree((void *)virt_addr);
> }
>
> if (ibb_sp->ushared)
> ibb_sp->ushared = NULL;
> }
>
> static int
> alloc_ibb_usr_shared(IbbSoftDev *ibb_sp)
> {
> u_long virt_addr;
> u_long ushared_addr;
> int order = 0;
>
> /*
> * From Linux Device Drivers - memory that is going to
> * be mmaped must be PAGE_SIZE grained. Since we do mmap
> * ushared to user space, we need to allocated it in
> * PAGE_SIZE chunks.
> */
>
> int size = PAGE_ALIGN(IBB_SHARED_SIZE);
>
> dcmn_err(1, ("<1>" "ibb::size is %d\n", size));
> dcmn_err(1, ("<1>" "ibb::order is %d\n", order));
>
> do {
> dcmn_err(1, ("<1>" "ibb::size is %d\n", size));
> dcmn_err(1, ("<1>" "ibb::order is %d\n", order));
>
> order++;
> } while (size > (PAGE_SIZE * (1 << order)));
>
> // ibb_sp->ushared = (IbbUserShared *) __get_free_pages(GFP_KERNEL,
> order);
> ibb_sp->ushared = (IbbUserShared *) vmalloc(4096 * 1024);

Shouldn't this be vmalloc(size)?

> static void
> free_ibb_image_table_mem(IbbSoftDev *ibb_sp)
> {
> uint32_t *virt_addr;
> struct page *page;
>
> dcmn_err(1, ("<1>" "free_ibb_image_table_mem(%d): Free size %d.\n",
> ibb_sp->dev_num,ibb_sp->image_table_size));
>
> /* unreserve all pages */
> for( virt_addr = ibb_sp->image_table;
> virt_addr < ibb_sp->image_table + ibb_sp->image_table_size;
> virt_addr += PAGE_SIZE)
> {
> page = virt_to_page(virt_addr);
> ClearPageReserved(page);
>
> atomic_set(&page->_count, 1);

ditto
>
> // ibb_sp->image_table = (uint32_t *) __get_free_pages(GFP_KERNEL,
> order);
> ibb_sp->ushared = (IbbUserShared *) vmalloc(4096 * 1024);

shouldn't this be ->image_table? and vmalloc(size)?

[...]

> if (ibb_sp->image_table != NULL && vsize <=
> ibb_sp->image_table_size) {
> const u_long start = vma->vm_start;
> const u_long size = (vma->vm_end - vma->vm_start);
> const u_long page = (void *)ibb_sp->image_table;
>
> if (io_remap_pfn_range(vma, start, page, size,
> vma->vm_page_prot)) {

I'm pretty sure you can't remap_pfn_range vmalloced memory because
it doesn't use contiguous page frames.

You'd be better off going through the pages one by one and running
vm_insert_page on each (or remap_pfn_range on each pageframe, if you
need to be compatible with kernels before 2.6.16).

Also, don't use io_remap on vmalloc memory, just remap.

> dcmn_err(1, ("<1>" "ibb_mmap(%d): image table remap
> failed\n",
> ibb_sp->dev_num));
>
> return(-EAGAIN);
> }
> }
>
> else {
> dcmn_err(1, ("<1>"
> "ibb_mmap(%d): image table params failed vs %d is %d\n",
> ibb_sp->dev_num, (int)vsize,
> (int)ibb_sp->image_table_size));
>
> return(-EAGAIN);
> }
> }
> else if (offset == IBB_SHARED_ADDR) {
> if (ibb_sp->ushared != NULL && vsize <=
> PAGE_ALIGN(IBB_SHARED_SIZE)) {
> const u_long start = vma->vm_start;
> const u_long size = (vma->vm_end - vma->vm_start);
> const u_long page = (void *)ibb_sp->ushared;
>
> if (io_remap_pfn_range(vma, start, page, size,
> vma->vm_page_prot)) {

ditto

> dcmn_err(1, ("<1>" "ibb_mmap(%d): ushared remap
> failed\n",
> ibb_sp->dev_num));
>
> return(-EAGAIN);
> }
>
> dcmn_err(1, ("<1>" "ibb_mmap(%d): \
> ushared remap: ibb_sp->ushared: %lx vma->vm_start:
> %lx\n",
> ibb_sp->dev_num,
> (u_long)ibb_sp->ushared,
> (u_long)vma->vm_start) );
> }
> else {
> dcmn_err(1, ("<1>" "ibb_mmap(%d): ushared mmap failed\n",
> ibb_sp->dev_num));
>
> return(-EAGAIN);
> }
> }
>
> /*
> * vsize is PAGE_SIZE at minimum. For registers and other
> * small memory areas we need to check for VSIZE. For large
> * chunks, vsize should equal the chunk size
> */
>
> else if (offset == IBB_CONTROL_OFF && vsize == PAGE_SIZE) {
> const u_long start = vma->vm_start;
> const u_long size = (vma->vm_end - vma->vm_start);
> const u_long page = ibb_sp->creg_phys;
>
> if (remap_pfn_range(vma, start, page, size, vma->vm_page_prot))

If creg_phys is io memory, that's when you'd use io_remap, I think.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-04-18 00:52    [W:0.083 / U:0.432 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site