[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080314113336.63af1e7a@bluebox.local>
Date: Fri, 14 Mar 2008 11:33:36 +0100
From: "Hans J. Koch" <hjk@...utronix.de>
To: Hans-Jürgen Koch <hjk@...utronix.de>
Cc: "Jean-Samuel Chenard" <jsamch@...il.com>,
"Thomas Gleixner" <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>, Greg KH <greg@...ah.com>,
grant.likely@...retlab.ca,
Juergen Beisert <juergen127@...uzholzen.de>
Subject: Re: Linux UIO driver cache problem in PowerPC (fix)
Am Thu, 13 Mar 2008 08:19:32 +0100
schrieb Hans-Jürgen Koch <hjk@...utronix.de>:
> Am Wed, 12 Mar 2008 15:22:59 -0400
> schrieb "Jean-Samuel Chenard" <jsamch@...il.com>:
>
> > Hi,
Hi Jean-Samuel,
I investigated a bit and found your patch to be correct. I cleaned it
up a bit and sent it to Greg and LKML. I added you as author and also
took the freedom to add your Signed-off-by.
Juergen, Thomas, Grant, if you can confirm this, please add your
signature as well.
Thanks,
Hans
>
> Hi Jean-Samuel,
> thanks for your report. Please CC LKML if you find bugs like this. I
> also added Grant Likely (PPC Xilinx Virtex maintainer), as it seems to
> be a platform specific problem.
>
> >
> > Experimenting with your userspace I/O driver on a Xilinx FPGA
> > (PowerPC 405), I was having problems when reading hardware device
> > registers mapped on my platform using the user-space mmap() call.
> >
> > After some investigation (looking at the driver/char/mspec.c file),
> > I found that preventing caching of the page in uio_mmap_physical
> > fixed my issues with userspace mmap() reading messed-up values.
>
> Hm, we already mark the page with VM_IO. That's sufficient on x86 and
> arm. I'm not sure whether this is a bug in PPC memory handling. Why do
> they cache VM_IO pages?
>
> >
> > This patch is against your initial commit of the UIO driver in the
> > mainline kernel tree.
> >
> > ====
> > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> > index 865f32b..36e1123 100644
> > --- a/drivers/uio/uio.c
> > +++ b/drivers/uio/uio.c
> > @@ -22,6 +22,7 @@
> > #include <linux/string.h>
> > #include <linux/kobject.h>
> > #include <linux/uio_driver.h>
> > +#include <asm/pgtable.h>
> >
> > #define UIO_MAX_DEVICES 255
> >
> > @@ -447,6 +448,8 @@ static int uio_mmap_physical(struct
> > vm_area_struct *vma)
> >
> > vma->vm_flags |= VM_IO | VM_RESERVED;
> >
> > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> > +
> > return remap_pfn_range(vma,
> > vma->vm_start,
> > idev->info->mem[mi].addr >>
> > PAGE_SHIFT, ====
> >
> > I am a bit unsure if this will break something in any of your uses
> > of the UIO driver (on other platforms than ppc), but it really fixes
> > things for me.
>
> It should be OK, but unneccesary on other platforms. I have no
> objections, but I fear in 6 months we'll see a patch removing that
> line again because it's not needed...
>
> > Let me know if you need more details on what was
> > happening on my platform before I added this statement.
>
> I'd like to hear the opinion of people really involved in PPC memory
> handling.
>
> >
> > Thanks for the great driver for user-space I/O, it will save me lots
> > of time in my research.
> >
> > Regards,
> >
> > Jean-Samuel
>
> Thanks,
> Hans
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists