Re: [PATCH v2] hw/arm: add versioning to sbsa-ref machine DT

[Rob Herring <robh@kernel.org>]

On Thu, May 5, 2022 at 6:41 AM Leif Lindholm <quic_llindhol@quicinc.com> wrote:
>
> The sbsa-ref machine is continuously evolving. Some of the changes we
> want to make in the near future, to align with real components (e.g.
> the GIC-700), will break compatibility for existing firmware.
>
> Introduce two new properties to the DT generated on machine generation:
> - machine-version-major
>   To be incremented when a platform change makes the machine
>   incompatible with existing firmware.
> - machine-version-minor
>   To be incremented when functionality is added to the machine
>   without causing incompatibility with existing firmware.
>   to be reset to 0 when machine-version-major is incremented.

Where's the binding documentation for this?

We already have a way to version DTs and that's with compatible. I'm
not completely opposed to a version number though, but I am opposed to
it not being common. We've rejected vendors (QCom in fact) doing their
own thing here.


> This versioning scheme is *neither*:
> - A QEMU versioned machine type; a given version of QEMU will emulate
>   a given version of the platform.
> - A reflection of level of SBSA (now SystemReady SR) support provided.

FYI, it's planned to certify the virt machine for SR-IR which will
include DT schema validation. Undocumented properties are a problem
for that.

> The version will increment on guest-visible functional changes only,
> akin to a revision ID register found on a physical platform.
>
> These properties are both introduced with the value 0.
> (Hence, a machine where the DT is lacking these nodes is equivalent
> to version 0.0.)
>
> Signed-off-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Radoslaw Biernacki <rad@semihalf.com>
> Cc: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes since v1:
> - More detail added to commit message.
> - Verbose comment added adjacent to code additions.
>
>  hw/arm/sbsa-ref.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 2387401963..d0c25ddfcf 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -190,6 +190,20 @@ static void create_fdt(SBSAMachineState *sms)
>      qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
>      qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
>
> +    /*
> +     * This versioning scheme is for informing platform fw only. It is neither:
> +     * - A QEMU versioned machine type; a given version of QEMU will emulate
> +     *   a given version of the platform.
> +     * - A reflection of level of SBSA (now SystemReady SR) support provided.
> +     *
> +     * machine-version-major: updated when changes breaking fw compatibility
> +     *                        are introduced.
> +     * machine-version-minor: updated when features are added that don't break
> +     *                        fw compatibility.
> +     */
> +    qemu_fdt_setprop_cell(fdt, "/", "machine-version-major", 0);
> +    qemu_fdt_setprop_cell(fdt, "/", "machine-version-minor", 0);
> +
>      if (ms->numa_state->have_numa_distance) {
>          int size = nb_numa_nodes * nb_numa_nodes * 3 * sizeof(uint32_t);
>          uint32_t *matrix = g_malloc0(size);
> --
> 2.30.2
>
>