On Fri, Aug 22, 2025 at 11:17 PM Jesse Taube <je...@rivosinc.com> wrote: > > Various parts of the kernel decode and read instruction from memory. > Functions like get_insn, GET_INSN_LENGTH and riscv_insn_is_c are defined > in multiple places. Consolidate these functions into the insn.h and the > newly added insn.c. > > Signed-off-by: Jesse Taube <je...@rivosinc.com> > --- > RFC -> V1: > - No change > V2 -> V1: > - No change > --- > arch/riscv/include/asm/bug.h | 12 --- > arch/riscv/include/asm/insn.h | 131 ++++++++++++++++++++++- > arch/riscv/kernel/Makefile | 1 + > arch/riscv/kernel/insn.c | 151 +++++++++++++++++++++++++++ > arch/riscv/kernel/kgdb.c | 102 +----------------- > arch/riscv/kernel/probes/kprobes.c | 1 + > arch/riscv/kernel/traps.c | 5 +- > arch/riscv/kernel/traps_misaligned.c | 93 ++++------------- > 8 files changed, 309 insertions(+), 187 deletions(-) > create mode 100644 arch/riscv/kernel/insn.c > > diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h > index 1aaea81fb141..a2777eb67ad1 100644 > --- a/arch/riscv/include/asm/bug.h > +++ b/arch/riscv/include/asm/bug.h > @@ -12,21 +12,9 @@ > > #include <asm/asm.h> > > -#define __INSN_LENGTH_MASK _UL(0x3) > -#define __INSN_LENGTH_32 _UL(0x3) > -#define __COMPRESSED_INSN_MASK _UL(0xffff) > - > #define __BUG_INSN_32 _UL(0x00100073) /* ebreak */ > #define __BUG_INSN_16 _UL(0x9002) /* c.ebreak */ > > -#define GET_INSN_LENGTH(insn) \ > -({ \ > - unsigned long __len; \ > - __len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) ? \ > - 4UL : 2UL; \ > - __len; \ > -}) > - > typedef u32 bug_insn_t; > > #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h > index 09fde95a5e8f..ba74e5b8262c 100644 > --- a/arch/riscv/include/asm/insn.h > +++ b/arch/riscv/include/asm/insn.h > @@ -64,6 +64,7 @@ > #define RVG_RS2_OPOFF 20 > #define RVG_RD_OPOFF 7 > #define RVG_RS1_MASK GENMASK(4, 0) > +#define RVG_RS2_MASK GENMASK(4, 0) > #define RVG_RD_MASK GENMASK(4, 0) > > /* The bit field of immediate value in RVC J instruction */ > @@ -121,17 +122,27 @@ > #define RVC_C0_RS1_OPOFF 7 > #define RVC_C0_RS2_OPOFF 2 > #define RVC_C0_RD_OPOFF 2 > +#define RVC_C0_RS1_MASK GENMASK(2, 0) > +#define RVC_C0_RS2_MASK GENMASK(2, 0) > +#define RVC_C0_RD_MASK GENMASK(2, 0) > +#define RVC_C0_REG_OFFSET 8 > > /* The register offset in RVC op=C1 instruction */ > #define RVC_C1_RS1_OPOFF 7 > #define RVC_C1_RS2_OPOFF 2 > #define RVC_C1_RD_OPOFF 7 > +#define RVC_C1_RS1_MASK GENMASK(2, 0) > +#define RVC_C1_RS2_MASK GENMASK(2, 0) > +#define RVC_C1_RD_MASK GENMASK(2, 0) > +#define RVC_C1_REG_OFFSET 8 > > /* The register offset in RVC op=C2 instruction */ > #define RVC_C2_RS1_OPOFF 7 > #define RVC_C2_RS2_OPOFF 2 > #define RVC_C2_RD_OPOFF 7 > #define RVC_C2_RS1_MASK GENMASK(4, 0) > +#define RVC_C2_RS2_MASK GENMASK(4, 0) > +#define RVC_C2_RD_MASK GENMASK(4, 0) > > /* parts of opcode for RVG*/ > #define RVG_OPCODE_FENCE 0x0f > @@ -226,12 +237,26 @@ > #define RVC_MASK_C_EBREAK 0xffff > #define RVG_MASK_EBREAK 0xffffffff > #define RVG_MASK_SRET 0xffffffff > +#define RVC_MASK_C GENMASK(15, 0) > > #define __INSN_LENGTH_MASK _UL(0x3) > #define __INSN_LENGTH_GE_32 _UL(0x3) > #define __INSN_OPCODE_MASK _UL(0x7F) > #define __INSN_BRANCH_OPCODE _UL(RVG_OPCODE_BRANCH) > > +#define GET_INSN_LENGTH(insn) \ > +({ \ > + unsigned long __len; \ > + __len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_GE_32) ? \ > + 4UL : 2UL; \ > + __len; \ > +}) > + > +static __always_inline bool riscv_insn_is_c(u32 code) > +{ > + return (code & (__INSN_LENGTH_MASK)) != (__INSN_LENGTH_GE_32); > +} > + > #define __RISCV_INSN_FUNCS(name, mask, val) \ > static __always_inline bool riscv_insn_is_##name(u32 code) \ > { \ > @@ -260,7 +285,7 @@ __RISCV_INSN_FUNCS(c_bnez, RVC_MASK_C_BNEZ, > RVC_MATCH_C_BNEZ) > __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK) > __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK) > __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET) > -__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE); > +__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE) > > /* special case to catch _any_ system instruction */ > static __always_inline bool riscv_insn_is_system(u32 code) > @@ -295,6 +320,10 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 > code) > ({typeof(x) x_ = (x); \ > (RV_X(x_, RVG_RS1_OPOFF, RVG_RS1_MASK)); }) > > +#define RV_EXTRACT_RS2_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RV_X(x_, RVG_RS2_OPOFF, RVG_RS2_MASK)); }) > + > #define RV_EXTRACT_RD_REG(x) \ > ({typeof(x) x_ = (x); \ > (RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); }) > @@ -322,9 +351,41 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 > code) > (RV_X(x_, RV_B_IMM_11_OPOFF, RV_B_IMM_11_MASK) << RV_B_IMM_11_OFF) | \ > (RV_IMM_SIGN(x_) << RV_B_IMM_SIGN_OFF); }) > > +#define RVC_EXTRACT_C0_RS1_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C0_RS1_OPOFF, RVC_C0_RS1_MASK)); }) > + > +#define RVC_EXTRACT_C0_RS2_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C0_RS2_OPOFF, RVC_C0_RS2_MASK)); }) > + > +#define RVC_EXTRACT_C0_RD_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C0_RD_OPOFF, RVC_C0_RD_MASK)); }) > + > +#define RVC_EXTRACT_C1_RS1_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C1_RS1_OPOFF, RVC_C1_RS1_MASK)); }) > + > +#define RVC_EXTRACT_C1_RS2_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C1_RS2_OPOFF, RVC_C1_RS2_MASK)); }) > + > +#define RVC_EXTRACT_C1_RD_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C1_RD_OPOFF, RVC_C1_RD_MASK)); }) > + > #define RVC_EXTRACT_C2_RS1_REG(x) \ > ({typeof(x) x_ = (x); \ > - (RV_X(x_, RVC_C2_RS1_OPOFF, RVC_C2_RS1_MASK)); }) > + (RVC_X(x_, RVC_C2_RS1_OPOFF, RVC_C2_RS1_MASK)); }) > + > +#define RVC_EXTRACT_C2_RS2_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C2_RS2_OPOFF, RVC_C2_RS2_MASK)); }) > + > +#define RVC_EXTRACT_C2_RD_REG(x) \ > + ({typeof(x) x_ = (x); \ > + (RVC_X(x_, RVC_C2_RD_OPOFF, RVC_C2_RD_MASK)); }) > > #define RVC_EXTRACT_JTYPE_IMM(x) \ > ({typeof(x) x_ = (x); \ > @@ -354,6 +415,66 @@ static __always_inline bool riscv_insn_is_c_jalr(u32 > code) > > #define RVV_EXRACT_VL_VS_WIDTH(x) RVFDQ_EXTRACT_FL_FS_WIDTH(x) > > +/* > + * Get the rs1 register number from RV or RVC instruction. > + * > + * @insn: instruction to process > + * Return: rs1 register > + */ > +static inline unsigned int riscv_insn_extract_rs1_reg(u32 insn) > +{ > + switch (RVC_INSN_OPCODE_MASK & insn) { > + case RVC_OPCODE_C0: > + return RVC_EXTRACT_C0_RS1_REG(insn) + RVC_C0_REG_OFFSET; > + case RVC_OPCODE_C1: > + return RVC_EXTRACT_C1_RS1_REG(insn) + RVC_C1_REG_OFFSET; > + case RVC_OPCODE_C2: > + return RVC_EXTRACT_C2_RS1_REG(insn); > + default: > + return RV_EXTRACT_RS1_REG(insn); > + } > +} > + > +/* > + * Get the rs2 register number from RV or RVC instruction. > + * > + * @insn: instruction to process > + * Return: rs2 register > + */ > +static inline unsigned int riscv_insn_extract_rs2_reg(u32 insn) > +{ > + switch (RVC_INSN_OPCODE_MASK & insn) { > + case RVC_OPCODE_C0: > + return RVC_EXTRACT_C0_RS2_REG(insn) + RVC_C0_REG_OFFSET; > + case RVC_OPCODE_C1: > + return RVC_EXTRACT_C1_RS2_REG(insn) + RVC_C1_REG_OFFSET; > + case RVC_OPCODE_C2: > + return RVC_EXTRACT_C2_RS2_REG(insn); > + default: > + return RV_EXTRACT_RS2_REG(insn); > + } > +} > + > +/* > + * Get the rd register number from RV or RVC instruction. > + * > + * @insn: instruction to process > + * Return: rd register > + */ > +static inline unsigned int riscv_insn_extract_rd_reg(u32 insn) > +{ > + switch (RVC_INSN_OPCODE_MASK & insn) { > + case RVC_OPCODE_C0: > + return RVC_EXTRACT_C0_RD_REG(insn) + RVC_C0_REG_OFFSET; > + case RVC_OPCODE_C1: > + return RVC_EXTRACT_C1_RD_REG(insn) + RVC_C1_REG_OFFSET; > + case RVC_OPCODE_C2: > + return RVC_EXTRACT_C2_RD_REG(insn); > + default: > + return RV_EXTRACT_RD_REG(insn); > + } > +} > + > /* > * Get the immediate from a J-type instruction. > * > @@ -428,4 +549,10 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 > *utype_insn, u32 *itype > *utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1); > *itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF); > } > + > +#include <asm/ptrace.h> > + > +int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn); > +unsigned long get_step_address(struct pt_regs *regs, u32 code); > + > #endif /* _ASM_RISCV_INSN_H */ > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile > index f7480c9c6f8d..4f719b09e5ad 100644 > --- a/arch/riscv/kernel/Makefile > +++ b/arch/riscv/kernel/Makefile > @@ -51,6 +51,7 @@ obj-$(CONFIG_RISCV_ALTERNATIVE) += alternative.o > obj-y += cpu.o > obj-y += cpufeature.o > obj-y += entry.o > +obj-y += insn.o > obj-y += irq.o > obj-y += process.o > obj-y += ptrace.o > diff --git a/arch/riscv/kernel/insn.c b/arch/riscv/kernel/insn.c > new file mode 100644 > index 000000000000..dd2a6ef9fd25 > --- /dev/null > +++ b/arch/riscv/kernel/insn.c > @@ -0,0 +1,151 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2025 Rivos, Inc > + */ > +#include <asm/insn.h> > +#include <asm/ptrace.h> > +#include <asm/uaccess.h> > + > +#define __read_insn(regs, insn, insn_addr, type) \ > +({ \ > + int __ret; \ > + \ > + if (user_mode(regs)) { \ > + __ret = get_user(insn, (type __user *) insn_addr); \ > + } else { \ > + insn = *(type *)insn_addr; \ > + __ret = 0; \ > + } \ > + \ > + __ret; \ > +}) > + > +/* > + * Update a set of two instructions (U-type + I-type) with an immediate > value. > + * > + * Used for example in auipc+jalrs pairs the U-type instructions contains > + * a 20bit upper immediate representing bits[31:12], while the I-type > + * instruction contains a 12bit immediate representing bits[11:0]. > + * > + * This also takes into account that both separate immediates are > + * considered as signed values, so if the I-type immediate becomes > + * negative (BIT(11) set) the U-type part gets adjusted. > + * > + * @regs: pointer to the utype instruction of the pair > + * @epc: pointer to the itype instruction of the pair > + * @r_insn: the immediate to insert into the two instructions > + * Return: combined immediate > + */ > +int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn) > +{ > + ulong insn = 0; > + > + if (epc & 0x2) { > + ulong tmp = 0; > + > + if (__read_insn(regs, insn, epc, u16)) > + return -EFAULT; > + /* __get_user() uses regular "lw" which sign extend the loaded > + * value make sure to clear higher order bits in case we "or" > it > + * below with the upper 16 bits half. > + */ > + insn &= RVC_MASK_C; > + if (riscv_insn_is_c(insn)) { > + *r_insn = insn; > + return 0; > + } > + epc += sizeof(u16); > + if (__read_insn(regs, tmp, epc, u16)) > + return -EFAULT; > + *r_insn = (tmp << 16) | insn; > + > + return 0; > + } else { > + if (__read_insn(regs, insn, epc, u32)) > + return -EFAULT; > + if (!riscv_insn_is_c(insn)) { > + *r_insn = insn; > + return 0; > + } > + insn &= RVC_MASK_C; > + *r_insn = insn; > + > + return 0; > + } > +} > + > +/* Calculate the new address for after a step */ > +unsigned long get_step_address(struct pt_regs *regs, u32 code) > +{ > + unsigned long pc = regs->epc; > + unsigned int rs1_num, rs2_num; > + > + if ((code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) { > + if (riscv_insn_is_c_jalr(code) || > + riscv_insn_is_c_jr(code)) { > + rs1_num = riscv_insn_extract_rs1_reg(code); > + return regs_get_register(regs, rs1_num); > + } else if (riscv_insn_is_c_j(code) || > + riscv_insn_is_c_jal(code)) { > + return RVC_EXTRACT_JTYPE_IMM(code) + pc; > + } else if (riscv_insn_is_c_beqz(code)) { > + rs1_num = riscv_insn_extract_rs1_reg(code); > + if (!rs1_num || regs_get_register(regs, rs1_num) == 0) > + return RVC_EXTRACT_BTYPE_IMM(code) + pc; > + else > + return pc + 2; > + } else if (riscv_insn_is_c_bnez(code)) { > + rs1_num = > riscv_insn_extract_rs1_reg(RVC_C1_RS1_OPOFF); > + if (rs1_num && regs_get_register(regs, rs1_num) != 0) > + return RVC_EXTRACT_BTYPE_IMM(code) + pc; > + else > + return pc + 2; > + } else { > + return pc + 2; > + } > + } else { > + if ((code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) { > + bool result = false; > + long imm = RV_EXTRACT_BTYPE_IMM(code); > + unsigned long rs1_val = 0, rs2_val = 0; > + > + rs1_num = riscv_insn_extract_rs1_reg(code); > + rs2_num = riscv_insn_extract_rs2_reg(code); > + if (rs1_num) > + rs1_val = regs_get_register(regs, rs1_num); > + if (rs2_num) > + rs2_val = regs_get_register(regs, rs2_num); > + > + if (riscv_insn_is_beq(code)) > + result = (rs1_val == rs2_val) ? true : false; > + else if (riscv_insn_is_bne(code)) > + result = (rs1_val != rs2_val) ? true : false; > + else if (riscv_insn_is_blt(code)) > + result = > + ((long)rs1_val < > + (long)rs2_val) ? true : false; > + else if (riscv_insn_is_bge(code)) > + result = > + ((long)rs1_val >= > + (long)rs2_val) ? true : false; > + else if (riscv_insn_is_bltu(code)) > + result = (rs1_val < rs2_val) ? true : false; > + else if (riscv_insn_is_bgeu(code)) > + result = (rs1_val >= rs2_val) ? true : false; > + if (result) > + return imm + pc; > + else > + return pc + 4; > + } else if (riscv_insn_is_jal(code)) { > + return RV_EXTRACT_JTYPE_IMM(code) + pc; > + } else if (riscv_insn_is_jalr(code)) { > + rs1_num = riscv_insn_extract_rs1_reg(code); > + return RV_EXTRACT_ITYPE_IMM(code) + > + (rs1_num ? regs_get_register(regs, rs1_num) : > 0); > + } else if (riscv_insn_is_sret(code)) { > + return pc; > + } else { > + return pc + 4; > + } > + } > +} > diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c > index 9f3db3503dab..aafc1424fc81 100644 > --- a/arch/riscv/kernel/kgdb.c > +++ b/arch/riscv/kernel/kgdb.c > @@ -23,111 +23,19 @@ enum { > static unsigned long stepped_address; > static unsigned int stepped_opcode; > > -static int decode_register_index(unsigned long opcode, int offset) > -{ > - return (opcode >> offset) & 0x1F; > -} > - > -static int decode_register_index_short(unsigned long opcode, int offset) > -{ > - return ((opcode >> offset) & 0x7) + 8; > -} > - > -/* Calculate the new address for after a step */ > -static int get_step_address(struct pt_regs *regs, unsigned long *next_addr) > -{ > - unsigned long pc = regs->epc; > - unsigned long *regs_ptr = (unsigned long *)regs; > - unsigned int rs1_num, rs2_num; > - int op_code; > - > - if (get_kernel_nofault(op_code, (void *)pc)) > - return -EINVAL; > - if ((op_code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) { > - if (riscv_insn_is_c_jalr(op_code) || > - riscv_insn_is_c_jr(op_code)) { > - rs1_num = decode_register_index(op_code, > RVC_C2_RS1_OPOFF); > - *next_addr = regs_ptr[rs1_num]; > - } else if (riscv_insn_is_c_j(op_code) || > - riscv_insn_is_c_jal(op_code)) { > - *next_addr = RVC_EXTRACT_JTYPE_IMM(op_code) + pc; > - } else if (riscv_insn_is_c_beqz(op_code)) { > - rs1_num = decode_register_index_short(op_code, > - > RVC_C1_RS1_OPOFF); > - if (!rs1_num || regs_ptr[rs1_num] == 0) > - *next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + > pc; > - else > - *next_addr = pc + 2; > - } else if (riscv_insn_is_c_bnez(op_code)) { > - rs1_num = > - decode_register_index_short(op_code, > RVC_C1_RS1_OPOFF); > - if (rs1_num && regs_ptr[rs1_num] != 0) > - *next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + > pc; > - else > - *next_addr = pc + 2; > - } else { > - *next_addr = pc + 2; > - } > - } else { > - if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) { > - bool result = false; > - long imm = RV_EXTRACT_BTYPE_IMM(op_code); > - unsigned long rs1_val = 0, rs2_val = 0; > - > - rs1_num = decode_register_index(op_code, > RVG_RS1_OPOFF); > - rs2_num = decode_register_index(op_code, > RVG_RS2_OPOFF); > - if (rs1_num) > - rs1_val = regs_ptr[rs1_num]; > - if (rs2_num) > - rs2_val = regs_ptr[rs2_num]; > - > - if (riscv_insn_is_beq(op_code)) > - result = (rs1_val == rs2_val) ? true : false; > - else if (riscv_insn_is_bne(op_code)) > - result = (rs1_val != rs2_val) ? true : false; > - else if (riscv_insn_is_blt(op_code)) > - result = > - ((long)rs1_val < > - (long)rs2_val) ? true : false; > - else if (riscv_insn_is_bge(op_code)) > - result = > - ((long)rs1_val >= > - (long)rs2_val) ? true : false; > - else if (riscv_insn_is_bltu(op_code)) > - result = (rs1_val < rs2_val) ? true : false; > - else if (riscv_insn_is_bgeu(op_code)) > - result = (rs1_val >= rs2_val) ? true : false; > - if (result) > - *next_addr = imm + pc; > - else > - *next_addr = pc + 4; > - } else if (riscv_insn_is_jal(op_code)) { > - *next_addr = RV_EXTRACT_JTYPE_IMM(op_code) + pc; > - } else if (riscv_insn_is_jalr(op_code)) { > - rs1_num = decode_register_index(op_code, > RVG_RS1_OPOFF); > - if (rs1_num) > - *next_addr = ((unsigned long *)regs)[rs1_num]; > - *next_addr += RV_EXTRACT_ITYPE_IMM(op_code); > - } else if (riscv_insn_is_sret(op_code)) { > - *next_addr = pc; > - } else { > - *next_addr = pc + 4; > - } > - } > - return 0; > -} > - > static int do_single_step(struct pt_regs *regs) > { > /* Determine where the target instruction will send us to */ > - unsigned long addr = 0; > - int error = get_step_address(regs, &addr); > + unsigned long addr, insn; > + int error = get_insn(regs, regs->epc, &insn); > > if (error) > return error; > > + addr = get_step_address(regs, insn); > + > /* Store the op code in the stepped address */ > - error = get_kernel_nofault(stepped_opcode, (void *)addr); > + error = get_insn(regs, addr, stepped_opcode);
This line gives following compile error: arch/riscv/kernel/kgdb.c: In function 'do_single_step': arch/riscv/kernel/kgdb.c:38:38: error: passing argument 3 of 'get_insn' makes pointer from integer without a cast [-Wint-conversion] 38 | error = get_insn(regs, addr, stepped_opcode); | ^~~~~~~~~~~~~~ | | | unsigned int In file included from arch/riscv/kernel/kgdb.c:14: ./arch/riscv/include/asm/insn.h:555:54: note: expected 'ulong *' {aka 'long unsigned int *'} but argument is of type 'unsigned int' 555 | int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn); | ~~~~~~~^~~~~~ If you are touching some source then at least compile test it. Regards, Anup