| Message ID | 20240713174325.107685-1-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/riscv/virt.c: re-insert and deprecate 'riscv,delegate' | expand |
On Sat, Jul 13, 2024 at 02:43:25PM -0300, Daniel Henrique Barboza wrote: > Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since > it is the correct name as per dt-bindings, and the absence of the > correct name will result in validation fails when dumping the dtb and > using dt-validate. > > But this change has a side-effect: every other firmware available that > is AIA capable is using 'riscv,delegate', and it will fault/misbehave if > this property isn't present. The property was added back in QEMU 7.0, > meaning we have 2 years of firmware development using the wrong > property. > > Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' will > make current firmwares to keep booting with the 'virt' machine and > dt-validate won't complain about it since we're still using the expected > property 'riscv,delegation'. 'riscv,delegate' is then marked for future > deprecation and its use is being discouraged. dt-validate /should/ complain about it - does yours not? It's probably a bug if it doesn't. Whether dt-validate complains or not here I don't think is relevant though, only the impact on firmware of removing it.
On Sun, Jul 14, 2024 at 3:44 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since > it is the correct name as per dt-bindings, and the absence of the > correct name will result in validation fails when dumping the dtb and > using dt-validate. > > But this change has a side-effect: every other firmware available that > is AIA capable is using 'riscv,delegate', and it will fault/misbehave if > this property isn't present. The property was added back in QEMU 7.0, > meaning we have 2 years of firmware development using the wrong > property. > > Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' will > make current firmwares to keep booting with the 'virt' machine and > dt-validate won't complain about it since we're still using the expected > property 'riscv,delegation'. 'riscv,delegate' is then marked for future > deprecation and its use is being discouraged. > > Cc: Conor Dooley <conor@kernel.org> > Cc: Anup Patel <apatel@ventanamicro.com> > Fixes: b1f1e9dcfa ("hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'") > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > docs/about/deprecated.rst | 11 +++++++++++ > hw/riscv/virt.c | 9 +++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 20b7a17cf0..88f0f03786 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -479,6 +479,17 @@ versions, aliases will point to newer CPU model versions > depending on the machine type, so management software must > resolve CPU model aliases before starting a virtual machine. > > +RISC-V "virt" board "riscv,delegate" DT property (since 9.1) > +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > + > +The "riscv,delegate" DT property was added in QEMU 7.0 as part of > +the AIA APLIC support. The property changed name during the review > +process in Linux and the correct name ended up being > +"riscv,delegation". Changing the DT property name will break all > +available firmwares that are using the current (wrong) name. The > +property is kept as is in 9.1, together with "riscv,delegation", to > +give more time for firmware developers to change their code. > + > Migration > --------- > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index bc0893e087..9981e0f6c9 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -651,6 +651,15 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket, > qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation", > aplic_child_phandle, 0x1, > VIRT_IRQCHIP_NUM_SOURCES); > + /* > + * DEPRECATED_9.1: Compat property kept temporarily > + * to allow old firmwares to work with AIA. Do *not* > + * use 'riscv,delegate' in new code: use > + * 'riscv,delegation' instead. > + */ > + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate", > + aplic_child_phandle, 0x1, > + VIRT_IRQCHIP_NUM_SOURCES); > } > > riscv_socket_fdt_write_id(ms, aplic_name, socket); > -- > 2.45.2 > >
On Sun, Jul 14, 2024 at 3:44 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since > it is the correct name as per dt-bindings, and the absence of the > correct name will result in validation fails when dumping the dtb and > using dt-validate. > > But this change has a side-effect: every other firmware available that > is AIA capable is using 'riscv,delegate', and it will fault/misbehave if > this property isn't present. The property was added back in QEMU 7.0, > meaning we have 2 years of firmware development using the wrong > property. > > Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' will > make current firmwares to keep booting with the 'virt' machine and > dt-validate won't complain about it since we're still using the expected > property 'riscv,delegation'. 'riscv,delegate' is then marked for future > deprecation and its use is being discouraged. > > Cc: Conor Dooley <conor@kernel.org> > Cc: Anup Patel <apatel@ventanamicro.com> > Fixes: b1f1e9dcfa ("hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'") > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Thanks! Applied to riscv-to-apply.next Alistair > --- > docs/about/deprecated.rst | 11 +++++++++++ > hw/riscv/virt.c | 9 +++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 20b7a17cf0..88f0f03786 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -479,6 +479,17 @@ versions, aliases will point to newer CPU model versions > depending on the machine type, so management software must > resolve CPU model aliases before starting a virtual machine. > > +RISC-V "virt" board "riscv,delegate" DT property (since 9.1) > +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' > + > +The "riscv,delegate" DT property was added in QEMU 7.0 as part of > +the AIA APLIC support. The property changed name during the review > +process in Linux and the correct name ended up being > +"riscv,delegation". Changing the DT property name will break all > +available firmwares that are using the current (wrong) name. The > +property is kept as is in 9.1, together with "riscv,delegation", to > +give more time for firmware developers to change their code. > + > Migration > --------- > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c > index bc0893e087..9981e0f6c9 100644 > --- a/hw/riscv/virt.c > +++ b/hw/riscv/virt.c > @@ -651,6 +651,15 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket, > qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation", > aplic_child_phandle, 0x1, > VIRT_IRQCHIP_NUM_SOURCES); > + /* > + * DEPRECATED_9.1: Compat property kept temporarily > + * to allow old firmwares to work with AIA. Do *not* > + * use 'riscv,delegate' in new code: use > + * 'riscv,delegation' instead. > + */ > + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate", > + aplic_child_phandle, 0x1, > + VIRT_IRQCHIP_NUM_SOURCES); > } > > riscv_socket_fdt_write_id(ms, aplic_name, socket); > -- > 2.45.2 > >
On 7/13/24 3:57 PM, Conor Dooley wrote: > On Sat, Jul 13, 2024 at 02:43:25PM -0300, Daniel Henrique Barboza wrote: >> Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since >> it is the correct name as per dt-bindings, and the absence of the >> correct name will result in validation fails when dumping the dtb and >> using dt-validate. >> >> But this change has a side-effect: every other firmware available that >> is AIA capable is using 'riscv,delegate', and it will fault/misbehave if >> this property isn't present. The property was added back in QEMU 7.0, >> meaning we have 2 years of firmware development using the wrong >> property. >> >> Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' will >> make current firmwares to keep booting with the 'virt' machine and >> dt-validate won't complain about it since we're still using the expected >> property 'riscv,delegation'. 'riscv,delegate' is then marked for future >> deprecation and its use is being discouraged. > > dt-validate /should/ complain about it - does yours not? It's probably a > bug if it doesn't. Whether dt-validate complains or not here I don't > think is relevant though, only the impact on firmware of removing it. We'll keep the property as a temporary documented band-aid that we'll remove a few releases from now. OpenSBI was also updated to use the right property recently so we shouldn't have any problems. Oh, and dt-validate will complain about it if we use the right command line that creates the controller ... something that I failed to do in v1 :( I'll re-send the patch with an updated commit msg. Thanks, Daniel >
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 20b7a17cf0..88f0f03786 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -479,6 +479,17 @@ versions, aliases will point to newer CPU model versions depending on the machine type, so management software must resolve CPU model aliases before starting a virtual machine. +RISC-V "virt" board "riscv,delegate" DT property (since 9.1) +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The "riscv,delegate" DT property was added in QEMU 7.0 as part of +the AIA APLIC support. The property changed name during the review +process in Linux and the correct name ended up being +"riscv,delegation". Changing the DT property name will break all +available firmwares that are using the current (wrong) name. The +property is kept as is in 9.1, together with "riscv,delegation", to +give more time for firmware developers to change their code. + Migration --------- diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index bc0893e087..9981e0f6c9 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -651,6 +651,15 @@ static void create_fdt_one_aplic(RISCVVirtState *s, int socket, qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegation", aplic_child_phandle, 0x1, VIRT_IRQCHIP_NUM_SOURCES); + /* + * DEPRECATED_9.1: Compat property kept temporarily + * to allow old firmwares to work with AIA. Do *not* + * use 'riscv,delegate' in new code: use + * 'riscv,delegation' instead. + */ + qemu_fdt_setprop_cells(ms->fdt, aplic_name, "riscv,delegate", + aplic_child_phandle, 0x1, + VIRT_IRQCHIP_NUM_SOURCES); } riscv_socket_fdt_write_id(ms, aplic_name, socket);
Commit b1f1e9dcfa renamed 'riscv,delegate' to 'riscv,delegation' since it is the correct name as per dt-bindings, and the absence of the correct name will result in validation fails when dumping the dtb and using dt-validate. But this change has a side-effect: every other firmware available that is AIA capable is using 'riscv,delegate', and it will fault/misbehave if this property isn't present. The property was added back in QEMU 7.0, meaning we have 2 years of firmware development using the wrong property. Re-introducing 'riscv,delegate' while keeping 'riscv,delegation' will make current firmwares to keep booting with the 'virt' machine and dt-validate won't complain about it since we're still using the expected property 'riscv,delegation'. 'riscv,delegate' is then marked for future deprecation and its use is being discouraged. Cc: Conor Dooley <conor@kernel.org> Cc: Anup Patel <apatel@ventanamicro.com> Fixes: b1f1e9dcfa ("hw/riscv/virt.c: aplic DT: rename prop to 'riscv, delegation'") Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- docs/about/deprecated.rst | 11 +++++++++++ hw/riscv/virt.c | 9 +++++++++ 2 files changed, 20 insertions(+)