Re: [XEN v10 3/4] xen/arm64: io: Handle the abort due to access to stage1 translation table

[Julien Grall <julien@xxxxxxx>]

Hi,

On 10/03/2022 17:45, 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 the 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 the
> direct MMIO region).
>
> If that fails as well, then there are the two following scenarios:-
> 1. Stage 1 translation table being in the emulated MMIO region - Xen
We have multiple emulated MMIO regions. So I would s/the/an/

> 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>
> ---
>
> 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.
>
>   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..e6c77e16bf 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 
> the
Same here.

> +     * emulated MMIO region or its address is invalid . This is not expected by
s/ . /. /

All of these can be fixed on commit. So:

Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>

Cheers,

--
Julien Grall