Hi Julien,
On 26/01/2022 17:22, Julien Grall wrote:
Hi,
On Wed, 26 Jan 2022, 16:58 Ayan Kumar Halder,
<ayan.kumar.hal...@xilinx.com> wrote:
Refer to Armv8 ARM DDI 0487G.b, Page D13-3219 "ISS encoding for an
exception
from a Data Abort" :-
ISV - ISS[23:14] holds a valid instruction syndrome
When the ISV is 0, the instruction could not be decoded by the
hardware (ie ISS
is invalid). One should immediately return an error to the caller
with an
appropriate error message.
That's going to be very spammy because we have use-case where it could
trap without a valid ISV (e.g. when break-before-make happens). So
please don't had a message.
There is already a logging statement in traps.c :-
inject_abt:
gdprintk(XENLOG_DEBUG,
"HSR=%#"PRIregister" pc=%#"PRIregister" gva=%#"PRIvaddr"
gpa=%#"PRIpaddr"\n",
hsr.bits, regs->pc, gva, gpa);
The reason for the error is to give the user some hint that an IO abort
is triggered by Xen. Can we keep the error message ?
Also, I think this is a duplicate check
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/ioreq.c;h=308650b40051825fdea78bb33bfbcc87ef5deaff;hb=HEAD#l79
, I will remove this in v2 if it makes sense.
- Ayan
That makes me think that the same will be valid for your other patch.
There is no use of the MMIO handler. This is the
reason why one should check for ISV before attempting to find a
MMIO handler.
Signed-off-by: Ayan Kumar Halder <ayank...@xilinx.com>
---
Suggested by Julien Grall in
https://lists.xenproject.org/archives/html/xen-devel/2022-01/msg01245.html
xen/arch/arm/io.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c
index 729287e37c..14d39222f2 100644
--- a/xen/arch/arm/io.c
+++ b/xen/arch/arm/io.c
@@ -109,6 +109,13 @@ enum io_state try_handle_mmio(struct
cpu_user_regs *regs,
ASSERT(hsr.ec <http://hsr.ec> == HSR_EC_DATA_ABORT_LOWER_EL);
+ /* All the instructions used on emulated MMIO region should
be valid */
+ if ( !dabt.valid )
+ {
+ gprintk(XENLOG_DEBUG, "No valid instruction syndrome for
data abort\n");
+ return IO_ABORT;
+ }
+
handler = find_mmio_handler(v->domain, info.gpa);
if ( !handler )
{
@@ -121,10 +128,6 @@ enum io_state try_handle_mmio(struct
cpu_user_regs *regs,
return rc;
}
- /* All the instructions used on emulated MMIO region should
be valid */
- if ( !dabt.valid )
- return IO_ABORT;
-
/*
* Erratum 766422: Thumb store translation fault to
Hypervisor may
* not have correct HSR Rt value.
--
2.17
