Re: [PATCH v11 2/3] xen/arm64: io: Handle the abort due to access to stage1 translation table

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 17 Mar 2022, Ayan Kumar Halder wrote:
> If the abort was caused due to access to stage1 translation table, Xen
> will try to set the p2m entry (assuming that the Stage 1 translation
> table is in a non MMIO region).
> If there is no such entry found, then Xen will try to map the address as
> a MMIO region (assuming that the Stage 1 translation table is in a
> direct MMIO region).
> 
> If that fails as well, then there are the two following scenarios:-
> 1. Stage 1 translation table being in an emulated MMIO region - Xen
> can read the region, but it has no way to return the value read to the
> CPU page table walker (which tries to go through the stage1 tables to
> resolve the translation fault).
> 
> 2. Stage 1 translation table address is invalid.
> 
> In both the above scenarios, Xen will forward the abort to the guest.
> 
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxxxxx>

Tested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> 
> Changelog :-
> 
> v1..v8 - NA
> 
> v9 - 1. Extracted this change from "[XEN v8 2/2] xen/arm64: io: Support
> instructions (for which ISS is not..." into a separate patch of its own.
> The reason being this is an existing bug in the codebase.
> 
> v10 - 1. Enabled checking for stage1 translation table address in the
> MMIO region. The reason being Arm Arm does not have any restrictions.
> 2. Updated the commit message to explain all the possible scenarios.
> 
> v11 - 1. Fixed some wordings in comments and commit message (pointed
> by Julien in v10).
> 
>  xen/arch/arm/io.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
> index fd903b7b03..6f458ee7fd 100644
> --- a/xen/arch/arm/io.c
> +++ b/xen/arch/arm/io.c
> @@ -128,6 +128,17 @@ void try_decode_instruction(const struct cpu_user_regs 
> *regs,
>          return;
>      }
>  
> +    /*
> +     * At this point, we know that the stage1 translation table is either in 
> an
> +     * emulated MMIO region or its address is invalid . This is not expected 
> by
> +     * Xen and thus it forwards the abort to the guest.
> +     */
> +    if ( info->dabt.s1ptw )
> +    {
> +        info->dabt_instr.state = INSTR_ERROR;
> +        return;
> +    }
> +
>      /*
>       * Armv8 processor does not provide a valid syndrome for decoding some
>       * instructions. So in order to process these instructions, Xen must
> -- 
> 2.17.1
> 
>