[PATCH] Fix VIS emulation bugs

[PATCH] Fix VIS emulation bugs

[Joseph S. Myers]

This patch fixes some bugs in VIS emulation that cause the GCC test
failure

FAIL: gcc.target/sparc/pdist-3.c execution test

for both 32-bit and 64-bit testing on hardware lacking these
instructions.  The emulation code for the pdist instruction uses
RS1(insn) for both source registers rs1 and rs2, which is obviously
wrong and leads to the instruction doing nothing (the observed
problem), and further inspection of the code shows that RS1 uses a
shift of 24 and RD a shift of 25, which clearly cannot both be right;
examining SPARC documentation indicates the correct shift for RS1 is
14.

This patch fixes the bug if single-stepping over the affected
instruction in the debugger, but not if the testcase is run
standalone.  For that, Wind River has another patch I hope they will
send as a followup to this patch submission.

Signed-off-by: Joseph Myers <joseph@codesourcery.com>
---
diff --git a/arch/sparc64/kernel/visemul.c b/arch/sparc64/kernel/visemul.c
index 9e05cb5..69f8a35 100644
--- a/arch/sparc64/kernel/visemul.c
+++ b/arch/sparc64/kernel/visemul.c
@@ -131,7 +131,7 @@
 #define VIS_OPF_SHIFT	5
 #define VIS_OPF_MASK	(0x1ff << VIS_OPF_SHIFT)
 
-#define RS1(INSN)	(((INSN) >> 24) & 0x1f)
+#define RS1(INSN)	(((INSN) >> 14) & 0x1f)
 #define RS2(INSN)	(((INSN) >>  0) & 0x1f)
 #define RD(INSN)	(((INSN) >> 25) & 0x1f)
 
@@ -445,7 +445,7 @@ static void pdist(struct pt_regs *regs, unsigned int insn)
 	unsigned long i;
 
 	rs1 = fpd_regval(f, RS1(insn));
-	rs2 = fpd_regval(f, RS1(insn));
+	rs2 = fpd_regval(f, RS2(insn));
 	rd = fpd_regaddr(f, RD(insn));
 
 	rd_val = *rd;

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix VIS emulation bugs

[David Miller]

From: "Joseph S. Myers" <joseph@codesourcery.com>
Date: Thu, 4 Dec 2008 00:41:50 +0000 (UTC)

> This patch fixes some bugs in VIS emulation that cause the GCC test
> failure
> 
> FAIL: gcc.target/sparc/pdist-3.c execution test

Thanks a lot for this fix, I'd been noticing that failure
and had been meaning to look into it.

> for both 32-bit and 64-bit testing on hardware lacking these
> instructions.  The emulation code for the pdist instruction uses
> RS1(insn) for both source registers rs1 and rs2, which is obviously
> wrong and leads to the instruction doing nothing (the observed
> problem), and further inspection of the code shows that RS1 uses a
> shift of 24 and RD a shift of 25, which clearly cannot both be right;
> examining SPARC documentation indicates the correct shift for RS1 is
> 14.
> 
> This patch fixes the bug if single-stepping over the affected
> instruction in the debugger, but not if the testcase is run
> standalone.  For that, Wind River has another patch I hope they will
> send as a followup to this patch submission.
> 
> Signed-off-by: Joseph Myers <joseph@codesourcery.com>

Patch applied, thanks!

Re: [PATCH] Fix VIS emulation bugs

[Hong H. Pham]

Hi,

>> This patch fixes the bug if single-stepping over the affected
>> instruction in the debugger, but not if the testcase is run
>> standalone.  For that, Wind River has another patch I hope they will
>> send as a followup to this patch submission.
>>
>> Signed-off-by: Joseph Myers <joseph@codesourcery.com>
> 
> Patch applied, thanks!

Here's the follow up patch to Joseph's VIS emulation fix.  The pdist
testcase runs successfully as a standalone with this patch.

Best regards,
Hong


Copy the FPU state to the task's thread_info->fpregs for the VIS emulation
functions to access.

Signed-off-by: Hong H. Pham <hong.pham@windriver.com>
---
  arch/sparc64/kernel/visemul.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/sparc64/kernel/visemul.c b/arch/sparc64/kernel/visemul.c
index 4e285f9..a4428fd 100644
--- a/arch/sparc64/kernel/visemul.c
+++ b/arch/sparc64/kernel/visemul.c
@@ -802,16 +802,18 @@ int vis_emul(struct pt_regs *regs, unsigned int insn)
  	BUG_ON(regs->tstate & TSTATE_PRIV);

  	if (test_thread_flag(TIF_32BIT))
  		pc = (u32)pc;

  	if (get_user(insn, (u32 __user *) pc))
  		return -EFAULT;

+	save_and_clear_fpu();
+
  	opf = (insn & VIS_OPF_MASK) >> VIS_OPF_SHIFT;
  	switch (opf) {
  	default:
  		return -EINVAL;

  	/* Pixel Formatting Instructions.  */
  	case FPACK16_OPF:
  	case FPACK32_OPF:
-- 
1.5.5.1

Re: [PATCH] Fix VIS emulation bugs

[David Miller]

From: "Hong H. Pham" <hong.pham@windriver.com>
Date: Thu, 04 Dec 2008 09:51:28 -0500

> Hi,
> 
> >> This patch fixes the bug if single-stepping over the affected
> >> instruction in the debugger, but not if the testcase is run
> >> standalone.  For that, Wind River has another patch I hope they will
> >> send as a followup to this patch submission.
> >>
> >> Signed-off-by: Joseph Myers <joseph@codesourcery.com>
> > Patch applied, thanks!
> 
> Here's the follow up patch to Joseph's VIS emulation fix.  The pdist
> testcase runs successfully as a standalone with this patch.
 ...
> Copy the FPU state to the task's thread_info->fpregs for the VIS emulation
> functions to access.
> 
> Signed-off-by: Hong H. Pham <hong.pham@windriver.com>

Also applied, thanks a lot!