|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN v7 1/2] xen/arm64: Decode ldr/str post increment operations
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@xxxxxxxxxx>
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
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |