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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.03.2022 18:44, Ayan Kumar Halder wrote:
> When an instruction is trapped in Xen due to translation fault, Xen
> checks if the ISS is invalid (for data abort) or it is an instruction
> abort. If so, Xen tries to resolve the translation fault using p2m page
> tables. In case of data abort, Xen will try to map the mmio region to
> the guest (ie tries to emulate the mmio region).
> 
> If the ISS is not valid and it is a data abort, then Xen tries to
> decode the instruction. In case of ioreq, Xen  saves the decoding state,
> rn and imm9 to vcpu_io. Whenever the vcpu handles the ioreq successfully,
> it will read the decoding state to determine if the instruction decoded
> was a ldr/str post indexing (ie INSTR_LDR_STR_POSTINDEXING). If so, it
> uses these details to post increment rn.
> 
> In case of mmio handler, if the mmio operation was successful, then Xen
> retrives the decoding state, rn and imm9. For state ==
> INSTR_LDR_STR_POSTINDEXING, Xen will update rn.
> 
> If there is an error encountered while decoding/executing the instruction,
> Xen will forward the abort to the guest.
> 
> Also, the logic to infer the type of instruction has been moved from
> try_handle_mmio() to try_decode_instruction() which is called before.
> try_handle_mmio() is solely responsible for handling the mmio operation.
> 
> Signed-off-by: Ayan Kumar Halder <ayankuma@xxxxxxxxxx>

In addition to the boot crash on 32-bit Arm there's also an early
build failure on x86:

In file included from arch/x86/x86_64/asm-offsets.c:11:
./include/xen/sched.h:164:26: error: field 'info' has incomplete type
     struct arch_vcpu_io  info;
                          ^~~~

Note how on Arm you have ...

> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -281,6 +281,10 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>  /* vPCI is not available on Arm */
>  #define has_vpci(d)    ({ (void)(d); false; })
>  
> +struct arch_vcpu_io {
> +    struct instr_details dabt_instr; /* when the instruction is decoded */
> +};

... this in asm/domain.h, and thus visible to xen/sched.h, while on x86
you have ...

> --- a/xen/arch/x86/include/asm/ioreq.h
> +++ b/xen/arch/x86/include/asm/ioreq.h
> @@ -26,6 +26,9 @@
>  #include <asm/hvm/ioreq.h>
>  #endif
>  
> +struct arch_vcpu_io {
> +};

... this in a header which xen/sched.h doesn't end up including. I would
like to ask that you arrange locally for at least an x86 build test
whenever you touch code which might also affect x86.

Jan