| Message ID | 20220310174501.62040-4-ayankuma@xilinx.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm64: io: Decode ldr/str post-indexing instruction | expand |
On Thu, 10 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 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 > 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@xilinx.com> > --- > > 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 > + * emulated MMIO region or its address is invalid . This is not expected by ^ This looks OK to me now, but I'll let Julien ack/comment as he has been following more closely. > + * 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 > >
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@xilinx.com> > --- > > 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@amazon.com> Cheers,
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 + * 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
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 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@xilinx.com> --- 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(+)