Re: [RFC PATCH 04/23] hw/arm: add SVD file for NXP i.MX RT595

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Aug 06, 2024 at 01:31:51PM -0700, Octavian Purdila wrote:
> On Tue, Aug 6, 2024 at 7:06 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> >
> > Octavian Purdila <tavip@google.com> writes:
> >
> > > Picked from:
> > >
> > > https://github.com/nxp-mcuxpresso/mcux-soc-svd/blob/main/MIMXRT595S/MIMXRT595S_cm33.xml
> > >
> > > NOTE: the file is truncated to keep the email size reasonable. Please
> > > use the link above and download the full file if you want to try out
> > > the patch.

"the file is truncated" wins understatement of the week.

The full XML file that would need to be in QEMU git for this is 8.6 MB in
size.

The overall generated headers that we get from it are ~16k lines,
or ~0.5 MB

$ wc -l build/hw/arm/svd/*.h
   135 build/hw/arm/svd/flexcomm.h
  1227 build/hw/arm/svd/flexcomm_i2c.h
  1161 build/hw/arm/svd/flexcomm_spi.h
  1231 build/hw/arm/svd/flexcomm_usart.h
  2243 build/hw/arm/svd/flexspi.h
  3100 build/hw/arm/svd/rt500_clkctl0.h
  4022 build/hw/arm/svd/rt500_clkctl1.h
    64 build/hw/arm/svd/rt500.h
  1073 build/hw/arm/svd/rt500_rstctl0.h
  1697 build/hw/arm/svd/rt500_rstctl1.h
 15953 total

$ wc -c build/hw/arm/svd/*.h
  4349 build/hw/arm/svd/flexcomm.h
 51135 build/hw/arm/svd/flexcomm_i2c.h
 43822 build/hw/arm/svd/flexcomm_spi.h
 46331 build/hw/arm/svd/flexcomm_usart.h
 89224 build/hw/arm/svd/flexspi.h
113952 build/hw/arm/svd/rt500_clkctl0.h
141885 build/hw/arm/svd/rt500_clkctl1.h
  1881 build/hw/arm/svd/rt500.h
 38881 build/hw/arm/svd/rt500_rstctl0.h
 61449 build/hw/arm/svd/rt500_rstctl1.h
592909 total



> > >
> > > Signed-off-by: Octavian Purdila <tavip@google.com>
> > > ---
> > >  hw/arm/svd/MIMXRT595S_cm33.xml | 224052
> > > ++++++++++++++++++++++++++++++
> >
> > I guess one thing we need to decide is if the source XML should live in
> > the repository as the preferred method of making changes or just the
> > translations generated by the tool.
> >
> 
> I think we might want to store the XML in the qemu repo, even if we
> don't use it to generate the header files at compile time. This avoids
> issues with the original XML moving, going away, changed in
> incompatible ways, etc.
>
> As for generating the headers at compile time, I don't have a strong
> preference. I like it because there is slightly less work to do and it
> avoids dealing with resolving changes on both the SVD and the
> generated headers. For example, the initial headers are committed,
> then some changes are done directly to the headers and then we want to
> pick up a new SVD from the vendor to support a new hardware revision.

IIUC the structs/enums/etc are defining guest ABI. So if we want to
preserve guest ABI for these devices across QEMU releases, we don't
want the generated output to be arbitrarily changing. If there are
different revisions of a device, we might need separate structs for
each maintained in parallel.



> 
> There are disadvantages as well: pysvd dependency for building qemu,
> hard to review if the vendor dumps a new version with lots of changes
> and we want to update to it for a new hardware revision, slight
> increase in build time.
> 
> > >  1 file changed, 224052 insertions(+)
> > >  create mode 100644 hw/arm/svd/MIMXRT595S_cm33.xml
> > >
> > > diff --git a/hw/arm/svd/MIMXRT595S_cm33.xml b/hw/arm/svd/MIMXRT595S_cm33.xml
> > > new file mode 100644
> > > index 0000000000..8943aa3555
> > > --- /dev/null
> > > +++ b/hw/arm/svd/MIMXRT595S_cm33.xml
> > > @@ -0,0 +1,1725 @@
> > > +<?xml version="1.0" encoding="UTF-8"?>
> > > +<device schemaVersion="1.3" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="CMSIS-SVD.xsd">
> > > +  <vendor>nxp.com</vendor>
> > > +  <name>MIMXRT595S_cm33</name>
> > > +  <version>1.0</version>
> > > +  <description>MIMXRT595SFAWC,MIMXRT595SFFOC</description>
> > > +  <licenseText>
> > > +Copyright 2016-2023 NXP
> > > +SPDX-License-Identifier: BSD-3-Clause
> > > +  </licenseText>
> >
> > This certainly seems compatible. XML is not the medium I personally
> > would have chosen as a register specification language but I guess there
> > are no other alternatives?
> >
> 
> I agree that the choice of XML is unfortunate but I am not aware of
> alternatives, this is what vendors will provide.

Given the size of the XML I'm inclined to say that we should just be
committing the generated header files to qemu.git

Then add https://github.com/nxp-mcuxpresso/mcux-soc-svd as a git
submodule, and provide some meson rules for triggering re-generation
that are off-by-default.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|