Re: [RFC PATCH 03/23] scripts: add script to generate C header files from SVD XML files

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 9/8/24 00:30, Octavian Purdila wrote:
> On Thu, Aug 8, 2024 at 2:56 PM John Snow <jsnow@redhat.com> wrote:


>>> diff --git a/configure b/configure
>>> index 5ad1674ca5..811bfa5d54 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -956,7 +956,7 @@ mkvenv="$python ${source_path}/python/scripts/mkvenv.py"
>>>   # Finish preparing the virtual environment using vendored .whl files
>>>
>>>   $mkvenv ensuregroup --dir "${source_path}/python/wheels" \
>>> -     ${source_path}/pythondeps.toml meson || exit 1
>>> +     ${source_path}/pythondeps.toml meson svd-gen-header || exit 1
>>
> 
> Hi John,
> 
> Thanks for reviewing!
> 
>>
>> I haven't read the rest of this series; I'm chiming in solely from the build/python maintainer angle. Do we *always* need pysvd, no matter how QEMU was configured? Adding it to the meson line here is a very big hammer.
>>
> 
> I think the minimum we can do is to install it only if CONFIG_ARM is
> enabled. That might change in the future if the models we create with
> pysvd are enabled for other architectures.

Similarly on how we manage libfdt, you can have meson defines
SVDGEN as:

   config_host_data.set('CONFIG_SVDGEN', svd_gen_header.found())

Then declare SVDGEN in Kconfig.host, and finally use in the Kconfigs:

   config FLEXCOMM_UART
       bool
       depends on SVDGEN

   config RT500
       bool
       depends on ARM
       select FLEXCOMM_UART

See for FDT examples:

$ git grep 'depends on.*FDT'
hw/core/Kconfig:10:    depends on FDT
hw/i386/Kconfig:118:    depends on I386 && FDT
hw/loongarch/Kconfig:4:    depends on LOONGARCH64 && FDT
hw/mips/Kconfig:84:    depends on MIPS64 && !TARGET_BIG_ENDIAN && FDT
hw/ppc/Kconfig:4:    depends on PPC64 && FDT
hw/ppc/Kconfig:29:    depends on PPC64 && FDT
hw/ppc/Kconfig:58:    depends on PPC && FDT
hw/ppc/Kconfig:77:    depends on PPC && FDT
hw/ppc/Kconfig:174:    depends on PPC && FDT
hw/ppc/Kconfig:180:    depends on PPC && FDT
hw/ppc/Kconfig:186:    depends on PPC && FDT
hw/rx/Kconfig:11:    depends on RX && FDT
hw/xtensa/Kconfig:17:    depends on XTENSA && FDT

>> If not, consider looking at how sphinx (the "docs" group) is only conditionally installed into the configure venv and mimic that using the appropriate configure flags that necessitate the availability of pyvsd for the QEMU build.
> 
> Thanks for the pointer, I'll take a look.
> 
>>
>> We also need to provide a way for pysvd to be available offline; some packages are available via distro libs and if this package is available for every distro we officially support, that's sufficient (but requires updates to our various docker and VM test configuration files to add the new dependency). Otherwise, like we do for meson, we need to vendor the wheel in the tree so offline tarball builds will continue to work.
>>
>> It looks like pysvd is a pure python package with no dependencies, so it should be OK to vendor it in qemu.git/python/wheels/ - look at qemu.git/python/scripts/vendor.py and consider updating and running this script.
> 
> Thanks, I'll look at it and add it in v2.
> 
>>
>> (The real blocker here is that RPM builds are performed offline and dependencies that cannot be satisfied via rpm can't be added through pip. We need any one of these to be true: (A) pyvsd is available (of a sufficient version) in all distro repositories we target; (B) This build feature is conditional and nobody minds if it never gets enabled for RPM builds; (C) The package can be vendored.)
>>
>> ~~js
>>
>> That said, you might be the first person I've seen outside of Paolo and I to brave mucking around with the python build venv. You deserve a bravery sticker :)
>>
>>>
>>>   # At this point, we expect Meson to be installed and available.
>>>   # We expect mkvenv or pip to have created pyvenv/bin/meson for us.
>>> diff --git a/meson.build b/meson.build
>>> index ec59effca2..dee587483b 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -3235,6 +3235,10 @@ tracetool_depends = files(
>>>     'scripts/tracetool/vcpu.py'
>>>   )
>>>
>>> +svd_gen_header = [
>>> +  python, files('scripts/svd-gen-header.py')
>>> +]
>>> +
>>>   qemu_version_cmd = [find_program('scripts/qemu-version.sh'),
>>>                       meson.current_source_dir(),
>>>                       get_option('pkgversion'), meson.project_version()]
>>> diff --git a/python/setup.cfg b/python/setup.cfg
>>> index 48668609d3..bc830c541a 100644
>>> --- a/python/setup.cfg
>>> +++ b/python/setup.cfg
>>> @@ -45,6 +45,7 @@ devel =
>>>       urwid >= 2.1.2
>>>       urwid-readline >= 0.13
>>>       Pygments >= 2.9.0
>>> +    pysvd >= 0.2.3
>>>