[PATCH 1/2] powerpc/signal: Fix confusing header documentation in sigcontext.h

[PATCH 1/2] powerpc/signal: Fix confusing header documentation in sigcontext.h

[Anshuman Khandual]

As 'vmx_reserve' array element had been expanded to contain 101 double
words, the comment block above that needs to be updated. Also changed
the array size declaration to reflect the logic mentioned in the comment
block above. This change helps in explaining how the HW registers are
represented in the array.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/include/uapi/asm/sigcontext.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/uapi/asm/sigcontext.h b/arch/powerpc/include/uapi/asm/sigcontext.h
index 9c1f24f..b0c5c46 100644
--- a/arch/powerpc/include/uapi/asm/sigcontext.h
+++ b/arch/powerpc/include/uapi/asm/sigcontext.h
@@ -28,7 +28,7 @@ struct sigcontext {
 /*
  * To maintain compatibility with current implementations the sigcontext is
  * extended by appending a pointer (v_regs) to a quadword type (elf_vrreg_t)
- * followed by an unstructured (vmx_reserve) field of 69 doublewords.  This
+ * followed by an unstructured (vmx_reserve) field of 101 doublewords. This
  * allows the array of vector registers to be quadword aligned independent of
  * the alignment of the containing sigcontext or ucontext. It is the
  * responsibility of the code setting the sigcontext to set this pointer to
@@ -80,7 +80,7 @@ struct sigcontext {
  * registers and vscr/vrsave.
  */
 	elf_vrreg_t	__user *v_regs;
-	long		vmx_reserve[ELF_NVRREG+ELF_NVRREG+32+1];
+	long		vmx_reserve[ELF_NVRREG+ELF_NVRREG+1+32];
 #endif
 };
 
-- 
2.1.0

[PATCH 2/2] powerpc/signal: Add helper function to fetch quad word aligned pointer

[Anshuman Khandual]

This patch adds one helper function 'vmx_reserve_addr' which computes
quad word aligned pointer for vmx_reserve array element in sigcontext
structure making the code more readable.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/signal_64.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index c7c24d2..e8762f5 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -73,6 +73,12 @@ static const char fmt32[] = KERN_INFO \
 static const char fmt64[] = KERN_INFO \
 	"%s[%d]: bad frame in %s: %016lx nip %016lx lr %016lx\n";
 
+static elf_vrreg_t __user *vmx_reserve_addr(struct sigcontext __user *sc)
+{
+		return (elf_vrreg_t __user *)
+			(((unsigned long)sc->vmx_reserve + 15) & ~0xful);
+}
+
 /*
  * Set up the sigcontext for the signal frame.
  */
@@ -90,7 +96,7 @@ static long setup_sigcontext(struct sigcontext __user *sc, struct pt_regs *regs,
 	 * v_regs pointer or not
 	 */
 #ifdef CONFIG_ALTIVEC
-	elf_vrreg_t __user *v_regs = (elf_vrreg_t __user *)(((unsigned long)sc->vmx_reserve + 15) & ~0xful);
+	elf_vrreg_t __user *v_regs = vmx_reserve_addr(sc);
 #endif
 	unsigned long msr = regs->msr;
 	long err = 0;
@@ -181,10 +187,8 @@ static long setup_tm_sigcontexts(struct sigcontext __user *sc,
 	 * v_regs pointer or not.
 	 */
 #ifdef CONFIG_ALTIVEC
-	elf_vrreg_t __user *v_regs = (elf_vrreg_t __user *)
-		(((unsigned long)sc->vmx_reserve + 15) & ~0xful);
-	elf_vrreg_t __user *tm_v_regs = (elf_vrreg_t __user *)
-		(((unsigned long)tm_sc->vmx_reserve + 15) & ~0xful);
+	elf_vrreg_t __user *v_regs = vmx_reserve_addr(sc);
+	elf_vrreg_t __user *tm_v_regs = vmx_reserve_addr(tm_sc);
 #endif
 	unsigned long msr = regs->msr;
 	long err = 0;
-- 
2.1.0

Re: [2/2] powerpc/signal: Add helper function to fetch quad word aligned pointer

[Michael Ellerman]

On Mon, 2015-06-07 at 10:25:34 UTC, Anshuman Khandual wrote:
> This patch adds one helper function 'vmx_reserve_addr' which computes
> quad word aligned pointer for vmx_reserve array element in sigcontext
> structure making the code more readable.
> 
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/signal_64.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index c7c24d2..e8762f5 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -73,6 +73,12 @@ static const char fmt32[] = KERN_INFO \
>  static const char fmt64[] = KERN_INFO \
>  	"%s[%d]: bad frame in %s: %016lx nip %016lx lr %016lx\n";
>  
> +static elf_vrreg_t __user *vmx_reserve_addr(struct sigcontext __user *sc)
> +{
> +		return (elf_vrreg_t __user *)
> +			(((unsigned long)sc->vmx_reserve + 15) & ~0xful);

You have more tabs here than you need. Please put it on one line, I don't care
if it's a bit more than 80 chars.

Please add a comment describing what it's doing and why.

Also I think I'd prefer if it was named sigcontext_vmx_regs() or something more
like that, ie. it's about sigcontext primarily.

cheers