On 6 Aug 2021, at 14:54, Bin Meng <bmeng...@gmail.com> wrote:
> On Fri, Aug 6, 2021 at 8:58 PM Jessica Clarke <jrt...@jrtc27.com> wrote:
>>
>>> On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng...@gmail.com> wrote:
>>>>
>>>> On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
>>>> <ruinl...@andestech.com> wrote:
>>>>>
>>>>> From: Ruinland ChuanTzu Tsai <ruinl...@andestech.com>
>>>>>
>>>>> Adding option `riscv_custom` to configure script, meson.build and
>>>>> meson_options.txt so as to toggle custom CSR and will-be-upstreamed custom
>>>>> instructions handling logic.
>>>>>
>>>>> Signed-off-by: Dylan Jhong <dy...@andestech.com>
>>>>> ---
>>>>> configure | 6 ++++++
>>>>> meson.build | 2 ++
>>>>> meson_options.txt | 2 ++
>>>>> 3 files changed, 10 insertions(+)
>>>>>
>>>>
>>>> This sounds like unnecessary to bring such a config option to the meson
>>>> level.
>>>>
>>>> I believe a Kconfig option should just be fine.
>>>
>>> +Alistair
>>
>> I don’t see why this is even a config option. If you request a vendor’s
>> CPU you should get any custom CSRs defined for that vendor’s CPU. If
>> you don’t you don’t. This should be purely a run-time thing, no?
>
> In a generic use case where we build all RISC-V machines into one
> qemu-system-riscv{32,64} executable this makes no difference. The
> Kconfig option will be turned on if any one of the machines requires
> it. It only gets benefits when we build a QEMU executable on a
> per-machine basis.
The machines live in hw, the current patch is using the config option
in target, and hw depends on target not the other way round. I don’t
see how your example fits with that; it’d result in a layering
violation (target depending on a hw config option), no?
In your example you still wouldn’t have a config option for the
feature, I believe. You’d always have the hooks in target, and then if
you enable support for a specific CPU at compile time you get its CSRs
along with all its other bits, and at run-time you choose which to use.
So I still don’t see how this option is useful.
Jess
