Re: [XEN v9 4/4] xen/arm64: io: Handle data abort due to cache maintenance instructions

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 1 Mar 2022, Ayan Kumar Halder wrote:
> When the data abort is caused due to cache maintenance for an address,
> there are two scenarios:-
> 
> 1. Address belonging to a non emulated region - For this, Xen should
> set the corresponding bit in the translation table entry to valid and
> return to the guest to retry the instruction. This can happen sometimes
> as Xen need to set the translation table entry to invalid. (for eg
> 'Break-Before-Make' sequence).
> 
> 2. Address belongs to an emulated region - Xen should ignore the
> instruction (ie increment the PC) and return to the guest.
> 
> We try to deal with scenario#1, by invoking check_p2m(). If this is
> unsuccessful, then we assume scenario#2.
> 
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> 
> Changelog:-
> 
> v1...v8 - NA
> 
> v9 - Extracted this change from "[XEN v7 2/2] xen/arm64: io: Support
> instructions (for which ISS is not ..." into a separate patch of its
> own. The reason being this addresses an existing bug in the codebase.
> 
>  xen/arch/arm/include/asm/mmio.h |  3 ++-
>  xen/arch/arm/io.c               | 11 +++++++++++
>  xen/arch/arm/traps.c            |  6 ++++++
>  3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/include/asm/mmio.h b/xen/arch/arm/include/asm/mmio.h
> index ef2c57a2d5..75d362d5f5 100644
> --- a/xen/arch/arm/include/asm/mmio.h
> +++ b/xen/arch/arm/include/asm/mmio.h
> @@ -34,7 +34,8 @@ enum instr_decode_state
>       * Instruction is decoded successfully. It is a ldr/str post indexing
>       * instruction.
>       */
> -    INSTR_LDR_STR_POSTINDEXING
> +    INSTR_LDR_STR_POSTINDEXING,
> +    INSTR_IGNORE                    /* Instruction is ignored */
>  };
>  
>  typedef struct
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index ebcb8ed548..7e9dd4bb08 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -139,6 +139,17 @@ void try_decode_instruction(const struct cpu_user_regs 
> *regs,
>          return;
>      }
>  
> +    /*
> +     * When the data abort is caused due to cache maintenance, Xen should 
> ignore
> +     * this instruction as the cache maintenance was caused on an address 
> belonging
> +     * to the emulated region.
> +     */
> +    if ( info->dabt.cache )
> +    {
> +        info->dabt_instr.state = INSTR_IGNORE;
> +        return;
> +    }
> +
>      /*
>       * Armv8 processor does not provide a valid syndrome for decoding some
>       * instructions. So in order to process these instructions, Xen must
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index e491ca15d7..5879640b73 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2011,6 +2011,12 @@ static void do_trap_stage2_abort_guest(struct 
> cpu_user_regs *regs,
>  
>          try_decode_instruction(regs, &info);
>  
> +        if ( info.dabt_instr.state == INSTR_IGNORE )
> +        {
> +            advance_pc(regs, hsr);
> +            return;
> +        }
> +
>          /*
>           * If Xen could not decode the instruction or encountered an error
>           * while decoding, then it should forward the abort to the guest.
> -- 
> 2.17.1
>