This RFC fixup is POC to demonstrate how the SFrame reading code
could benefit from introducing an internal FDE representation (struct
sframe_fde_internal) similar to the used internal FRE representation
(struct sframe_fre).  The goal is to eliminate the passing through of
fde_start_base in many places as well as the various computations of the
effective function start address (= *fde_start_base + fde->start_addr)
throughout this module.  The internal FDE representation simply conveys
the effective function start address via the "unsigned long
func_start_addr" field.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 kernel/unwind/sframe.c | 52 ++++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 3d7ac4eaa8b7..f88fc2c92c58 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -17,6 +17,15 @@
 #include "sframe.h"
 #include "sframe_debug.h"
 
+struct sframe_fde_internal {
+	unsigned long	func_start_addr;
+	u32		func_size;
+	u32		fres_off;
+	u32		fres_num;
+	u8		info;
+	u8		rep_size;
+};
+
 struct sframe_fre {
 	unsigned int	size;
 	u32		ip_off;
@@ -45,20 +54,26 @@ static __always_inline unsigned char offset_size_enum_to_size(unsigned char off_
 
 static __always_inline int __read_fde(struct sframe_section *sec,
 				      unsigned int fde_num,
-				      struct sframe_fde *fde,
-				      unsigned long *fde_start_base)
+				      struct sframe_fde_internal *fde)
 {
-	unsigned long fde_addr, ip;
+	unsigned long fde_addr, func_addr;
+	struct sframe_fde _fde;
 
 	fde_addr = sec->fdes_start + (fde_num * sizeof(struct sframe_fde));
-	unsafe_copy_from_user(fde, (void __user *)fde_addr,
+	unsafe_copy_from_user(&_fde, (void __user *)fde_addr,
 			      sizeof(struct sframe_fde), Efault);
 
-	ip = fde_addr + fde->start_addr;
-	if (ip < sec->text_start || ip > sec->text_end)
+	func_addr = fde_addr + _fde.start_addr;
+	if (func_addr < sec->text_start || func_addr > sec->text_end)
 		return -EINVAL;
 
-	*fde_start_base = fde_addr;
+	fde->func_start_addr	= func_addr;
+	fde->func_size		= _fde.func_size;
+	fde->fres_off		= _fde.fres_off;
+	fde->fres_num		= _fde.fres_num;
+	fde->info		= _fde.info;
+	fde->rep_size		= _fde.rep_size;
+
 	return 0;
 
 Efault:
@@ -67,8 +82,7 @@ static __always_inline int __read_fde(struct sframe_section *sec,
 
 static __always_inline int __find_fde(struct sframe_section *sec,
 				      unsigned long ip,
-				      struct sframe_fde *fde,
-				      unsigned long *fde_start_base)
+				      struct sframe_fde_internal *fde)
 {
 	unsigned long func_addr_low = 0, func_addr_high = ULONG_MAX;
 	struct sframe_fde __user *first, *low, *high, *found = NULL;
@@ -109,13 +123,13 @@ static __always_inline int __find_fde(struct sframe_section *sec,
 	if (!found)
 		return -EINVAL;
 
-	ret = __read_fde(sec, found - first, fde, fde_start_base);
+	ret = __read_fde(sec, found - first, fde);
 	if (ret)
 		return ret;
 
 	/* make sure it's not in a gap */
-	if (ip < *fde_start_base + fde->start_addr ||
-	    ip >= *fde_start_base + fde->start_addr + fde->func_size)
+	if (ip < fde->func_start_addr ||
+	    ip >= fde->func_start_addr + fde->func_size)
 		return -EINVAL;
 
 	return 0;
@@ -165,7 +179,7 @@ static __always_inline int __find_fde(struct sframe_section *sec,
 		 s32:	UNSAFE_GET_USER_SIGNED_INC(to, from, size, label))
 
 static __always_inline int __read_fre(struct sframe_section *sec,
-				      struct sframe_fde *fde,
+				      struct sframe_fde_internal *fde,
 				      unsigned long fre_addr,
 				      struct sframe_fre *fre)
 {
@@ -244,8 +258,7 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 }
 
 static __always_inline int __find_fre(struct sframe_section *sec,
-				      struct sframe_fde *fde,
-				      unsigned long fde_start_base,
+				      struct sframe_fde_internal *fde,
 				      unsigned long ip,
 				      struct unwind_user_frame *frame)
 {
@@ -257,7 +270,7 @@ static __always_inline int __find_fre(struct sframe_section *sec,
 	unsigned int i;
 	u32 ip_off;
 
-	ip_off = ip - (fde_start_base + fde->start_addr);
+	ip_off = ip - fde->func_start_addr;
 
 	if (fde_type == SFRAME_FDE_TYPE_PCMASK)
 		ip_off %= fde->rep_size;
@@ -306,8 +319,7 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 {
 	struct mm_struct *mm = current->mm;
 	struct sframe_section *sec;
-	struct sframe_fde fde;
-	unsigned long fde_start_base;
+	struct sframe_fde_internal fde;
 	int ret;
 
 	if (!mm)
@@ -323,11 +335,11 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 				    sec->sframe_end - sec->sframe_start))
 		return -EFAULT;
 
-	ret = __find_fde(sec, ip, &fde, &fde_start_base);
+	ret = __find_fde(sec, ip, &fde);
 	if (ret)
 		goto end;
 
-	ret = __find_fre(sec, &fde, fde_start_base, ip, frame);
+	ret = __find_fre(sec, &fde, ip, frame);
 end:
 	user_read_access_end();
 
-- 
2.48.1

This RFC fixup is POC to demonstrate how the SFrame validation code
would adjust if introducing an internal FDE representation (struct
sframe_fde_internal) similar to the used internal FRE representation
(struct sframe_fre) in the SFrame reading code.  The goal is to
eliminate the passing through of fde_start_base in many places as well
as the various computations of the effective function start address
(= *fde_start_base + fde->start_addr) throughout this module.  The
internal FDE representation simply conveys the effective function start
address via the "unsigned long func_start_addr" field.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 kernel/unwind/sframe.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index f88fc2c92c58..f2977c010117 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -354,21 +354,21 @@ int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
 #ifdef CONFIG_SFRAME_VALIDATION
 
 static int safe_read_fde(struct sframe_section *sec,
-			 unsigned int fde_num, struct sframe_fde *fde,
-			 unsigned long *fde_start_base)
+			 unsigned int fde_num, struct sframe_fde_internal *fde)
 {
 	int ret;
 
 	if (!user_read_access_begin((void __user *)sec->sframe_start,
 				    sec->sframe_end - sec->sframe_start))
 		return -EFAULT;
-	ret = __read_fde(sec, fde_num, fde, fde_start_base);
+	ret = __read_fde(sec, fde_num, fde);
 	user_read_access_end();
 	return ret;
 }
 
 static int safe_read_fre(struct sframe_section *sec,
-			 struct sframe_fde *fde, unsigned long fre_addr,
+			 struct sframe_fde_internal *fde,
+			 unsigned long fre_addr,
 			 struct sframe_fre *fre)
 {
 	int ret;
@@ -388,18 +388,18 @@ static int sframe_validate_section(struct sframe_section *sec)
 
 	for (i = 0; i < sec->num_fdes; i++) {
 		struct sframe_fre *fre, *prev_fre = NULL;
-		unsigned long ip, fde_start_base, fre_addr;
-		struct sframe_fde fde;
+		unsigned long ip, fre_addr;
+		struct sframe_fde_internal fde;
 		struct sframe_fre fres[2];
 		bool which = false;
 		unsigned int j;
 		int ret;
 
-		ret = safe_read_fde(sec, i, &fde, &fde_start_base);
+		ret = safe_read_fde(sec, i, &fde);
 		if (ret)
 			return ret;
 
-		ip = fde_start_base + fde.start_addr;
+		ip = fde.func_start_addr;
 		if (ip <= prev_ip) {
 			dbg_sec("fde %u not sorted\n", i);
 			return -EFAULT;
@@ -416,8 +416,8 @@ static int sframe_validate_section(struct sframe_section *sec)
 			ret = safe_read_fre(sec, &fde, fre_addr, fre);
 			if (ret) {
 				dbg_sec("fde %u: __read_fre(%u) failed\n", i, j);
-				dbg_sec("FDE: start_addr:0x%x func_size:0x%x fres_off:0x%x fres_num:%d info:%u rep_size:%u\n",
-					fde.start_addr, fde.func_size,
+				dbg_sec("FDE: func_start_addr:0x%lx func_size:0x%x fres_off:0x%x fres_num:%d info:%u rep_size:%u\n",
+					fde.func_start_addr, fde.func_size,
 					fde.fres_off, fde.fres_num,
 					fde.info, fde.rep_size);
 				return ret;
-- 
2.48.1