Re: [XEN v10 2/4] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler

[Julien Grall <julien@xxxxxxx>]

Hi Ayan,

On 10/03/2022 17:44, Ayan Kumar Halder wrote:
> diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c
> index 9c9790a6d1..159e3cef8b 100644
> --- a/xen/arch/arm/arm32/traps.c
> +++ b/xen/arch/arm/arm32/traps.c
> @@ -18,9 +18,11 @@
>   
>   #include <xen/lib.h>
>   #include <xen/kernel.h>
> +#include <xen/sched.h>
>   
>   #include <public/xen.h>
>   
> +#include <asm/mmio.h>
>   #include <asm/processor.h>
>   #include <asm/traps.h>
>   
> @@ -82,6 +84,15 @@ void do_trap_data_abort(struct cpu_user_regs *regs)
>           do_unexpected_trap("Data Abort", regs);
>   }
>   
> +void post_increment_register(const struct instr_details *instr)
> +{
> +    /*
> +     * We have not implemented decoding of post indexing instructions for 32 
> bit.
> +     * Thus, this should be unreachable.
> +     */
> +    domain_crash(current->domain);
Osstest [1] is reporting a crash here. This is because 
post_increment_register() is called unconditionally.
I think you need to add:

if ( instr->state != INSTR_LDR_STR_POSTINDEXING )

Also, given this is called unconditionally, I think the function needs 
to be renamed to some more generic. Maybe finalize_instr_emulation() or 
similar.
I have decided to revert this patch and patch #3 for now. Please test it 
on arm32 and respin it.
While you are at it, please make sure the title is no longer than
80 characters.

Cheers,


> +}
> +

[1] 
http://logs.test-lab.xenproject.org/osstest/logs/168589/test-armhf-armhf-xl/info.html
--
Julien Grall