[PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture

Thomas Weißschuh posted 4 patches 2 months ago
There is a newer version of this series
[PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Posted by Thomas Weißschuh 2 months ago
Only a small subset of all architectures supported by qemu make use of
firmware files. Introduce and use a new enum to represent this.

This also removes the dependency to machine.json from the global qapi
definitions.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/interop/firmware.json | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index a26fe81bf2fe..2eb0be11d595 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -14,7 +14,10 @@
 # = Firmware
 ##
 
-{ 'include' : 'machine.json' }
+{ 'pragma': {
+    'member-name-exceptions': [
+        'FirmwareArchitecture' # x86_64
+    ] } }
 
 ##
 # @FirmwareOSInterface:
@@ -59,6 +62,28 @@
 { 'enum' : 'FirmwareDevice',
   'data' : [ 'flash', 'kernel', 'memory' ] }
 
+##
+# @FirmwareArchitecture:
+#
+# Enumerations of architectures for which Qemu uses additional firmware files.
+# The values are a subset of the enum SysEmuTarget.
+#
+# @aarch64: 64-bit Arm.
+#
+# @arm: 32-bit Arm.
+#
+# @i386: 32-bit x86.
+#
+# @loongarch64: 64-bit LoongArch.
+#
+# @x86_64: 64-bit x86.
+#
+# Since: 9.1
+##
+{ 'enum' : 'FirmwareArchitecture',
+  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
+
+
 ##
 # @FirmwareTarget:
 #
@@ -80,7 +105,7 @@
 # Since: 3.0
 ##
 { 'struct' : 'FirmwareTarget',
-  'data'   : { 'architecture' : 'SysEmuTarget',
+  'data'   : { 'architecture' : 'FirmwareArchitecture',
                'machines'     : [ 'str' ] } }
 
 ##

-- 
2.45.2


Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Posted by Markus Armbruster 2 months ago
Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:

> Only a small subset of all architectures supported by qemu make use of
> firmware files. Introduce and use a new enum to represent this.
>
> This also removes the dependency to machine.json from the global qapi
> definitions.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  docs/interop/firmware.json | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index a26fe81bf2fe..2eb0be11d595 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -14,7 +14,10 @@
>  # = Firmware
>  ##
>  
> -{ 'include' : 'machine.json' }
> +{ 'pragma': {
> +    'member-name-exceptions': [
> +        'FirmwareArchitecture' # x86_64
> +    ] } }
>  
>  ##
>  # @FirmwareOSInterface:
> @@ -59,6 +62,28 @@
>  { 'enum' : 'FirmwareDevice',
>    'data' : [ 'flash', 'kernel', 'memory' ] }
>  
> +##
> +# @FirmwareArchitecture:
> +#
> +# Enumerations of architectures for which Qemu uses additional firmware files.

docs/devel/qapi-code-gen.rst section "Documentation markup":

    For legibility, wrap text paragraphs so every line is at most 70
    characters long.

> +# The values are a subset of the enum SysEmuTarget.

Will consumers of firmware.json care for this?  Or is it just a reminder
for developers to keep the two enums in sync?

> +#
> +# @aarch64: 64-bit Arm.
> +#
> +# @arm: 32-bit Arm.
> +#
> +# @i386: 32-bit x86.
> +#
> +# @loongarch64: 64-bit LoongArch.
> +#
> +# @x86_64: 64-bit x86.
> +#
> +# Since: 9.1

The enum type is indeed since 9.1, but its members are since 3.0, and
that's what matters.  Except for @loongarch, which is since 7.1.0 (not
documented in qapi/machine.json; I'll fix that).

> +##
> +{ 'enum' : 'FirmwareArchitecture',
> +  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> +
> +

Drop one blank line, please.

>  ##
>  # @FirmwareTarget:
>  #
> @@ -80,7 +105,7 @@
>  # Since: 3.0
>  ##
>  { 'struct' : 'FirmwareTarget',
> -  'data'   : { 'architecture' : 'SysEmuTarget',
> +  'data'   : { 'architecture' : 'FirmwareArchitecture',
>                 'machines'     : [ 'str' ] } }
>  
>  ##
Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Posted by Thomas Weißschuh 2 months ago
On Thu, Jul 18, 2024 at 03:18:07PM GMT, Markus Armbruster wrote:
> Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:
> 
> > Only a small subset of all architectures supported by qemu make use of
> > firmware files. Introduce and use a new enum to represent this.
> >
> > This also removes the dependency to machine.json from the global qapi
> > definitions.
> >
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> >  docs/interop/firmware.json | 29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> > index a26fe81bf2fe..2eb0be11d595 100644
> > --- a/docs/interop/firmware.json
> > +++ b/docs/interop/firmware.json
> > @@ -14,7 +14,10 @@
> >  # = Firmware
> >  ##
> >  
> > -{ 'include' : 'machine.json' }
> > +{ 'pragma': {
> > +    'member-name-exceptions': [
> > +        'FirmwareArchitecture' # x86_64
> > +    ] } }
> >  
> >  ##
> >  # @FirmwareOSInterface:
> > @@ -59,6 +62,28 @@
> >  { 'enum' : 'FirmwareDevice',
> >    'data' : [ 'flash', 'kernel', 'memory' ] }
> >  
> > +##
> > +# @FirmwareArchitecture:
> > +#
> > +# Enumerations of architectures for which Qemu uses additional firmware files.
> 
> docs/devel/qapi-code-gen.rst section "Documentation markup":
> 
>     For legibility, wrap text paragraphs so every line is at most 70
>     characters long.
> 
> > +# The values are a subset of the enum SysEmuTarget.

Ack.

> Will consumers of firmware.json care for this?

Most probably not.

> Or is it just a reminder for developers to keep the two enums in sync?

I guess so.
Should I drop it?

> 
> > +#
> > +# @aarch64: 64-bit Arm.
> > +#
> > +# @arm: 32-bit Arm.
> > +#
> > +# @i386: 32-bit x86.
> > +#
> > +# @loongarch64: 64-bit LoongArch.
> > +#
> > +# @x86_64: 64-bit x86.
> > +#
> > +# Since: 9.1
> 
> The enum type is indeed since 9.1, but its members are since 3.0, and
> that's what matters.  Except for @loongarch, which is since 7.1.0 (not
> documented in qapi/machine.json; I'll fix that).
> 
> > +##
> > +{ 'enum' : 'FirmwareArchitecture',
> > +  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> > +
> > +
> 
> Drop one blank line, please.

Ack.

> 
> >  ##
> >  # @FirmwareTarget:
> >  #
> > @@ -80,7 +105,7 @@
> >  # Since: 3.0
> >  ##
> >  { 'struct' : 'FirmwareTarget',
> > -  'data'   : { 'architecture' : 'SysEmuTarget',
> > +  'data'   : { 'architecture' : 'FirmwareArchitecture',
> >                 'machines'     : [ 'str' ] } }
> >  
> >  ##
> 
Re: [PATCH v5 2/4] docs/interop/firmware.json: add new enum FirmwareArchitecture
Posted by Markus Armbruster 2 months ago
Thomas Weißschuh <t-8ch@linutronix.de> writes:

> On Thu, Jul 18, 2024 at 03:18:07PM GMT, Markus Armbruster wrote:
>> Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:
>> 
>> > Only a small subset of all architectures supported by qemu make use of
>> > firmware files. Introduce and use a new enum to represent this.
>> >
>> > This also removes the dependency to machine.json from the global qapi
>> > definitions.
>> >
>> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
>> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>> > ---
>> >  docs/interop/firmware.json | 29 +++++++++++++++++++++++++++--
>> >  1 file changed, 27 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
>> > index a26fe81bf2fe..2eb0be11d595 100644
>> > --- a/docs/interop/firmware.json
>> > +++ b/docs/interop/firmware.json
>> > @@ -14,7 +14,10 @@
>> >  # = Firmware
>> >  ##
>> >  
>> > -{ 'include' : 'machine.json' }
>> > +{ 'pragma': {
>> > +    'member-name-exceptions': [
>> > +        'FirmwareArchitecture' # x86_64
>> > +    ] } }
>> >  
>> >  ##
>> >  # @FirmwareOSInterface:
>> > @@ -59,6 +62,28 @@
>> >  { 'enum' : 'FirmwareDevice',
>> >    'data' : [ 'flash', 'kernel', 'memory' ] }
>> >  
>> > +##
>> > +# @FirmwareArchitecture:
>> > +#
>> > +# Enumerations of architectures for which Qemu uses additional firmware files.
>> 
>> docs/devel/qapi-code-gen.rst section "Documentation markup":
>> 
>>     For legibility, wrap text paragraphs so every line is at most 70
>>     characters long.
>> 
>> > +# The values are a subset of the enum SysEmuTarget.
>
> Ack.
>
>> Will consumers of firmware.json care for this?
>
> Most probably not.
>
>> Or is it just a reminder for developers to keep the two enums in sync?
>
> I guess so.
> Should I drop it?

You can drop it.

If you think it's useful for developers, you can instead make it a
non-doc comment, like ...

>> 
>> > +#
>> > +# @aarch64: 64-bit Arm.
>> > +#
>> > +# @arm: 32-bit Arm.
>> > +#
>> > +# @i386: 32-bit x86.
>> > +#
>> > +# @loongarch64: 64-bit LoongArch.
>> > +#
>> > +# @x86_64: 64-bit x86.
>> > +#
>> > +# Since: 9.1
>> 
>> The enum type is indeed since 9.1, but its members are since 3.0, and
>> that's what matters.  Except for @loongarch, which is since 7.1.0 (not
>> documented in qapi/machine.json; I'll fix that).
>> 
>> > +##
>> > +{ 'enum' : 'FirmwareArchitecture',
>> > +  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }

... here:

      # The values are a subset of the enum SysEmuTarget defined in
      # qapi/machine.json.

If you do, consider adding a similar note to FirmwareFormat in the
previous patch.

>> > +
>> > +
>> 
>> Drop one blank line, please.
>
> Ack.
>
>> 
>> >  ##
>> >  # @FirmwareTarget:
>> >  #
>> > @@ -80,7 +105,7 @@
>> >  # Since: 3.0
>> >  ##
>> >  { 'struct' : 'FirmwareTarget',
>> > -  'data'   : { 'architecture' : 'SysEmuTarget',
>> > +  'data'   : { 'architecture' : 'FirmwareArchitecture',
>> >                 'machines'     : [ 'str' ] } }
>> >  
>> >  ##
>>