Re: [PATCH v2 01/20] mcd: Introduce Multi-Core Debug (MCD) API

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

On Wed, Apr 30, 2025 at 02:47:07PM +0200, Mario Fleischmann wrote:
> On 30.04.2025 10:20, Daniel P. Berrangé wrote:
> 
> > On Wed, Apr 30, 2025 at 07:27:22AM +0200, Mario Fleischmann wrote:
> >> Formatting changes to mcd_api.h to compily with QEMU's coding style guidelines:
> >>
> >> * limit line width to 80
> >> * convert Doxygen to kernel-doc comments
> >> * avoid architecture specific defines
> >>
> >> The original MCD API version can be found at:
> >> https://repo.lauterbach.com/sprint_mcd_api_v1_0.zip
> > 
> > The commit message needs to call out the license choice of the
> > imported file.
> 
> Thanks for pointing that out. Will be added in the next version.
> 
> >> Signed-off-by: Mario Fleischmann <mario.fleischmann@lauterbach.com>
> >> ---
> >>  MAINTAINERS            |    6 +
> >>  docs/interop/index.rst |    1 +
> >>  docs/interop/mcd.rst   |   44 +
> >>  mcd/mcd_api.h          | 3963 ++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 4014 insertions(+)
> >>  create mode 100644 docs/interop/mcd.rst
> >>  create mode 100644 mcd/mcd_api.h
> > 
> >> diff --git a/mcd/mcd_api.h b/mcd/mcd_api.h
> >> new file mode 100644
> >> index 0000000..8c89353
> >> --- /dev/null
> >> +++ b/mcd/mcd_api.h
> >> @@ -0,0 +1,3963 @@
> >> +/*
> >> + * Copyright (c) 2008, ARM Ltd., Infineon Technologies, NXP Semiconductors,
> >> + * Lauterbach, STMicroelectronics and TIMA Laboratory.
> >> + * All rights reserved.
> >> + *
> >> + * PREAMBLE
> >> + *
> >> + * The MCD API (Multi-Core Debug) has been designed as an interface between
> >> + * software development tools and simulated or real systems with multi-core
> >> + * SoCs. The target is to allow consistent software tooling throughout the
> >> + * whole SoC development flow.
> >> + * The MCD API (the "SOFTWARE") has been developed jointly by ARM Ltd.,
> >> + * Infineon Technologies, NXP Semiconductors, Lauterbach,
> >> + * STMicroelectronics and TIMA Laboratory as part of the SPRINT project
> >> + * (www.sprint-project.net).
> >> + * The SPRINT project has been funded by the European Commission.
> >> + *
> >> + * LICENSE
> >> + *
> >> + *  Any redistribution and use of the SOFTWARE in source and binary forms,
> >> + *  with or without modification constitutes the full acceptance of the
> >> + *  following disclaimer as well as of the license herein and is permitted
> >> + *  provided that the following conditions are met:
> >> + *  - Redistributions of source code must retain the above copyright notice,
> >> + *    this list of conditions and the disclaimer detailed below.
> >> + *  - Redistributions in binary form must reproduce the above copyright notice,
> >> + *    this list of conditions and the disclaimer detailed below in the
> >> + *    documentation and/or other materials provided with the distribution.
> >> + *  - Neither the name of its copyright holders nor the names of its
> >> + *    contributors may be used to endorse or promote products derived from the
> >> + *    Software without specific prior written permission.
> >> + *  - Modification of any or all of the source code, documentation and other
> >> + *    materials provided under this license are subject to acknowledgement of
> >> + *    the modification(s) by including a prominent notice on the modification(s)
> >> + *    stating the change(s) to the file(s), identifying the date of such change
> >> + *    and stating the name of the publisher of any such modification(s).
> > 
> > This 4th clause is rather obnoxious and....
> > 
> >> + * VERSION HISTORY
> >> + *
> >> + *  1.0 "SPRINT Release"     : SPRINT reference version
> >> + *
> >> + *  1.1 "Lauterbach Release" :
> >> + *  - forces all boolean types to 8-bit on Linux and Mac-OS-X,
> >> + *    but 32-bit on all other OS forces 32-bit enumeration types
> >> + *  - additional memory spaces MCD_MEM_SPACE_IS_PHYSICAL,
> >> + *    MCD_MEM_SPACE_IS_LOGICAL, MCD_MEM_SPACE_IS_AUX
> >> + *  - changed type of 2nd argument of mcd_qry_input_handle_f from "int" to
> >> + *    "uint32_t"
> >> + *  - changed type of element "data" of of mcd_tx_st from "unsigned char" to
> >> + *     "uint8_t"
> >> + *  - specifying the calling convention for MS Windows (x86) to __cdecl
> >> + *
> >> + *  1.2 "QEMU Release"       :
> >> + *  - changes formatting to accommodate QEMU's coding style guidelines
> >> + *  - includes qemu/osdep.h instead of mcd_types.h
> > 
> > ....this appears to not be compliant with the license since it fails
> > to include the dates
> 
> We've got the dates of modifications available at our version control.
> They will be added in the next version of this patch.
> 
> > AFAICT this license is derived from a classic BSD 3 clause, with the
> > extra 4th clause added.
> > 
> > I don't see any SPDX license matching this, which is painful as it
> > means it is going to need license approval before it can be included
> > by distributions downstream.
> > 
> > Does this really have to be under a custom license instead of a
> > well known standard license ? There's really no good reason for
> > inventing new open source licenses.
> 
> BSD 3-Clause Modification matches the license of the MCD API and is
> listed under the SPDX License List. Is it possible to extend
> checkpatch's @valid with "BSD-3-Clause-Modification"? That way, we can
> also add the SPDX-License-Identifier to mcd_api.h without an error being
> generated by the script.

Conceptually this is similar to BSD-3-Clause-Modification, but the
actual text of the license does not match

  https://spdx.org/licenses/BSD-3-Clause-Modification.html

so IMHO we can't claim this is BSD-3-Clause-Modification currently.

IIUC, it would have to be submitted to SPDX for review to decide
whether it needs to be a new license, or whether the permitted text
for BSD-3-Clause-Modification can allow a choice of matches.


> Thank you very much for taking the time to review the patch! I will add
> the changes in the next version of this series but will wait for further
> reviews until submitting v3 in order to keep the mailing list less busy.

Yep, makes sense.

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 :|