Re: [Xen-devel] [PATCH 7/9] xen/arm: traps: MMIO should only be emulated for fault translation

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 22 Jun 2016, Julien Grall wrote:
> The function do_trap_data_abort_guest assumes that a stage-2 data abort
> can only be taken for a translation fault or permission fault today.
> 
> Whilst this is true today, it might not be in the future. Rather than
> emulating the MMIO for any fault other than the permission one, print
> a warning message when the fault is not handled by Xen.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
>  xen/arch/arm/traps.c | 51 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 28 insertions(+), 23 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 785e3e9..591de3c 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2468,35 +2468,40 @@ static void do_trap_data_abort_guest(struct 
> cpu_user_regs *regs,
>          /* Trap was triggered by mem_access, work here is done */
>          if ( !rc )
>              return;
> +        break;
>      }
> -    break;
> -    }
> -
> -    if ( dabt.s1ptw )
> -        goto bad_data_abort;
> +    case FSC_FLT_TRANS:
> +        if ( dabt.s1ptw )
> +            goto bad_data_abort;
>  
> -    /* XXX: Decode the instruction if ISS is not valid */
> -    if ( !dabt.valid )
> -        goto bad_data_abort;
> +        /* XXX: Decode the instruction if ISS is not valid */
> +        if ( !dabt.valid )
> +            goto bad_data_abort;
>  
> -    /*
> -     * Erratum 766422: Thumb store translation fault to Hypervisor may
> -     * not have correct HSR Rt value.
> -     */
> -    if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) && dabt.write 
> )
> -    {
> -        rc = decode_instruction(regs, &info.dabt);
> -        if ( rc )
> +        /*
> +         * Erratum 766422: Thumb store translation fault to Hypervisor may
> +         * not have correct HSR Rt value.
> +         */
> +        if ( check_workaround_766422() && (regs->cpsr & PSR_THUMB) &&
> +             dabt.write )
>          {
> -            gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
> -            goto bad_data_abort;
> +            rc = decode_instruction(regs, &info.dabt);
> +            if ( rc )
> +            {
> +                gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
> +                goto bad_data_abort;
> +            }
>          }
> -    }
>  
> -    if (handle_mmio(&info))
> -    {
> -        advance_pc(regs, hsr);
> -        return;
> +        if ( handle_mmio(&info) )
> +        {
> +            advance_pc(regs, hsr);
> +            return;
> +        }
> +        break;
> +    default:
> +        gprintk(XENLOG_WARNING, "Unsupported DFSC: HSR=%#x DFSC=%#x\n",
> +                hsr.bits, dabt.dfsc);

Given that bad_data_abort, which is right after, will print HSR again, I
would remove it from this message as it's redundant.


>      }
>  
>  bad_data_abort:
> -- 
> 1.9.1
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel