| Message ID | 20220317140046.64563-4-ayankuma@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm64: io: Decode ldr/str post-indexing instruction | expand |
On Thu, 17 Mar 2022, Ayan Kumar Halder wrote: > When the data abort is caused due to cache maintenance for an address, > there are three scenarios:- > > 1. Address belonging to a non emulated region - For this, Xen should > set the corresponding bit in the translation table entry to valid and > return to the guest to retry the instruction. This can happen sometimes > as Xen need to set the translation table entry to invalid. (for eg > 'Break-Before-Make' sequence). Xen returns to the guest to retry the > instruction. > > 2. Address belongs to an emulated region - Xen should ignore the > instruction (ie increment the PC) and return to the guest. > > 3. Address is invalid - Xen should forward the data abort to the guest. > > Signed-off-by: Ayan Kumar Halder <ayankuma@xilinx.com> Tested-by: Stefano Stabellini <sstabellini@kernel.org> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
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,
On 18/03/2022 18:26, Julien Grall wrote: > Hi Ayan, Hi Julien, > > 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? Yes, you are correct. I tested with the following (dis)assembly snippet :- 0x3001000 is the base address of GIC Distributor base. __asm__ __volatile__("ldr x1, =0x3001000"); 40000ca8: 58000301 ldr x1, 40000d08 <main+0x70> __asm __volatile__("DC CVAU, x1"); 40000cac: d50b7b21 dc cvau, x1 This resulting in hitting the assertion :- (XEN) Assertion 'unreachable' failed at arch/arm/io.c:178 I dumped the registers as follows, to determine that the fault is caused by the instruction at 40000cac. HSR=0x00000092000147 regs->pc = 0x40000cac info.gpa = 0x3001000 So, my patch needs to be modified as follows:- @@ -172,7 +173,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs, ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL); - if ( !info->dabt.valid ) + if ( !(info->dabt.valid || (info->dabt_instr.state == INSTR_CACHE)) ) { ASSERT_UNREACHABLE(); return IO_ABORT; I will send a v12 patch with this change. - Ayan > > Cheers, >
On 22/03/2022 12:06, Ayan Kumar Halder wrote: > > On 18/03/2022 18:26, Julien Grall wrote: >> Hi Ayan, > Hi Julien, >> >> 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? > > Yes, you are correct. > > I tested with the following (dis)assembly snippet :- > > 0x3001000 is the base address of GIC Distributor base. > > __asm__ __volatile__("ldr x1, =0x3001000"); > 40000ca8: 58000301 ldr x1, 40000d08 <main+0x70> > __asm __volatile__("DC CVAU, x1"); > 40000cac: d50b7b21 dc cvau, x1 > > This resulting in hitting the assertion :- > > (XEN) Assertion 'unreachable' failed at arch/arm/io.c:178 > > I dumped the registers as follows, to determine that the fault is > caused by the instruction at 40000cac. > > HSR=0x00000092000147 regs->pc = 0x40000cac info.gpa = 0x3001000 > > > So, my patch needs to be modified as follows:- > > @@ -172,7 +173,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs > *regs, > > ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL); > > - if ( !info->dabt.valid ) > + if ( !(info->dabt.valid || (info->dabt_instr.state == > INSTR_CACHE)) ) Actually this is not needed. The following change is sufficient :- @@ -146,7 +146,9 @@ void try_decode_instruction(const struct cpu_user_regs *regs, */ if ( info->dabt.cache ) { info->dabt_instr.state = INSTR_CACHE; + info->dabt.valid = 1; return; } "info->dabt.valid == 1" means the instruction is valid or decoded successfully (this holds true for INSTR_CACHE as well).
Hi Ayan, On 22/03/2022 12:38, Ayan Kumar Halder wrote: > > On 22/03/2022 12:06, Ayan Kumar Halder wrote: >> >> On 18/03/2022 18:26, Julien Grall wrote: >>> Hi Ayan, >> Hi Julien, >>> >>> 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? >> >> Yes, you are correct. >> >> I tested with the following (dis)assembly snippet :- >> >> 0x3001000 is the base address of GIC Distributor base. >> >> __asm__ __volatile__("ldr x1, =0x3001000"); >> 40000ca8: 58000301 ldr x1, 40000d08 <main+0x70> >> __asm __volatile__("DC CVAU, x1"); >> 40000cac: d50b7b21 dc cvau, x1 >> >> This resulting in hitting the assertion :- >> >> (XEN) Assertion 'unreachable' failed at arch/arm/io.c:178 >> >> I dumped the registers as follows, to determine that the fault is >> caused by the instruction at 40000cac. >> >> HSR=0x00000092000147 regs->pc = 0x40000cac info.gpa = 0x3001000 >> >> >> So, my patch needs to be modified as follows:- >> >> @@ -172,7 +173,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs >> *regs, >> >> ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL); >> >> - if ( !info->dabt.valid ) >> + if ( !(info->dabt.valid || (info->dabt_instr.state == >> INSTR_CACHE)) ) > > Actually this is not needed. > > The following change is sufficient :- > > @@ -146,7 +146,9 @@ void try_decode_instruction(const struct > cpu_user_regs *regs, > */ > if ( info->dabt.cache ) > { > info->dabt_instr.state = INSTR_CACHE; > + info->dabt.valid = 1; To me, 'info->dabt.valid' indicates whether the syndrome is valid. We set to 1 for emulated instruction because the syndrome will be updated. But this is not the case for the cache instructions. So I would prefer if it is kept as 0 and use your previous suggestion. Furthermore, I think try_fwd_ioserv() need to be adapted because the function will use the fields SAS and SRT. From the Arm Arm they are RES0, so while they are 0 today, we should not rely on this. Therefore, to be fully compliant with the Arm, we want to reorder a bit the code: * The field data could be set past ioreq_select_server(). * The field size should be set to the cache line size. Cheers,
Hi Julien, On 22/03/2022 13:22, Julien Grall wrote: > Hi Ayan, > > On 22/03/2022 12:38, Ayan Kumar Halder wrote: >> >> On 22/03/2022 12:06, Ayan Kumar Halder wrote: >>> >>> On 18/03/2022 18:26, Julien Grall wrote: >>>> Hi Ayan, >>> Hi Julien, >>>> >>>> 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? >>> >>> Yes, you are correct. >>> >>> I tested with the following (dis)assembly snippet :- >>> >>> 0x3001000 is the base address of GIC Distributor base. >>> >>> __asm__ __volatile__("ldr x1, =0x3001000"); >>> 40000ca8: 58000301 ldr x1, 40000d08 <main+0x70> >>> __asm __volatile__("DC CVAU, x1"); >>> 40000cac: d50b7b21 dc cvau, x1 >>> >>> This resulting in hitting the assertion :- >>> >>> (XEN) Assertion 'unreachable' failed at arch/arm/io.c:178 >>> >>> I dumped the registers as follows, to determine that the fault is >>> caused by the instruction at 40000cac. >>> >>> HSR=0x00000092000147 regs->pc = 0x40000cac info.gpa = 0x3001000 >>> >>> >>> So, my patch needs to be modified as follows:- >>> >>> @@ -172,7 +173,7 @@ enum io_state try_handle_mmio(struct >>> cpu_user_regs *regs, >>> >>> ASSERT(info->dabt.ec == HSR_EC_DATA_ABORT_LOWER_EL); >>> >>> - if ( !info->dabt.valid ) >>> + if ( !(info->dabt.valid || (info->dabt_instr.state == >>> INSTR_CACHE)) ) >> >> Actually this is not needed. >> >> The following change is sufficient :- >> >> @@ -146,7 +146,9 @@ void try_decode_instruction(const struct >> cpu_user_regs *regs, >> */ >> if ( info->dabt.cache ) >> { >> info->dabt_instr.state = INSTR_CACHE; >> + info->dabt.valid = 1; > > To me, 'info->dabt.valid' indicates whether the syndrome is valid. We > set to 1 for emulated instruction because the syndrome will be updated. > > But this is not the case for the cache instructions. So I would prefer > if it is kept as 0 and use your previous suggestion. > > Furthermore, I think try_fwd_ioserv() need to be adapted because the > function will use the fields SAS and SRT. From the Arm Arm they are > RES0, so while they are 0 today, we should not rely on this. > > Therefore, to be fully compliant with the Arm, we want to reorder a > bit the code: > > * The field data could be set past ioreq_select_server(). > * The field size should be set to the cache line size. I am assuming that we need to invoke dcache_line_size() (from xen/arch/arm/arm64/cache.S ) to get the cache line size. I think the cache line may be 32 or 64 bytes. In which case, this cannot be represented by SAS (as it can represent 1, 2, 4 and 8 bytes). Also, we are invoking ioreq_select_server() to determine if the address is emulated or not. So, can we use an assumed size (= 1 byte) ? If it is emulated, Xen will ignore the instruction. If it is not emulated, Xen will forward the abort to the guest. Thus, Xen will never execute the instruction. So the correctness of the size should not matter here. - Ayan > > Cheers, >
Hi, On 22/03/2022 16:16, Ayan Kumar Halder wrote: > On 22/03/2022 13:22, Julien Grall wrote: >> Furthermore, I think try_fwd_ioserv() need to be adapted because the >> function will use the fields SAS and SRT. From the Arm Arm they are >> RES0, so while they are 0 today, we should not rely on this. >> >> Therefore, to be fully compliant with the Arm, we want to reorder a >> bit the code: >> >> * The field data could be set past ioreq_select_server(). >> * The field size should be set to the cache line size. > > I am assuming that we need to invoke dcache_line_size() (from > xen/arch/arm/arm64/cache.S ) to get the cache line size. You would want to use dcache_line_bytes. > > I think the cache line may be 32 or 64 bytes. In which case, this cannot > be represented by SAS (as it can represent 1, 2, 4 and 8 bytes). You are correct that this cannot be represented by SAS. However, I was referring to the field 'size' in the ioreq structure. It is a 32-bit integer and could therefore represent the size of the cache line. > > Also, we are invoking ioreq_select_server() to determine if the address > is emulated or not. So, can we use an assumed size (= 1 byte) ? I thought about this. This is technically incorrect but would be OK if we cannot find the correct size. Per above, I think the correct size could be found. Cheers,
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 ) + return IO_HANDLED; + /* * At this point, we know that the instruction is either valid or has been * decoded successfully. Thus, Xen should be allowed to execute the diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c index 54167aebcb..cc66713386 100644 --- a/xen/arch/arm/ioreq.c +++ b/xen/arch/arm/ioreq.c @@ -47,7 +47,7 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs, struct vcpu *v, mmio_info_t *info) { struct vcpu_io *vio = &v->io; - struct instr_details instr = info->dabt_instr; + const struct instr_details instr = info->dabt_instr; struct hsr_dabt dabt = info->dabt; ioreq_t p = { .type = IOREQ_TYPE_COPY, @@ -78,6 +78,13 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs, if ( !s ) return IO_UNHANDLED; + /* + * When the data abort is caused due to cache maintenance and the address + * belongs to an emulated region, Xen should ignore this instruction. + */ + if ( instr.state == INSTR_CACHE ) + return IO_HANDLED; + ASSERT(dabt.valid); vio->req = p;
When the data abort is caused due to cache maintenance for an address, there are three scenarios:- 1. Address belonging to a non emulated region - For this, Xen should set the corresponding bit in the translation table entry to valid and return to the guest to retry the instruction. This can happen sometimes as Xen need to set the translation table entry to invalid. (for eg 'Break-Before-Make' sequence). Xen returns to the guest to retry the instruction. 2. Address belongs to an emulated region - Xen should ignore the instruction (ie increment the PC) and return to the guest. 3. Address is invalid - Xen should forward the data abort to the guest. Signed-off-by: Ayan Kumar Halder <ayankuma@xilinx.com> --- Changelog:- v1...v8 - NA v9 - Extracted this change from "[XEN v7 2/2] xen/arm64: io: Support instructions (for which ISS is not ..." into a separate patch of its own. The reason being this addresses an existing bug in the codebase. v10 - 1. To check if the address belongs to an emulated region, one needs to check if it has a mmio handler or an ioreq server. In this case, Xen should increment the PC 2. If the address is invalid (niether emulated MMIO nor the translation could be resolved via p2m or mapping the MMIO region), then Xen should forward the abort to the guest. v11 - 1. Removed the un-necessary check "( instr.state == INSTR_CACHE )" in handle_ioserv(). The reason being the ioserv request is not forwarded by try_fwd_ioserv() when instr.state == INSTR_CACHE. xen/arch/arm/include/asm/mmio.h | 1 + xen/arch/arm/io.c | 18 ++++++++++++++++++ xen/arch/arm/ioreq.c | 9 ++++++++- 3 files changed, 27 insertions(+), 1 deletion(-)