Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Jessica Clarke]

> On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
> > <ruinland@andestech.com> wrote:
> > >
> > > From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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?

Jess

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Bin Meng]

On Fri, Aug 6, 2021 at 8:58 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>
> > On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
> > > <ruinland@andestech.com> wrote:
> > > >
> > > > From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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.

Regards,
Bin

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Jessica Clarke]

On 6 Aug 2021, at 14:54, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Fri, Aug 6, 2021 at 8:58 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
>> 
>>> On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> 
>>>> On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
>>>> <ruinland@andestech.com> wrote:
>>>>> 
>>>>> From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Alistair Francis]

On Fri, Aug 6, 2021 at 11:54 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Aug 6, 2021 at 8:58 PM Jessica Clarke <jrtc27@jrtc27.com> wrote:
> >
> > > On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
> > > > <ruinland@andestech.com> wrote:
> > > > >
> > > > > From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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.

I agree with Bin that this could be a Kconfig option, that is selected
when a vendor CPU is enabled.

It also doesn't have to be a config and could just be built all the
time. I don't see much of an advantage in allowing it to be disabled,
it's just another thing we would need to test. Maybe is a user was
just interested in the virt machine/KVM they could disable it to avoid
any overhead.

Alistair

>
> Regards,
> Bin
>

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Bin Meng]

On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Bin Meng]

On Fri, Aug 6, 2021 at 10:39 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
> <ruinland@andestech.com> wrote:
> >
> > From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Bin Meng]

On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
<ruinland@andestech.com> wrote:
>
> From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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.

Regards,
Bin

Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Bin Meng]

On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai
<ruinland@andestech.com> wrote:
>
> From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@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.

Regards,
Bin

[RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Ruinland Chuan-Tzu Tsai]

From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@andestech.com>
---
 configure         | 6 ++++++
 meson.build       | 2 ++
 meson_options.txt | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/configure b/configure
index 34fccaa..88f6584 100755
--- a/configure
+++ b/configure
@@ -324,6 +324,7 @@ virtiofsd="auto"
 virtfs="auto"
 libudev="auto"
 mpath="auto"
+riscv_custom="auto"
 vnc="enabled"
 sparse="auto"
 vde="$default_feature"
@@ -1016,6 +1017,10 @@ for opt do
   ;;
   --enable-vnc) vnc="enabled"
   ;;
+  --enable-riscv-custom) riscv_custom="enabled"
+  ;;
+  --disable-riscv-custom) riscv_custom="disabled"
+  ;;
   --disable-gettext) gettext="disabled"
   ;;
   --enable-gettext) gettext="enabled"
@@ -6416,6 +6421,7 @@ NINJA=$ninja $meson setup \
         -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
+        -Driscv_custom=$riscv_custom \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
         -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
diff --git a/meson.build b/meson.build
index adeec15..736810e 100644
--- a/meson.build
+++ b/meson.build
@@ -1151,6 +1151,7 @@ config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
 
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
+config_host_data.set('CONFIG_RISCV_CUSTOM', get_option('riscv_custom').enabled())
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST']
@@ -2694,6 +2695,7 @@ summary_info += {'libpmem support':   config_host.has_key('CONFIG_LIBPMEM')}
 summary_info += {'libdaxctl support': config_host.has_key('CONFIG_LIBDAXCTL')}
 summary_info += {'libudev':           libudev.found()}
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
+summary_info += {'RISC-V custom CSRs/instructions': get_option('riscv_custom').enabled()}
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
 if not supported_cpus.contains(cpu)
diff --git a/meson_options.txt b/meson_options.txt
index 9734019..470ef23 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -125,3 +125,5 @@ option('slirp', type: 'combo', value: 'auto',
 option('fdt', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
        description: 'Whether and how to find the libfdt library')
+option('riscv_custom', type: 'feature', value: 'auto',
+       description: 'RISC-V custom')
-- 
2.32.0

[RFC PATCH v4 1/4] Add options to config/meson files for custom CSR

[Ruinland Chuan-Tzu Tsai]

From: Ruinland ChuanTzu Tsai <ruinland@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 <dylan@andestech.com>
---
 configure         | 6 ++++++
 meson.build       | 2 ++
 meson_options.txt | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/configure b/configure
index 34fccaa..88f6584 100755
--- a/configure
+++ b/configure
@@ -324,6 +324,7 @@ virtiofsd="auto"
 virtfs="auto"
 libudev="auto"
 mpath="auto"
+riscv_custom="auto"
 vnc="enabled"
 sparse="auto"
 vde="$default_feature"
@@ -1016,6 +1017,10 @@ for opt do
   ;;
   --enable-vnc) vnc="enabled"
   ;;
+  --enable-riscv-custom) riscv_custom="enabled"
+  ;;
+  --disable-riscv-custom) riscv_custom="disabled"
+  ;;
   --disable-gettext) gettext="disabled"
   ;;
   --enable-gettext) gettext="enabled"
@@ -6416,6 +6421,7 @@ NINJA=$ninja $meson setup \
         -Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
         -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png \
         -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f -Dvirtiofsd=$virtiofsd \
+        -Driscv_custom=$riscv_custom \
         -Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
         -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
         -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
diff --git a/meson.build b/meson.build
index adeec15..736810e 100644
--- a/meson.build
+++ b/meson.build
@@ -1151,6 +1151,7 @@ config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h'))
 config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
 
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>'))
+config_host_data.set('CONFIG_RISCV_CUSTOM', get_option('riscv_custom').enabled())
 
 ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
 arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST']
@@ -2694,6 +2695,7 @@ summary_info += {'libpmem support':   config_host.has_key('CONFIG_LIBPMEM')}
 summary_info += {'libdaxctl support': config_host.has_key('CONFIG_LIBDAXCTL')}
 summary_info += {'libudev':           libudev.found()}
 summary_info += {'FUSE lseek':        fuse_lseek.found()}
+summary_info += {'RISC-V custom CSRs/instructions': get_option('riscv_custom').enabled()}
 summary(summary_info, bool_yn: true, section: 'Dependencies')
 
 if not supported_cpus.contains(cpu)
diff --git a/meson_options.txt b/meson_options.txt
index 9734019..470ef23 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -125,3 +125,5 @@ option('slirp', type: 'combo', value: 'auto',
 option('fdt', type: 'combo', value: 'auto',
        choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
        description: 'Whether and how to find the libfdt library')
+option('riscv_custom', type: 'feature', value: 'auto',
+       description: 'RISC-V custom')
-- 
2.32.0