On Fri, Aug 9, 2024 at 2:13 AM Daniel P. Berrangé <berra...@redhat.com> wrote: > > 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.ben...@linaro.org> wrote: > > > > > > Octavian Purdila <ta...@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 <ta...@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. >
I think we can review changes to prevent ABI breakages both for XML and generated headers - unless we use meson git-wrap submodules. > > > > > > 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 > Fine with me. I think we can also reconsider later with minimal effort if this gets widely used and there is lots of churn. Peter, what do you think? > 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. > Sounds good. I was thinking of adding mcux-soc-svd as a meson subproject via git-wrap as suggested by Philippe. > 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 :| >
