[Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[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 <azu@sysgo.de>
---
 target-sparc/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 41a3319..63440dd 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -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
-- 
1.9.1

Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[Fabien Chouteau]

Hello Alex,

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

On 12/04/2015 04:01 PM, Alex Zuepke wrote:
> 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 <azu@sysgo.de>
> ---
>  target-sparc/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 41a3319..63440dd 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -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.

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

That would give you something like this:

diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 41a3319..b93faea 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -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;
@@ -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


to be tested of course ;)

Regards,

Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

["Züpke, Alexander"]

Hi Fabien,

> On December 7, 2015 at 12:35 PM Fabien Chouteau <chouteau@adacore.com> wrote:
> 
> 
> Hello Alex,
> 
> Thanks for your patch! I have a couple of comments. 
> 
> On 12/04/2015 04:01 PM, Alex Zuepke wrote:
> > 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 <azu@sysgo.de>
> > ---
> >  target-sparc/translate.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> > index 41a3319..63440dd 100644
> > --- a/target-sparc/translate.c
> > +++ b/target-sparc/translate.c
> > @@ -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.
> 
> > +                    if ((GET_FIELD(insn, 19, 26) != 0xa) &&
> > !supervisor(dc)) {
> >                          goto priv_insn;
> >                      }
> 
> That would give you something like this:
> 
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 41a3319..b93faea 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -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;
> @@ -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
> 
> 
> 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

Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[Fabien Chouteau]

On 12/07/2015 01:41 PM, "Züpke, Alexander" wrote:
> 
> 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,

Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

["Züpke, Alexander"]

Hi Fabien,

> On December 7, 2015 at 1:56 PM Fabien Chouteau <chouteau@adacore.com> wrote:
> On 12/07/2015 01:41 PM, "Züpke, Alexander" wrote:
> > 
> > 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,
> 

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

Re: [Qemu-devel] [PATCH] sparc: allow CASA with ASI 0xa from user space

[Fabien Chouteau]

On 12/07/2015 02:07 PM, "Züpke, Alexander" wrote:
> Hi Fabien,
> 
>> On December 7, 2015 at 1:56 PM Fabien Chouteau <chouteau@adacore.com> wrote:
>> On 12/07/2015 01:41 PM, "Züpke, Alexander" wrote:
>>>
>>> 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,
>>
> 
> 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,

[Qemu-devel] [PATCH for-2.5] sparc: allow CASA with ASI 0xa from user space

[Richard Henderson]

On 12/04/2015 07:01 AM, Alex Zuepke wrote:
> 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 <azu@sysgo.de>
> ---
>  target-sparc/translate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
> index 41a3319..63440dd 100644
> --- a/target-sparc/translate.c
> +++ b/target-sparc/translate.c
> @@ -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 <rth@twiddle.net>

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


r~

Re: [Qemu-devel] [PATCH for-2.5] sparc: allow CASA with ASI 0xa from user space

[Peter Maydell]

On 8 December 2015 at 19:59, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2015 07:01 AM, Alex Zuepke wrote:
>> 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 <azu@sysgo.de>
>> ---
>>  target-sparc/translate.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>> index 41a3319..63440dd 100644
>> --- a/target-sparc/translate.c
>> +++ b/target-sparc/translate.c
>> @@ -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 <rth@twiddle.net>
>
> 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

Re: [Qemu-devel] [PATCH for-2.5] sparc: allow CASA with ASI 0xa from user space

[Mark Cave-Ayland]

On 08/12/15 21:28, Peter Maydell wrote:

> On 8 December 2015 at 19:59, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/04/2015 07:01 AM, Alex Zuepke wrote:
>>> 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 <azu@sysgo.de>
>>> ---
>>>  target-sparc/translate.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>>> index 41a3319..63440dd 100644
>>> --- a/target-sparc/translate.c
>>> +++ b/target-sparc/translate.c
>>> @@ -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 <rth@twiddle.net>
>>
>> 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.

Re: [Qemu-devel] [PATCH for-2.5] sparc: allow CASA with ASI 0xa from user space

[Peter Maydell]

On 9 December 2015 at 23:24, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 08/12/15 21:28, Peter Maydell wrote:
>
>> On 8 December 2015 at 19:59, Richard Henderson <rth@twiddle.net> wrote:
>>> On 12/04/2015 07:01 AM, Alex Zuepke wrote:
>>>> 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 <azu@sysgo.de>
>>>> ---
>>>>  target-sparc/translate.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/target-sparc/translate.c b/target-sparc/translate.c
>>>> index 41a3319..63440dd 100644
>>>> --- a/target-sparc/translate.c
>>>> +++ b/target-sparc/translate.c
>>>> @@ -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 <rth@twiddle.net>
>>>
>>> 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).

OK. I have applied this patch to master.

thanks
-- PMM