diff mbox series

[linux,dev-5.3,2/2] fsi: core: Fix small accesses and unaligned offsets via sysfs

Message ID 20191031051438.28589-3-andrew@aj.id.au
State Changes Requested, archived
Headers show
Series fsi: aspeed: Fix unaligned raw accesses | expand

Commit Message

Andrew Jeffery Oct. 31, 2019, 5:14 a.m. UTC 
Subtracting the offset delta from four-byte alignment lead to wrapping
of the requested length where `count` is less than `off`. Generalise the
length handling to enable all valid offset and size combinations.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Acked-by: Jeremy Kerr <jk@ozlabs.org>
---
 drivers/fsi/fsi-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Andrew Jeffery Nov. 1, 2019, 1:14 a.m. UTC | #1 
On Thu, 31 Oct 2019, at 15:44, Andrew Jeffery wrote:
> Subtracting the offset delta from four-byte alignment lead to wrapping
> of the requested length where `count` is less than `off`. Generalise the
> length handling to enable all valid offset and size combinations.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Acked-by: Jeremy Kerr <jk@ozlabs.org>
> ---
>  drivers/fsi/fsi-core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 1ea15621e588..889349beb284 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -559,8 +559,8 @@ static ssize_t fsi_slave_sysfs_raw_read(struct file *file,
>  		return -EINVAL;
>  
>  	for (total_len = 0; total_len < count; total_len += read_len) {
> -		read_len = min_t(size_t, count, 4);
> -		read_len -= off & 0x3;
> +		read_len = ((count | off) & 1) ?
> +				1 : min_t(size_t, count, 4 - (off & 3));
> 

Actually, I've cooked up something that's optimal and still fairly compact.
Assuming some macros like:

#define BIT(x) (1 << (x))
#define clz(x) __builtin_clz(x)
#define ctz(x) __builtin_ctz(x)

we can do:

-		read_len = min_t(size_t, count, 4);
-		read_len -= off & 0x3;
+		read_len =  BIT(ctz(BIT(ctz(off | 4)) | BIT(31 - clz(count))));

This gives the following lengths for input offset and sizes:

offset  request length
0       1       1
0       2       2
0       3       2
0       4       4
0       5       4
1       1       1
1       2       1
1       3       1
1       4       1
1       5       1
2       1       1
2       2       2
2       3       2
2       4       2
2       5       2
3       1       1
3       2       1
3       3       1
3       4       1
3       5       1

The implementation in the current patch generates some sub-optimal
lengths for certain offset/size pairs (i.e. a wider access could be performed).

Andrew
diff mbox series

Patch

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 1ea15621e588..889349beb284 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -559,8 +559,8 @@  static ssize_t fsi_slave_sysfs_raw_read(struct file *file,
 		return -EINVAL;
 
 	for (total_len = 0; total_len < count; total_len += read_len) {
-		read_len = min_t(size_t, count, 4);
-		read_len -= off & 0x3;
+		read_len = ((count | off) & 1) ?
+				1 : min_t(size_t, count, 4 - (off & 3));
 
 		rc = fsi_slave_read(slave, off, buf + total_len, read_len);
 		if (rc)
@@ -587,8 +587,8 @@  static ssize_t fsi_slave_sysfs_raw_write(struct file *file,
 		return -EINVAL;
 
 	for (total_len = 0; total_len < count; total_len += write_len) {
-		write_len = min_t(size_t, count, 4);
-		write_len -= off & 0x3;
+		write_len = ((count | off) & 1) ?
+				1 : min_t(size_t, count, 4 - (off & 3));
 
 		rc = fsi_slave_write(slave, off, buf + total_len, write_len);
 		if (rc)