[PATCH v2] RISC-V: Generate ELF priv attributes if priv instruction are explicited used.

Nelson Chu nelson.chu@sifive.com
Mon Jun 22 08:29:42 GMT 2020


Thanks for all your feedback, I update what you suggested in this v2 patch.

> +/* Return True if it is a privileged instruction.  Otherwise, return FALSE.
> +
> +   uret is actually a N-ext instruction.  So it is better to regard it as
> +   an user instruction rather than the priv instruction.
> +
> +   hret is used to return from traps in H-mode.  H-mode is removed since
> +   the v1.10 priv spec, but probably be added in the new hypervisor spec.
> +   Therefore, hret should be controlled by the hypervisor spec rather than
> +   priv spec in the future.
> +
> +   dret is defined in the debug spec, so it should be checked in the future,
> +   too.  */
> +
> +static bfd_boolean
> +riscv_is_priv_insn (insn_t insn)
> +{
> +  return (((insn ^ MATCH_SRET) & MASK_SRET) == 0
> +         || ((insn ^ MATCH_MRET) & MASK_MRET) == 0
> +         || ((insn ^ MATCH_SFENCE_VMA) & MASK_SFENCE_VMA) == 0
> +         || ((insn ^ MATCH_WFI) & MASK_WFI) == 0
> +  /* The sfence.vm is dropped in the v1.10 priv specs, but we still need to
> +     check it here to keep the compatible.  Maybe we should issue warning
> +     if sfence.vm is used, but the priv spec newer than v1.10 is choosen.
> +     We aleady have a similar check for CSR, but not yet for instructions.
> +     It would be good if we could check the spec versions both for CSR and
> +     instructions, but not here.  */
> +         || ((insn ^ MATCH_SFENCE_VM) & MASK_SFENCE_VM) == 0);
> +}

* Some typo I will fix when committing. "aleady -> already". "choosen
-> chosen".

* hret is actually a hypervisor instruction, so I think it should be
controlled by the new hypervisor specs rather than the priv spec.

* I also remove uret instruction in the riscv_is_priv_insn since it
should be a N-ext instruction.

* Two options when handling sfence.vm in the future,
1. Encode the max/min priv specs in the elf attributes.
2. Check the spec versions, including ISA , priv and debug specs, both
for CSR and instruction.  We already have the similar check for CSR on
upstream, so maybe we could add the check for instruction in the
future.


>             case 'E':           /* Control register.  */
>               insn_with_csr = TRUE;
> -             explicit_csr = TRUE;
> +             explicit_priv_attr = TRUE;
>               if (reg_lookup (&s, RCLASS_CSR, &regno))
>                 INSERT_OPERAND (CSR, *ip, regno);
>               else

This will be updated when I support the unprivileged CSR in the future
patches.  The unprivileged CSR should be controlled by the other specs
rather than the priv spec, so the explicit_priv_attr shouldn't be set.
Besides, we haven't checked the CSR_CLASS when the CSR is used by the
CSR address directly, rather than the CSR name.

Thanks
Nelson


More information about the Binutils mailing list