| Message ID | 20220205225816.5952-2-ayankuma@xilinx.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm64: io: Decode ldr/str post-indexing instruction | expand |
On Sat, 5 Feb 2022, Ayan Kumar Halder wrote: > At the moment, Xen does not decode any of the arm64 instructions. This > means that hsr_dabt.isv = 0, Xen cannot handle those instructions. This > will lead to Xen abort the guests (from which those instructions > originated). > > 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. ldrh w2, [x1], #2 > 6. ldrb w2, [x1], #1 > 7. str x2, [x1], #8 > 8. str w2, [x1], #-4 > 9. strh w2, [x1], #2 > 10. strb w2, [x1], #1 > > In the subsequent patches, decode_arm64() will get invoked when > hsr_dabt.isv=0. > > Signed-off-by: Ayan Kumar Halder <ayankuma@xilinx.com> This patch looks good to me. Only minor comments below. > --- > > Changelog :- > > v2..v5 - Mentioned in the cover letter. > > v6 - 1. Fixed the code style issues as mentioned in v5. > > v7 - No change. > > xen/arch/arm/decode.c | 80 ++++++++++++++++++++++++++++++++- > xen/arch/arm/decode.h | 49 +++++++++++++++++--- > xen/arch/arm/include/asm/mmio.h | 4 ++ > xen/arch/arm/io.c | 2 +- > 4 files changed, 125 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c > index 792c2e92a7..3f2d2a3f62 100644 > --- a/xen/arch/arm/decode.c > +++ b/xen/arch/arm/decode.c > @@ -23,6 +23,7 @@ > #include <xen/types.h> > > #include <asm/current.h> > +#include <asm/mmio.h> it doesn't look like this is needed > #include "decode.h" > > @@ -84,6 +85,78 @@ bad_thumb2: > return 1; > } > > +static int decode_arm64(register_t pc, mmio_info_t *info) > +{ > + union instr opcode = {0}; > + struct hsr_dabt *dabt = &info->dabt; > + struct instr_details *dabt_instr = &info->dabt_instr; > + > + if ( raw_copy_from_guest(&opcode.value, (void * __user)pc, sizeof (opcode)) ) > + { > + gprintk(XENLOG_ERR, "Could not copy the instruction from PC\n"); > + goto bad_loadstore; this should just return 1 (no need to print the other error message after the label bad_loadstore). > + } > + > + /* > + * 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", 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 +223,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, &info->dabt); > + > + if ( !psr_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..fe7512a053 100644 > --- a/xen/arch/arm/decode.h > +++ b/xen/arch/arm/decode.h > @@ -23,19 +23,54 @@ > #include <asm/regs.h> > #include <asm/processor.h> > > -/** > - * Decode an instruction from pc > - * /!\ This function is not intended to fully decode an instruction. It > - * considers that the instruction is valid. > +/* > + * Refer to the ARMv8 ARM (DDI 0487G.b), Section C4.1.4 Loads and Stores > + * Page 318 specifies the following bit pattern for > + * "load/store register (immediate post-indexed)". > + * > + * 31 30 29 27 26 25 23 21 20 11 9 4 0 > + * ___________________________________________________________________ > + * |size|1 1 1 |V |0 0 |opc |0 | imm9 |0 1 | Rn | Rt | > + * |____|______|__|____|____|__|_______________|____|_________|_______| > + */ > +union instr { > + uint32_t value; > + struct { > + unsigned int rt:5; /* Rt register */ > + unsigned int rn:5; /* Rn register */ > + unsigned int fixed1:2; /* value == 01b */ > + signed int imm9:9; /* imm9 */ > + unsigned int fixed2:1; /* value == 0b */ > + unsigned int opc:2; /* opc */ > + unsigned int fixed3:2; /* value == 00b */ > + unsigned int v:1; /* vector */ > + unsigned int fixed4:3; /* value == 111b */ > + unsigned int size:2; /* size */ > + } ldr_str; > +}; > + > +#define POST_INDEX_FIXED_MASK 0x3B200C00 > +#define POST_INDEX_FIXED_VALUE 0x38000400 > + > +/* Decode an instruction from pc code style: /* * Decode an instruction from pc > + * /!\ This function is intended to decode an instruction. It considers that the > + * instruction is valid. > * > - * This function will get: > - * - The transfer register > + * In case of thumb mode, this function will get: > + * - The transfer register (ie Rt) > * - Sign bit > * - Size > + * > + * In case of arm64 mode, this function will get: > + * - The transfer register (ie Rt) > + * - The source register (ie Rn) > + * - Size > + * - Immediate offset > + * - Read or write > */ > > int decode_instruction(const struct cpu_user_regs *regs, > - struct hsr_dabt *dabt); > + mmio_info_t *info); > > #endif /* __ARCH_ARM_DECODE_H_ */ > > diff --git a/xen/arch/arm/include/asm/mmio.h b/xen/arch/arm/include/asm/mmio.h > index 7ab873cb8f..3354d9c635 100644 > --- a/xen/arch/arm/include/asm/mmio.h > +++ b/xen/arch/arm/include/asm/mmio.h > @@ -29,6 +29,10 @@ > typedef struct > { > struct hsr_dabt dabt; > + struct instr_details { > + unsigned long rn:5; > + signed int imm9:9; > + } dabt_instr; > paddr_t gpa; > } mmio_info_t; > > diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c > index 729287e37c..a289d393f9 100644 > --- a/xen/arch/arm/io.c > +++ b/xen/arch/arm/io.c > @@ -134,7 +134,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs, > { > int rc; > > - rc = decode_instruction(regs, &info.dabt); > + rc = decode_instruction(regs, &info); > if ( rc ) > { > gprintk(XENLOG_DEBUG, "Unable to decode instruction\n"); > -- > 2.17.1 >
diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c index 792c2e92a7..3f2d2a3f62 100644 --- a/xen/arch/arm/decode.c +++ b/xen/arch/arm/decode.c @@ -23,6 +23,7 @@ #include <xen/types.h> #include <asm/current.h> +#include <asm/mmio.h> #include "decode.h" @@ -84,6 +85,78 @@ bad_thumb2: return 1; } +static int decode_arm64(register_t pc, mmio_info_t *info) +{ + union instr opcode = {0}; + struct hsr_dabt *dabt = &info->dabt; + struct instr_details *dabt_instr = &info->dabt_instr; + + if ( raw_copy_from_guest(&opcode.value, (void * __user)pc, sizeof (opcode)) ) + { + gprintk(XENLOG_ERR, "Could not copy the instruction from PC\n"); + goto bad_loadstore; + } + + /* + * 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", 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 +223,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, &info->dabt); + + if ( !psr_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..fe7512a053 100644 --- a/xen/arch/arm/decode.h +++ b/xen/arch/arm/decode.h @@ -23,19 +23,54 @@ #include <asm/regs.h> #include <asm/processor.h> -/** - * Decode an instruction from pc - * /!\ This function is not intended to fully decode an instruction. It - * considers that the instruction is valid. +/* + * Refer to the ARMv8 ARM (DDI 0487G.b), Section C4.1.4 Loads and Stores + * Page 318 specifies the following bit pattern for + * "load/store register (immediate post-indexed)". + * + * 31 30 29 27 26 25 23 21 20 11 9 4 0 + * ___________________________________________________________________ + * |size|1 1 1 |V |0 0 |opc |0 | imm9 |0 1 | Rn | Rt | + * |____|______|__|____|____|__|_______________|____|_________|_______| + */ +union instr { + uint32_t value; + struct { + unsigned int rt:5; /* Rt register */ + unsigned int rn:5; /* Rn register */ + unsigned int fixed1:2; /* value == 01b */ + signed int imm9:9; /* imm9 */ + unsigned int fixed2:1; /* value == 0b */ + unsigned int opc:2; /* opc */ + unsigned int fixed3:2; /* value == 00b */ + unsigned int v:1; /* vector */ + unsigned int fixed4:3; /* value == 111b */ + unsigned int size:2; /* size */ + } ldr_str; +}; + +#define POST_INDEX_FIXED_MASK 0x3B200C00 +#define POST_INDEX_FIXED_VALUE 0x38000400 + +/* Decode an instruction from pc + * /!\ This function is intended to decode an instruction. It considers that the + * instruction is valid. * - * This function will get: - * - The transfer register + * In case of thumb mode, this function will get: + * - The transfer register (ie Rt) * - Sign bit * - Size + * + * In case of arm64 mode, this function will get: + * - The transfer register (ie Rt) + * - The source register (ie Rn) + * - Size + * - Immediate offset + * - Read or write */ int decode_instruction(const struct cpu_user_regs *regs, - struct hsr_dabt *dabt); + mmio_info_t *info); #endif /* __ARCH_ARM_DECODE_H_ */ diff --git a/xen/arch/arm/include/asm/mmio.h b/xen/arch/arm/include/asm/mmio.h index 7ab873cb8f..3354d9c635 100644 --- a/xen/arch/arm/include/asm/mmio.h +++ b/xen/arch/arm/include/asm/mmio.h @@ -29,6 +29,10 @@ typedef struct { struct hsr_dabt dabt; + struct instr_details { + unsigned long rn:5; + signed int imm9:9; + } dabt_instr; paddr_t gpa; } mmio_info_t; diff --git a/xen/arch/arm/io.c b/xen/arch/arm/io.c index 729287e37c..a289d393f9 100644 --- a/xen/arch/arm/io.c +++ b/xen/arch/arm/io.c @@ -134,7 +134,7 @@ enum io_state try_handle_mmio(struct cpu_user_regs *regs, { int rc; - rc = decode_instruction(regs, &info.dabt); + rc = decode_instruction(regs, &info); if ( rc ) { gprintk(XENLOG_DEBUG, "Unable to decode instruction\n");
At the moment, Xen does not decode any of the arm64 instructions. This means that hsr_dabt.isv = 0, Xen cannot handle those instructions. This will lead to Xen abort the guests (from which those instructions originated). 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. ldrh w2, [x1], #2 6. ldrb w2, [x1], #1 7. str x2, [x1], #8 8. str w2, [x1], #-4 9. strh w2, [x1], #2 10. strb w2, [x1], #1 In the subsequent patches, decode_arm64() will get invoked when hsr_dabt.isv=0. Signed-off-by: Ayan Kumar Halder <ayankuma@xilinx.com> --- Changelog :- v2..v5 - Mentioned in the cover letter. v6 - 1. Fixed the code style issues as mentioned in v5. v7 - No change. xen/arch/arm/decode.c | 80 ++++++++++++++++++++++++++++++++- xen/arch/arm/decode.h | 49 +++++++++++++++++--- xen/arch/arm/include/asm/mmio.h | 4 ++ xen/arch/arm/io.c | 2 +- 4 files changed, 125 insertions(+), 10 deletions(-)