On Thu, 20 Jan 2022 21:55:27 +0000
Ayan Kumar Halder <ayan.kumar.hal...@xilinx.com> wrote:
Hi,
At the moment, Xen is only handling data abort with valid syndrome
(i.e.
ISV=0). Unfortunately, this doesn't cover all the instructions a
domain
could use to access MMIO regions.
For instance, a baremetal OS can use any of the following
instructions, where
x1 contains the address of the MMIO region:
1. ldr x2, [x1], #4
That looks dodgy, since is misaligns the pointer afterwards. MMIO
access typically go to device memory, which must be naturally aligned.
Just don't give a bad example here and change that to a multiple of 8.
2. ldr w2, [x1], #-4
(this one is fine, btw, because it's a 32-bit read)
3. ldr x2, [x1], #-8
4. ldr w2, [x1], #4
5. ldrh w2, [x1], #8
6. ldrb w2, [x1], #16
More naturally I'd use the data size of the postindex value ...
ldr x2 ... #-8
ldr w2 ... #4
ldrh w2 ... #2
ldrb w2 ... #1
7. str x2, [x1], #4
This is a again problematic, because x1 is not 8-byte aligned anymore
after that.
8. str w2, [x1], #-4
9. strh w2, [x1], #8
10. strb w2, [x1], #16
In the following two instructions, sp contains the address of the
MMIO region:-
Really? I don't think you should give people funny ideas, just mention
that the Rn register could theoretically be the stack pointer.
11. ldrb w2, [sp], #16
12. ldrb wzr, [sp], #16
In order to handle post-indexing store/load instructions (like
those mentioned
above), Xen will need to fetch and decode the instruction.
This patch only cover post-index store/load instructions from
AArch64 mode.
For now, this is left unimplemented for trap from AArch32 mode.
Signed-off-by: Ayan Kumar Halder <ayank...@xilinx.com>
---
Changelog :-
v2 - 1. Updated the rn register after reading from it. (Pointed by
Julien,
Stefano)
2. Used a union to represent the instruction opcode
(Suggestd by Bertrand)
3. Fixed coding style issues (Pointed by Julien)
4. In the previous patch, I was updating dabt->sign based on
the signedness
of imm9. This was incorrect. As mentioned in ARMv8 ARM
DDI 0487G.b,
Page 3221, SSE indicates the signedness of the data item
loaded. In our
case, the data item loaded is always unsigned.
v3- 1. Handled all the variants of ldr/str (ie 64, 32, 16, 8 bit
variants).
Thus, I have removed the check for "instr->code.opc == 0"
(Suggested by
Andre)
2. Handled the scenario when rn = SP, rt = XZR (Suggested by
Jan, Andre)
3. Added restriction for "rt != rn" (Suggested by Andre)
4. Moved union ldr_str_instr_class {} to decode.h. This is
the header included
by io.c and decode.c (where the union is referred).
(Suggested by Jan)
5. Indentation and typo fixes (Suggested by Jan)
Changes suggested but could not be considered due to reasons :-
1. Using accessor macros instead of bitfields for
"ldr_str_instr_class". (Andre)
Reason - I could not find a simple way to represent 9 bit
signed integer
(ie imm9) without using bitfields. If I use accessor
macros, then I need
to manually calculate two's complement to obtain the value
when signed
bit is present.
2. I/D cache cohenerncy (Andre)
Reason :- I could not see any instruction to flush the I
cache.
First, please try to avoid the term "flush", because it is somewhat
overloaded. The architecture speaks of "clean" and "invalidate", which
are more precise.
Assuming you mean "clean" here: conceptually there is no such thing for
the I cache, because it's always clean. The I$ will only be read from
the CPU side - from the instruction fetcher - there is nothing written
back through it. Every store goes through the data path - always.
That is the problem that I tried to sketch you previously: you don't
have a guarantee that the instruction you read from memory is the same
that the CPU executed. The guest could have changed the instruction
after the I$ fetched that. So the CPU will execute (and trap) on
instruction X, but you will read Y. I leave it up to your imagination
if that could be exploited.