Re: [RFC patch 4/4] riscv: sifive: apply errata "cip-453" patch

[Ruinland ChuanTzu Tsai <ruinland@andestech.com>]

Hi Vincent,

Thanks for introducing the alternative mechanism to RISC-V, with which
vendors could provide fixes for each erratum in a more elegant way.

Somehow, I'm a bit sketchy about these parts of your proposal :

>     /* Exception vector table */
>  ENTRY(excp_vect_table)
>     RISCV_PTR do_trap_insn_misaligned
> -   RISCV_PTR do_trap_insn_fault
> +   ALTERNATIVE(__stringify(RISCV_PTR do_trap_insn_fault),
> +           __stringify(RISCV_PTR do_trap_insn_fault_trampoline),
> +           SIFIVE_VENDOR_ID, ERRATA_CIP_453, CONFIG_ERRATA_SIFIVE_CIP_453)
>     RISCV_PTR do_trap_insn_illegal
>     RISCV_PTR do_trap_break
>     RISCV_PTR do_trap_load_misaligned
> @@ -461,7 +466,10 @@ ENTRY(excp_vect_table)
>     RISCV_PTR do_trap_ecall_s
>     RISCV_PTR do_trap_unknown
>     RISCV_PTR do_trap_ecall_m
> -   RISCV_PTR do_page_fault   /* instruction page fault */
> +   /* instruciton page fault */
> +   ALTERNATIVE(__stringify(RISCV_PTR do_page_fault),
> +           __stringify(RISCV_PTR do_page_fault_trampoline),
> +           SIFIVE_VENDOR_ID, ERRATA_CIP_453, CONFIG_ERRATA_SIFIVE_CIP_453)
>     RISCV_PTR do_page_fault   /* load page fault */

As far as I can tell, `ALTERNATIVE(...)` seems a bit like a mixture of
ARM's version of ALTERNATIVE and `alternative_insn`. However, ARM's
ALTERNATIVE takes a vardatic macro and yours here doesn't, which makes
me wonder if another vendor needs to patch the same location as yours,
will they be able to multiplex the same probe ?


Secondly, I think it's a bit intrusive to patch directly on exception
vector table here.

I'm not sure about whether it's possible to introduce your alternative
probe inside do_trap_insn_fault() & do_page_fault(), do the inspection
the reason of trap (e.g. instruction/load/store page fault) there and
then, perform the software workaround.

If that's not feasible, maybe we shall make a new macro with a name
like "RISCV_TRAP_ENTRY" which encompass the alternative probes ?

Last but not least, is it possible that in the near future,
`alternative_if` macro family from ARM could be ported to RISC-V ?

Best regards,
Ruinland

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv