Re: [PATCH v11 3/3] xen/arm64: io: Handle data abort due to cache maintenance instructions

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 17/03/2022 14:00, Ayan Kumar Halder wrote:
> diff --git a/xen/arch/arm/include/asm/mmio.h b/xen/arch/arm/include/asm/mmio.h
> index ca259a79c2..79e64d9af8 100644
> --- a/xen/arch/arm/include/asm/mmio.h
> +++ b/xen/arch/arm/include/asm/mmio.h
> @@ -35,6 +35,7 @@ enum instr_decode_state
>        * instruction.
>        */
>       INSTR_LDR_STR_POSTINDEXING,
> +    INSTR_CACHE,                    /* Cache Maintenance instr */
>   };
>   
>   typedef struct
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index 6f458ee7fd..26c716b4a5 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 check
> +     * if the address belongs to an emulated MMIO region or not. The behavior
> +     * will differ accordingly.
> +     */
> +    if ( info->dabt.cache )
> +    {
> +        info->dabt_instr.state = INSTR_CACHE;
> +        return;
> +    }
> +
>       /*
>        * Armv8 processor does not provide a valid syndrome for decoding some
>        * instructions. So in order to process these instructions, Xen must
> @@ -177,6 +188,13 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs,
>           return rc;
>       }
>   
> +    /*
> +     * When the data abort is caused due to cache maintenance and the address
> +     * belongs to an emulated region, Xen should ignore this instruction.
> +     */
> +    if ( info->dabt_instr.state == INSTR_CACHE )
Reading the Arm Arm, the ISS should be invalid for cache instructions. 
So, I think the check at the beginning of try_handle_mmio() would 
prevent us to reach this check.
Can you check that cache instructions on emulated region will 
effectively be ignored?
Cheers,

--
Julien Grall