On Wed, Dec 07, 2022 at 10:35:50AM +0100, Heiko Stübner wrote: > Am Dienstag, 6. Dezember 2022, 23:10:01 CET schrieb Heiko Stübner: > > Am Donnerstag, 1. Dezember 2022, 20:33:53 CET schrieb Andrew Jones: > > > On Wed, Nov 30, 2022 at 11:56:11PM +0100, Heiko Stuebner wrote: > > > > From: Heiko Stuebner <heiko.stuebner@xxxxxxxx> ... > > > > +#define to_auipc_imm(value) \ > > > > + ((value & JALR_SIGN_MASK) ? \ > > > > + ((value & RV_U_IMM_31_12_MASK) + AUIPC_PAD) : \ > > > > + (value & RV_U_IMM_31_12_MASK)) > > > > > > I know RV_U_IMM_31_12_OPOFF is 0, but it looks odd not shifting > > > RV_U_IMM_31_12_MASK when we do shift RV_I_IMM_11_0_MASK. > > > > > > So, it looks like to_auipc_imm() is doing > > > > > > offset[31:12] + ((value & BIT(11)) ? (1 << 12) : 0) > > > > > > but the spec says the auipc part of the 'call' pseudoinstruction should be > > > > can you point me to that part of the spec? > > > > For educational purposes, because in the main riscv-spec I only found > > the main auipc + jalr descriptions, but nothing about the actual > > "call" pseudoinstruction. > > > > [and I'm probably just looking at the wrong document] > > > > > > > offset[31:12] + offset[11] > > > > > > which I think would be written as > > > > > > ((((value) & RV_U_IMM_31_12_MASK) << RV_U_IMM_31_12_OPOFF) + ((value) & BIT(11))) > > > > > > or what am I missing? > > So far I've found the riscv-asm-manual [0], which only states for call > auipc x1, offset[31:12] > jalr x1, x1, offset[11:0] > > and the psABI spec [1], neither mention your "offset[31:12] + offset[11]" ? > > But [1] does contain that tiny sentence > "The successive instruction has a signed 12-bit immediate so the value > of the preceding high 20-bit relocation may have 1 added to it." > > > I.e. the lower 12 bits become themself a signed number [-2048:2047] > > Take an example: > - address is 1862116 ( 0b 111000110 100111100100 ) > - address[31:12] becomes 1859584 (as 0b 111000110 000000000000) > - while address[11:0] is 2532 as part of the bigger number > - as lone 12bit signed IMM it becomes -1564 > - so you need to add this 4096 to the auipc IMM to compensate > > > Heiko > > > [0] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md > [1] https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/tag/v1.0 Yeah, I got this figured out yesterday and sent another mail confirming the way you had it was right. Did you receive that mail? Where I see call offset auipc x1, offset[31 : 12] + offset[11] jalr x1, offset[11:0](x1) is in chapter 25 of the unprivileged ISA. > > > > > that whole thing came from the ftrace parts, also doing call fixup voodoo > > And I can't really say that I understand every nook and cranny of it. > > > > But for practical purposes, the addresses generated now work, > > and also seem to work for the ftrace counterpart (see include/asm/ftrace.h) > > > > [another place that will profit from more generalization :-) ] > > > > > > > > + > > > > +void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len, > > > > + int patch_offset) > > > > +{ > > > > + int num_instr = len / sizeof(u32); > > > > + unsigned int call[2]; > > > > + int i; > > > > + int imm; > > > > + u32 rd1; > > > > + > > > > + /* > > > > + * stop one instruction before the end, as we're checking > > > > + * for auipc + jalr > > > > + */ > > > > + for (i = 0; i < num_instr - 1; i++) { > > > > > > If we change riscv_instruction_at() to just take a pointer then we can do > > > the math in the for() and actually just use pointer arithmetic. > > > > > > uint32_t *p = alt_ptr; > > > for (i = 0; i < num_instr - 1; i++, p++) { > > > > Wasn't not using uint32 pointers the whole point of going with the accessor? Oh, right. So maybe stick to the offsetting, but still do the math in the caller. Thanks, drew
