Re: [XEN v9 1/4] xen/arm64: Decode ldr/str post increment operations
On Tue, 1 Mar 2022, Ayan Kumar Halder wrote:
> At the moment, Xen does not decode any of the arm64 instructions. This
> means that when hsr_dabt.isv == 0, Xen cannot handle those instructions.
> This will lead to Xen to abort the guests (from which those instructions
> originate).
>
> With this patch, Xen is able to decode ldr/str post indexing instructions.
> These are a subset of instructions for which hsr_dabt.isv == 0.
>
> The following instructions are now supported by Xen :-
> 1. ldr x2,[x1],#8
> 2. ldr w2,[x1],#-4
> 3. ldr x2,[x1],#-8
> 4. ldr w2,[x1],#4
> 5. ldrhw2,[x1],#2
> 6. ldrbw2,[x1],#1
> 7. str x2,[x1],#8
> 8. str w2,[x1],#-4
> 9. strhw2,[x1],#2
> 10. strbw2,[x1],#1
>
> In the subsequent patch, decode_arm64() will get invoked when
> hsr_dabt.isv == 0.
>
> Signed-off-by: Ayan Kumar Halder
Reviewed-by: Stefano Stabellini
> ---
>
> Changelog :-
>
> v2..v5 - Mentioned in the cover letter.
>
> v6 - 1. Fixed the code style issues as mentioned in v5.
>
> v7 - No change.
>
> v8 - 1. Removed some un-necessary header files inclusion.
> 2. Some style changes pointed out in v7.
>
> v9 - 1. Rebased on top of the master.
> 2. Renamed psr_mode_is_32bit to regs_mode_is_32bit.
>
> xen/arch/arm/decode.c | 79 -
> xen/arch/arm/decode.h | 48 +---
> xen/arch/arm/include/asm/mmio.h | 4 ++
> xen/arch/arm/io.c | 2 +-
> 4 files changed, 124 insertions(+), 9 deletions(-)
>
> diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
> index 792c2e92a7..3add87e83a 100644
> --- a/xen/arch/arm/decode.c
> +++ b/xen/arch/arm/decode.c
> @@ -84,6 +84,78 @@ bad_thumb2:
> return 1;
> }
>
> +static int decode_arm64(register_t pc, mmio_info_t *info)
> +{
> +union instr opcode = {0};
> +struct hsr_dabt *dabt = >dabt;
> +struct instr_details *dabt_instr = >dabt_instr;
> +
> +if ( raw_copy_from_guest(, (void * __user)pc, sizeof
> (opcode)) )
> +{
> +gprintk(XENLOG_ERR, "Could not copy the instruction from PC\n");
> +return 1;
> +}
> +
> +/*
> + * Refer Arm v8 ARM DDI 0487G.b, Page - C6-1107
> + * "Shared decode for all encodings" (under ldr immediate)
> + * If n == t && n != 31, then the return value is implementation defined
> + * (can be WBSUPPRESS, UNKNOWN, UNDEFINED or NOP). Thus, we do not
> support
> + * this. This holds true for ldrb/ldrh immediate as well.
> + *
> + * Also refer, Page - C6-1384, the above described behaviour is same for
> + * str immediate. This holds true for strb/strh immediate as well
> + */
> +if ( (opcode.ldr_str.rn == opcode.ldr_str.rt) && (opcode.ldr_str.rn !=
> 31) )
> +{
> +gprintk(XENLOG_ERR, "Rn should not be equal to Rt except for r31\n");
> +goto bad_loadstore;
> +}
> +
> +/* First, let's check for the fixed values */
> +if ( (opcode.value & POST_INDEX_FIXED_MASK) != POST_INDEX_FIXED_VALUE )
> +{
> +gprintk(XENLOG_ERR,
> +"Decoding instruction 0x%x is not supported\n",
> opcode.value);
> +goto bad_loadstore;
> +}
> +
> +if ( opcode.ldr_str.v != 0 )
> +{
> +gprintk(XENLOG_ERR,
> +"ldr/str post indexing for vector types are not
> supported\n");
> +goto bad_loadstore;
> +}
> +
> +/* Check for STR (immediate) */
> +if ( opcode.ldr_str.opc == 0 )
> +dabt->write = 1;
> +/* Check for LDR (immediate) */
> +else if ( opcode.ldr_str.opc == 1 )
> +dabt->write = 0;
> +else
> +{
> +gprintk(XENLOG_ERR,
> +"Decoding ldr/str post indexing is not supported for this
> variant\n");
> +goto bad_loadstore;
> +}
> +
> +gprintk(XENLOG_INFO,
> +"opcode->ldr_str.rt = 0x%x, opcode->ldr_str.size = 0x%x,
> opcode->ldr_str.imm9 = %d\n",
> +opcode.ldr_str.rt, opcode.ldr_str.size, opcode.ldr_str.imm9);
> +
> +update_dabt(dabt, opcode.ldr_str.rt, opcode.ldr_str.size, false);
> +
> +dabt_instr->rn = opcode.ldr_str.rn;
> +dabt_instr->imm9 = opcode.ldr_str.imm9;
> +
> +return 0;
> +
> + bad_loadstore:
> +gprintk(XENLOG_ERR, "unhandled Arm instruction 0x%x\n", opcode.value);
> +return 1;
> +}
> +
> static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
> {
> uint16_t instr;
> @@ -150,10 +222,13 @@ bad_thumb:
> return 1;
> }
>
> -int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt
> *dabt)
> +int decode_instruction(const struct cpu_user_regs *regs, mmio_info_t *info)
> {
> if ( is_32bit_domain(current->domain) && regs->cpsr & PSR_THUMB )
> -return decode_thumb(regs->pc, dabt);
> +return decode_thumb(regs->pc, >dabt);
> +
> +if (
[XEN v9 1/4] xen/arm64: Decode ldr/str post increment operations
At the moment, Xen does not decode any of the arm64 instructions. This
means that when hsr_dabt.isv == 0, Xen cannot handle those instructions.
This will lead to Xen to abort the guests (from which those instructions
originate).
With this patch, Xen is able to decode ldr/str post indexing instructions.
These are a subset of instructions for which hsr_dabt.isv == 0.
The following instructions are now supported by Xen :-
1. ldr x2,[x1],#8
2. ldr w2,[x1],#-4
3. ldr x2,[x1],#-8
4. ldr w2,[x1],#4
5. ldrhw2,[x1],#2
6. ldrbw2,[x1],#1
7. str x2,[x1],#8
8. str w2,[x1],#-4
9. strhw2,[x1],#2
10. strbw2,[x1],#1
In the subsequent patch, decode_arm64() will get invoked when
hsr_dabt.isv == 0.
Signed-off-by: Ayan Kumar Halder
---
Changelog :-
v2..v5 - Mentioned in the cover letter.
v6 - 1. Fixed the code style issues as mentioned in v5.
v7 - No change.
v8 - 1. Removed some un-necessary header files inclusion.
2. Some style changes pointed out in v7.
v9 - 1. Rebased on top of the master.
2. Renamed psr_mode_is_32bit to regs_mode_is_32bit.
xen/arch/arm/decode.c | 79 -
xen/arch/arm/decode.h | 48 +---
xen/arch/arm/include/asm/mmio.h | 4 ++
xen/arch/arm/io.c | 2 +-
4 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 792c2e92a7..3add87e83a 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -84,6 +84,78 @@ bad_thumb2:
return 1;
}
+static int decode_arm64(register_t pc, mmio_info_t *info)
+{
+union instr opcode = {0};
+struct hsr_dabt *dabt = >dabt;
+struct instr_details *dabt_instr = >dabt_instr;
+
+if ( raw_copy_from_guest(, (void * __user)pc, sizeof
(opcode)) )
+{
+gprintk(XENLOG_ERR, "Could not copy the instruction from PC\n");
+return 1;
+}
+
+/*
+ * Refer Arm v8 ARM DDI 0487G.b, Page - C6-1107
+ * "Shared decode for all encodings" (under ldr immediate)
+ * If n == t && n != 31, then the return value is implementation defined
+ * (can be WBSUPPRESS, UNKNOWN, UNDEFINED or NOP). Thus, we do not support
+ * this. This holds true for ldrb/ldrh immediate as well.
+ *
+ * Also refer, Page - C6-1384, the above described behaviour is same for
+ * str immediate. This holds true for strb/strh immediate as well
+ */
+if ( (opcode.ldr_str.rn == opcode.ldr_str.rt) && (opcode.ldr_str.rn != 31)
)
+{
+gprintk(XENLOG_ERR, "Rn should not be equal to Rt except for r31\n");
+goto bad_loadstore;
+}
+
+/* First, let's check for the fixed values */
+if ( (opcode.value & POST_INDEX_FIXED_MASK) != POST_INDEX_FIXED_VALUE )
+{
+gprintk(XENLOG_ERR,
+"Decoding instruction 0x%x is not supported\n", opcode.value);
+goto bad_loadstore;
+}
+
+if ( opcode.ldr_str.v != 0 )
+{
+gprintk(XENLOG_ERR,
+"ldr/str post indexing for vector types are not supported\n");
+goto bad_loadstore;
+}
+
+/* Check for STR (immediate) */
+if ( opcode.ldr_str.opc == 0 )
+dabt->write = 1;
+/* Check for LDR (immediate) */
+else if ( opcode.ldr_str.opc == 1 )
+dabt->write = 0;
+else
+{
+gprintk(XENLOG_ERR,
+"Decoding ldr/str post indexing is not supported for this
variant\n");
+goto bad_loadstore;
+}
+
+gprintk(XENLOG_INFO,
+"opcode->ldr_str.rt = 0x%x, opcode->ldr_str.size = 0x%x,
opcode->ldr_str.imm9 = %d\n",
+opcode.ldr_str.rt, opcode.ldr_str.size, opcode.ldr_str.imm9);
+
+update_dabt(dabt, opcode.ldr_str.rt, opcode.ldr_str.size, false);
+
+dabt_instr->rn = opcode.ldr_str.rn;
+dabt_instr->imm9 = opcode.ldr_str.imm9;
+
+return 0;
+
+ bad_loadstore:
+gprintk(XENLOG_ERR, "unhandled Arm instruction 0x%x\n", opcode.value);
+return 1;
+}
+
static int decode_thumb(register_t pc, struct hsr_dabt *dabt)
{
uint16_t instr;
@@ -150,10 +222,13 @@ bad_thumb:
return 1;
}
-int decode_instruction(const struct cpu_user_regs *regs, struct hsr_dabt *dabt)
+int decode_instruction(const struct cpu_user_regs *regs, mmio_info_t *info)
{
if ( is_32bit_domain(current->domain) && regs->cpsr & PSR_THUMB )
-return decode_thumb(regs->pc, dabt);
+return decode_thumb(regs->pc, >dabt);
+
+if ( !regs_mode_is_32bit(regs) )
+return decode_arm64(regs->pc, info);
/* TODO: Handle ARM instruction */
gprintk(XENLOG_ERR, "unhandled ARM instruction\n");
diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
index 4613763bdb..13db8ac968 100644
--- a/xen/arch/arm/decode.h
+++ b/xen/arch/arm/decode.h
@@ -23,19 +23,55 @@
#include
#include
-/**
+/*
+ * Refer to the ARMv8
