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 <address@hidden> 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 <address@hidden>
> > ---
> >  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