[Qemu-devel] Re: sparc32 do_unassigned_access overhaul

[Blue Swirl]

On Fri, Jan 15, 2010 at 6:46 PM, Artyom Tarasenko
<address@hidden> wrote:
> According to pages 9-31 - 9-34 of "SuperSPARC & MultiCache Controller
> User's Manual":
>
> 1. "A lower priority fault may not overwrite the
>    MFSR status of a higher priority fault."
> 2. The MFAR is overwritten according to the policy defined for the MFSR
> 3. The overwrite bit is asserted if the fault status register (MFSR)
>   has been written more than once by faults of the same class
> 4. SuperSPARC will never place instruction fault addresses in the MFAR.
>
> Implementation of points 1-3 allows booting Solaris 2.6 and 2.5.1.

Nice work! This also passes my tests. However, there are some
CODING_STYLE issues.

>
> Signed-off-by: Artyom Tarasenko <address@hidden>
> ---
> diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
> index 381e6c4..3a56ce9 100644
> --- a/target-sparc/op_helper.c
> +++ b/target-sparc/op_helper.c
> @@ -3714,6 +3714,7 @@ void do_unassigned_access(target_phys_addr_t addr, int 
> is_write, int is_exec,
>                           int is_asi, int size)
>  {
>     CPUState *saved_env;
> +    int fault_type;
>
>     /* XXX: hack to restore env in all cases, even if not called from
>        generated code */
> @@ -3731,18 +3732,27 @@ void do_unassigned_access(target_phys_addr_t addr, 
> int is_write, int is_exec,
>                is_exec ? "exec" : is_write ? "write" : "read", size,
>                size == 1 ? "" : "s", addr, env->pc);
>  #endif
> -    if (env->mmuregs[3]) /* Fault status register */
> -        env->mmuregs[3] = 1; /* overflow (not read before another fault) */
> -    if (is_asi)
> -        env->mmuregs[3] |= 1 << 16;
> -    if (env->psrs)
> -        env->mmuregs[3] |= 1 << 5;
> -    if (is_exec)
> -        env->mmuregs[3] |= 1 << 6;
> -    if (is_write)
> -        env->mmuregs[3] |= 1 << 7;
> -    env->mmuregs[3] |= (5 << 2) | 2;
> -    env->mmuregs[4] = addr; /* Fault address register */
> +    /* Don't overwrite translation and access faults */
> +    fault_type=(env->mmuregs[3]&0x1c)>>2;

Must have spaces around '=', '&' and '>>'.

> +    if ((fault_type > 4) || (fault_type==0)) {

Must have spaces around '=='.

> +        env->mmuregs[3]=0; /* Fault status register */

and '='

> +        if (is_asi)
> +            env->mmuregs[3] |= 1 << 16;
> +        if (env->psrs)
> +            env->mmuregs[3] |= 1 << 5;
> +        if (is_exec)
> +            env->mmuregs[3] |= 1 << 6;
> +        if (is_write)
> +            env->mmuregs[3] |= 1 << 7;

Here you could add the {} which the original lacked, but as this is
only code movement it's not needed.

> +        env->mmuregs[3] |= (5 << 2) | 2;
> +        /* SuperSPARC will never place instruction fault addresses in the 
> FAR */
> +        if (!is_exec)
> +            env->mmuregs[4] = addr; /* Fault address register */

But this is new code so {} must be added.

> +    }
> +    /* overflow (same type fault was not read before another fault) */
> +    if (fault_type==((env->mmuregs[3]&0x1c))>>2)

Must have spaces around '=', '&' and '>>'.

> +        env->mmuregs[3] |= 1;
> +
>     if ((env->mmuregs[0] & MMU_E) && !(env->mmuregs[0] & MMU_NF)) {
>         if (is_exec)
>             raise_exception(TT_CODE_ACCESS);
> @@ -3750,6 +3760,10 @@ void do_unassigned_access(target_phys_addr_t addr, int 
> is_write, int is_exec,
>             raise_exception(TT_DATA_ACCESS);
>     }
>     env = saved_env;
> +    /* flush neverland mappings created during no-fault mode,
> +       so the sequential MMU faults report proper fault types */
> +    if (env->mmuregs[0] & MMU_NF)
> +        tlb_flush(env, 1);

New code, {}.

>  }
>  #else
>  void do_unassigned_access(target_phys_addr_t addr, int is_write, int is_exec,
>