mmap works page aligned. If uio-mem areas were set up unaligned, mmap
would silently align it and the corresponding attributes in sysfs would
not reflect it. This patch fixes such values during init to what the
kernel will do anyway and adds a warning.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
---
drivers/uio/uio.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Index: drivers/uio/uio.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/uio/uio.c.orig
+++ drivers/uio/uio.c
@@ -656,6 +656,8 @@
struct uio_info *info)
{
struct uio_device *idev;
+ struct uio_mem *mem;
+ unsigned int offset;
int ret =3D 0;
=20
if (!parent || !info || !info->name || !info->version)
@@ -691,6 +693,16 @@
goto err_device_create;
}
=20
+ for (mem =3D info->mem; mem->size; mem++) {
+ offset =3D mem->addr & ~PAGE_MASK;
+ if (offset) {
+ mem->addr -=3D offset;
+ mem->size +=3D offset;
+ dev_warn(idev->dev, "mem[%d] not page aligned!"
+ "Fixing values.\n", mem - info->mem);
+ }
+ }
+
ret =3D uio_dev_add_attributes(idev);
if (ret)
goto err_uio_dev_add_attributes;
--=20
Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
Pengutronix - Linux Solutions for Science and Industry
Minor nit: please add a space to the message. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." --
Hi Wolfram, thanks for mentioning this. I already know of one case where such a situation lead to ugly workarounds, so the problem needs to be addressed. However, I'd like to choose a different approach. Let's take the following situation as an example: - A chip with 16 bytes of registers - Its base address is 0x12345080 (4k page size, not aligned) With your approach, sysfs would report a base address of 0x12345000 and a size of 0x90. Both is a lie. We don't want to encourage the user to access addresses outside the 16 bytes range the driver author originally announced. UIO drivers are often used in embedded devices, where developers usually know the physical addresses of their devices and have them hardcoded in a #define. It's confusing if sysfs suddenly reports something different. The userspace part of the driver expects a 16 bytes range but is told there are 0x90 bytes at his disposal. It has to guess where the devices registers are (or if this is a completely different device...). It might also check the physical base address and find that this is wrong, too. The situation becomes even worse if you have two such chips on your board, and each reports a different size. If their addresses were in the same page, both would have the same base address, but different sizes... Another issue: You print a warning if a mem->addr is not page aligned. Why? Either the driver (both kernel and userspace) can handle this, or it can't. In the first case, nothing needs to be printed, in the second case it's not a warning but an error. IMO a warning only makes sense if there's something the driver author can do to fix it. But if you've got hardware with certain addresses, there's usually nothing you can do. I'd like to suggest the following solution (patch at the end of this mail): Let's leave it to userspace. All userspace needs is the "offset" information you calculate in your patch. If we add a new sysfs attribute for that, userspace can simply add it to the address ...
Hello Hans, True. It's actually quite dangerous to be able to write outside that region at all, but I guess this can't be helped due to the nature of I'm perfectly fine with this approach. I tested it and it worked as expected. The only thing I'd like to add is that 'offset' could be renamed to 'map_offset' or 'mmap_offset' to be a bit more precise what You're welcome. Thanks for dealing with the issue. All the best, Wolfram --=20 Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de Pengutronix - Linux Solutions for Science and Industry
