pussuw commented on code in PR #5940: URL: https://github.com/apache/incubator-nuttx/pull/5940#discussion_r843928281
########## arch/risc-v/src/common/riscv_misaligned.c: ########## @@ -0,0 +1,448 @@ +/**************************************************************************** + * arch/risc-v/src/common/riscv_misaligned.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <assert.h> +#include <debug.h> +#include <nuttx/compiler.h> +#include <nuttx/config.h> +#include <nuttx/arch.h> +#include <nuttx/irq.h> + +#include <stdbool.h> +#include <stdint.h> +#include <string.h> +#include <sys/types.h> +#include <syslog.h> + +#include "riscv_internal.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* Instruction mask for load/store */ + +#define INSN_MASK 0x707f +#define INSN_C_MASK 0xe003 + +/* Load instruction opcode */ + +#define INSN_LB 0x0003 +#define INSN_LH 0x1003 +#define INSN_LW 0x2003 +#define INSN_LD 0x3003 +#define INSN_LBU 0x4003 +#define INSN_LHU 0x5003 +#define INSN_LWU 0x6003 + +#define INSN_C_LW 0x4000 +#define INSN_C_LWSP 0x4002 +#define INSN_C_LD 0x6000 +#define INSN_C_LDSP 0x6002 + +/* Store instruction opcode */ + +#define INSN_SB 0x0023 +#define INSN_SH 0x1023 +#define INSN_SW 0x2023 +#define INSN_SD 0x3023 + +#define INSN_C_SW 0xc000 +#define INSN_C_SWSP 0xc002 +#define INSN_C_SD 0xe000 +#define INSN_C_SDSP 0xe002 + +/* Float load instruction opcode */ + +#define INSN_FLW 0x2007 +#define INSN_FLD 0x3007 + +#define INSN_C_FLW 0x6000 +#define INSN_C_FLWSP 0x6002 +#define INSN_C_FLD 0x2000 +#define INSN_C_FLDSP 0x2002 + +/* Float store instruction opcode */ + +#define INSN_FSW 0x2027 +#define INSN_FSD 0x3027 + +#define INSN_C_FSW 0xe000 +#define INSN_C_FSWSP 0xe002 +#define INSN_C_FSD 0xa000 +#define INSN_C_FSDSP 0xa002 + +/**************************************************************************** + * Public Type Definitions + ****************************************************************************/ + +/* Compressed instruction encoding */ + +typedef union +{ + uint16_t insn; + + /* Load */ + + struct + { + uint16_t op : 2; + uint16_t rd : 3; + uint16_t imm6 : 1; /* IMM bit [6] */ + uint16_t imm2 : 1; /* IMM bit [2] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } lw; + + struct + { + uint16_t op : 2; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t imm42 : 3; /* IMM bit [4:2] */ + uint16_t rd : 5; + uint16_t imm5 : 1; /* IMM bit [5] */ + uint16_t funct3 : 3; + } lwsp; + + struct + { + uint16_t op : 2; + uint16_t rd : 3; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } ld; + + struct + { + uint16_t op : 2; + uint16_t imm86 : 3; /* IMM bit [6:6] */ + uint16_t imm43 : 2; /* IMM bit [4:3] */ + uint16_t rd : 5; + uint16_t imm5 : 1; /* IMM bit [5] */ + uint16_t funct3 : 3; + } ldsp; + + /* Store */ + + struct + { + uint16_t op : 2; + uint16_t rs2 : 3; + uint16_t imm6 : 1; /* IMM bit [6] */ + uint16_t imm2 : 1; /* IMM bit [2] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sw; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 5; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t imm52 : 4; /* IMM bit [5:2] */ + uint16_t funct3 : 3; + } swsp; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 3; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sd; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 5; + uint16_t imm86 : 3; /* IMM bit [8:6] */ + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sdsp; +} rv_insn_c_t; + +/* Normal instruction encoding */ + +typedef union +{ + uint32_t insn; + + /* Load */ + + struct + { + uint32_t op : 7; + uint32_t rd : 5; + uint32_t funct3 : 3; + uint32_t rs1 : 5; + int32_t imm : 12; + } l; + + /* Store */ + + struct + { + uint32_t op : 7; + uint32_t imm2 : 5; + uint32_t funct3 : 3; + uint32_t rs1 : 5; + uint32_t rs2 : 5; + uint32_t imm1 : 7; + } s; +} rv_insn_t; + +typedef struct +{ + uint8_t *dest; + uint8_t *src; + int len; +} rv_insn_ctx_t; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: decode_insn_compressed + * + * Description: + * Try to decode a compressed instruction to normal encoding + * + ****************************************************************************/ + +static bool decode_insn_compressed(uintptr_t *regs, rv_insn_ctx_t *ctx) +{ +#ifdef CONFIG_ARCH_RV_ISA_C + uint16_t in; + uint32_t imm = 0; + rv_insn_c_t insn; + + /* Fetch instruction */ + + insn.insn = *(uint16_t *)regs[REG_EPC]; + + in = insn.insn & INSN_C_MASK; + + switch (in & INSN_C_MASK) + { + case INSN_C_LW: + imm = insn.lw.imm2 << 2 | insn.lw.imm53 << 3 | insn.lw.imm6 << 6; + ctx->dest = (uint8_t *)®s[REG_X8 + insn.lw.rd]; + ctx->src = (uint8_t *)regs[REG_X8 + insn.lw.rs1] + imm; + ctx->len = 4; + break; + + case INSN_C_LWSP: + imm = insn.lwsp.imm42 << 2 | insn.lwsp.imm5 << 5 | + insn.lwsp.imm76 << 6; + ctx->dest = (uint8_t *)®s[insn.lwsp.rd]; + ctx->src = (uint8_t *)regs[REG_SP] + imm; + ctx->len = 4; + break; + + case INSN_C_LD: + imm = insn.ld.imm53 << 3 | insn.ld.imm76 << 6; + ctx->dest = (uint8_t *)®s[REG_X8 + insn.ld.rd]; + ctx->src = (uint8_t *)regs[REG_X8 + insn.ld.rs1] + imm; + ctx->len = 8; + break; + + case INSN_C_LDSP: + imm = insn.ldsp.imm43 << 3 | insn.ldsp.imm5 << 5 | + insn.ldsp.imm86 << 6; + ctx->dest = (uint8_t *)®s[insn.ld.rd]; + ctx->src = (uint8_t *)regs[REG_SP] + imm; + ctx->len = 8; + break; + + case INSN_C_SW: + imm = insn.sw.imm2 << 2 | insn.sw.imm53 << 3 | insn.sw.imm6 << 6; + ctx->dest = (uint8_t *)regs[REG_X8 + insn.sw.rs1] + imm; + ctx->src = (uint8_t *)®s[REG_X8 + insn.sd.rs2]; + ctx->len = 4; + break; + + case INSN_C_SWSP: + imm = insn.swsp.imm52 << 2 | insn.swsp.imm76 << 6; + ctx->dest = (uint8_t *)regs[REG_SP] + imm; + ctx->src = (uint8_t *)®s[insn.swsp.rs2]; + ctx->len = 4; + break; + + case INSN_C_SD: + imm = insn.sd.imm53 << 3 | insn.sd.imm76 << 6; + ctx->dest = (uint8_t *)regs[REG_X8 + insn.sd.rs1] + imm; + ctx->src = (uint8_t *)®s[REG_X8 + insn.sd.rs2]; + ctx->len = 8; + break; + + case INSN_C_SDSP: + imm = insn.sdsp.imm53 << 3 | insn.sdsp.imm86 << 6; + ctx->dest = (uint8_t *)regs[REG_SP] + imm; + ctx->src = (uint8_t *)®s[insn.sdsp.rs2]; + ctx->len = 8; + break; + + default: + _alert("Compressed: %x\n", insn.insn); + return false; + } + + regs[REG_EPC] += 2; Review Comment: This looks a bit dubious, does this work ? I does this not result in an "Instruction address misaligned" fault ? ########## arch/risc-v/src/common/riscv_misaligned.c: ########## @@ -0,0 +1,448 @@ +/**************************************************************************** + * arch/risc-v/src/common/riscv_misaligned.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <assert.h> +#include <debug.h> +#include <nuttx/compiler.h> +#include <nuttx/config.h> +#include <nuttx/arch.h> +#include <nuttx/irq.h> + +#include <stdbool.h> +#include <stdint.h> +#include <string.h> +#include <sys/types.h> +#include <syslog.h> + +#include "riscv_internal.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/* Instruction mask for load/store */ + +#define INSN_MASK 0x707f +#define INSN_C_MASK 0xe003 + +/* Load instruction opcode */ + +#define INSN_LB 0x0003 +#define INSN_LH 0x1003 +#define INSN_LW 0x2003 +#define INSN_LD 0x3003 +#define INSN_LBU 0x4003 +#define INSN_LHU 0x5003 +#define INSN_LWU 0x6003 + +#define INSN_C_LW 0x4000 +#define INSN_C_LWSP 0x4002 +#define INSN_C_LD 0x6000 +#define INSN_C_LDSP 0x6002 + +/* Store instruction opcode */ + +#define INSN_SB 0x0023 +#define INSN_SH 0x1023 +#define INSN_SW 0x2023 +#define INSN_SD 0x3023 + +#define INSN_C_SW 0xc000 +#define INSN_C_SWSP 0xc002 +#define INSN_C_SD 0xe000 +#define INSN_C_SDSP 0xe002 + +/* Float load instruction opcode */ + +#define INSN_FLW 0x2007 +#define INSN_FLD 0x3007 + +#define INSN_C_FLW 0x6000 +#define INSN_C_FLWSP 0x6002 +#define INSN_C_FLD 0x2000 +#define INSN_C_FLDSP 0x2002 + +/* Float store instruction opcode */ + +#define INSN_FSW 0x2027 +#define INSN_FSD 0x3027 + +#define INSN_C_FSW 0xe000 +#define INSN_C_FSWSP 0xe002 +#define INSN_C_FSD 0xa000 +#define INSN_C_FSDSP 0xa002 + +/**************************************************************************** + * Public Type Definitions + ****************************************************************************/ + +/* Compressed instruction encoding */ + +typedef union +{ + uint16_t insn; + + /* Load */ + + struct + { + uint16_t op : 2; + uint16_t rd : 3; + uint16_t imm6 : 1; /* IMM bit [6] */ + uint16_t imm2 : 1; /* IMM bit [2] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } lw; + + struct + { + uint16_t op : 2; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t imm42 : 3; /* IMM bit [4:2] */ + uint16_t rd : 5; + uint16_t imm5 : 1; /* IMM bit [5] */ + uint16_t funct3 : 3; + } lwsp; + + struct + { + uint16_t op : 2; + uint16_t rd : 3; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } ld; + + struct + { + uint16_t op : 2; + uint16_t imm86 : 3; /* IMM bit [6:6] */ + uint16_t imm43 : 2; /* IMM bit [4:3] */ + uint16_t rd : 5; + uint16_t imm5 : 1; /* IMM bit [5] */ + uint16_t funct3 : 3; + } ldsp; + + /* Store */ + + struct + { + uint16_t op : 2; + uint16_t rs2 : 3; + uint16_t imm6 : 1; /* IMM bit [6] */ + uint16_t imm2 : 1; /* IMM bit [2] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sw; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 5; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t imm52 : 4; /* IMM bit [5:2] */ + uint16_t funct3 : 3; + } swsp; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 3; + uint16_t imm76 : 2; /* IMM bit [7:6] */ + uint16_t rs1 : 3; + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sd; + + struct + { + uint16_t op : 2; + uint16_t rs2 : 5; + uint16_t imm86 : 3; /* IMM bit [8:6] */ + uint16_t imm53 : 3; /* IMM bit [5:3] */ + uint16_t funct3 : 3; + } sdsp; +} rv_insn_c_t; + +/* Normal instruction encoding */ + +typedef union +{ + uint32_t insn; + + /* Load */ + + struct + { + uint32_t op : 7; + uint32_t rd : 5; + uint32_t funct3 : 3; + uint32_t rs1 : 5; + int32_t imm : 12; + } l; + + /* Store */ + + struct + { + uint32_t op : 7; + uint32_t imm2 : 5; + uint32_t funct3 : 3; + uint32_t rs1 : 5; + uint32_t rs2 : 5; + uint32_t imm1 : 7; + } s; +} rv_insn_t; + +typedef struct +{ + uint8_t *dest; + uint8_t *src; + int len; +} rv_insn_ctx_t; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: decode_insn_compressed + * + * Description: + * Try to decode a compressed instruction to normal encoding + * + ****************************************************************************/ + +static bool decode_insn_compressed(uintptr_t *regs, rv_insn_ctx_t *ctx) +{ +#ifdef CONFIG_ARCH_RV_ISA_C + uint16_t in; + uint32_t imm = 0; + rv_insn_c_t insn; + + /* Fetch instruction */ + + insn.insn = *(uint16_t *)regs[REG_EPC]; + + in = insn.insn & INSN_C_MASK; + + switch (in & INSN_C_MASK) + { + case INSN_C_LW: + imm = insn.lw.imm2 << 2 | insn.lw.imm53 << 3 | insn.lw.imm6 << 6; + ctx->dest = (uint8_t *)®s[REG_X8 + insn.lw.rd]; + ctx->src = (uint8_t *)regs[REG_X8 + insn.lw.rs1] + imm; + ctx->len = 4; + break; + + case INSN_C_LWSP: + imm = insn.lwsp.imm42 << 2 | insn.lwsp.imm5 << 5 | + insn.lwsp.imm76 << 6; + ctx->dest = (uint8_t *)®s[insn.lwsp.rd]; + ctx->src = (uint8_t *)regs[REG_SP] + imm; + ctx->len = 4; + break; + + case INSN_C_LD: + imm = insn.ld.imm53 << 3 | insn.ld.imm76 << 6; + ctx->dest = (uint8_t *)®s[REG_X8 + insn.ld.rd]; + ctx->src = (uint8_t *)regs[REG_X8 + insn.ld.rs1] + imm; + ctx->len = 8; + break; + + case INSN_C_LDSP: + imm = insn.ldsp.imm43 << 3 | insn.ldsp.imm5 << 5 | + insn.ldsp.imm86 << 6; + ctx->dest = (uint8_t *)®s[insn.ld.rd]; + ctx->src = (uint8_t *)regs[REG_SP] + imm; + ctx->len = 8; + break; + + case INSN_C_SW: + imm = insn.sw.imm2 << 2 | insn.sw.imm53 << 3 | insn.sw.imm6 << 6; + ctx->dest = (uint8_t *)regs[REG_X8 + insn.sw.rs1] + imm; + ctx->src = (uint8_t *)®s[REG_X8 + insn.sd.rs2]; + ctx->len = 4; + break; + + case INSN_C_SWSP: + imm = insn.swsp.imm52 << 2 | insn.swsp.imm76 << 6; + ctx->dest = (uint8_t *)regs[REG_SP] + imm; + ctx->src = (uint8_t *)®s[insn.swsp.rs2]; + ctx->len = 4; + break; + + case INSN_C_SD: + imm = insn.sd.imm53 << 3 | insn.sd.imm76 << 6; + ctx->dest = (uint8_t *)regs[REG_X8 + insn.sd.rs1] + imm; + ctx->src = (uint8_t *)®s[REG_X8 + insn.sd.rs2]; + ctx->len = 8; + break; + + case INSN_C_SDSP: + imm = insn.sdsp.imm53 << 3 | insn.sdsp.imm86 << 6; + ctx->dest = (uint8_t *)regs[REG_SP] + imm; + ctx->src = (uint8_t *)®s[insn.sdsp.rs2]; + ctx->len = 8; + break; + + default: + _alert("Compressed: %x\n", insn.insn); + return false; + } + + regs[REG_EPC] += 2; + + return true; +#else + return false; +#endif +} + +/**************************************************************************** + * Name: decode_insn_ctx + * + * Description: + * Try to decode a normal encoding + * + ****************************************************************************/ + +static bool decode_insn_ctx(uintptr_t *regs, rv_insn_ctx_t *ctx) +{ + uint32_t in; + rv_insn_t insn; + int32_t imm; + + insn.insn = *(uint32_t *)regs[REG_EPC]; + + /* Get load/store instruction encoding */ + + in = insn.insn & INSN_MASK; + + switch (in) + { + case INSN_LH: + case INSN_LHU: + case INSN_LW: + case INSN_LWU: + case INSN_LD: + + /* Load a value from memory to register */ + + ctx->dest = (uint8_t *)®s[insn.l.rd]; + ctx->src = (uint8_t *)regs[insn.l.rs1] + insn.l.imm; + + /* Zero the target register */ + + *(uintptr_t *)ctx->dest = 0; + + /* Get data wide bit */ + + in &= (~0x4000); + in >>= 12; + ctx->len = 2 << (in - 1); + + break; + + case INSN_SH: + case INSN_SW: + case INSN_SD: + + /* Fetch sign extended offset */ + + *(uint32_t *)&imm = insn.s.imm2 | (insn.s.imm1 << 5); + + ctx->dest = (uint8_t *)regs[insn.s.rs1] + imm; + ctx->src = (uint8_t *)®s[insn.s.rs2]; + + /* Get data wide bit */ + + in >>= 12; + ctx->len = 2 << (in - 1); + + break; + + case INSN_FLW: + case INSN_FLD: + case INSN_FSW: + case INSN_FSD: + default: + _alert("Uncompressed: %x\n", insn.insn); + return false; + } + + /* Adjust EPC */ + + regs[REG_EPC] += 4; + + return true; +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: riscv_misaligned + * + * Description: + * This is software interrupt exception handler that handle load/store + * address misaligned exception. + * + * Input Parameters: + * regs - The current exception context + * + ****************************************************************************/ + +int riscv_misaligned(int irq, void *context, void *arg) +{ + uintptr_t *regs = context; + rv_insn_ctx_t ctx; + bool ret; + + /* Try to decode compressed instruction if it is */ + + ret = decode_insn_compressed(regs, &ctx); + + /* Decode instruction context */ + + if (ret == false) + { + if (decode_insn_ctx(regs, &ctx) == false) + { + /* Decode failed, we can't handle a invalid instruction */ + + PANIC(); Review Comment: PANIC() here is fine for now but another option is to return from this function and let the IRQ dispatching logic decide what to do. In flat mode there is nothing more to do but later with kernel mode we could kill the faulting process. But this is fine like it is ! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
