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

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

[Leif Lindholm]

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.

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.

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

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

[Peter Maydell]

On Thu, 5 May 2022 at 12:39, 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.
>
> 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.
>
> 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.)
>



Applied to target-arm.next, thanks.

-- PMM

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

[Rob Herring]

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

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

[Peter Maydell]

On Tue, 17 May 2022 at 14:27, Rob Herring <robh@kernel.org> wrote:
>
> 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.

This isn't the 'virt' machine :-)

This dtb fragment is a purely private communication between
the QEMU model and the sbsa-ref EL3 firmware. We could in
theory equally replace it with a set of hardwired
"board revision" registers. There's a comment in the existing
sources about this:

/*
 * Firmware on this machine only uses ACPI table to load OS, these limited
 * device tree nodes are just to let firmware know the info which varies from
 * command line parameters, so it is not necessary to be fully compatible
 * with the kernel CPU and NUMA binding rules.
 */

Kernels running on sbsa-ref won't see a dtb (let alone one with this
version information in it), because firmware will always boot them
with ACPI.

thanks
-- PMM

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

[Rob Herring]

On Tue, May 17, 2022 at 11:50 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 17 May 2022 at 14:27, Rob Herring <robh@kernel.org> wrote:
> >
> > 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.
>
> This isn't the 'virt' machine :-)

Ah, okay.

> This dtb fragment is a purely private communication between
> the QEMU model and the sbsa-ref EL3 firmware. We could in

And that interface is tightly coupled and always in sync?

> theory equally replace it with a set of hardwired
> "board revision" registers. There's a comment in the existing
> sources about this:
>
> /*
>  * Firmware on this machine only uses ACPI table to load OS, these limited
>  * device tree nodes are just to let firmware know the info which varies from
>  * command line parameters, so it is not necessary to be fully compatible
>  * with the kernel CPU and NUMA binding rules.
>  */
>
> Kernels running on sbsa-ref won't see a dtb (let alone one with this
> version information in it), because firmware will always boot them
> with ACPI.

Okay, if there's no chance this moves up the stack, NM.

Rob