sparc: allow CASA with ASI 0xa from user space

PATCH In review Alex Zuepke 6 participants 10 messages Dec 4, 2015 – Dec 10, 2015

lore

Share & tools

lore

Alex Zuepke

author

2015-12-04 15:01 UTC #0 lore raw

PATCH 1/1 sparc: allow CASA with ASI 0xa from user space

LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).

Signed-off-by Alex Zuepke <[email protected]>

---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

target-sparc/translate.c +2 -1

index 41a3319..63440dd 100644
@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
                         goto priv_insn;
                     }
 #endif

Signature · 1 line

1.9.1

Züpke, Alexander

2015-12-07 12:41 UTC #2 lore raw

Hi Fabien,

Quoted 69 lines · Fabien Chouteau

Hello Alex,

Thanks for your patch! I have a couple of comments.

Quoted 13 lines · Alex Zuepke

LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).
Signed-off-by Alex Zuepke <[email protected]>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-sparc/translate.c +0 -0
index 41a3319..63440dd 100644

target-sparc/translate.c static void disas_sparc_insn(DisasContext * dc, · line 5101

Quoted 6 lines

target-sparc/translate.c +1 -1

@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc,
 unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */

Since this is a LEON3 specific feature, when you check the ASI you should also check CPU_FEATURE_CASA.

Quoted 4 lines

target-sparc/translate.c +1 -0

+                    if ((GET_FIELD(insn, 19, 26) != 0xa) &&
 !supervisor(dc)) {
                         goto priv_insn;

That would give you something like this:

target-sparc/translate.c +3 -0

index 41a3319..b93faea 100644
@@ -2463,6 +2463,9 @@ static void gen_faligndata(TCGv dst, TCGv gsr, TCGv s1,
 TCGv s2)
 }
 #endif
 
+#define IU_FEATURE(dc, FEATURE)                            \
+    (((dc)->def->features & CPU_FEATURE_ ## FEATURE))
+
 #define CHECK_IU_FEATURE(dc, FEATURE)                      \
     if (!((dc)->def->features & CPU_FEATURE_ ## FEATURE))  \

         goto illegal_insn;

target-sparc/translate.c +5 -1

@@ -5097,7 +5100,11 @@ static void disas_sparc_insn(DisasContext * dc,
 unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if (!((IU_FEATURE(dc, CASA) &&
+                           (GET_FIELD(insn, 19, 26) == 0xa))
+                          || supervisor(dc))) {
                         goto priv_insn;
                     }

#endif

Quoted 3 lines

to be tested of course ;)
Regards,

The check for the CASA feature on SPARC v8 is already in the existng code, just 3 lines above the second hunk in the patch:

  #if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
                  case 0x3c: /* V9 or LEON3 casa */
  #ifndef TARGET_SPARC64
                      CHECK_IU_FEATURE(dc, CASA);
                      if (IS_IMM) {
                          goto illegal_insn;
                      }
                      if (!supervisor(dc)) {
                          goto priv_insn;
                      }
                      ...

So I don't think it's necessary to check it again.

Best regards
Alex

Fabien Chouteau

2015-12-07 12:56 UTC #3 lore raw

Quoted 4 lines · Alexander

The check for the CASA feature on SPARC v8 is already in the existng code, just 3 lines above the second hunk in the patch:

Yes but you add a special case that allow casa from user space, this special case is only available on LEON3 so that should be taken into account in the priv_insn check.

Regards,

Züpke, Alexander

2015-12-07 13:07 UTC #4 lore raw

Hi Fabien,

Quoted 7 lines · Fabien Chouteau

Quoted 4 lines · Alexander
The check for the CASA feature on SPARC v8 is already in the existng code, just 3 lines above the second hunk in the patch:

Quoted 6 lines

Yes but you add a special case that allow casa from user space, this special case is only available on LEON3 so that should be taken into account in the priv_insn check.
Regards,

No, other v8 SPARCs do not have CASA at all, only LEON3 and 4 have CASA backported from v9. But when CASA is available on LEON, it's available in both user and supervisor mode. The only restriction for user mode is that one must use ASI 0xa.

Best regards
Alex

Fabien Chouteau

2015-12-07 15:03 UTC #5 lore raw

Quoted 13 lines · Alexander

Hi Fabien,
Quoted 11 lines · Fabien Chouteau
Quoted 4 lines · Alexander
The check for the CASA feature on SPARC v8 is already in the existng code, just 3 lines above the second hunk in the patch:
Yes but you add a special case that allow casa from user space, this special case is only available on LEON3 so that should be taken into account in the priv_insn check.

Quoted 9 lines

Quoted 2 lines
Regards,
No, other v8 SPARCs do not have CASA at all, only LEON3 and 4 have CASA backported from v9. But when CASA is available on LEON, it's available in both user and supervisor mode. The only restriction for user mode is that one must use ASI 0xa.

You're right, and ASI 0xA is also user data in SPARKv9.

Patch looks good then :)

Thanks,

Richard Henderson

2015-12-08 19:59 UTC #6 lore raw

PATCH 1/1 sparc: allow CASA with ASI 0xa from user space

In reply to Alex Zuepke

Quoted 14 lines · Alex Zuepke

LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).
Signed-off-by Alex Zuepke <[email protected]>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-sparc/translate.c +0 -0
index 41a3319..63440dd 100644

target-sparc/translate.c static void disas_sparc_insn(DisasContext * dc, unsigned int insn) · line 5104

Quoted 9 lines

target-sparc/translate.c +2 -1

@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
                         goto priv_insn;
                     }
 #endif

Reviewed-by Richard Henderson <[email protected]>

This should probably be merged for 2.5.

For 2.6, I have a branch with substantial changes for the sparc backend. Part of which totally revamps the way ASIs are handled. I believe it gets this right. See

git://github.com/rth7680/qemu.git tgt-sparc

Peter Maydell

2015-12-08 21:29 UTC #7 lore raw

Quoted 27 lines · Richard Henderson

Quoted 14 lines · Alex Zuepke

LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).
Signed-off-by Alex Zuepke <[email protected]>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-sparc/translate.c +0 -0
index 41a3319..63440dd 100644

target-sparc/translate.c static void disas_sparc_insn(DisasContext * dc, unsigned int insn) · line 5104

Quoted 9 lines

target-sparc/translate.c +2 -1

@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
                         goto priv_insn;
                     }
 #endif

Reviewed-by Richard Henderson <[email protected]>

Quoted 1 line

This should probably be merged for 2.5.

Very late, but a very small patch which only affects TCG SPARC, so I'm OK with applying it to master. (Mark, did you want to ack/review?)

thanks
-- PMM

Mark Cave-Ayland

2015-12-09 23:25 UTC #8 lore raw

Quoted 28 lines · Peter Maydell

Quoted 27 lines · Richard Henderson

Quoted 14 lines · Alex Zuepke

LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).
Signed-off-by Alex Zuepke <[email protected]>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-sparc/translate.c +0 -0
index 41a3319..63440dd 100644

target-sparc/translate.c static void disas_sparc_insn(DisasContext * dc, unsigned int insn) · line 5104

Quoted 9 lines

target-sparc/translate.c +2 -1

@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
                         goto priv_insn;
                     }
 #endif

Reviewed-by Richard Henderson <[email protected]>

Quoted 4 lines

Quoted 1 line
This should probably be merged for 2.5.
Very late, but a very small patch which only affects TCG SPARC, so I'm OK with applying it to master. (Mark, did you want to ack/review?)

My understanding from this thread is that it only affects LEON3 - while I don't have any such images to test with, given that it's restricted to LEON3 then I'd be fine if you were to apply this to master with an R-B from Richard and Fabien (I'll do another OpenBIOS test suite run over the weekend just to be sure).

ATB,

Mark.

Peter Maydell

2015-12-10 11:43 UTC #9 lore raw

Quoted 32 lines · Mark Cave-Ayland

Quoted 30 lines · Peter Maydell
Quoted 28 lines · Richard Henderson
Quoted 14 lines · Alex Zuepke
LEON3 allows the CASA instruction to be used from user space if the ASI is set to 0xa (user data).
Signed-off-by Alex Zuepke <[email protected]>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
target-sparc/translate.c +0 -0
index 41a3319..63440dd 100644
target-sparc/translate.c static void disas_sparc_insn(DisasContext * dc, unsigned int insn) · line 5104
Quoted 9 lines
target-sparc/translate.c +2 -1
@@ -5097,7 +5097,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn)
                     if (IS_IMM) {
                         goto illegal_insn;
                     }
-                    if (!supervisor(dc)) {
+                    /* LEON3 allows CASA from user space with ASI 0xa */
+                    if ((GET_FIELD(insn, 19, 26) != 0xa) && !supervisor(dc)) {
                         goto priv_insn;
                     }
 #endif
Reviewed-by Richard Henderson <[email protected]>
This should probably be merged for 2.5.

Quoted 8 lines

Quoted 2 lines
Very late, but a very small patch which only affects TCG SPARC, so I'm OK with applying it to master. (Mark, did you want to ack/review?)
My understanding from this thread is that it only affects LEON3 - while I don't have any such images to test with, given that it's restricted to LEON3 then I'd be fine if you were to apply this to master with an R-B from Richard and Fabien (I'll do another OpenBIOS test suite run over the weekend just to be sure).

OK. I have applied this patch to master.

thanks
-- PMM