Re: Can we unpoison CONFIG_FOO macros?

[Thomas Huth]

On 07/02/2023 16.39, Markus Armbruster wrote:
> We have a boatload of CONFIG_FOO macros that may only be used in
> target-dependent code.  We use generated config-poison.h to enforce.
>
> This is a bit annoying in the QAPI schema.  Let me demonstrate with an
> example: QMP commands query-rocker, query-rocker-ports, and so forth.
> These commands are useful only with "rocker" devices.  They are
> compile-time optional.  hw/net/Kconfig:
>
>      config ROCKER
>          bool
>          default y if PCI_DEVICES
>          depends on PCI && MSI_NONBROKEN
>
> The rocker device and QMP code is actually target-independent:
> hw/net/meson.build puts it into softmmu_ss.
>
> Disabling the "rocker" device type ideally disables the rocker QMP
> commands, too.  Should be easy enough: 'if': 'CONFIG_FOO' in the QAPI
> schema.
>
> Except that makes the entire code QAPI generates for rocker.json
> device-dependent: it now contains #if defined(CONFIG_ROCKER), and
> CONFIG_ROCKER is poisoned.  The rocker code implementing monitor
> commands also becomes device-dependent, because it includes generated
> headers.  We compile all that per target for no sane reason at all.

Well, CONFIG_ROCKER depends on the target config, so that is a sane reason 
in my eyes. Depending on which target you currently compile, it's either set 
or not set - there is no reasonable way to compile generic code and still 
test the CONFIG_ROCKER macro.

So adding such a switch to rocker.json itself just cannot work.

But at the "make" level, we distinguish between config-all-devices.mak and 
*-softmmu-config-devices.mak , so if you want to disable the rocker QMP code 
depending on whether CONFIG_ROCKER is set in *any* target binary or not, you 
would need a similar mechanism to config-all-devices.mak here, e.g. 
something that would switch the inclusion of rocker.json on or off depending 
on the switch in config-all-devices.mak ...
Would it be possible to add such a switch to qapi/meson.build ?
... and to qapi/qapi-schema.json, I guess, since the rocker.json gets 
included from there? Something like that:

diff --git a/qapi/meson.build b/qapi/meson.build
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -59,9 +59,11 @@ if have_system
     'qdev',
     'pci',
     'rdma',
-    'rocker',
     'tpm',
   ]
+  if config_all_devices.has_key('CONFIG_ROCKER')
+    qapi_all_modules += [ 'rocker', ]
+  endif
 endif
 if have_system or have_tools
   qapi_all_modules += [
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -72,7 +72,7 @@
 { 'include': 'job.json' }
 { 'include': 'net.json' }
 { 'include': 'rdma.json' }
-{ 'include': 'rocker.json' }
+{ 'include': 'rocker.json', 'if': 'CONFIG_ROCKER' }
 { 'include': 'tpm.json' }
 { 'include': 'ui.json' }
 { 'include': 'authz.json' }

No clue whether the qapi parser could be extended like that, though...

 Thomas