[PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI

Markus Armbruster posted 12 patches 3 years, 2 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
[PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Markus Armbruster 3 years, 2 months ago
We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
!CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci/pci-stub.c  | 5 +++++
 hw/pci/meson.build | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index f29ecc999e..01d20a2f67 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "qapi/qapi-commands-pci.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/msi.h"
@@ -34,6 +35,10 @@ PciInfoList *qmp_query_pci(Error **errp)
     return NULL;
 }
 
+void hmp_info_pci(Monitor *mon, const QDict *qdict)
+{
+}
+
 void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
 {
     monitor_printf(mon, "PCI devices not supported\n");
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index e42a133f3a..4fcd888b27 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -5,6 +5,7 @@ pci_ss.add(files(
   'pci.c',
   'pci_bridge.c',
   'pci_host.c',
+  'pci-hmp-cmds.c',
   'pci-qmp-cmds.c',
   'pcie_sriov.c',
   'shpc.c',
@@ -20,4 +21,3 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
 
 softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
 softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
-softmmu_ss.add(files('pci-hmp-cmds.c'))
-- 
2.37.3
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Dr. David Alan Gilbert 3 years, 2 months ago
* Markus Armbruster (armbru@redhat.com) wrote:
> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Had you considered wrapping the hmp-commands-info.hx entry
with a #if defined instead?

Dave

> ---
>  hw/pci/pci-stub.c  | 5 +++++
>  hw/pci/meson.build | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index f29ecc999e..01d20a2f67 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/sysemu.h"
>  #include "monitor/monitor.h"
> +#include "monitor/hmp.h"
>  #include "qapi/qapi-commands-pci.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci/msi.h"
> @@ -34,6 +35,10 @@ PciInfoList *qmp_query_pci(Error **errp)
>      return NULL;
>  }
>  
> +void hmp_info_pci(Monitor *mon, const QDict *qdict)
> +{
> +}
> +
>  void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
>  {
>      monitor_printf(mon, "PCI devices not supported\n");
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> index e42a133f3a..4fcd888b27 100644
> --- a/hw/pci/meson.build
> +++ b/hw/pci/meson.build
> @@ -5,6 +5,7 @@ pci_ss.add(files(
>    'pci.c',
>    'pci_bridge.c',
>    'pci_host.c',
> +  'pci-hmp-cmds.c',
>    'pci-qmp-cmds.c',
>    'pcie_sriov.c',
>    'shpc.c',
> @@ -20,4 +21,3 @@ softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>  
>  softmmu_ss.add(when: 'CONFIG_PCI', if_false: files('pci-stub.c'))
>  softmmu_ss.add(when: 'CONFIG_ALL', if_true: files('pci-stub.c'))
> -softmmu_ss.add(files('pci-hmp-cmds.c'))
> -- 
> 2.37.3
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Markus Armbruster 3 years, 2 months ago
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
>> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
>> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Had you considered wrapping the hmp-commands-info.hx entry
> with a #if defined instead?

No.  Would you prefer that?

Code containing #ifdef CONFIG_PCI is target-dependent.  Looks like the
affected monitor code already is, so no new headaches.

Aside: splitting off its target-independent parts could be nice.  Not
today.

[...]
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Dr. David Alan Gilbert 3 years, 2 months ago
* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> >> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
> >> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >
> > Had you considered wrapping the hmp-commands-info.hx entry
> > with a #if defined instead?
> 
> No.  Would you prefer that?

It seemed a bit simpler to me, but I'm not too fussed.
I kind of preferred the idea of the command giving an error if there's
no PCI built in.

> Code containing #ifdef CONFIG_PCI is target-dependent.  Looks like the
> affected monitor code already is, so no new headaches.

> Aside: splitting off its target-independent parts could be nice.  Not
> today.

Yeh.

Dave
> [...]
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Philippe Mathieu-Daudé 3 years, 2 months ago
On 28/11/22 09:01, Markus Armbruster wrote:
> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/pci/pci-stub.c  | 5 +++++
>   hw/pci/meson.build | 2 +-
>   2 files changed, 6 insertions(+), 1 deletion(-)

Squash with patch #3?
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Markus Armbruster 3 years, 2 months ago
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 28/11/22 09:01, Markus Armbruster wrote:
>> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
>> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
>> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   hw/pci/pci-stub.c  | 5 +++++
>>   hw/pci/meson.build | 2 +-
>>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> Squash with patch #3?

Could do, but the combined patch isn't pure code motion anymore, and I
get to explain that in the commit message.
Re: [PATCH 05/12] pci: Build hw/pci/pci-hmp-cmds.c only when CONFIG_PCI
Posted by Michael S. Tsirkin 3 years, 2 months ago
On Mon, Nov 28, 2022 at 11:21:36AM +0100, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
> > On 28/11/22 09:01, Markus Armbruster wrote:
> >> We compile pci-hmp-cmds.c always, but pci-qmp-cmds.c only when
> >> CONFIG_PCI.  hw/pci/pci-stub.c keeps the linker happy when
> >> !CONFIG_PCI.  Build pci-hmp-cmds.c that way, too.
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>   hw/pci/pci-stub.c  | 5 +++++
> >>   hw/pci/meson.build | 2 +-
> >>   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > Squash with patch #3?
> 
> Could do, but the combined patch isn't pure code motion anymore, and I
> get to explain that in the commit message.

Yes I like it the way it is.

-- 
MST