[PATCH v2 00/19] 32-bit lseek and /dev/mem fixes/improvements

[PATCH v2 00/19] 32-bit lseek and /dev/mem fixes/improvements

[Andrey Smirnov]

This series is a result of my attempt to fix a regression in lseek()
on 32-bit platforms. The regression manifested in lseek() unable to
seek past ~4GiB mark caused and was caused by usage of IS_ERR_VALUE()
(see commit messages for more detailed explanation).

Feedback is welcome!

Thanks,
Andrey Smirnov

Changes since [v1]:

    - /dev/mem is split into /dev/mem and /dev/highmem on 64-bit
      platforms

    - .lseek() hooks are converted to return error code or 0 on
      success

    - A number of .lseek() related simplifcations that were done as a
      percursor to the change above added to the series

    - Added a fix for a bugs discovered when testing corener cases of
      accessing /dev/mem (/dev/highmem)

[v1] http://lists.infradead.org/pipermail/barebox/2019-January/036756.html

Andrey Smirnov (19):
  commands: Move mem_parse_options() to lib/misc.c
  commands: Get rid of mem_rw_buf
  commands: Move /dev/mem driver to drivers/misc
  nvmem: Do not use DEVFS_IS_CHARACTER_DEV
  common: firmware: Don't use FILE_SIZE_STREAM directly
  devfs: Fix incorrect error check for cdev->ops->lseek()
  fs: Update FILE position in lseek()
  fs: Drop trivial .lseek() implementaitons in FS drivers
  devfs: Drop dev_lseek_default()
  fs: devfs: Change .lseek callbacks to return 'int'
  fs: Do not use IS_ERR_VALUE() to validate offset in lseek()
  fs: Simplify new position calculation in lseek()
  fs: Share code between mem_write()/mem_read()
  fs: Avoid division in mem_copy()
  fs: Report actual data processed by mem_copy()
  fs: Introduce mem_read_nofail()
  commands: md: Do not use memmap()
  drivers: mem: Create file to access second half of 64-bit memory
  libfile: Fix incorrect lseek check in open_and_lseek()

 arch/arm/mach-mxs/ocotp.c     |   1 -
 arch/sandbox/board/hostfile.c |   1 -
 commands/Kconfig              |  17 ++---
 commands/Makefile             |   1 -
 commands/md.c                 |  18 ++---
 commands/mem.c                | 123 ----------------------------------
 commands/memcmp.c             |  16 ++---
 commands/memcpy.c             |  10 +--
 commands/memset.c             |   2 -
 commands/stddev.c             |   4 --
 common/block.c                |   1 -
 common/firmware.c             |   2 +-
 common/ratp/md.c              |   9 +--
 drivers/base/regmap/regmap.c  |   1 -
 drivers/eeprom/at24.c         |   1 -
 drivers/eeprom/at25.c         |   1 -
 drivers/hw_random/core.c      |   1 -
 drivers/mfd/act8846.c         |   1 -
 drivers/mfd/lp3972.c          |   1 -
 drivers/mfd/mc34704.c         |   1 -
 drivers/mfd/mc9sdz60.c        |   1 -
 drivers/mfd/stmpe-i2c.c       |   1 -
 drivers/mfd/twl-core.c        |   1 -
 drivers/misc/Kconfig          |   3 +
 drivers/misc/Makefile         |   1 +
 drivers/misc/mem.c            |  61 +++++++++++++++++
 drivers/misc/sram.c           |   1 -
 drivers/mtd/core.c            |   1 -
 drivers/mtd/mtdoob.c          |   1 -
 drivers/mtd/mtdraw.c          |   1 -
 drivers/mtd/nand/nand-bb.c    |   5 +-
 drivers/mtd/ubi/barebox.c     |   4 +-
 drivers/net/e1000/eeprom.c    |   2 -
 drivers/net/ksz8864rmn.c      |   1 -
 drivers/net/phy/mdio_bus.c    |   1 -
 drivers/nvmem/core.c          |   2 -
 drivers/video/fb.c            |   1 -
 drivers/w1/slaves/w1_ds2431.c |   1 -
 drivers/w1/slaves/w1_ds2433.c |   1 -
 fs/bpkfs.c                    |   4 +-
 fs/cramfs/cramfs.c            |   7 --
 fs/devfs-core.c               | 110 +++++++++++++++++++++---------
 fs/devfs.c                    |  14 ++--
 fs/efi.c                      |   6 +-
 fs/efivarfs.c                 |   8 ---
 fs/ext4/ext_barebox.c         |   8 ---
 fs/fat/fat.c                  |   5 +-
 fs/fs.c                       |  33 ++++-----
 fs/nfs.c                      |   5 +-
 fs/omap4_usbbootfs.c          |   7 --
 fs/pstore/fs.c                |   4 +-
 fs/ramfs.c                    |   7 --
 fs/ratpfs.c                   |   9 ---
 fs/smhfs.c                    |  10 ++-
 fs/squashfs/squashfs.c        |   8 ---
 fs/tftp.c                     |  27 +++++---
 fs/ubifs/ubifs.c              |   8 ---
 fs/uimagefs.c                 |   4 +-
 include/driver.h              |   9 +--
 include/fs.h                  |   2 +-
 lib/libfile.c                 |   3 +-
 lib/misc.c                    |  42 ++++++++++++
 62 files changed, 283 insertions(+), 359 deletions(-)
 delete mode 100644 commands/mem.c
 create mode 100644 drivers/misc/mem.c

-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 01/19] commands: Move mem_parse_options() to lib/misc.c

[Andrey Smirnov]

As a first step of de-cluttering /dev/mem related code, move
mem_parse_options() out of commands/mem.c into lib/misc.c where it
seem to fit better. With this change we no longer explicitly turn this
code off using CONFIG_COMPILE_MEMORY and instead rely on LTO to get
rid of it when it's not being used.

While at it, also fix return value by replacing COMMAND_ERROR_USAGE
with -EINVAL. All of the callers of mem_parse_options() expect
negative error code as a sign of failure and COMMAND_ERROR_USAGE is
not negative.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/mem.c | 40 ----------------------------------------
 lib/misc.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/commands/mem.c b/commands/mem.c
index a9e12f3e5..62488bf52 100644
--- a/commands/mem.c
+++ b/commands/mem.c
@@ -41,46 +41,6 @@
 
 char *mem_rw_buf;
 
-/*
- * Common function for parsing options for the 'md', 'mw', 'memcpy', 'memcmp'
- * commands.
- */
-int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
-		char **sourcefile, char **destfile, int *swab)
-{
-	int opt;
-
-	while((opt = getopt(argc, argv, optstr)) > 0) {
-		switch(opt) {
-		case 'b':
-			*mode = O_RWSIZE_1;
-			break;
-		case 'w':
-			*mode = O_RWSIZE_2;
-			break;
-		case 'l':
-			*mode = O_RWSIZE_4;
-			break;
-		case 'q':
-			*mode = O_RWSIZE_8;
-			break;
-		case 's':
-			*sourcefile = optarg;
-			break;
-		case 'd':
-			*destfile = optarg;
-			break;
-		case 'x':
-			*swab = 1;
-			break;
-		default:
-			return COMMAND_ERROR_USAGE;
-		}
-	}
-
-	return 0;
-}
-
 static struct cdev_operations memops = {
 	.read  = mem_read,
 	.write = mem_write,
diff --git a/lib/misc.c b/lib/misc.c
index 1d20e1b09..cd420a57d 100644
--- a/lib/misc.c
+++ b/lib/misc.c
@@ -23,6 +23,7 @@
 #include <fs.h>
 #include <string.h>
 #include <linux/ctype.h>
+#include <getopt.h>
 
 /*
  * Like simple_strtoull() but handles an optional G, M, K or k
@@ -129,3 +130,44 @@ success:
 	return 0;
 }
 EXPORT_SYMBOL(parse_area_spec);
+
+/*
+ * Common function for parsing options for the 'md', 'mw', 'memcpy', 'memcmp'
+ * commands.
+ */
+int mem_parse_options(int argc, char *argv[], char *optstr, int *mode,
+		      char **sourcefile, char **destfile, int *swab)
+{
+	int opt;
+
+	while((opt = getopt(argc, argv, optstr)) > 0) {
+		switch(opt) {
+		case 'b':
+			*mode = O_RWSIZE_1;
+			break;
+		case 'w':
+			*mode = O_RWSIZE_2;
+			break;
+		case 'l':
+			*mode = O_RWSIZE_4;
+			break;
+		case 'q':
+			*mode = O_RWSIZE_8;
+			break;
+		case 's':
+			*sourcefile = optarg;
+			break;
+		case 'd':
+			*destfile = optarg;
+			break;
+		case 'x':
+			*swab = 1;
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 02/19] commands: Get rid of mem_rw_buf

[Andrey Smirnov]

There doesn't seem to be any good reason for all of the memory
commands (md, mw, etc.) to rely on a shared pre-allocated buffer
anymore. So, to simplify things, drop the shared buffer and adjust all
of the utilites to allocate needed memory.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/md.c     | 12 +++++++-----
 commands/mem.c    |  6 ------
 commands/memcmp.c | 16 ++++++++--------
 commands/memcpy.c | 10 ++++++----
 commands/memset.c |  2 --
 common/ratp/md.c  |  9 +++++----
 6 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/commands/md.c b/commands/md.c
index a495fc8b4..2389c12d1 100644
--- a/commands/md.c
+++ b/commands/md.c
@@ -34,8 +34,6 @@
 #include <linux/stat.h>
 #include <xfuncs.h>
 
-extern char *mem_rw_buf;
-
 static int do_mem_md(int argc, char *argv[])
 {
 	loff_t	start = 0, size = 0x100;
@@ -46,6 +44,7 @@ static int do_mem_md(int argc, char *argv[])
 	int mode = O_RWSIZE_4;
 	int swab = 0;
 	void *map;
+	void *buf = NULL;
 
 	if (argc < 2)
 		return COMMAND_ERROR_USAGE;
@@ -74,9 +73,11 @@ static int do_mem_md(int argc, char *argv[])
 		goto out;
 	}
 
+	buf = xmalloc(RW_BUF_SIZE);
+
 	do {
 		now = min(size, (loff_t)RW_BUF_SIZE);
-		r = read(fd, mem_rw_buf, now);
+		r = read(fd, buf, now);
 		if (r < 0) {
 			perror("read");
 			goto out;
@@ -84,8 +85,8 @@ static int do_mem_md(int argc, char *argv[])
 		if (!r)
 			goto out;
 
-		if ((ret = memory_display(mem_rw_buf, start, r,
-				mode >> O_RWSIZE_SHIFT, swab)))
+		if ((ret = memory_display(buf, start, r,
+					  mode >> O_RWSIZE_SHIFT, swab)))
 			goto out;
 
 		start += r;
@@ -93,6 +94,7 @@ static int do_mem_md(int argc, char *argv[])
 	} while (size);
 
 out:
+	free(buf);
 	close(fd);
 
 	return ret ? 1 : 0;
diff --git a/commands/mem.c b/commands/mem.c
index 62488bf52..8a47e1fe1 100644
--- a/commands/mem.c
+++ b/commands/mem.c
@@ -39,8 +39,6 @@
 #define PRINTF(fmt,args...)
 #endif
 
-char *mem_rw_buf;
-
 static struct cdev_operations memops = {
 	.read  = mem_read,
 	.write = mem_write,
@@ -73,10 +71,6 @@ static struct driver_d mem_drv = {
 
 static int mem_init(void)
 {
-	mem_rw_buf = malloc(RW_BUF_SIZE);
-	if(!mem_rw_buf)
-		return -ENOMEM;
-
 	add_mem_device("mem", 0, ~0, IORESOURCE_MEM_WRITEABLE);
 	return platform_driver_register(&mem_drv);
 }
diff --git a/commands/memcmp.c b/commands/memcmp.c
index 981c8cb38..48957b450 100644
--- a/commands/memcmp.c
+++ b/commands/memcmp.c
@@ -34,8 +34,6 @@
 #include <linux/stat.h>
 #include <xfuncs.h>
 
-extern char *mem_rw_buf;
-
 static char *devmem = "/dev/mem";
 
 static int do_memcmp(int argc, char *argv[])
@@ -45,7 +43,7 @@ static int do_memcmp(int argc, char *argv[])
 	char   *sourcefile = devmem;
 	char   *destfile = devmem;
 	int     sourcefd, destfd;
-	char   *rw_buf1;
+	char   *buf, *source_data, *dest_data;
 	int     ret = 1;
 	int     offset = 0;
 	struct  stat statbuf;
@@ -84,20 +82,22 @@ static int do_memcmp(int argc, char *argv[])
 		return 1;
 	}
 
-	rw_buf1 = xmalloc(RW_BUF_SIZE);
+	buf = xmalloc(RW_BUF_SIZE + RW_BUF_SIZE);
+	source_data = buf;
+	dest_data   = buf + RW_BUF_SIZE;
 
 	while (count > 0) {
 		int now, r1, r2, i;
 
 		now = min((loff_t)RW_BUF_SIZE, count);
 
-		r1 = read_full(sourcefd, mem_rw_buf, now);
+		r1 = read_full(sourcefd, source_data, now);
 		if (r1 < 0) {
 			perror("read");
 			goto out;
 		}
 
-		r2 = read_full(destfd, rw_buf1, now);
+		r2 = read_full(destfd, dest_data, now);
 		if (r2 < 0) {
 			perror("read");
 			goto out;
@@ -109,7 +109,7 @@ static int do_memcmp(int argc, char *argv[])
 		}
 
 		for (i = 0; i < now; i++) {
-			if (mem_rw_buf[i] != rw_buf1[i]) {
+			if (source_data[i] != dest_data[i]) {
 				printf("files differ at offset %d\n", offset);
 				goto out;
 			}
@@ -124,7 +124,7 @@ static int do_memcmp(int argc, char *argv[])
 out:
 	close(sourcefd);
 	close(destfd);
-	free(rw_buf1);
+	free(buf);
 
 	return ret;
 }
diff --git a/commands/memcpy.c b/commands/memcpy.c
index 168ef3b4f..ddaf767ea 100644
--- a/commands/memcpy.c
+++ b/commands/memcpy.c
@@ -34,8 +34,6 @@
 #include <linux/stat.h>
 #include <xfuncs.h>
 
-extern char *mem_rw_buf;
-
 static char *devmem = "/dev/mem";
 
 static int do_memcpy(int argc, char *argv[])
@@ -47,6 +45,7 @@ static int do_memcpy(int argc, char *argv[])
 	int mode = 0;
 	struct stat statbuf;
 	int ret = 0;
+	char *buf;
 
 	if (mem_parse_options(argc, argv, "bwlqs:d:", &mode, &sourcefile,
 			&destfile, NULL) < 0)
@@ -82,12 +81,14 @@ static int do_memcpy(int argc, char *argv[])
 		return 1;
 	}
 
+	buf = xmalloc(RW_BUF_SIZE);
+
 	while (count > 0) {
 		int now, r, w, tmp;
 
 		now = min((loff_t)RW_BUF_SIZE, count);
 
-		r = read(sourcefd, mem_rw_buf, now);
+		r = read(sourcefd, buf, now);
 		if (r < 0) {
 			perror("read");
 			goto out;
@@ -99,7 +100,7 @@ static int do_memcpy(int argc, char *argv[])
 		tmp = 0;
 		now = r;
 		while (now) {
-			w = write(destfd, mem_rw_buf + tmp, now);
+			w = write(destfd, buf + tmp, now);
 			if (w < 0) {
 				perror("write");
 				goto out;
@@ -123,6 +124,7 @@ static int do_memcpy(int argc, char *argv[])
 	}
 
 out:
+	free(buf);
 	close(sourcefd);
 	close(destfd);
 
diff --git a/commands/memset.c b/commands/memset.c
index f99bf86c0..b0770159f 100644
--- a/commands/memset.c
+++ b/commands/memset.c
@@ -34,8 +34,6 @@
 #include <linux/stat.h>
 #include <xfuncs.h>
 
-extern char *mem_rw_buf;
-
 static int do_memset(int argc, char *argv[])
 {
 	loff_t	s, c, n;
diff --git a/common/ratp/md.c b/common/ratp/md.c
index ce343d7c7..a25cbf112 100644
--- a/common/ratp/md.c
+++ b/common/ratp/md.c
@@ -59,8 +59,6 @@ struct ratp_bb_md_response {
 	uint8_t  buffer[];
 } __packed;
 
-extern char *mem_rw_buf;
-
 static int do_ratp_mem_md(const char *filename,
 			  loff_t start,
 			  loff_t size,
@@ -70,6 +68,7 @@ static int do_ratp_mem_md(const char *filename,
 	int ret = 0;
 	int fd;
 	void *map;
+	char *buf = NULL;
 
 	fd = open_and_lseek(filename, O_RWSIZE_1 | O_RDONLY, start);
 	if (fd < 0)
@@ -81,10 +80,11 @@ static int do_ratp_mem_md(const char *filename,
 		goto out;
 	}
 
+	buf = xmalloc(RW_BUF_SIZE);
 	t = 0;
 	do {
 		now = min(size, (loff_t)RW_BUF_SIZE);
-		r = read(fd, mem_rw_buf, now);
+		r = read(fd, buf, now);
 		if (r < 0) {
 			ret = -errno;
 			perror("read");
@@ -93,13 +93,14 @@ static int do_ratp_mem_md(const char *filename,
 		if (!r)
 			goto out;
 
-		memcpy(output + t, (uint8_t *)(mem_rw_buf), r);
+		memcpy(output + t, buf, r);
 
 		size  -= r;
 		t     += r;
 	} while (size);
 
 out:
+	free(buf);
 	close(fd);
 
 	return ret;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 03/19] commands: Move /dev/mem driver to drivers/misc

[Andrey Smirnov]

With all other code gone from commands/mem.c, move it into
driver/misc, where it fits better. While at it, expose it directly via
a Kconfig options instead of relying on CONFIG_COMPILE_MEMORY

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/Kconfig      | 17 ++++------
 commands/Makefile     |  1 -
 commands/mem.c        | 77 -------------------------------------------
 drivers/misc/Kconfig  |  3 ++
 drivers/misc/Makefile |  1 +
 drivers/misc/mem.c    | 45 +++++++++++++++++++++++++
 6 files changed, 55 insertions(+), 89 deletions(-)
 delete mode 100644 commands/mem.c
 create mode 100644 drivers/misc/mem.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 45592f26c..901036f59 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -14,11 +14,6 @@ config COMPILE_HASH
 	help
 	  Turns on compilation of digest.c
 
-config COMPILE_MEMORY
-	bool
-	help
-	  Turns on compilation of mem.c
-
 menu "Commands"
 
 
@@ -1493,7 +1488,7 @@ config CMD_CRC_CMP
 config CMD_MD
 	tristate
 	default y
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "md"
 	help
 	  Memory display
@@ -1517,7 +1512,7 @@ config CMD_MD
 config CMD_MEMCMP
 	tristate
 	default y
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "memcmp"
 	help
 	  Memory compare
@@ -1539,7 +1534,7 @@ config CMD_MEMCMP
 config CMD_MEMCPY
 	tristate
 	default y
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "memcpy"
 	help
 	  Memory copy
@@ -1558,7 +1553,7 @@ config CMD_MEMCPY
 config CMD_MEMSET
 	tristate
 	default y
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "memset"
 	help
 	  Memory fill
@@ -1591,7 +1586,7 @@ config CMD_MEMTEST
 
 config CMD_MM
 	tristate
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "memory modify (mm)"
 	help
 	  Memory modify with mask
@@ -1609,7 +1604,7 @@ config CMD_MM
 config CMD_MW
 	tristate
 	default y
-	select COMPILE_MEMORY
+	select DEV_MEM
 	prompt "mw"
 	help
 	  Memory write
diff --git a/commands/Makefile b/commands/Makefile
index ecd2c99e1..ce4c4f6eb 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -1,7 +1,6 @@
 obj-$(CONFIG_STDDEV)		+= stddev.o
 obj-$(CONFIG_CMD_DIGEST)	+= digest.o
 obj-$(CONFIG_COMPILE_HASH)	+= hashsum.o
-obj-$(CONFIG_COMPILE_MEMORY)	+= mem.o
 obj-$(CONFIG_CMD_BOOTM)		+= bootm.o
 obj-$(CONFIG_CMD_UIMAGE)	+= uimage.o
 obj-$(CONFIG_CMD_LINUX16)	+= linux16.o
diff --git a/commands/mem.c b/commands/mem.c
deleted file mode 100644
index 8a47e1fe1..000000000
--- a/commands/mem.c
+++ /dev/null
@@ -1,77 +0,0 @@
-/*
- * Copyright (c) 2011 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- */
-
-/*
- * Memory Functions
- *
- * Copied from FADS ROM, Dan Malek (dmalek@jlc.net)
- */
-
-#include <common.h>
-#include <command.h>
-#include <init.h>
-#include <driver.h>
-#include <malloc.h>
-#include <errno.h>
-#include <fs.h>
-#include <fcntl.h>
-#include <getopt.h>
-#include <linux/stat.h>
-#include <xfuncs.h>
-
-#ifdef	CMD_MEM_DEBUG
-#define	PRINTF(fmt,args...)	printf (fmt ,##args)
-#else
-#define PRINTF(fmt,args...)
-#endif
-
-static struct cdev_operations memops = {
-	.read  = mem_read,
-	.write = mem_write,
-	.memmap = generic_memmap_rw,
-	.lseek = dev_lseek_default,
-};
-
-static int mem_probe(struct device_d *dev)
-{
-	struct cdev *cdev;
-
-	cdev = xzalloc(sizeof (*cdev));
-	dev->priv = cdev;
-
-	cdev->name = (char*)dev->resource[0].name;
-	cdev->size = min_t(unsigned long long, resource_size(&dev->resource[0]),
-			   S64_MAX);
-	cdev->ops = &memops;
-	cdev->dev = dev;
-
-	devfs_create(cdev);
-
-	return 0;
-}
-
-static struct driver_d mem_drv = {
-	.name  = "mem",
-	.probe = mem_probe,
-};
-
-static int mem_init(void)
-{
-	add_mem_device("mem", 0, ~0, IORESOURCE_MEM_WRITEABLE);
-	return platform_driver_register(&mem_drv);
-}
-device_initcall(mem_init);
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index df74cca97..f547024bd 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -24,4 +24,7 @@ config BLINK_ENCODER
         tristate "Blinking LED encoder"
 	depends on LED && LED_TRIGGERS
 
+config DEV_MEM
+        bool "Generic memory I/O device (/dev/mem)"
+
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 2e80f1a63..8b5140536 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_JTAG)		+= jtag.o
 obj-$(CONFIG_SRAM)		+= sram.o
 obj-$(CONFIG_STATE_DRV)		+= state.o
 obj-$(CONFIG_BLINK_ENCODER)	+= blink-encoder.o
+obj-$(CONFIG_DEV_MEM)		+= mem.o
diff --git a/drivers/misc/mem.c b/drivers/misc/mem.c
new file mode 100644
index 000000000..d829af724
--- /dev/null
+++ b/drivers/misc/mem.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2011 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <init.h>
+
+static struct cdev_operations memops = {
+	.read  = mem_read,
+	.write = mem_write,
+	.memmap = generic_memmap_rw,
+	.lseek = dev_lseek_default,
+};
+
+static int mem_probe(struct device_d *dev)
+{
+	struct cdev *cdev;
+
+	cdev = xzalloc(sizeof (*cdev));
+	dev->priv = cdev;
+
+	cdev->name = (char*)dev->resource[0].name;
+	cdev->size = min_t(unsigned long long, resource_size(&dev->resource[0]),
+			   S64_MAX);
+	cdev->ops = &memops;
+	cdev->dev = dev;
+
+	devfs_create(cdev);
+
+	return 0;
+}
+
+static struct driver_d mem_drv = {
+	.name  = "mem",
+	.probe = mem_probe,
+};
+
+static int mem_init(void)
+{
+	add_mem_device("mem", 0, ~0, IORESOURCE_MEM_WRITEABLE);
+	return platform_driver_register(&mem_drv);
+}
+device_initcall(mem_init);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 04/19] nvmem: Do not use DEVFS_IS_CHARACTER_DEV

[Andrey Smirnov]

There doesn't appear any good reason to mark NVMEM cdevs as
DEVFS_IS_CHARACTER_DEV, since NVMEM devices should be able to
read/write data at arbitrary offsets and are not different from any
other cdevs representing non-volatile storage.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/nvmem/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 63c0f997b..9fd599095 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -96,7 +96,6 @@ static int nvmem_register_cdev(struct nvmem_device *nvmem, const char *name)
 	alias = of_alias_get(dev->device_node);
 
 	nvmem->cdev.name = xstrdup(alias ?: name);
-	nvmem->cdev.flags = DEVFS_IS_CHARACTER_DEV;
 	nvmem->cdev.ops = &nvmem_chrdev_ops;
 	nvmem->cdev.dev = &nvmem->dev;
 	nvmem->cdev.size = nvmem->size;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 05/19] common: firmware: Don't use FILE_SIZE_STREAM directly

[Andrey Smirnov]

Don't use FILE_SIZE_STREAM directly, use DEVFS_IS_CHARACTER_DEV to
denote that underlying cdev is a "character device".

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 common/firmware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/firmware.c b/common/firmware.c
index 250fef537..9d55d73e7 100644
--- a/common/firmware.c
+++ b/common/firmware.c
@@ -177,7 +177,7 @@ int firmwaremgr_register(struct firmware_handler *fh)
 	cdev = &mgr->cdev;
 
 	cdev->name = xstrdup(fh->id);
-	cdev->size = FILE_SIZE_STREAM;
+	cdev->flags = DEVFS_IS_CHARACTER_DEV;
 	cdev->ops = &firmware_ops;
 	cdev->priv = mgr;
 	cdev->dev = fh->dev;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 06/19] devfs: Fix incorrect error check for cdev->ops->lseek()

[Andrey Smirnov]

Cdev->ops->lseek() will either return a negative error code on failure
or requested position on success. In order to properly check for
errors we need to test if the return value is negative, not just that
it's not -1.

Returning ret - cdev->offset, doesn't appear to be correct either,
even if ret is -1, since on failure this will lead us to return (-1 -
cdev->offset). Simplify that part by just returning 'pos', which is
what we'd end up returning on success in original code as well.

Third, make sure to return -ENOSYS, when no .lseek() callback is
provided.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/devfs.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/fs/devfs.c b/fs/devfs.c
index 81ae2c25a..ae5e6475b 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -60,15 +60,19 @@ static int devfs_write(struct device_d *_dev, FILE *f, const void *buf, size_t s
 static loff_t devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
 {
 	struct cdev *cdev = f->priv;
-	loff_t ret = -1;
+	loff_t ret;
 
-	if (cdev->ops->lseek)
+	if (cdev->ops->lseek) {
 		ret = cdev->ops->lseek(cdev, pos + cdev->offset);
+		if (ret < 0)
+			return ret;
+	} else {
+		return -ENOSYS;
+	}
 
-	if (ret != -1)
-		f->pos = pos;
+	f->pos = pos;
 
-	return ret - cdev->offset;
+	return pos;
 }
 
 static int devfs_erase(struct device_d *_dev, FILE *f, loff_t count, loff_t offset)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 07/19] fs: Update FILE position in lseek()

[Andrey Smirnov]

Instead on relying on driver callbacks to update 'pos' in FILE, do it
as a part of lseek() code. This allows us to drop a bit of repeating
code as well as making lseek() implementation consistent with write()
and read().

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/cramfs/cramfs.c     |  3 +--
 fs/devfs.c             |  2 --
 fs/efi.c               |  4 +---
 fs/efivarfs.c          |  4 +---
 fs/ext4/ext_barebox.c  |  4 +---
 fs/fat/fat.c           |  1 -
 fs/fs.c                |  2 ++
 fs/nfs.c               |  3 +--
 fs/omap4_usbbootfs.c   |  1 -
 fs/ramfs.c             |  3 +--
 fs/ratpfs.c            |  3 +--
 fs/smhfs.c             |  8 +++-----
 fs/squashfs/squashfs.c |  2 --
 fs/tftp.c              | 17 +++++++++++++----
 fs/ubifs/ubifs.c       |  2 --
 15 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/fs/cramfs/cramfs.c b/fs/cramfs/cramfs.c
index 093657f62..1cce04fce 100644
--- a/fs/cramfs/cramfs.c
+++ b/fs/cramfs/cramfs.c
@@ -168,8 +168,7 @@ static int cramfs_read(struct device_d *_dev, FILE *f, void *buf, size_t size)
 
 static loff_t cramfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-	return f->pos;
+	return pos;
 }
 
 #if 0
diff --git a/fs/devfs.c b/fs/devfs.c
index ae5e6475b..6acbbd7ad 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -70,8 +70,6 @@ static loff_t devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
 		return -ENOSYS;
 	}
 
-	f->pos = pos;
-
 	return pos;
 }
 
diff --git a/fs/efi.c b/fs/efi.c
index 692556b26..074ef6b53 100644
--- a/fs/efi.c
+++ b/fs/efi.c
@@ -297,14 +297,12 @@ static loff_t efifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 	struct efifs_file *ufile = f->priv;
 	efi_status_t efiret;
 
-	f->pos = pos;
-
 	efiret = ufile->entry->set_position(ufile->entry, pos);
 	if (EFI_ERROR(efiret)) {
 		return -efi_errno(efiret);
 	}
 
-	return f->pos;
+	return pos;
 }
 
 static int efifs_truncate(struct device_d *dev, FILE *f, unsigned long size)
diff --git a/fs/efivarfs.c b/fs/efivarfs.c
index bf7351e6d..34a261935 100644
--- a/fs/efivarfs.c
+++ b/fs/efivarfs.c
@@ -309,9 +309,7 @@ static int efivarfs_truncate(struct device_d *dev, FILE *f, ulong size)
 
 static loff_t efivarfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-
-	return f->pos;
+	return pos;
 }
 
 static DIR *efivarfs_opendir(struct device_d *dev, const char *pathname)
diff --git a/fs/ext4/ext_barebox.c b/fs/ext4/ext_barebox.c
index 1e7da2a4b..6e41b8345 100644
--- a/fs/ext4/ext_barebox.c
+++ b/fs/ext4/ext_barebox.c
@@ -61,9 +61,7 @@ static int ext_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
 
 static loff_t ext_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-
-	return f->pos;
+	return pos;
 }
 
 static struct inode *ext_alloc_inode(struct super_block *sb)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index 49cd78ff9..ee7751e94 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -277,7 +277,6 @@ static loff_t fat_lseek(struct device_d *dev, FILE *f, loff_t pos)
 	if (ret)
 		return ret;
 
-	f->pos = pos;
 	return pos;
 }
 
diff --git a/fs/fs.c b/fs/fs.c
index a304bf186..7729fe8b1 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -448,6 +448,8 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 		return -1;
 	}
 
+	f->pos = pos;
+
 	return pos;
 
 out:
diff --git a/fs/nfs.c b/fs/nfs.c
index 07a82ec60..d83f25007 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -1092,10 +1092,9 @@ static loff_t nfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 {
 	struct file_priv *priv = file->priv;
 
-	file->pos = pos;
 	kfifo_reset(priv->fifo);
 
-	return file->pos;
+	return pos;
 }
 
 static int nfs_iterate(struct file *file, struct dir_context *ctx)
diff --git a/fs/omap4_usbbootfs.c b/fs/omap4_usbbootfs.c
index b35f411cb..51038c705 100644
--- a/fs/omap4_usbbootfs.c
+++ b/fs/omap4_usbbootfs.c
@@ -151,7 +151,6 @@ static int omap4_usbbootfs_read(
 
 static loff_t omap4_usbbootfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
 	return pos;
 }
 
diff --git a/fs/ramfs.c b/fs/ramfs.c
index 84ecfa0dd..f571cd5ca 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -349,8 +349,7 @@ static int ramfs_write(struct device_d *_dev, FILE *f, const void *buf, size_t i
 
 static loff_t ramfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-	return f->pos;
+	return pos;
 }
 
 static int ramfs_truncate(struct device_d *dev, FILE *f, ulong size)
diff --git a/fs/ratpfs.c b/fs/ratpfs.c
index 902289bab..e316289d7 100644
--- a/fs/ratpfs.c
+++ b/fs/ratpfs.c
@@ -288,8 +288,7 @@ static loff_t ratpfs_lseek(struct device_d __always_unused *dev,
 			  FILE *f, loff_t pos)
 {
 	pr_debug("%s\n", __func__);
-	f->pos = pos;
-	return f->pos;
+	return pos;
 }
 
 static DIR* ratpfs_opendir(struct device_d __always_unused *dev,
diff --git a/fs/smhfs.c b/fs/smhfs.c
index f1b6d6bb1..18eaa9dfc 100644
--- a/fs/smhfs.c
+++ b/fs/smhfs.c
@@ -112,12 +112,10 @@ static int smhfs_read(struct device_d __always_unused *dev,
 static loff_t smhfs_lseek(struct device_d __always_unused *dev,
 			  FILE *f, loff_t pos)
 {
-	if (semihosting_seek(file_to_fd(f), pos)) {
+	if (semihosting_seek(file_to_fd(f), pos))
 		return -semihosting_errno();
-	} else {
-		f->pos = pos;
-		return f->pos;
-	}
+
+	return pos;
 }
 
 static DIR* smhfs_opendir(struct device_d __always_unused *dev,
diff --git a/fs/squashfs/squashfs.c b/fs/squashfs/squashfs.c
index d9049b752..87b182a78 100644
--- a/fs/squashfs/squashfs.c
+++ b/fs/squashfs/squashfs.c
@@ -233,8 +233,6 @@ static int squashfs_read(struct device_d *_dev, FILE *f, void *buf,
 
 static loff_t squashfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-
 	return pos;
 }
 
diff --git a/fs/tftp.c b/fs/tftp.c
index 1b50ba84f..f9e204db5 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -576,12 +576,14 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 static loff_t tftp_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
 	/* We cannot seek backwards without reloading or caching the file */
-	if (pos >= f->pos) {
+	loff_t f_pos = f->pos;
+
+	if (pos >= f_pos) {
 		loff_t ret;
 		char *buf = xmalloc(1024);
 
-		while (pos > f->pos) {
-			size_t len = min_t(size_t, 1024, pos - f->pos);
+		while (pos > f_pos) {
+			size_t len = min_t(size_t, 1024, pos - f_pos);
 
 			ret = tftp_read(dev, f, buf, len);
 
@@ -591,13 +593,20 @@ static loff_t tftp_lseek(struct device_d *dev, FILE *f, loff_t pos)
 			if (ret < 0)
 				goto out_free;
 
-			f->pos += ret;
+			f_pos += ret;
 		}
 
 		ret = pos;
 
 out_free:
 		free(buf);
+		if (ret < 0) {
+			/*
+			 * Update f->pos even if the overall request
+			 * failed since we can't move backwards
+			 */
+			f->pos = f_pos;
+		}
 		return ret;
 	}
 
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index 7545dd4c9..ec6d00890 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -396,8 +396,6 @@ static int ubifs_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
 
 static loff_t ubifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
-	f->pos = pos;
-
 	return pos;
 }
 
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 08/19] fs: Drop trivial .lseek() implementaitons in FS drivers

[Andrey Smirnov]

There are no FS drivers that do not implement .lseek callback in the
codebase, so there doesn't seem to exist a use-case where lseek()
would return -ENOSYS due to fsdrv->lseek being NULL. At the same time
a large number of FS drivers implement only the most basic "always
succeeds" custom .lseek() hook.

Change the code of lseek() to treat absense of .lseek() to mean that
no special actions needs to be taken by FS driver and seek is always
successful and drop all of the trivial .lseek() implementations.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/cramfs/cramfs.c     |  6 ------
 fs/efivarfs.c          |  6 ------
 fs/ext4/ext_barebox.c  |  6 ------
 fs/fs.c                | 14 ++++++--------
 fs/omap4_usbbootfs.c   |  6 ------
 fs/ramfs.c             |  6 ------
 fs/ratpfs.c            |  8 --------
 fs/squashfs/squashfs.c |  6 ------
 fs/ubifs/ubifs.c       |  6 ------
 9 files changed, 6 insertions(+), 58 deletions(-)

diff --git a/fs/cramfs/cramfs.c b/fs/cramfs/cramfs.c
index 1cce04fce..99cbdb920 100644
--- a/fs/cramfs/cramfs.c
+++ b/fs/cramfs/cramfs.c
@@ -166,11 +166,6 @@ static int cramfs_read(struct device_d *_dev, FILE *f, void *buf, size_t size)
 	return cramfs_read_file(f->f_inode, f->pos, buf, size);
 }
 
-static loff_t cramfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 #if 0
 static int cramfs_info (struct device_d *dev)
 {
@@ -490,7 +485,6 @@ static void cramfs_remove(struct device_d *dev)
 
 static struct fs_driver_d cramfs_driver = {
 	.read		= cramfs_read,
-	.lseek		= cramfs_lseek,
 	.drv = {
 		.probe = cramfs_probe,
 		.remove = cramfs_remove,
diff --git a/fs/efivarfs.c b/fs/efivarfs.c
index 34a261935..d2615774e 100644
--- a/fs/efivarfs.c
+++ b/fs/efivarfs.c
@@ -307,11 +307,6 @@ static int efivarfs_truncate(struct device_d *dev, FILE *f, ulong size)
 	return 0;
 }
 
-static loff_t efivarfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 static DIR *efivarfs_opendir(struct device_d *dev, const char *pathname)
 {
 	struct efivarfs_priv *priv = dev->priv;
@@ -435,7 +430,6 @@ static struct fs_driver_d efivarfs_driver = {
 	.read      = efivarfs_read,
 	.write     = efivarfs_write,
 	.truncate  = efivarfs_truncate,
-	.lseek     = efivarfs_lseek,
 	.opendir   = efivarfs_opendir,
 	.readdir   = efivarfs_readdir,
 	.closedir  = efivarfs_closedir,
diff --git a/fs/ext4/ext_barebox.c b/fs/ext4/ext_barebox.c
index 6e41b8345..82d4c581e 100644
--- a/fs/ext4/ext_barebox.c
+++ b/fs/ext4/ext_barebox.c
@@ -59,11 +59,6 @@ static int ext_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
 	return ext4fs_read_file(node, f->pos, insize, buf);
 }
 
-static loff_t ext_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 static struct inode *ext_alloc_inode(struct super_block *sb)
 {
 	struct fs_device_d *fsdev = container_of(sb, struct fs_device_d, sb);
@@ -302,7 +297,6 @@ static void ext_remove(struct device_d *dev)
 
 static struct fs_driver_d ext_driver = {
 	.read      = ext_read,
-	.lseek     = ext_lseek,
 	.type      = filetype_ext,
 	.flags     = 0,
 	.drv = {
diff --git a/fs/fs.c b/fs/fs.c
index 7729fe8b1..34de0669a 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -413,10 +413,6 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 
 	f = &files[fildes];
 	fsdrv = f->fsdev->driver;
-	if (!fsdrv->lseek) {
-		ret = -ENOSYS;
-		goto out;
-	}
 
 	ret = -EINVAL;
 
@@ -442,10 +438,12 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 		goto out;
 	}
 
-	pos = fsdrv->lseek(&f->fsdev->dev, f, pos);
-	if (IS_ERR_VALUE(pos)) {
-		errno = -pos;
-		return -1;
+	if (fsdrv->lseek) {
+		pos = fsdrv->lseek(&f->fsdev->dev, f, pos);
+		if (IS_ERR_VALUE(pos)) {
+			errno = -pos;
+			return -1;
+		}
 	}
 
 	f->pos = pos;
diff --git a/fs/omap4_usbbootfs.c b/fs/omap4_usbbootfs.c
index 51038c705..cc33287b9 100644
--- a/fs/omap4_usbbootfs.c
+++ b/fs/omap4_usbbootfs.c
@@ -149,11 +149,6 @@ static int omap4_usbbootfs_read(
 	return size;
 }
 
-static loff_t omap4_usbbootfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 static DIR *omap4_usbbootfs_opendir(struct device_d *dev, const char *pathname)
 {
 	return NULL;
@@ -190,7 +185,6 @@ static struct fs_driver_d omap4_usbbootfs_driver = {
 	.open    = omap4_usbbootfs_open,
 	.close   = omap4_usbbootfs_close,
 	.read    = omap4_usbbootfs_read,
-	.lseek   = omap4_usbbootfs_lseek,
 	.opendir = omap4_usbbootfs_opendir,
 	.stat    = omap4_usbbootfs_stat,
 /*
diff --git a/fs/ramfs.c b/fs/ramfs.c
index f571cd5ca..153b9b614 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -347,11 +347,6 @@ static int ramfs_write(struct device_d *_dev, FILE *f, const void *buf, size_t i
 	return insize;
 }
 
-static loff_t ramfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 static int ramfs_truncate(struct device_d *dev, FILE *f, ulong size)
 {
 	struct inode *inode = f->f_inode;
@@ -447,7 +442,6 @@ static void ramfs_remove(struct device_d *dev)
 static struct fs_driver_d ramfs_driver = {
 	.read      = ramfs_read,
 	.write     = ramfs_write,
-	.lseek     = ramfs_lseek,
 	.truncate  = ramfs_truncate,
 	.flags     = FS_DRIVER_NO_DEV,
 	.drv = {
diff --git a/fs/ratpfs.c b/fs/ratpfs.c
index e316289d7..dffd27654 100644
--- a/fs/ratpfs.c
+++ b/fs/ratpfs.c
@@ -284,13 +284,6 @@ out:
 	return ret;
 }
 
-static loff_t ratpfs_lseek(struct device_d __always_unused *dev,
-			  FILE *f, loff_t pos)
-{
-	pr_debug("%s\n", __func__);
-	return pos;
-}
-
 static DIR* ratpfs_opendir(struct device_d __always_unused *dev,
 			  const char *pathname)
 {
@@ -449,7 +442,6 @@ static struct fs_driver_d ratpfs_driver = {
 	.open      = ratpfs_open,
 	.close     = ratpfs_close,
 	.read      = ratpfs_read,
-	.lseek     = ratpfs_lseek,
 	.opendir   = ratpfs_opendir,
 	.readdir   = ratpfs_readdir,
 	.closedir  = ratpfs_closedir,
diff --git a/fs/squashfs/squashfs.c b/fs/squashfs/squashfs.c
index 87b182a78..38aff6d5b 100644
--- a/fs/squashfs/squashfs.c
+++ b/fs/squashfs/squashfs.c
@@ -231,11 +231,6 @@ static int squashfs_read(struct device_d *_dev, FILE *f, void *buf,
 	return insize;
 }
 
-static loff_t squashfs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 struct squashfs_dir {
 	struct file file;
 	struct dentry dentry;
@@ -251,7 +246,6 @@ static struct fs_driver_d squashfs_driver = {
 	.open		= squashfs_open,
 	.close		= squashfs_close,
 	.read		= squashfs_read,
-	.lseek		= squashfs_lseek,
 	.type		= filetype_squashfs,
 	.drv = {
 		.probe = squashfs_probe,
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
index ec6d00890..494b1f261 100644
--- a/fs/ubifs/ubifs.c
+++ b/fs/ubifs/ubifs.c
@@ -394,11 +394,6 @@ static int ubifs_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
 	return insize;
 }
 
-static loff_t ubifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
-{
-	return pos;
-}
-
 static void ubifs_set_rootarg(struct ubifs_priv *priv, struct fs_device_d *fsdev)
 {
 	struct ubi_volume_info vi = {};
@@ -475,7 +470,6 @@ static struct fs_driver_d ubifs_driver = {
 	.open      = ubifs_open,
 	.close     = ubifs_close,
 	.read      = ubifs_read,
-	.lseek     = ubifs_lseek,
 	.type = filetype_ubifs,
 	.flags     = 0,
 	.drv = {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 09/19] devfs: Drop dev_lseek_default()

[Andrey Smirnov]

Only the following cdevs do not declare an .lseek() operation:

  - Console devices in common/console.c
  - Firmware framework in common/firmware.c
  - JTAG driver in drivers/misc/jtag.c
  - UBI in drivers/mtd/ubi/barebox.c

Of those four, first two are marked DEVFS_IS_CHARACTER_DEV and
implement only .write() operation and the last two don't implement
anything but .ioctl(). While there's probably no meaningful way to use
lseek() against any of those devices, there doesn't seem to be any
harm in allowing it either.

Change devfs_lseek() to ignore absense of .lseek() callback and drop
dev_lseek_default() and all references to it in the codebase.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 arch/arm/mach-mxs/ocotp.c     | 1 -
 arch/sandbox/board/hostfile.c | 1 -
 commands/stddev.c             | 4 ----
 common/block.c                | 1 -
 drivers/base/regmap/regmap.c  | 1 -
 drivers/eeprom/at24.c         | 1 -
 drivers/eeprom/at25.c         | 1 -
 drivers/hw_random/core.c      | 1 -
 drivers/mfd/act8846.c         | 1 -
 drivers/mfd/lp3972.c          | 1 -
 drivers/mfd/mc34704.c         | 1 -
 drivers/mfd/mc9sdz60.c        | 1 -
 drivers/mfd/stmpe-i2c.c       | 1 -
 drivers/mfd/twl-core.c        | 1 -
 drivers/misc/mem.c            | 1 -
 drivers/misc/sram.c           | 1 -
 drivers/mtd/core.c            | 1 -
 drivers/mtd/mtdoob.c          | 1 -
 drivers/mtd/mtdraw.c          | 1 -
 drivers/net/e1000/eeprom.c    | 2 --
 drivers/net/ksz8864rmn.c      | 1 -
 drivers/net/phy/mdio_bus.c    | 1 -
 drivers/nvmem/core.c          | 1 -
 drivers/video/fb.c            | 1 -
 drivers/w1/slaves/w1_ds2431.c | 1 -
 drivers/w1/slaves/w1_ds2433.c | 1 -
 fs/devfs-core.c               | 1 -
 fs/devfs.c                    | 2 --
 include/driver.h              | 5 -----
 29 files changed, 38 deletions(-)

diff --git a/arch/arm/mach-mxs/ocotp.c b/arch/arm/mach-mxs/ocotp.c
index 01db73116..f230d9ad8 100644
--- a/arch/arm/mach-mxs/ocotp.c
+++ b/arch/arm/mach-mxs/ocotp.c
@@ -174,7 +174,6 @@ free_mem:
 
 static struct cdev_operations mxs_ocotp_ops = {
 	.read	= mxs_ocotp_cdev_read,
-	.lseek	= dev_lseek_default,
 };
 
 static int mxs_ocotp_probe(struct device_d *dev)
diff --git a/arch/sandbox/board/hostfile.c b/arch/sandbox/board/hostfile.c
index 3fc150379..745f078d1 100644
--- a/arch/sandbox/board/hostfile.c
+++ b/arch/sandbox/board/hostfile.c
@@ -67,7 +67,6 @@ static void hf_info(struct device_d *dev)
 static struct cdev_operations hf_fops = {
 	.read  = hf_read,
 	.write = hf_write,
-	.lseek = dev_lseek_default,
 };
 
 static int hf_probe(struct device_d *dev)
diff --git a/commands/stddev.c b/commands/stddev.c
index 4d1b6f510..2b3d084c8 100644
--- a/commands/stddev.c
+++ b/commands/stddev.c
@@ -27,7 +27,6 @@ static ssize_t zero_read(struct cdev *cdev, void *buf, size_t count, loff_t offs
 
 static struct cdev_operations zeroops = {
 	.read  = zero_read,
-	.lseek = dev_lseek_default,
 };
 
 static int zero_init(void)
@@ -55,7 +54,6 @@ static ssize_t full_read(struct cdev *cdev, void *buf, size_t count, loff_t offs
 
 static struct cdev_operations fullops = {
 	.read  = full_read,
-	.lseek = dev_lseek_default,
 };
 
 static int full_init(void)
@@ -82,7 +80,6 @@ static ssize_t null_write(struct cdev *cdev, const void *buf, size_t count, loff
 
 static struct cdev_operations nullops = {
 	.write = null_write,
-	.lseek = dev_lseek_default,
 };
 
 static int null_init(void)
@@ -110,7 +107,6 @@ static ssize_t prng_read(struct cdev *cdev, void *buf, size_t count, loff_t offs
 
 static struct cdev_operations prngops = {
 	.read  = prng_read,
-	.lseek = dev_lseek_default,
 };
 
 static int prng_init(void)
diff --git a/common/block.c b/common/block.c
index 291721876..97cf5dc4d 100644
--- a/common/block.c
+++ b/common/block.c
@@ -349,7 +349,6 @@ static struct cdev_operations block_ops = {
 #endif
 	.close	= block_op_close,
 	.flush	= block_op_flush,
-	.lseek	= dev_lseek_default,
 };
 
 int blockdevice_register(struct block_device *blk)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 8bbc2373f..d2f8ec70e 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -353,7 +353,6 @@ static ssize_t regmap_cdev_write(struct cdev *cdev, const void *buf, size_t coun
 }
 
 static struct cdev_operations regmap_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= regmap_cdev_read,
 	.write	= regmap_cdev_write,
 };
diff --git a/drivers/eeprom/at24.c b/drivers/eeprom/at24.c
index 1f6676eb5..d2fcd645b 100644
--- a/drivers/eeprom/at24.c
+++ b/drivers/eeprom/at24.c
@@ -440,7 +440,6 @@ static int at24_probe(struct device_d *dev)
 	at24->cdev.priv = at24;
 	at24->cdev.dev = dev;
 	at24->cdev.ops = &at24->fops;
-	at24->fops.lseek = dev_lseek_default;
 	at24->fops.read	= at24_cdev_read,
 	at24->fops.protect = at24_cdev_protect,
 	at24->cdev.size = chip.byte_len;
diff --git a/drivers/eeprom/at25.c b/drivers/eeprom/at25.c
index a9050d6c1..1c9ef1232 100644
--- a/drivers/eeprom/at25.c
+++ b/drivers/eeprom/at25.c
@@ -235,7 +235,6 @@ static ssize_t at25_ee_write(struct cdev *cdev,
 static struct cdev_operations at25_fops = {
 	.read	= at25_ee_read,
 	.write	= at25_ee_write,
-	.lseek	= dev_lseek_default,
 };
 
 static int at25_np_to_chip(struct device_d *dev,
diff --git a/drivers/hw_random/core.c b/drivers/hw_random/core.c
index f870a15b5..a1c414984 100644
--- a/drivers/hw_random/core.c
+++ b/drivers/hw_random/core.c
@@ -63,7 +63,6 @@ static ssize_t rng_dev_read(struct cdev *cdev, void *buf, size_t size,
 
 static struct cdev_operations rng_chrdev_ops = {
 	.read  = rng_dev_read,
-	.lseek = dev_lseek_default,
 };
 
 static int hwrng_register_cdev(struct hwrng *rng)
diff --git a/drivers/mfd/act8846.c b/drivers/mfd/act8846.c
index 53ab70f5c..b7a64c739 100644
--- a/drivers/mfd/act8846.c
+++ b/drivers/mfd/act8846.c
@@ -117,7 +117,6 @@ static ssize_t act8846_write(struct cdev *cdev, const void *_buf, size_t count,
 }
 
 static struct cdev_operations act8846_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= act8846_read,
 	.write	= act8846_write,
 };
diff --git a/drivers/mfd/lp3972.c b/drivers/mfd/lp3972.c
index 42b28070a..3ae9d1ac6 100644
--- a/drivers/mfd/lp3972.c
+++ b/drivers/mfd/lp3972.c
@@ -70,7 +70,6 @@ static ssize_t lp_read(struct cdev *cdev, void *_buf, size_t count, loff_t offse
 }
 
 static struct cdev_operations lp_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= lp_read,
 };
 
diff --git a/drivers/mfd/mc34704.c b/drivers/mfd/mc34704.c
index f15f37ef6..4aa02b74f 100644
--- a/drivers/mfd/mc34704.c
+++ b/drivers/mfd/mc34704.c
@@ -100,7 +100,6 @@ static ssize_t mc34704_write(struct cdev *cdev, const void *_buf, size_t count,
 }
 
 static struct cdev_operations mc34704_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= mc34704_read,
 	.write	= mc34704_write,
 };
diff --git a/drivers/mfd/mc9sdz60.c b/drivers/mfd/mc9sdz60.c
index 2cb38d978..408d74645 100644
--- a/drivers/mfd/mc9sdz60.c
+++ b/drivers/mfd/mc9sdz60.c
@@ -112,7 +112,6 @@ static ssize_t mc_write(struct cdev *cdev, const void *_buf, size_t count, loff_
 }
 
 static struct cdev_operations mc_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= mc_read,
 	.write	= mc_write,
 };
diff --git a/drivers/mfd/stmpe-i2c.c b/drivers/mfd/stmpe-i2c.c
index 084e4b43b..f140f1bbc 100644
--- a/drivers/mfd/stmpe-i2c.c
+++ b/drivers/mfd/stmpe-i2c.c
@@ -101,7 +101,6 @@ static ssize_t stmpe_write(struct cdev *cdev, const void *_buf, size_t count, lo
 }
 
 static struct cdev_operations stmpe_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= stmpe_read,
 	.write	= stmpe_write,
 };
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index fb435f510..c3240b854 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -150,7 +150,6 @@ static ssize_t twl_write(struct cdev *cdev, const void *_buf, size_t count,
 }
 
 struct cdev_operations twl_fops = {
-	.lseek	= dev_lseek_default,
 	.read	= twl_read,
 	.write	= twl_write,
 };
diff --git a/drivers/misc/mem.c b/drivers/misc/mem.c
index d829af724..60981a3e9 100644
--- a/drivers/misc/mem.c
+++ b/drivers/misc/mem.c
@@ -11,7 +11,6 @@ static struct cdev_operations memops = {
 	.read  = mem_read,
 	.write = mem_write,
 	.memmap = generic_memmap_rw,
-	.lseek = dev_lseek_default,
 };
 
 static int mem_probe(struct device_d *dev)
diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
index 27b4c681f..053b35150 100644
--- a/drivers/misc/sram.c
+++ b/drivers/misc/sram.c
@@ -29,7 +29,6 @@ static struct cdev_operations memops = {
 	.read  = mem_read,
 	.write = mem_write,
 	.memmap = generic_memmap_rw,
-	.lseek = dev_lseek_default,
 };
 
 static int sram_probe(struct device_d *dev)
diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index f44c6cfc6..881b5f486 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -462,7 +462,6 @@ static struct cdev_operations mtd_ops = {
 	.protect = mtd_op_protect,
 #endif
 	.ioctl  = mtd_ioctl,
-	.lseek  = dev_lseek_default,
 };
 
 static int mtd_partition_set(struct param_d *p, void *priv)
diff --git a/drivers/mtd/mtdoob.c b/drivers/mtd/mtdoob.c
index ffaf9506f..4aef84448 100644
--- a/drivers/mtd/mtdoob.c
+++ b/drivers/mtd/mtdoob.c
@@ -66,7 +66,6 @@ static ssize_t mtd_op_read_oob(struct cdev *cdev, void *buf, size_t count,
 static struct cdev_operations mtd_ops_oob = {
 	.read   = mtd_op_read_oob,
 	.ioctl  = mtd_ioctl,
-	.lseek  = dev_lseek_default,
 };
 
 static int add_mtdoob_device(struct mtd_info *mtd, const char *devname, void **priv)
diff --git a/drivers/mtd/mtdraw.c b/drivers/mtd/mtdraw.c
index 6e36dc533..f63da7b3b 100644
--- a/drivers/mtd/mtdraw.c
+++ b/drivers/mtd/mtdraw.c
@@ -291,7 +291,6 @@ static const struct cdev_operations mtd_raw_fops = {
 	.read		= mtdraw_read,
 	.write		= mtdraw_write,
 	.erase		= mtdraw_erase,
-	.lseek		= dev_lseek_default,
 };
 
 static int add_mtdraw_device(struct mtd_info *mtd, const char *devname, void **priv)
diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 36d818b3f..5b34e9b8d 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -1326,7 +1326,6 @@ exit:
 static struct cdev_operations e1000_invm_ops = {
 	.read	= e1000_invm_cdev_read,
 	.write	= e1000_invm_cdev_write,
-	.lseek	= dev_lseek_default,
 };
 
 static ssize_t e1000_eeprom_cdev_read(struct cdev *cdev, void *buf,
@@ -1351,7 +1350,6 @@ static ssize_t e1000_eeprom_cdev_read(struct cdev *cdev, void *buf,
 
 static struct cdev_operations e1000_eeprom_ops = {
 	.read = e1000_eeprom_cdev_read,
-	.lseek = dev_lseek_default,
 };
 
 static int e1000_mtd_read_or_write(bool read,
diff --git a/drivers/net/ksz8864rmn.c b/drivers/net/ksz8864rmn.c
index 4a19dd873..85063ff0d 100644
--- a/drivers/net/ksz8864rmn.c
+++ b/drivers/net/ksz8864rmn.c
@@ -113,7 +113,6 @@ static ssize_t micel_switch_write(struct cdev *cdev, const void *_buf, size_t co
 static struct cdev_operations micrel_switch_ops = {
 	.read  = micel_switch_read,
 	.write = micel_switch_write,
-	.lseek = dev_lseek_default,
 };
 
 static int micrel_switch_probe(struct device_d *dev)
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index e1dd8f0ae..3480e2ffb 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -392,7 +392,6 @@ static ssize_t phydev_write(struct cdev *cdev, const void *_buf, size_t count, l
 static struct cdev_operations phydev_ops = {
 	.read  = phydev_read,
 	.write = phydev_write,
-	.lseek = dev_lseek_default,
 };
 
 static void of_set_phy_supported(struct phy_device *phydev)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 9fd599095..6cf98f62a 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -85,7 +85,6 @@ static ssize_t nvmem_cdev_write(struct cdev *cdev, const void *buf, size_t count
 static struct cdev_operations nvmem_chrdev_ops = {
 	.read  = nvmem_cdev_read,
 	.write  = nvmem_cdev_write,
-	.lseek = dev_lseek_default,
 };
 
 static int nvmem_register_cdev(struct nvmem_device *nvmem, const char *name)
diff --git a/drivers/video/fb.c b/drivers/video/fb.c
index 72f33a6db..2d82bc01f 100644
--- a/drivers/video/fb.c
+++ b/drivers/video/fb.c
@@ -228,7 +228,6 @@ static struct cdev_operations fb_ops = {
 	.read	= mem_read,
 	.write	= mem_write,
 	.memmap	= generic_memmap_rw,
-	.lseek	= dev_lseek_default,
 	.ioctl	= fb_ioctl,
 	.close  = fb_close,
 	.flush  = fb_op_flush,
diff --git a/drivers/w1/slaves/w1_ds2431.c b/drivers/w1/slaves/w1_ds2431.c
index 13691d7ba..6446f4ba0 100644
--- a/drivers/w1/slaves/w1_ds2431.c
+++ b/drivers/w1/slaves/w1_ds2431.c
@@ -260,7 +260,6 @@ out_up:
 static struct cdev_operations ds2431_ops = {
 	.read	= ds2431_cdev_read,
 	.write	= ds2431_cdev_write,
-	.lseek	= dev_lseek_default,
 };
 
 static int ds2431_probe(struct w1_device *dev)
diff --git a/drivers/w1/slaves/w1_ds2433.c b/drivers/w1/slaves/w1_ds2433.c
index f521a46a7..b24fb5b3b 100644
--- a/drivers/w1/slaves/w1_ds2433.c
+++ b/drivers/w1/slaves/w1_ds2433.c
@@ -159,7 +159,6 @@ out_up:
 static struct cdev_operations ds2433_ops = {
 	.read	= ds2433_cdev_read,
 	.write	= ds2433_cdev_write,
-	.lseek	= dev_lseek_default,
 };
 
 static int ds2433_cdev_create(struct w1_device *dev, int size, int id)
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 74e1b37b4..5dbb67b43 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -476,7 +476,6 @@ static const struct cdev_operations loop_ops = {
 	.read = loop_read,
 	.write = loop_write,
 	.memmap = generic_memmap_rw,
-	.lseek = dev_lseek_default,
 };
 
 struct cdev *cdev_create_loop(const char *path, ulong flags, loff_t offset)
diff --git a/fs/devfs.c b/fs/devfs.c
index 6acbbd7ad..5599f39e8 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -66,8 +66,6 @@ static loff_t devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
 		ret = cdev->ops->lseek(cdev, pos + cdev->offset);
 		if (ret < 0)
 			return ret;
-	} else {
-		return -ENOSYS;
 	}
 
 	return pos;
diff --git a/include/driver.h b/include/driver.h
index 3d9970df5..72741a964 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -360,11 +360,6 @@ int dummy_probe(struct device_d *);
 int generic_memmap_ro(struct cdev *dev, void **map, int flags);
 int generic_memmap_rw(struct cdev *dev, void **map, int flags);
 
-static inline loff_t dev_lseek_default(struct cdev *cdev, loff_t ofs)
-{
-	return ofs;
-}
-
 static inline int dev_open_default(struct device_d *dev, struct filep *f)
 {
 	return 0;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 10/19] fs: devfs: Change .lseek callbacks to return 'int'

[Andrey Smirnov]

Returning requested offset from .lseek() callback doesn't really give
us any new information while bringing unnecessary
complications. Change all .lseek() types (both in struct struct
cdev_operations and in struct fs_driver_d) to return 'int' and adjust
the rest of the codebase accordingly.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/mtd/nand/nand-bb.c |  5 ++---
 drivers/mtd/ubi/barebox.c  |  4 ++--
 fs/bpkfs.c                 |  4 ++--
 fs/devfs.c                 |  6 +++---
 fs/efi.c                   |  4 ++--
 fs/fat/fat.c               |  4 ++--
 fs/fs.c                    |  8 +++-----
 fs/nfs.c                   |  4 ++--
 fs/pstore/fs.c             |  4 ++--
 fs/smhfs.c                 |  4 ++--
 fs/tftp.c                  | 10 +++++-----
 fs/uimagefs.c              |  4 ++--
 include/driver.h           |  2 +-
 include/fs.h               |  2 +-
 14 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/drivers/mtd/nand/nand-bb.c b/drivers/mtd/nand/nand-bb.c
index 012163ebb..eea4bb43e 100644
--- a/drivers/mtd/nand/nand-bb.c
+++ b/drivers/mtd/nand/nand-bb.c
@@ -236,11 +236,10 @@ static int nand_bb_calc_size(struct nand_bb *bb)
 	return 0;
 }
 
-static loff_t nand_bb_lseek(struct cdev *cdev, loff_t __offset)
+static int nand_bb_lseek(struct cdev *cdev, loff_t offset)
 {
 	struct nand_bb *bb = cdev->priv;
 	loff_t raw_pos = 0;
-	uint32_t offset = __offset;
 
 	/* lseek only in readonly mode */
 	if (bb->flags & O_ACCMODE)
@@ -257,7 +256,7 @@ static loff_t nand_bb_lseek(struct cdev *cdev, loff_t __offset)
 
 		if (!offset) {
 			bb->offset = raw_pos;
-			return __offset;
+			return 0;
 		}
 	}
 
diff --git a/drivers/mtd/ubi/barebox.c b/drivers/mtd/ubi/barebox.c
index 65f545645..781061d9a 100644
--- a/drivers/mtd/ubi/barebox.c
+++ b/drivers/mtd/ubi/barebox.c
@@ -151,7 +151,7 @@ static int ubi_volume_cdev_close(struct cdev *cdev)
 	return 0;
 }
 
-static loff_t ubi_volume_cdev_lseek(struct cdev *cdev, loff_t ofs)
+static int ubi_volume_cdev_lseek(struct cdev *cdev, loff_t ofs)
 {
 	struct ubi_volume_cdev_priv *priv = cdev->priv;
 
@@ -159,7 +159,7 @@ static loff_t ubi_volume_cdev_lseek(struct cdev *cdev, loff_t ofs)
 	if (priv->written)
 		return -EINVAL;
 
-	return ofs;
+	return 0;
 }
 
 static int ubi_volume_cdev_truncate(struct cdev *cdev, size_t size)
diff --git a/fs/bpkfs.c b/fs/bpkfs.c
index f1db963d0..655cde09b 100644
--- a/fs/bpkfs.c
+++ b/fs/bpkfs.c
@@ -192,7 +192,7 @@ static int bpkfs_read(struct device_d *dev, FILE *file, void *buf, size_t insize
 	}
 }
 
-static loff_t bpkfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
+static int bpkfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 {
 	struct bpkfs_handle_data *d = file->priv;
 
@@ -201,7 +201,7 @@ static loff_t bpkfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 
 	d->pos = pos;
 
-	return pos;
+	return 0;
 }
 
 struct somfy_readdir {
diff --git a/fs/devfs.c b/fs/devfs.c
index 5599f39e8..007dea96d 100644
--- a/fs/devfs.c
+++ b/fs/devfs.c
@@ -57,10 +57,10 @@ static int devfs_write(struct device_d *_dev, FILE *f, const void *buf, size_t s
 	return cdev_write(cdev, buf, size, f->pos, f->flags);
 }
 
-static loff_t devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
+static int devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
 {
 	struct cdev *cdev = f->priv;
-	loff_t ret;
+	int ret;
 
 	if (cdev->ops->lseek) {
 		ret = cdev->ops->lseek(cdev, pos + cdev->offset);
@@ -68,7 +68,7 @@ static loff_t devfs_lseek(struct device_d *_dev, FILE *f, loff_t pos)
 			return ret;
 	}
 
-	return pos;
+	return 0;
 }
 
 static int devfs_erase(struct device_d *_dev, FILE *f, loff_t count, loff_t offset)
diff --git a/fs/efi.c b/fs/efi.c
index 074ef6b53..cd4fee79a 100644
--- a/fs/efi.c
+++ b/fs/efi.c
@@ -292,7 +292,7 @@ static int efifs_write(struct device_d *_dev, FILE *f, const void *buf, size_t i
 	return bufsize;
 }
 
-static loff_t efifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
+static int efifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
 	struct efifs_file *ufile = f->priv;
 	efi_status_t efiret;
@@ -302,7 +302,7 @@ static loff_t efifs_lseek(struct device_d *dev, FILE *f, loff_t pos)
 		return -efi_errno(efiret);
 	}
 
-	return pos;
+	return 0;
 }
 
 static int efifs_truncate(struct device_d *dev, FILE *f, unsigned long size)
diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index ee7751e94..136757772 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -268,7 +268,7 @@ static int fat_read(struct device_d *_dev, FILE *f, void *buf, size_t insize)
 	return outsize;
 }
 
-static loff_t fat_lseek(struct device_d *dev, FILE *f, loff_t pos)
+static int fat_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
 	FIL *f_file = f->priv;
 	int ret;
@@ -277,7 +277,7 @@ static loff_t fat_lseek(struct device_d *dev, FILE *f, loff_t pos)
 	if (ret)
 		return ret;
 
-	return pos;
+	return 0;
 }
 
 static DIR* fat_opendir(struct device_d *dev, const char *pathname)
diff --git a/fs/fs.c b/fs/fs.c
index 34de0669a..7e62b89e4 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -439,11 +439,9 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 	}
 
 	if (fsdrv->lseek) {
-		pos = fsdrv->lseek(&f->fsdev->dev, f, pos);
-		if (IS_ERR_VALUE(pos)) {
-			errno = -pos;
-			return -1;
-		}
+		ret = fsdrv->lseek(&f->fsdev->dev, f, pos);
+		if (ret < 0)
+			goto out;
 	}
 
 	f->pos = pos;
diff --git a/fs/nfs.c b/fs/nfs.c
index d83f25007..8112830f7 100644
--- a/fs/nfs.c
+++ b/fs/nfs.c
@@ -1088,13 +1088,13 @@ static int nfs_read(struct device_d *dev, FILE *file, void *buf, size_t insize)
 	return kfifo_get(priv->fifo, buf, insize);
 }
 
-static loff_t nfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
+static int nfs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 {
 	struct file_priv *priv = file->priv;
 
 	kfifo_reset(priv->fifo);
 
-	return pos;
+	return 0;
 }
 
 static int nfs_iterate(struct file *file, struct dir_context *ctx)
diff --git a/fs/pstore/fs.c b/fs/pstore/fs.c
index a879a6806..9a7e0b552 100644
--- a/fs/pstore/fs.c
+++ b/fs/pstore/fs.c
@@ -172,13 +172,13 @@ static int pstore_read(struct device_d *dev, FILE *file, void *buf,
 	return insize;
 }
 
-static loff_t pstore_lseek(struct device_d *dev, FILE *file, loff_t pos)
+static int pstore_lseek(struct device_d *dev, FILE *file, loff_t pos)
 {
 	struct pstore_private *d = file->priv;
 
 	d->pos = pos;
 
-	return pos;
+	return 0;
 }
 
 static DIR *pstore_opendir(struct device_d *dev, const char *pathname)
diff --git a/fs/smhfs.c b/fs/smhfs.c
index 18eaa9dfc..7a6933630 100644
--- a/fs/smhfs.c
+++ b/fs/smhfs.c
@@ -109,13 +109,13 @@ static int smhfs_read(struct device_d __always_unused *dev,
 		return -semihosting_errno();
 }
 
-static loff_t smhfs_lseek(struct device_d __always_unused *dev,
+static int smhfs_lseek(struct device_d __always_unused *dev,
 			  FILE *f, loff_t pos)
 {
 	if (semihosting_seek(file_to_fd(f), pos))
 		return -semihosting_errno();
 
-	return pos;
+	return 0;
 }
 
 static DIR* smhfs_opendir(struct device_d __always_unused *dev,
diff --git a/fs/tftp.c b/fs/tftp.c
index f9e204db5..41f904f29 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -573,13 +573,13 @@ static int tftp_read(struct device_d *dev, FILE *f, void *buf, size_t insize)
 	return outsize;
 }
 
-static loff_t tftp_lseek(struct device_d *dev, FILE *f, loff_t pos)
+static int tftp_lseek(struct device_d *dev, FILE *f, loff_t pos)
 {
 	/* We cannot seek backwards without reloading or caching the file */
 	loff_t f_pos = f->pos;
 
 	if (pos >= f_pos) {
-		loff_t ret;
+		int ret = 0;
 		char *buf = xmalloc(1024);
 
 		while (pos > f_pos) {
@@ -596,8 +596,6 @@ static loff_t tftp_lseek(struct device_d *dev, FILE *f, loff_t pos)
 			f_pos += ret;
 		}
 
-		ret = pos;
-
 out_free:
 		free(buf);
 		if (ret < 0) {
@@ -606,8 +604,10 @@ out_free:
 			 * failed since we can't move backwards
 			 */
 			f->pos = f_pos;
+			return ret;
 		}
-		return ret;
+
+		return 0;
 	}
 
 	return -ENOSYS;
diff --git a/fs/uimagefs.c b/fs/uimagefs.c
index c120944a4..e5ada82da 100644
--- a/fs/uimagefs.c
+++ b/fs/uimagefs.c
@@ -116,7 +116,7 @@ static int uimagefs_read(struct device_d *dev, FILE *file, void *buf, size_t ins
 	}
 }
 
-static loff_t uimagefs_lseek(struct device_d *dev, FILE *file, loff_t pos)
+static int uimagefs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 {
 	struct uimagefs_handle_data *d = file->priv;
 
@@ -125,7 +125,7 @@ static loff_t uimagefs_lseek(struct device_d *dev, FILE *file, loff_t pos)
 
 	d->pos = pos;
 
-	return pos;
+	return 0;
 }
 
 static DIR *uimagefs_opendir(struct device_d *dev, const char *pathname)
diff --git a/include/driver.h b/include/driver.h
index 72741a964..2db1cf1a8 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -434,7 +434,7 @@ struct cdev_operations {
 	ssize_t (*write)(struct cdev*, const void* buf, size_t count, loff_t offset, ulong flags);
 
 	int (*ioctl)(struct cdev*, int, void *);
-	loff_t (*lseek)(struct cdev*, loff_t);
+	int (*lseek)(struct cdev*, loff_t);
 	int (*open)(struct cdev*, unsigned long flags);
 	int (*close)(struct cdev*);
 	int (*flush)(struct cdev*);
diff --git a/include/fs.h b/include/fs.h
index f1514afa9..be8241fd4 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -53,7 +53,7 @@ struct fs_driver_d {
 	int (*read)(struct device_d *dev, FILE *f, void *buf, size_t size);
 	int (*write)(struct device_d *dev, FILE *f, const void *buf, size_t size);
 	int (*flush)(struct device_d *dev, FILE *f);
-	loff_t (*lseek)(struct device_d *dev, FILE *f, loff_t pos);
+	int (*lseek)(struct device_d *dev, FILE *f, loff_t pos);
 
 	int (*ioctl)(struct device_d *dev, FILE *f, int request, void *buf);
 	int (*erase)(struct device_d *dev, FILE *f, loff_t count,
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 10/19] fs: devfs: Change .lseek callbacks to return 'int'

[Sascha Hauer]

On Mon, Jan 28, 2019 at 10:55:40PM -0800, Andrey Smirnov wrote:
> Returning requested offset from .lseek() callback doesn't really give
> us any new information while bringing unnecessary
> complications. Change all .lseek() types (both in struct struct
> cdev_operations and in struct fs_driver_d) to return 'int' and adjust
> the rest of the codebase accordingly.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  drivers/mtd/nand/nand-bb.c |  5 ++---
>  drivers/mtd/ubi/barebox.c  |  4 ++--
>  fs/bpkfs.c                 |  4 ++--
>  fs/devfs.c                 |  6 +++---
>  fs/efi.c                   |  4 ++--
>  fs/fat/fat.c               |  4 ++--
>  fs/fs.c                    |  8 +++-----
>  fs/nfs.c                   |  4 ++--
>  fs/pstore/fs.c             |  4 ++--
>  fs/smhfs.c                 |  4 ++--
>  fs/tftp.c                  | 10 +++++-----
>  fs/uimagefs.c              |  4 ++--
>  include/driver.h           |  2 +-
>  include/fs.h               |  2 +-
>  14 files changed, 31 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand-bb.c b/drivers/mtd/nand/nand-bb.c
> index 012163ebb..eea4bb43e 100644
> --- a/drivers/mtd/nand/nand-bb.c
> +++ b/drivers/mtd/nand/nand-bb.c
> @@ -236,11 +236,10 @@ static int nand_bb_calc_size(struct nand_bb *bb)
>  	return 0;
>  }
>  
> -static loff_t nand_bb_lseek(struct cdev *cdev, loff_t __offset)
> +static int nand_bb_lseek(struct cdev *cdev, loff_t offset)
>  {
>  	struct nand_bb *bb = cdev->priv;
>  	loff_t raw_pos = 0;
> -	uint32_t offset = __offset;
>  
>  	/* lseek only in readonly mode */
>  	if (bb->flags & O_ACCMODE)
> @@ -257,7 +256,7 @@ static loff_t nand_bb_lseek(struct cdev *cdev, loff_t __offset)
>  
>  		if (!offset) {
>  			bb->offset = raw_pos;
> -			return __offset;
> +			return 0;

This introduces the warning

include/linux/kernel.h:111:17: warning: comparison of distinct pointer types lacks a cast
  (void) (&_min1 == &_min2);  \
                   ^
drivers/mtd/nand/nand-bb.c:248:15: note: in expansion of macro 'min'
		      off_t now = min(offset, bb->mtd->erasesize);

Changed to use min_t() rather than min()

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 11/19] fs: Do not use IS_ERR_VALUE() to validate offset in lseek()

[Andrey Smirnov]

On 32-bit systems, checking for IS_ERR_VALUE(pos) is not
correct. Expanding that code we get (loff_t cast is added for clarity):

 (loff_t)pos >= (unsigned long)-MAX_ERRNO

given that loff_t is a 64-bit signed value, any perfectly valid seek
offset that is greater than 0xffffc000 will result in false positive.

Moreso, as a part of fix introduced in e10efc5080 ("fs: fix memory
access via /dev/mem for MIPS64") it doesn't really solve the problem
completely on on 64-bit platforms, becuase it still leaves out a
number of perfectly valid offsets (e.g. "md 0xffffffffffffff00"
doesn't work)

Undo the original change and convert the check to simply test if
offset is negative.

Changes neccessary to alllow access to end of 64-bit address space
will be implemented in the follow-up patch.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/fs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index 7e62b89e4..2638e7458 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -420,7 +420,7 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 	case SEEK_SET:
 		if (f->size != FILE_SIZE_STREAM && offset > f->size)
 			goto out;
-		if (IS_ERR_VALUE(offset))
+		if (offset < 0)
 			goto out;
 		pos = offset;
 		break;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 12/19] fs: Simplify new position calculation in lseek()

[Andrey Smirnov]

All these checks are really testing is that resulting position is
within [0; f->size] interval. Convert all of the custom checks into a
signle one done after the switch statement to simplify the code.

Note this change also disables the validity check for f->size ==
FILE_SIZE_STREAM and whence == SEEK_END, but lseek(stream_fd, offset,
SEEK_END) wasn't a meaningful operation to begin with, so this
shouldn't be a problem.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/fs.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index 2638e7458..d785c0c3a 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -418,26 +418,23 @@ loff_t lseek(int fildes, loff_t offset, int whence)
 
 	switch (whence) {
 	case SEEK_SET:
-		if (f->size != FILE_SIZE_STREAM && offset > f->size)
-			goto out;
-		if (offset < 0)
-			goto out;
-		pos = offset;
+		pos = 0;
 		break;
 	case SEEK_CUR:
-		if (f->size != FILE_SIZE_STREAM && offset + f->pos > f->size)
-			goto out;
-		pos = f->pos + offset;
+		pos = f->pos;
 		break;
 	case SEEK_END:
-		if (offset > 0)
-			goto out;
-		pos = f->size + offset;
+		pos = f->size;
 		break;
 	default:
 		goto out;
 	}
 
+	pos += offset;
+
+	if (f->size != FILE_SIZE_STREAM && (pos < 0 || pos > f->size))
+		goto out;
+
 	if (fsdrv->lseek) {
 		ret = fsdrv->lseek(&f->fsdev->dev, f, pos);
 		if (ret < 0)
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 13/19] fs: Share code between mem_write()/mem_read()

[Andrey Smirnov]

Rename memcpy_sz() to mem_copy() and move all of the identical code
from mem_write()/mem_read() there.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/devfs-core.c | 46 ++++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 5dbb67b43..37c4bd0ad 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -528,12 +528,22 @@ void cdev_remove_loop(struct cdev *cdev)
 	free(cdev);
 }
 
-static void memcpy_sz(void *dst, const void *src, size_t count, int rwsize)
+static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
+			resource_size_t count, resource_size_t offset,
+			unsigned long flags)
 {
+	ssize_t size;
+	int rwsize = flags & O_RWSIZE_MASK;
+
+	if (!dev || dev->num_resources < 1)
+		return -1;
+
+	count = size = min(count, resource_size(&dev->resource[0]) - offset);
+
 	/* no rwsize specification given. Do whatever memcpy likes best */
 	if (!rwsize) {
 		memcpy(dst, src, count);
-		return;
+		goto out;
 	}
 
 	rwsize = rwsize >> O_RWSIZE_SHIFT;
@@ -558,40 +568,32 @@ static void memcpy_sz(void *dst, const void *src, size_t count, int rwsize)
 		dst += rwsize;
 		src += rwsize;
 	}
+out:
+	return size;
 }
 
 ssize_t mem_read(struct cdev *cdev, void *buf, size_t count, loff_t offset,
 		 unsigned long flags)
 {
-	unsigned long size;
-	struct device_d *dev;
+	struct device_d *dev = cdev->dev;
 
-	if (!cdev->dev || cdev->dev->num_resources < 1)
+	if (!dev)
 		return -1;
-	dev = cdev->dev;
 
-	size = min((resource_size_t)count,
-			resource_size(&dev->resource[0]) -
-			(resource_size_t)offset);
-	memcpy_sz(buf, dev_get_mem_region(dev, 0) + offset, size, flags & O_RWSIZE_MASK);
-	return size;
+	return mem_copy(dev, buf, dev_get_mem_region(dev, 0) + offset,
+			count, offset, flags);
 }
 EXPORT_SYMBOL(mem_read);
 
-ssize_t mem_write(struct cdev *cdev, const void *buf, size_t count, loff_t offset,
-		  unsigned long flags)
+ssize_t mem_write(struct cdev *cdev, const void *buf, size_t count,
+		  loff_t offset, unsigned long flags)
 {
-	unsigned long size;
-	struct device_d *dev;
+	struct device_d *dev = cdev->dev;
 
-	if (!cdev->dev || cdev->dev->num_resources < 1)
+	if (!dev)
 		return -1;
-	dev = cdev->dev;
 
-	size = min((resource_size_t)count,
-			resource_size(&dev->resource[0]) -
-			(resource_size_t)offset);
-	memcpy_sz(dev_get_mem_region(dev, 0) + offset, buf, size, flags & O_RWSIZE_MASK);
-	return size;
+	return mem_copy(dev, dev_get_mem_region(dev, 0) + offset, buf,
+			count, offset, flags);
 }
 EXPORT_SYMBOL(mem_write);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 14/19] fs: Avoid division in mem_copy()

[Andrey Smirnov]

Instead of dividing count by rwsize, use ALIGN_DOWN() and change the
loop to decrement by "rwsize" bytes.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/devfs-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 37c4bd0ad..8d5ff71d6 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -548,9 +548,9 @@ static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 
 	rwsize = rwsize >> O_RWSIZE_SHIFT;
 
-	count /= rwsize;
+	count = ALIGN_DOWN(count, rwsize);
 
-	while (count-- > 0) {
+	while (count) {
 		switch (rwsize) {
 		case 1:
 			*((u8 *)dst) = *((u8 *)src);
@@ -567,6 +567,7 @@ static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 		}
 		dst += rwsize;
 		src += rwsize;
+		count -= rwsize;
 	}
 out:
 	return size;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 15/19] fs: Report actual data processed by mem_copy()

[Andrey Smirnov]

If we are asked to copy amount of data that is not a multiple of
"rwsize", we won't process all "count" bytes, so we should reflect
that in the value we return.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/devfs-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 8d5ff71d6..b2f66578d 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -548,7 +548,7 @@ static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 
 	rwsize = rwsize >> O_RWSIZE_SHIFT;
 
-	count = ALIGN_DOWN(count, rwsize);
+	count = size = ALIGN_DOWN(count, rwsize);
 
 	while (count) {
 		switch (rwsize) {
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 16/19] fs: Introduce mem_read_nofail()

[Andrey Smirnov]

Introduce mem_read_nofail() to allow memory reads of
unused/not-implmented memory addresses.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 fs/devfs-core.c  | 64 ++++++++++++++++++++++++++++++++++++++++++------
 include/driver.h |  2 ++
 2 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index b2f66578d..84234fd38 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -29,6 +29,7 @@
 #include <linux/mtd/mtd.h>
 #include <unistd.h>
 #include <fs.h>
+#include <abort.h>
 
 LIST_HEAD(cdev_list);
 
@@ -528,9 +529,39 @@ void cdev_remove_loop(struct cdev *cdev)
 	free(cdev);
 }
 
+static u64 mem_read_ptr(const void *ptr, int rwsize)
+{
+	switch (rwsize) {
+	case 1:
+		return *((u8 *)ptr);
+	case 2:
+		return *((u16 *)ptr);
+	case 4:
+		return *((u32 *)ptr);
+	case 8:
+		return *((u64 *)ptr);
+	}
+
+	BUG();
+	return 0;
+}
+
+static u64 mem_read_ptr_nofail(const void *ptr, int rwsize)
+{
+	u64 data;
+
+	data_abort_mask();
+	data = mem_read_ptr(ptr, rwsize);
+
+	if (data_abort_unmask())
+		return U64_MAX;
+
+	return data;
+}
+
 static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 			resource_size_t count, resource_size_t offset,
-			unsigned long flags)
+			unsigned long flags, bool nofail)
 {
 	ssize_t size;
 	int rwsize = flags & O_RWSIZE_MASK;
@@ -541,7 +572,7 @@ static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 	count = size = min(count, resource_size(&dev->resource[0]) - offset);
 
 	/* no rwsize specification given. Do whatever memcpy likes best */
-	if (!rwsize) {
+	if (!nofail && !rwsize) {
 		memcpy(dst, src, count);
 		goto out;
 	}
@@ -551,18 +582,22 @@ static ssize_t mem_copy(struct device_d *dev, void *dst, const void *src,
 	count = size = ALIGN_DOWN(count, rwsize);
 
 	while (count) {
+		const u64 data = nofail ?
+			mem_read_ptr_nofail(src, rwsize) :
+			mem_read_ptr(src, rwsize);
+
 		switch (rwsize) {
 		case 1:
-			*((u8 *)dst) = *((u8 *)src);
+			*((u8 *)dst) = data;
 			break;
 		case 2:
-			*((u16 *)dst) = *((u16 *)src);
+			*((u16 *)dst) = data;
 			break;
 		case 4:
-			*((u32  *)dst) = *((u32  *)src);
+			*((u32  *)dst) = data;
 			break;
 		case 8:
-			*((u64  *)dst) = *((u64  *)src);
+			*((u64  *)dst) = data;
 			break;
 		}
 		dst += rwsize;
@@ -573,6 +608,19 @@ out:
 	return size;
 }
 
+ssize_t mem_read_nofail(struct cdev *cdev, void *buf, size_t count,
+			loff_t offset, unsigned long flags)
+{
+	struct device_d *dev = cdev->dev;
+
+	if (!dev)
+		return -1;
+
+	return mem_copy(dev, buf, dev_get_mem_region(dev, 0) + offset,
+			count, offset, flags, true);
+}
+EXPORT_SYMBOL(mem_read_nofail);
+
 ssize_t mem_read(struct cdev *cdev, void *buf, size_t count, loff_t offset,
 		 unsigned long flags)
 {
@@ -582,7 +630,7 @@ ssize_t mem_read(struct cdev *cdev, void *buf, size_t count, loff_t offset,
 		return -1;
 
 	return mem_copy(dev, buf, dev_get_mem_region(dev, 0) + offset,
-			count, offset, flags);
+			count, offset, flags, false);
 }
 EXPORT_SYMBOL(mem_read);
 
@@ -595,6 +643,6 @@ ssize_t mem_write(struct cdev *cdev, const void *buf, size_t count,
 		return -1;
 
 	return mem_copy(dev, dev_get_mem_region(dev, 0) + offset, buf,
-			count, offset, flags);
+			count, offset, flags, false);
 }
 EXPORT_SYMBOL(mem_write);
diff --git a/include/driver.h b/include/driver.h
index 2db1cf1a8..270b3805d 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -350,6 +350,8 @@ struct driver_d *get_driver_by_name(const char *name);
 struct cdev;
 
 /* These are used by drivers which work with direct memory accesses */
+ssize_t mem_read_nofail(struct cdev *cdev, void *buf, size_t count,
+			loff_t offset, unsigned long flags);
 ssize_t mem_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, ulong flags);
 ssize_t mem_write(struct cdev *cdev, const void *buf, size_t count, loff_t offset, ulong flags);
 int mem_memmap(struct cdev *cdev, void **map, int flags);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 17/19] commands: md: Do not use memmap()

[Andrey Smirnov]

Codepaths using memmap() in md.c don't do any boundary checks, so it
can be easily made to read past the underlying file's
boundary. For example on i.MX8MQ based board with 4GiB or RAM we get:

md -b -s /dev/ram0 0xfffffff0
fffffff0: 00 00 00 00 00 00 08 0c  00 02 20 00 20 00 05 20   .......... . ..
100000000: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000010: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000020: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000030: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000040: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000050: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000060: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000070: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000080: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
100000090: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
1000000a0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
1000000b0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
1000000c0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
1000000d0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
1000000e0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................

Drop the memmap() realted codepath, so we can realy on boundary
checking done by file I/O layer.

Note that this change has a cosmetic side effect, before:

md -b 0x7ffffffffffffffff000
7ffffffffffff000: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff010: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff020: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff030: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff040: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff050: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff060: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff070: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff080: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff090: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0a0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0b0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0c0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0d0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0e0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................
7ffffffffffff0f0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx     ................

after:

md -b 0x7ffffffffffffffff000
7ffffffffffff000: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff010: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff020: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff030: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff040: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff050: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff060: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff070: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff080: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff090: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0a0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0b0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0c0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0d0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0e0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................
7ffffffffffff0f0: ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff     ................

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 commands/md.c      | 8 --------
 drivers/misc/mem.c | 2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/commands/md.c b/commands/md.c
index 2389c12d1..507823c67 100644
--- a/commands/md.c
+++ b/commands/md.c
@@ -43,7 +43,6 @@ static int do_mem_md(int argc, char *argv[])
 	char *filename = "/dev/mem";
 	int mode = O_RWSIZE_4;
 	int swab = 0;
-	void *map;
 	void *buf = NULL;
 
 	if (argc < 2)
@@ -66,13 +65,6 @@ static int do_mem_md(int argc, char *argv[])
 	if (fd < 0)
 		return 1;
 
-	map = memmap(fd, PROT_READ);
-	if (map != MAP_FAILED) {
-		ret = memory_display(map + start, start, size,
-				mode >> O_RWSIZE_SHIFT, swab);
-		goto out;
-	}
-
 	buf = xmalloc(RW_BUF_SIZE);
 
 	do {
diff --git a/drivers/misc/mem.c b/drivers/misc/mem.c
index 60981a3e9..2f3316307 100644
--- a/drivers/misc/mem.c
+++ b/drivers/misc/mem.c
@@ -8,7 +8,7 @@
 #include <init.h>
 
 static struct cdev_operations memops = {
-	.read  = mem_read,
+	.read  = mem_read_nofail,
 	.write = mem_write,
 	.memmap = generic_memmap_rw,
 };
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 17/19] commands: md: Do not use memmap()

[Sascha Hauer]

On Mon, Jan 28, 2019 at 10:55:47PM -0800, Andrey Smirnov wrote:
> Codepaths using memmap() in md.c don't do any boundary checks, so it
> can be easily made to read past the underlying file's
> boundary. For example on i.MX8MQ based board with 4GiB or RAM we get:
> 
> md -b -s /dev/ram0 0xfffffff0
> fffffff0: 00 00 00 00 00 00 08 0c  00 02 20 00 20 00 05 20   .......... . ..
> 100000000: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000010: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000020: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000030: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000040: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000050: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000060: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000070: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000080: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 100000090: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 1000000a0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 1000000b0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 1000000c0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 1000000d0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 1000000e0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> 
> Drop the memmap() realted codepath, so we can realy on boundary
> checking done by file I/O layer.
> 
> Note that this change has a cosmetic side effect, before:

With this change we can no longer see that the underlying memory cannot
be accessed 'xx'. I find this feature very useful and don't want to
loose it.
If we want to do size checking we can do this, well, by checking the
size. We could add offset/size parameters to memmap().

So, dropped this patch for now.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 17/19] commands: md: Do not use memmap()

[Andrey Smirnov]

On Mon, Feb 4, 2019 at 5:57 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Mon, Jan 28, 2019 at 10:55:47PM -0800, Andrey Smirnov wrote:
> > Codepaths using memmap() in md.c don't do any boundary checks, so it
> > can be easily made to read past the underlying file's
> > boundary. For example on i.MX8MQ based board with 4GiB or RAM we get:
> >
> > md -b -s /dev/ram0 0xfffffff0
> > fffffff0: 00 00 00 00 00 00 08 0c  00 02 20 00 20 00 05 20   .......... . ..
> > 100000000: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000010: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000020: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000030: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000040: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000050: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000060: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000070: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000080: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 100000090: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 1000000a0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 1000000b0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 1000000c0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 1000000d0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> > 1000000e0: xx xx xx xx xx xx xx xx  xx xx xx xx xx xx xx xx  ................
> >
> > Drop the memmap() realted codepath, so we can realy on boundary
> > checking done by file I/O layer.
> >
> > Note that this change has a cosmetic side effect, before:
>
> With this change we can no longer see that the underlying memory cannot
> be accessed 'xx'. I find this feature very useful and don't want to
> loose it.

Sad to hear that. I was hoping we can use this as a stepping stone to
get rid of memory_display() altogether.

> If we want to do size checking we can do this, well, by checking the
> size. We could add offset/size parameters to memmap().
>

I'll leave that to next person who'll try to tackle this bug.

> So, dropped this patch for now.
>

"fs: Introduce mem_read_nofail()" should probably be dropped as well
since it was added in support of this.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Andrey Smirnov]

In order to allow access to second half of address space on 64-bit
machines, add code that creates /dev/highmem dedicated for that.

Note that due to maximum file size being limited to MAX_LFS_FILESIZE
or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
/dev/hightmem correspondingly.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/misc/mem.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/mem.c b/drivers/misc/mem.c
index 2f3316307..e3fb0d04a 100644
--- a/drivers/misc/mem.c
+++ b/drivers/misc/mem.c
@@ -7,6 +7,10 @@
 #include <driver.h>
 #include <init.h>
 
+#include <linux/pagemap.h>
+#include <linux/fs.h>
+
+
 static struct cdev_operations memops = {
 	.read  = mem_read_nofail,
 	.write = mem_write,
@@ -21,8 +25,7 @@ static int mem_probe(struct device_d *dev)
 	dev->priv = cdev;
 
 	cdev->name = (char*)dev->resource[0].name;
-	cdev->size = min_t(unsigned long long, resource_size(&dev->resource[0]),
-			   S64_MAX);
+	cdev->size = resource_size(&dev->resource[0]);
 	cdev->ops = &memops;
 	cdev->dev = dev;
 
@@ -38,7 +41,21 @@ static struct driver_d mem_drv = {
 
 static int mem_init(void)
 {
-	add_mem_device("mem", 0, ~0, IORESOURCE_MEM_WRITEABLE);
+	const resource_size_t size = min_t(unsigned long long,
+					   MAX_LFS_FILESIZE,
+					   (resource_size_t)~0);
+	add_mem_device("mem", 0, size, IORESOURCE_MEM_WRITEABLE);
+	/*
+	 * We can't fit all of 64-bit address space into a single
+	 * file, so we create a separate one to allow acces to the
+	 * second half
+	 */
+	if (IS_ENABLED(CONFIG_CPU_64))
+		add_mem_device("highmem",
+			       (resource_size_t)MAX_LFS_FILESIZE + 1,
+			       size,
+			       IORESOURCE_MEM_WRITEABLE);
+
 	return platform_driver_register(&mem_drv);
 }
 device_initcall(mem_init);
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Sascha Hauer]

On Mon, Jan 28, 2019 at 10:55:48PM -0800, Andrey Smirnov wrote:
> In order to allow access to second half of address space on 64-bit
> machines, add code that creates /dev/highmem dedicated for that.

Very nice series, I applied it right away with the exception of this
patch. I don't like naming it "highmem" because normally this describes
a hardware feature, namely a technique for reaching memory that is
outside the CPUs address space. What we have here is a technique for
reaching memory that is outside the range of a poorly chosen variable
type ;)

That said my alternative suggestions are not very striking and range
from /dev/mem1 to /dev/uppermem or /dev/mem-upper. Any other ideas?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Andrey Smirnov]

On Tue, Jan 29, 2019 at 12:48 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Mon, Jan 28, 2019 at 10:55:48PM -0800, Andrey Smirnov wrote:
> > In order to allow access to second half of address space on 64-bit
> > machines, add code that creates /dev/highmem dedicated for that.
>
> Very nice series, I applied it right away with the exception of this
> patch.

Cool, thanks!

> I don't like naming it "highmem" because normally this describes
> a hardware feature, namely a technique for reaching memory that is
> outside the CPUs address space. What we have here is a technique for
> reaching memory that is outside the range of a poorly chosen variable
> type ;)
>
> That said my alternative suggestions are not very striking and range
> from /dev/mem1 to /dev/uppermem or /dev/mem-upper. Any other ideas?
>

/dev/mem1 seems a bit less self-descriptive and might cause end user
confusion. Ether /dev/uppermem of /dev/mem-upper would probably be the
best. Let me know if you want me to re-spin the last patch with
either.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Sam Ravnborg]

Hi Andrey

On Tue, Jan 29, 2019 at 12:40:21PM -0800, Andrey Smirnov wrote:
> On Tue, Jan 29, 2019 at 12:48 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Mon, Jan 28, 2019 at 10:55:48PM -0800, Andrey Smirnov wrote:
> > > In order to allow access to second half of address space on 64-bit
> > > machines, add code that creates /dev/highmem dedicated for that.
> >
> > Very nice series, I applied it right away with the exception of this
> > patch.
> 
> Cool, thanks!
> 
> > I don't like naming it "highmem" because normally this describes
> > a hardware feature, namely a technique for reaching memory that is
> > outside the CPUs address space. What we have here is a technique for
> > reaching memory that is outside the range of a poorly chosen variable
> > type ;)
> >
> > That said my alternative suggestions are not very striking and range
> > from /dev/mem1 to /dev/uppermem or /dev/mem-upper. Any other ideas?

No good ideas here either.
/dev/mem-upper get my vote as it will sit next to /dev/mem in an ls.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Peter Mamonov]

Hello, Andrey,

> In order to allow access to second half of address space on 64-bit
> machines, add code that creates /dev/highmem dedicated for that.
> 
> Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> /dev/hightmem correspondingly.

Does it imply using `-s /dev/highmem` argument when accessing addresses beyond 
MAX_LFS_FILESIZE? That's not very convenient:

	$ git grep -l /dev/mem
	commands/crc.c
	commands/digest.c
	commands/disasm.c
	commands/md.c
	commands/memcmp.c
	commands/memcpy.c
	commands/memset.c
	commands/mm.c
	commands/mw.c

Regards,
Peter

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Peter Mamonov]

On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> Hello, Andrey,
> 
> > In order to allow access to second half of address space on 64-bit
> > machines, add code that creates /dev/highmem dedicated for that.
> > 
> > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > /dev/hightmem correspondingly.
> 
> Does it imply using `-s /dev/highmem` argument when accessing addresses beyond 
> MAX_LFS_FILESIZE? That's not very convenient:
> 
> 	$ git grep -l /dev/mem
> 	commands/crc.c
> 	commands/digest.c
> 	commands/disasm.c
> 	commands/md.c
> 	commands/memcmp.c
> 	commands/memcpy.c
> 	commands/memset.c
> 	commands/mm.c
> 	commands/mw.c

I've forgotten to mention, that all meaningful MIPS64 virtual addresses  
belong to the 2nd half of the address space. Except for user segment addresses 
which are not used in barebox.

> 
> Regards,
> Peter

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Sascha Hauer]

On Thu, Jan 31, 2019 at 03:50:28PM +0300, Peter Mamonov wrote:
> On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> > Hello, Andrey,
> > 
> > > In order to allow access to second half of address space on 64-bit
> > > machines, add code that creates /dev/highmem dedicated for that.
> > > 
> > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > /dev/hightmem correspondingly.
> > 
> > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond 
> > MAX_LFS_FILESIZE? That's not very convenient:
> > 
> > 	$ git grep -l /dev/mem
> > 	commands/crc.c
> > 	commands/digest.c
> > 	commands/disasm.c
> > 	commands/md.c
> > 	commands/memcmp.c
> > 	commands/memcpy.c
> > 	commands/memset.c
> > 	commands/mm.c
> > 	commands/mw.c
> 
> I've forgotten to mention, that all meaningful MIPS64 virtual addresses  
> belong to the 2nd half of the address space. Except for user segment addresses 
> which are not used in barebox.

In that case our only option seems to be to use an unsigned 64bit type
for filesize. We would need an additional lseek like function which
is able to reach the upper half of the address space and with regular
lseek simply only the lower half would be reachable.

Note that currently we don't seem to have a problem as even with Andreys
series applied we can still 'md' the whole address space. The test if we
lseek outside the file never triggers since the signed comparison of
offset > f->size never evaluates to true for f->size == S64_MAX.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Peter Mamonov]

On Fri, Feb 01, 2019 at 08:47:11AM +0100, Sascha Hauer wrote:
> On Thu, Jan 31, 2019 at 03:50:28PM +0300, Peter Mamonov wrote:
> > On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> > > Hello, Andrey,
> > > 
> > > > In order to allow access to second half of address space on 64-bit
> > > > machines, add code that creates /dev/highmem dedicated for that.
> > > > 
> > > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > > /dev/hightmem correspondingly.
> > > 
> > > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond 
> > > MAX_LFS_FILESIZE? That's not very convenient:
> > > 
> > > 	$ git grep -l /dev/mem
> > > 	commands/crc.c
> > > 	commands/digest.c
> > > 	commands/disasm.c
> > > 	commands/md.c
> > > 	commands/memcmp.c
> > > 	commands/memcpy.c
> > > 	commands/memset.c
> > > 	commands/mm.c
> > > 	commands/mw.c
> > 
> > I've forgotten to mention, that all meaningful MIPS64 virtual addresses  
> > belong to the 2nd half of the address space. Except for user segment addresses 
> > which are not used in barebox.
> 
> In that case our only option seems to be to use an unsigned 64bit type
> for filesize. We would need an additional lseek like function which
> is able to reach the upper half of the address space and with regular
> lseek simply only the lower half would be reachable.
> 
> Note that currently we don't seem to have a problem as even with Andreys
> series applied we can still 'md' the whole address space. The test if we
> lseek outside the file never triggers since the signed comparison of
> offset > f->size never evaluates to true for f->size == S64_MAX.

I can confirm that it is possible to enable access to the whole 64 bit address 
space via /dev/mem after adjusting /dev/mem size and eliminating checks for 
negative offset/pos in lseek(). Yet the proper approach seems to be to add a 
dedicated lseek implementation for /dev/mem, as in Linux.

Regards,
Peter

> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Andrey Smirnov]

On Fri, Feb 1, 2019 at 2:25 AM Peter Mamonov <pmamonov@gmail.com> wrote:
>
> On Fri, Feb 01, 2019 at 08:47:11AM +0100, Sascha Hauer wrote:
> > On Thu, Jan 31, 2019 at 03:50:28PM +0300, Peter Mamonov wrote:
> > > On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> > > > Hello, Andrey,
> > > >
> > > > > In order to allow access to second half of address space on 64-bit
> > > > > machines, add code that creates /dev/highmem dedicated for that.
> > > > >
> > > > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > > > /dev/hightmem correspondingly.
> > > >
> > > > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond
> > > > MAX_LFS_FILESIZE? That's not very convenient:
> > > >
> > > >   $ git grep -l /dev/mem
> > > >   commands/crc.c
> > > >   commands/digest.c
> > > >   commands/disasm.c
> > > >   commands/md.c
> > > >   commands/memcmp.c
> > > >   commands/memcpy.c
> > > >   commands/memset.c
> > > >   commands/mm.c
> > > >   commands/mw.c
> > >
> > > I've forgotten to mention, that all meaningful MIPS64 virtual addresses
> > > belong to the 2nd half of the address space. Except for user segment addresses
> > > which are not used in barebox.
> >
> > In that case our only option seems to be to use an unsigned 64bit type
> > for filesize. We would need an additional lseek like function which
> > is able to reach the upper half of the address space and with regular
> > lseek simply only the lower half would be reachable.
> >
> > Note that currently we don't seem to have a problem as even with Andreys
> > series applied we can still 'md' the whole address space. The test if we
> > lseek outside the file never triggers since the signed comparison of
> > offset > f->size never evaluates to true for f->size == S64_MAX.
>
> I can confirm that it is possible to enable access to the whole 64 bit address
> space via /dev/mem after adjusting /dev/mem size and eliminating checks for
> negative offset/pos in lseek().

Assuming you are running on top of this patchset, if you just set
/dev/mem's size to U64_MAX, that alone should disable any checks since
U64_MAX is also FILE_SIZE_STREAM and validation code in lseek() is:

if (f->size != FILE_SIZE_STREAM && (pos < 0 || pos > f->size))
    goto out;

Sascha, is this the direction you want to go? Should we rename
DEVFS_IS_CHARACTER_DEV to something more generic and use it on
/dev/mem?

>  Yet the proper approach seems to be to add a
> dedicated lseek implementation for /dev/mem, as in Linux.
>

The checks we are talking about are done before custom .lseek()
implementation is called. The execution goes: lseek() -> validity
checks -> cdev_lseek() -> cdev's (which /dev/mem is) custom .lseek()
callback. Following Linux's example would require quite a bit of
rework of how lseek() is handled.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Andrey Smirnov]

On Thu, Jan 31, 2019 at 11:47 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> On Thu, Jan 31, 2019 at 03:50:28PM +0300, Peter Mamonov wrote:
> > On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> > > Hello, Andrey,
> > >
> > > > In order to allow access to second half of address space on 64-bit
> > > > machines, add code that creates /dev/highmem dedicated for that.
> > > >
> > > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > > /dev/hightmem correspondingly.
> > >
> > > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond
> > > MAX_LFS_FILESIZE? That's not very convenient:
> > >
> > >     $ git grep -l /dev/mem
> > >     commands/crc.c
> > >     commands/digest.c
> > >     commands/disasm.c
> > >     commands/md.c
> > >     commands/memcmp.c
> > >     commands/memcpy.c
> > >     commands/memset.c
> > >     commands/mm.c
> > >     commands/mw.c
> >
> > I've forgotten to mention, that all meaningful MIPS64 virtual addresses
> > belong to the 2nd half of the address space. Except for user segment addresses
> > which are not used in barebox.
>
> In that case our only option seems to be to use an unsigned 64bit type
> for filesize. We would need an additional lseek like function which
> is able to reach the upper half of the address space and with regular
> lseek simply only the lower half would be reachable.
>
> Note that currently we don't seem to have a problem as even with Andreys
> series applied we can still 'md' the whole address space. The test if we
> lseek outside the file never triggers since the signed comparison of
> offset > f->size never evaluates to true for f->size == S64_MAX.
>

I am a bit confused by this statement. I agree The check for pos >
f->size won't evaluate to true if f->size == S64_MAX, but the pos < 0,
will. On my i.MX8MQ board with all but "drivers: mem: Create file to
access second half of 64-bit memory" applied I have:

barebox@ZII i.MX8MQ Ultra Zest Board:/ md -b 0x7ffffffffffffffe
7ffffffffffffffe: ff                                         .
barebox@ZII i.MX8MQ Ultra Zest Board:/ md -b 0x7fffffffffffffff
barebox@ZII i.MX8MQ Ultra Zest Board:/ md -b 0x8fffffffffffffff
lseek: Invalid argument
barebox@ZII i.MX8MQ Ultra Zest Board:/ md -b 0xfffffffffffffff0
lseek: Invalid argument

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Sascha Hauer]

On Fri, Feb 01, 2019 at 04:35:16PM -0800, Andrey Smirnov wrote:
> On Thu, Jan 31, 2019 at 11:47 PM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >
> > On Thu, Jan 31, 2019 at 03:50:28PM +0300, Peter Mamonov wrote:
> > > On Thu, Jan 31, 2019 at 01:54:52PM +0300, Peter Mamonov wrote:
> > > > Hello, Andrey,
> > > >
> > > > > In order to allow access to second half of address space on 64-bit
> > > > > machines, add code that creates /dev/highmem dedicated for that.
> > > > >
> > > > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > > > /dev/hightmem correspondingly.
> > > >
> > > > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond
> > > > MAX_LFS_FILESIZE? That's not very convenient:
> > > >
> > > >     $ git grep -l /dev/mem
> > > >     commands/crc.c
> > > >     commands/digest.c
> > > >     commands/disasm.c
> > > >     commands/md.c
> > > >     commands/memcmp.c
> > > >     commands/memcpy.c
> > > >     commands/memset.c
> > > >     commands/mm.c
> > > >     commands/mw.c
> > >
> > > I've forgotten to mention, that all meaningful MIPS64 virtual addresses
> > > belong to the 2nd half of the address space. Except for user segment addresses
> > > which are not used in barebox.
> >
> > In that case our only option seems to be to use an unsigned 64bit type
> > for filesize. We would need an additional lseek like function which
> > is able to reach the upper half of the address space and with regular
> > lseek simply only the lower half would be reachable.
> >
> > Note that currently we don't seem to have a problem as even with Andreys
> > series applied we can still 'md' the whole address space. The test if we
> > lseek outside the file never triggers since the signed comparison of
> > offset > f->size never evaluates to true for f->size == S64_MAX.
> >
> 
> I am a bit confused by this statement. I agree The check for pos >
> f->size won't evaluate to true if f->size == S64_MAX, but the pos < 0,
> will. On my i.MX8MQ board with all but "drivers: mem: Create file to
> access second half of 64-bit memory" applied I have:

Sorry, my bad. I accidently looked at a tree that didn't have the
patches applied. You're right of course.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Andrey Smirnov]

On Thu, Jan 31, 2019 at 2:55 AM Peter Mamonov <pmamonov@gmail.com> wrote:
>
> Hello, Andrey,
>
> > In order to allow access to second half of address space on 64-bit
> > machines, add code that creates /dev/highmem dedicated for that.
> >
> > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > /dev/hightmem correspondingly.
>
> Does it imply using `-s /dev/highmem` argument when accessing addresses beyond
> MAX_LFS_FILESIZE?

Yes, it does.

> That's not very convenient:
>
>         $ git grep -l /dev/mem
>         commands/crc.c
>         commands/digest.c
>         commands/disasm.c
>         commands/md.c
>         commands/memcmp.c
>         commands/memcpy.c
>         commands/memset.c
>         commands/mm.c
>         commands/mw.c
>

I don't disagree, but there isn't much that can be done about that
AFAICT. The first version of this fix kept single /dev/mem file, but
ended up introducing quite a bit of special cases all over the FS
layer, so it was rejected. The only two options to make it more
convenient that I can see would be to either modify "/dev/mem" users
to make "/dev/highmem" substitution automatically based on offset they
are given, or, assuming it is possible, create a mapping that would
expose all of the interesting registers on MIPS64 in the lower half of
64-bit address space.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 18/19] drivers: mem: Create file to access second half of 64-bit memory

[Peter Mamonov]

On Thu, Jan 31, 2019 at 12:17:08PM -0800, Andrey Smirnov wrote:
> On Thu, Jan 31, 2019 at 2:55 AM Peter Mamonov <pmamonov@gmail.com> wrote:
> >
> > Hello, Andrey,
> >
> > > In order to allow access to second half of address space on 64-bit
> > > machines, add code that creates /dev/highmem dedicated for that.
> > >
> > > Note that due to maximum file size being limited to MAX_LFS_FILESIZE
> > > or 0x7fff_ffff_ffff_ffff bytes at addresses 0x7fff_ffff_ffff_ffff and
> > > 0xffff_ffff_ffff_ffff cannot be access through /dev/mem and
> > > /dev/hightmem correspondingly.
> >
> > Does it imply using `-s /dev/highmem` argument when accessing addresses beyond
> > MAX_LFS_FILESIZE?
> 
> Yes, it does.
> 
> > That's not very convenient:
> >
> >         $ git grep -l /dev/mem
> >         commands/crc.c
> >         commands/digest.c
> >         commands/disasm.c
> >         commands/md.c
> >         commands/memcmp.c
> >         commands/memcpy.c
> >         commands/memset.c
> >         commands/mm.c
> >         commands/mw.c
> >
> 
> I don't disagree, but there isn't much that can be done about that
> AFAICT. The first version of this fix kept single /dev/mem file, but
> ended up introducing quite a bit of special cases all over the FS
> layer, so it was rejected. The only two options to make it more
> convenient that I can see would be to either modify "/dev/mem" users
> to make "/dev/highmem" substitution automatically based on offset they
> are given, or, assuming it is possible, create a mapping that would
> expose all of the interesting registers on MIPS64 in the lower half of
> 64-bit address space.

Guess we can just adopt Linux's approach. Linux recognizes /dev/mem's lseek as
a special case and has a dedicated implementation for it. Here is a relevant
comment from drivers/char/mem.c:

	/*
	 * The memory devices use the full 32/64 bits of the offset, and so we cannot
	 * check against negative addresses: they are ok. The return value is weird,
	 * though, in that case (0).
	 *
	 * also note that seeking relative to the "end of file" isn't supported:
	 * it has no meaning, so it returns -EINVAL.
	 */
	static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
	{

Regards,
Peter

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 19/19] libfile: Fix incorrect lseek check in open_and_lseek()

[Andrey Smirnov]

We can't use "int" to caputre output of lseek which is "loff_t", since
former does not cover all of the range of the values of the latter and
any pos >= 0xffff_ffff will result in a false positive "failure". To
avoid that replace the check to check that new position matches what
was requested.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 lib/libfile.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/libfile.c b/lib/libfile.c
index 8f2aed230..9a223d232 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -556,8 +556,7 @@ int open_and_lseek(const char *filename, int mode, loff_t pos)
 		}
 	}
 
-	ret = lseek(fd, pos, SEEK_SET);
-	if (ret == -1) {
+	if (lseek(fd, pos, SEEK_SET) != pos) {
 		perror("lseek");
 		close(fd);
 		return -errno;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox