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

[Cédric Le Goater]

Hello,

On 4/27/22 20:29, Leif Lindholm 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.
>
> 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.)

This raises a lot of questions. I am talking with my PAPR specs
experience there, which might be off topic for SBSA, but it's a
way to clarify my understanding.


If we need to introduce incompatible changes in the sbsa machine,
that would break existing firmwares, I think we should start versioning
the sbsa machine like the other do : arm/i440fx/m68k/q35/s390x/spapr
and add class attributes describing features being activated or not.
This to make sure that firmware already shipped can always be run.

Regarding the DT changes, we could also expose/advertise the new
platform features by name with property nodes. A version is OK but
literal names are more explicit and we might want to de/activate
features one by one for test purposes or for some other reason.

Also, if some reconfiguration of the platform is needed to activate
a new feature, software (firmware or OS) needs a way (a trap or
something) to negotiate with the platform what should be done.
I might be anticipating a bit too much but pSeries has such needs
for the MMU and interrupt modes.

What about the SBSA specs ? Do they need a change ? It is true that
there are a bit vague regarding the DT, only referencing the Arm
Base Boot Requirements document which references the Device Tree
specs v0.3. I didn't find anything about versioning.

Thanks,

C.


> 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>
> ---
>   hw/arm/sbsa-ref.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index 2387401963..e05f6056c7 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -190,6 +190,9 @@ static void create_fdt(SBSAMachineState *sms)
>       qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 0x2);
>       qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
>   
> +    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);