[PATCH 5/7] RISC-V: fix auipc-jalr addresses in patched alternatives

[Philipp Tomsich]

On Mon, 14 Nov 2022 at 12:38, Heiko Stübner <heiko at sntech.de> wrote:
>
> Am Montag, 14. November 2022, 12:35:53 CET schrieb Andrew Jones:
> > On Mon, Nov 14, 2022 at 11:57:29AM +0100, Emil Renner Berthing wrote:
> > > On Thu, 10 Nov 2022 at 17:50, Heiko Stuebner <heiko at sntech.de> wrote:
> > ...
> > > > @@ -316,8 +384,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > >                 }
> > > >
> > > >                 tmp = (1U << alt->errata_id);
> > > > -               if (cpu_req_feature & tmp)
> > > > -                       patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > > > +               if (cpu_req_feature & tmp) {
> > > > +                       /* do the basic patching */
> > > > +                       patch_text_nosync(alt->old_ptr, alt->alt_ptr,
> > > > +                                         alt->alt_len);
> > > > +
> > > > +                       riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > > +                                                        alt->alt_len,
> > > > +                                                        alt->old_ptr - alt->alt_ptr);
> > >
> > > Here you're casting a void pointer to an instruction to an unsigned
> > > int pointer, but since we enable compressed instructions this may
> > > result in an unaligned pointer. Using this pointer will work, but may
> > > be slow. Eg. fault to m-mode to be patched up. We already do that in
> > > other places in the arch/riscv, but I'd prefer not to add new
> > > instances of this.
> >
> > Alternative instruction sequences (old and new) have compression disabled.
>
> That was my first thought as well, but I think Emil was talking more about the
> placement of the alternative block inside the running kernel.
>
> i.e. I guess the starting point of an alternative sequence could also be unaligned.

Indeed. Instruction alignment is only guaranteed to be 16bits, even
for larger instructions.

> Though I don't _yet_ see how an improvement could look like.

The general strategy would be multiple smaller accesses that are then
stitched together.
This will require (for a 32bit opcode) at least 2 loads, 1 shift and 1
or — for a critical path of 3 instructions.

Given that we are running on RV64, you can handle up to 48bit by
aligning down, performing a 64bit load and (if needed) shifting.

Finally, profiles looks like it will give us support for misaligned
loads (see https://github.com/riscv/riscv-profiles/blob/main/profiles.adoc#rva22-profiles
and where the Zicclsm extension is mandated).

Philipp.