[PATCH v3 0/3] docs/interop/firmware.json: scripts/qapi-gen.py compatibility

[PATCH v3 0/3] docs/interop/firmware.json: scripts/qapi-gen.py compatibility

[Thomas Weißschuh]

docs/interop/firmware.json is currently not usable with qapi-gen.py due
to various non-functional issues.
Fix those issue to provide compatibility.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Changes in v3:
- Drop already picked up patches
- Drop include of pragma.json
- Introduce new enums FirmwareFormat and FirmwareArchitecture
- Link to v2: https://lore.kernel.org/r/20240307-qapi-firmware-json-v2-0-3b29eabb9b9a@linutronix.de

Changes in v2:
- Add review tag from Philippe
- Add Fixes tag (Philippe)
- Add testcase (Philippe)
- Link to v1: https://lore.kernel.org/r/20240306-qapi-firmware-json-v1-0-619f7122a249@linutronix.de

---
Thomas Weißschuh (3):
      docs/interop/firmware.json: add new enum FirmwareFormat
      docs/interop/firmware.json: add new enum FirmwareArchitecture
      docs: add test for firmware.json QAPI

 docs/interop/firmware.json | 36 ++++++++++++++++++++++++++++++++----
 docs/meson.build           |  5 +++++
 2 files changed, 37 insertions(+), 4 deletions(-)
---
base-commit: 7489f7f3f81dcb776df8c1b9a9db281fc21bf05f
change-id: 20240306-qapi-firmware-json-6fb1213936dd

Best regards,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>

[PATCH v3 1/3] docs/interop/firmware.json: add new enum FirmwareFormat

[Thomas Weißschuh]

Only a small subset of all blockdev drivers make sense for firmware
images. Introduce and use a new enum to represent this.

This also reduces the dependency of firmware.json on the global qapi
definitions.

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

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 54a1fc6c1041..0e619e8780e7 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -15,7 +15,11 @@
 ##
 
 { 'include' : 'machine.json' }
-{ 'include' : 'block-core.json' }
+
+{ 'pragma': {
+    'documentation-exceptions': [
+        'FirmwareFormat'
+    ] } }
 
 ##
 # @FirmwareOSInterface:
@@ -200,6 +204,16 @@
              'enrolled-keys', 'requires-smm', 'secure-boot',
              'verbose-dynamic', 'verbose-static' ] }
 
+##
+# @FirmwareFormat:
+#
+# Formats that are supported for firmware images.
+#
+# Since: 8.3
+##
+{ 'enum': 'FirmwareFormat',
+  'data': [ 'raw', 'qcow2' ] }
+
 ##
 # @FirmwareFlashFile:
 #
@@ -219,7 +233,7 @@
 ##
 { 'struct' : 'FirmwareFlashFile',
   'data'   : { 'filename' : 'str',
-               'format'   : 'BlockdevDriver' } }
+               'format'   : 'FirmwareFormat' } }
 
 
 ##

-- 
2.44.0

Re: [PATCH v3 1/3] docs/interop/firmware.json: add new enum FirmwareFormat

[Philippe Mathieu-Daudé]

On 11/3/24 12:46, Thomas Weißschuh wrote:
> Only a small subset of all blockdev drivers make sense for firmware
> images. Introduce and use a new enum to represent this.
> 
> This also reduces the dependency of firmware.json on the global qapi
> definitions.
> 
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   docs/interop/firmware.json | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)


> +##
> +# @FirmwareFormat:
> +#
> +# Formats that are supported for firmware images.
> +#
> +# Since: 8.3

That will be 9.0 :/

> +##
> +{ 'enum': 'FirmwareFormat',
> +  'data': [ 'raw', 'qcow2' ] }

Re: [PATCH v3 1/3] docs/interop/firmware.json: add new enum FirmwareFormat

[Thomas Weißschuh]

On Mon, Mar 11, 2024 at 01:08:19PM +0100, Philippe Mathieu-Daudé wrote:
> On 11/3/24 12:46, Thomas Weißschuh wrote:
> > Only a small subset of all blockdev drivers make sense for firmware
> > images. Introduce and use a new enum to represent this.
> > 
> > This also reduces the dependency of firmware.json on the global qapi
> > definitions.
> > 
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> >   docs/interop/firmware.json | 18 ++++++++++++++++--
> >   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> 
> > +##
> > +# @FirmwareFormat:
> > +#
> > +# Formats that are supported for firmware images.
> > +#
> > +# Since: 8.3
> 
> That will be 9.0 :/

Thanks.

IMO it doesn't warrant a new revision, the maintainer can change it when
applying.

Locally I changed it, so if a new revision is necessary for other
reasons it will have the change.

> > +##
> > +{ 'enum': 'FirmwareFormat',
> > +  'data': [ 'raw', 'qcow2' ] }
> 

Re: [PATCH v3 1/3] docs/interop/firmware.json: add new enum FirmwareFormat

[Markus Armbruster]

Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:

> Only a small subset of all blockdev drivers make sense for firmware
> images. Introduce and use a new enum to represent this.
>
> This also reduces the dependency of firmware.json on the global qapi
> definitions.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  docs/interop/firmware.json | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 54a1fc6c1041..0e619e8780e7 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -15,7 +15,11 @@
>  ##
>  
>  { 'include' : 'machine.json' }
> -{ 'include' : 'block-core.json' }
> +
> +{ 'pragma': {
> +    'documentation-exceptions': [
> +        'FirmwareFormat'
> +    ] } }

Necessary because ...
>  
>  ##
>  # @FirmwareOSInterface:
> @@ -200,6 +204,16 @@
>               'enrolled-keys', 'requires-smm', 'secure-boot',
>               'verbose-dynamic', 'verbose-static' ] }
>  
> +##
> +# @FirmwareFormat:
> +#
> +# Formats that are supported for firmware images.
> +#

... we don't document the members here.  Documenting them would be
nicer.  We'd do that if we could steal it from BlockdevDriver, but
there's nothing to steal there.

Mere observation; I'm not asking you to come up with documentation
BlockdevDriver doesn't have.

> +# Since: 8.3
> +##
> +{ 'enum': 'FirmwareFormat',
> +  'data': [ 'raw', 'qcow2' ] }

@raw or @file?  Kevin or Hanna, thoughts?

> +
>  ##
>  # @FirmwareFlashFile:
>  #
> @@ -219,7 +233,7 @@
>  ##
>  { 'struct' : 'FirmwareFlashFile',
>    'data'   : { 'filename' : 'str',
> -               'format'   : 'BlockdevDriver' } }
> +               'format'   : 'FirmwareFormat' } }
>  
>  
>  ##

Re: [PATCH v3 1/3] docs/interop/firmware.json: add new enum FirmwareFormat

[Thomas Weißschuh]

Hi Hanna and Kevin,

any updates?

On Mon, Mar 11, 2024 at 02:29:25PM +0100, Markus Armbruster wrote:
> Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:
> 
> > Only a small subset of all blockdev drivers make sense for firmware
> > images. Introduce and use a new enum to represent this.
> >
> > This also reduces the dependency of firmware.json on the global qapi
> > definitions.
> >
> > Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> >  docs/interop/firmware.json | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> > index 54a1fc6c1041..0e619e8780e7 100644
> > --- a/docs/interop/firmware.json
> > +++ b/docs/interop/firmware.json
> > @@ -15,7 +15,11 @@
> >  ##
> >  
> >  { 'include' : 'machine.json' }
> > -{ 'include' : 'block-core.json' }
> > +
> > +{ 'pragma': {
> > +    'documentation-exceptions': [
> > +        'FirmwareFormat'
> > +    ] } }
> 
> Necessary because ...
> >  
> >  ##
> >  # @FirmwareOSInterface:
> > @@ -200,6 +204,16 @@
> >               'enrolled-keys', 'requires-smm', 'secure-boot',
> >               'verbose-dynamic', 'verbose-static' ] }
> >  
> > +##
> > +# @FirmwareFormat:
> > +#
> > +# Formats that are supported for firmware images.
> > +#
> 
> ... we don't document the members here.  Documenting them would be
> nicer.  We'd do that if we could steal it from BlockdevDriver, but
> there's nothing to steal there.
> 
> Mere observation; I'm not asking you to come up with documentation
> BlockdevDriver doesn't have.
> 
> > +# Since: 8.3
> > +##
> > +{ 'enum': 'FirmwareFormat',
> > +  'data': [ 'raw', 'qcow2' ] }
> 
> @raw or @file?  Kevin or Hanna, thoughts?

The existing descriptors in pc-bios/descriptors/ are using @raw.

> > +
> >  ##
> >  # @FirmwareFlashFile:
> >  #
> > @@ -219,7 +233,7 @@
> >  ##
> >  { 'struct' : 'FirmwareFlashFile',
> >    'data'   : { 'filename' : 'str',
> > -               'format'   : 'BlockdevDriver' } }
> > +               'format'   : 'FirmwareFormat' } }
> >  
> >  
> >  ##

Thanks,
Thomas

[PATCH v3 2/3] docs/interop/firmware.json: add new enum FirmwareArchitecture

[Thomas Weißschuh]

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 of firmware.json on the global qapi
definitions.

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

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 0e619e8780e7..54cae6a51b43 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -14,11 +14,13 @@
 # = Firmware
 ##
 
-{ 'include' : 'machine.json' }
-
 { 'pragma': {
     'documentation-exceptions': [
+        'FirmwareArchitecture',
         'FirmwareFormat'
+    ],
+    'member-name-exceptions': [
+        'FirmwareArchitecture'
     ] } }
 
 ##
@@ -64,6 +66,18 @@
 { '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.
+#
+# Since: 8.3
+##
+{ 'enum' : 'FirmwareArchitecture',
+  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
+
+
 ##
 # @FirmwareTarget:
 #
@@ -85,7 +99,7 @@
 # Since: 3.0
 ##
 { 'struct' : 'FirmwareTarget',
-  'data'   : { 'architecture' : 'SysEmuTarget',
+  'data'   : { 'architecture' : 'FirmwareArchitecture',
                'machines'     : [ 'str' ] } }
 
 ##

-- 
2.44.0

Re: [PATCH v3 2/3] docs/interop/firmware.json: add new enum FirmwareArchitecture

[Markus Armbruster]

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 of firmware.json on the global qapi
> definitions.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  docs/interop/firmware.json | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
> index 0e619e8780e7..54cae6a51b43 100644
> --- a/docs/interop/firmware.json
> +++ b/docs/interop/firmware.json
> @@ -14,11 +14,13 @@
>  # = Firmware
>  ##
>  
> -{ 'include' : 'machine.json' }
> -
>  { 'pragma': {
>      'documentation-exceptions': [
> +        'FirmwareArchitecture',

Comment on the previous patch applies.

>          'FirmwareFormat'
> +    ],
> +    'member-name-exceptions': [
> +        'FirmwareArchitecture'

Needed for x86_64.  Fine.

>      ] } }
>  
>  ##
> @@ -64,6 +66,18 @@
>  { '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.
> +#
> +# Since: 8.3
> +##
> +{ 'enum' : 'FirmwareArchitecture',
> +  'data' : [ 'aarch64', 'arm', 'i386', 'loongarch64', 'x86_64' ] }
> +
> +
>  ##
>  # @FirmwareTarget:
>  #
> @@ -85,7 +99,7 @@
>  # Since: 3.0
>  ##
>  { 'struct' : 'FirmwareTarget',
> -  'data'   : { 'architecture' : 'SysEmuTarget',
> +  'data'   : { 'architecture' : 'FirmwareArchitecture',
>                 'machines'     : [ 'str' ] } }
>  
>  ##

[PATCH v3 3/3] docs: add test for firmware.json QAPI

[Thomas Weißschuh]

To make sure that the QAPI description stays valid add a testcase.

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/qemu-devel/d9ce0234-4beb-4b90-b14c-76810d3b81d7@linaro.org/
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 docs/meson.build | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/meson.build b/docs/meson.build
index 9040f860ae1a..bcca45a342a3 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -99,3 +99,8 @@ if build_docs
   alias_target('html', sphinxdocs)
   alias_target('man', sphinxmans)
 endif
+
+test('QAPI firmware.json regression tests', python,
+     args: [qapi_gen.full_path(), '-o', meson.current_build_dir() / 'qapi',
+            meson.current_source_dir() / 'interop/firmware.json'],
+     env: test_env, suite: ['qapi-schema', 'qapi-interop'])

-- 
2.44.0