Messages in this thread | | | Subject | Re: [PATCH] uio: Fix bus error that access memory mapped by physical | From | wubian <> | Date | Wed, 23 Jun 2021 16:49:16 +0800 |
| |
First,thanks for your reply
I haven’t found this problem on the x86 platform. I only found it on arm64. I used gdb to track memset and found that the bus error in glibc/sysdeps/aarch64/memset.S: dc zva, dst; reference "Architecture Reference ManualArmv8, for Armv8-A architecture profile" manual found that the dc assembly instruction(performance optimization) is related to the operation of the cache, that is to say, there is a bus error in the operation of the cache, and then check the "ARM Cortex-A Series Programmer's Guide for ARMv8-A " manual, found that the armv8 architecture cache has data cache and write buffer, and there are two operation modes write-back and write-through, write operations in these two modes, the data flow will pass through the write buffer, and pgprot_noncached will prohibit data Cache and write buffer, this causes the dc command in memset to fail (bus error), and pgprot_writecombine does not prohibit write buffer, so the dc command in memset is successfully operated when pgprot_writecombine is used.
Regarding the bugfix issue, this is my first kernel patch submission, I may not be too clear about the boundary of the kernel bugfix. I think this issue is a bug, so I classify this submission as a bugfix. At last hope to get your suggestion.
thanks,
wubian
On 2021/6/23 下午3:14, Greg KH wrote: > On Wed, Jun 23, 2021 at 02:52:14PM +0800, wubian wrote: >> On the arm64, register the uio driver and map a physical space >> on the pci device to user space, then use memset write data to >> the address space, a bus error will occur. This error is due to >> the dc instruction(cache operation) used in the assembly of memset, >> uio mapping physical memory will call pgprot_noncached() to set >> non-cached and non-buffered, while pgprot_writecombine() has fewer >> restrictions. It does not prohibit write buffer, so replacing >> pgprot_noncached() with pgprot_writecombine() can solve this problem. >> >> Signed-off-by: wubian <wubian@uniontech.com> >> --- >> drivers/uio/uio.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c >> index ea96e319c8a0..09b04b20fa30 100644 >> --- a/drivers/uio/uio.c >> +++ b/drivers/uio/uio.c >> @@ -739,7 +739,11 @@ static int uio_mmap_physical(struct vm_area_struct *vma) >> >> vma->vm_ops = &uio_physical_vm_ops; >> if (idev->info->mem[mi].memtype == UIO_MEM_PHYS) >> +#if defined(CONFIG_ARM64) >> + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); >> +#else >> vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); >> +#endif > This feels really wrong, shouldn't stuff like this be handled in the > platform itself and not in the driver? > > And why is ARM64 special here? Why not other arches? What is odd about > this platform? We almost never want to use #if in .c files, why is it > ok to do that here? > > And is this a bugfix? If so, what commit does it fix? Should it go to > stable kernels, and if so, how far back? > > I need more information here :) > > thanks, > > greg k-h > >
| |