mbox series

[RESEND,v2,0/5] target: Restrict 'qapi-commands-machine.h' to system emulation

Message ID 20221220111122.8966-1-philmd@linaro.org
Headers show
Series target: Restrict 'qapi-commands-machine.h' to system emulation | expand

Message

Philippe Mathieu-Daudé Dec. 20, 2022, 11:11 a.m. UTC
[resend fixing my last name typography...]

All series reviewed, can patches be picked by corresponding
maintainers?

The "qapi-commands-machine.h" header is not generated in user-only
emulation. This series removes its use in user-emu code by moving
the QMP code depending on this header into a separate sysemu unit.

Since v1:
- renamed cpu-monitor.c -> monitor.c on loongarch

Philippe Mathieu-Daudé (5):
  target/arm: Restrict 'qapi-commands-machine.h' to system emulation
  target/i386: Restrict 'qapi-commands-machine.h' to system emulation
  target/loongarch: Restrict 'qapi-commands-machine.h' to system
    emulation
  target/mips: Restrict 'qapi-commands-machine.h' to system emulation
  target/ppc: Restrict 'qapi-commands-machine.h' to system emulation

 target/arm/helper.c            | 29 -------------
 target/arm/m_helper.c          |  1 -
 target/arm/monitor.c           | 28 +++++++++++++
 target/i386/cpu.c              | 74 ++++++++++++++++++----------------
 target/loongarch/cpu.c         | 27 -------------
 target/loongarch/meson.build   |  1 +
 target/loongarch/monitor.c     | 37 +++++++++++++++++
 target/mips/cpu.c              | 29 -------------
 target/mips/sysemu/meson.build |  1 +
 target/mips/sysemu/monitor.c   | 39 ++++++++++++++++++
 target/ppc/cpu-qom.h           |  2 +
 target/ppc/cpu_init.c          | 48 +---------------------
 target/ppc/monitor.c           | 50 ++++++++++++++++++++++-
 13 files changed, 197 insertions(+), 169 deletions(-)
 create mode 100644 target/loongarch/monitor.c
 create mode 100644 target/mips/sysemu/monitor.c

Comments

Markus Armbruster Jan. 13, 2023, 1:57 p.m. UTC | #1
Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> [resend fixing my last name typography...]
>
> All series reviewed, can patches be picked by corresponding
> maintainers?
>
> The "qapi-commands-machine.h" header is not generated in user-only
> emulation. This series removes its use in user-emu code by moving
> the QMP code depending on this header into a separate sysemu unit.
>
> Since v1:
> - renamed cpu-monitor.c -> monitor.c on loongarch

Quick drive-by remark: we usually name C files containing just QMP
commands SUBSYSTEM-qmp-cmds.c, and files containing just HMP commands
SUBSYSTEM-hmp-cmds.c.  On the other hand, the existing monitor-related
files seem to be named target/TARGET/monitor.c.

Keeping QMP and HMP two separate is desirable, but not required.
monitor.c is a fine name for a file containing both.

Use your judgement.
Philippe Mathieu-Daudé Jan. 13, 2023, 2:42 p.m. UTC | #2
On 13/1/23 14:57, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> [resend fixing my last name typography...]
>>
>> All series reviewed, can patches be picked by corresponding
>> maintainers?
>>
>> The "qapi-commands-machine.h" header is not generated in user-only
>> emulation. This series removes its use in user-emu code by moving
>> the QMP code depending on this header into a separate sysemu unit.
>>
>> Since v1:
>> - renamed cpu-monitor.c -> monitor.c on loongarch
> 
> Quick drive-by remark: we usually name C files containing just QMP
> commands SUBSYSTEM-qmp-cmds.c, and files containing just HMP commands
> SUBSYSTEM-hmp-cmds.c.  On the other hand, the existing monitor-related
> files seem to be named target/TARGET/monitor.c.
> 
> Keeping QMP and HMP two separate is desirable, but not required.
> monitor.c is a fine name for a file containing both.

$ git ls-files | fgrep qmp-cmds.c
block/monitor/bitmap-qmp-cmds.c
hw/core/machine-qmp-cmds.c
hw/pci/pci-qmp-cmds.c
monitor/qmp-cmds.c
qom/qom-qmp-cmds.c
tests/unit/test-qmp-cmds.c

$ git ls-files | fgrep monitor.c
monitor/monitor.c
softmmu/qdev-monitor.c
stubs/monitor.c
target/arm/monitor.c
target/i386/monitor.c
target/m68k/monitor.c
target/mips/sysemu/monitor.c
target/nios2/monitor.c
target/ppc/monitor.c
target/riscv/monitor.c
target/sh4/monitor.c
target/sparc/monitor.c
target/xtensa/monitor.c
tests/unit/test-util-filemonitor.c

Do you rather 'cpu-qmp-cmds.c'?

Or is your SUBSYSTEM the $target here?
Because , target/arm/arm-qmp-cmds.c sounds redundant.
Philippe Mathieu-Daudé Jan. 13, 2023, 2:53 p.m. UTC | #3
On 13/1/23 15:42, Philippe Mathieu-Daudé wrote:
> On 13/1/23 14:57, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> [resend fixing my last name typography...]
>>>
>>> All series reviewed, can patches be picked by corresponding
>>> maintainers?
>>>
>>> The "qapi-commands-machine.h" header is not generated in user-only
>>> emulation. This series removes its use in user-emu code by moving
>>> the QMP code depending on this header into a separate sysemu unit.
>>>
>>> Since v1:
>>> - renamed cpu-monitor.c -> monitor.c on loongarch
>>
>> Quick drive-by remark: we usually name C files containing just QMP
>> commands SUBSYSTEM-qmp-cmds.c, and files containing just HMP commands
>> SUBSYSTEM-hmp-cmds.c.  On the other hand, the existing monitor-related
>> files seem to be named target/TARGET/monitor.c.
>>
>> Keeping QMP and HMP two separate is desirable, but not required.
>> monitor.c is a fine name for a file containing both.
> 
> $ git ls-files | fgrep qmp-cmds.c
> block/monitor/bitmap-qmp-cmds.c
> hw/core/machine-qmp-cmds.c
> hw/pci/pci-qmp-cmds.c
> monitor/qmp-cmds.c
> qom/qom-qmp-cmds.c
> tests/unit/test-qmp-cmds.c
> 
> $ git ls-files | fgrep monitor.c
> monitor/monitor.c
> softmmu/qdev-monitor.c
> stubs/monitor.c
> target/arm/monitor.c
> target/i386/monitor.c
> target/m68k/monitor.c
> target/mips/sysemu/monitor.c
> target/nios2/monitor.c
> target/ppc/monitor.c
> target/riscv/monitor.c
> target/sh4/monitor.c
> target/sparc/monitor.c
> target/xtensa/monitor.c
> tests/unit/test-util-filemonitor.c
> 
> Do you rather 'cpu-qmp-cmds.c'?
> 
> Or is your SUBSYSTEM the $target here?
> Because , target/arm/arm-qmp-cmds.c sounds redundant.

IIUC the SUBSYSTEM is "target" so maybe you meant target-qmp-cmds.c?
Philippe Mathieu-Daudé Jan. 13, 2023, 5:21 p.m. UTC | #4
On 13/1/23 15:53, Philippe Mathieu-Daudé wrote:
> On 13/1/23 15:42, Philippe Mathieu-Daudé wrote:
>> On 13/1/23 14:57, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> [resend fixing my last name typography...]
>>>>
>>>> All series reviewed, can patches be picked by corresponding
>>>> maintainers?
>>>>
>>>> The "qapi-commands-machine.h" header is not generated in user-only
>>>> emulation. This series removes its use in user-emu code by moving
>>>> the QMP code depending on this header into a separate sysemu unit.
>>>>
>>>> Since v1:
>>>> - renamed cpu-monitor.c -> monitor.c on loongarch
>>>
>>> Quick drive-by remark: we usually name C files containing just QMP
>>> commands SUBSYSTEM-qmp-cmds.c, and files containing just HMP commands
>>> SUBSYSTEM-hmp-cmds.c.  On the other hand, the existing monitor-related
>>> files seem to be named target/TARGET/monitor.c.
>>>
>>> Keeping QMP and HMP two separate is desirable, but not required.
>>> monitor.c is a fine name for a file containing both.
>>
>> $ git ls-files | fgrep qmp-cmds.c
>> block/monitor/bitmap-qmp-cmds.c
>> hw/core/machine-qmp-cmds.c
>> hw/pci/pci-qmp-cmds.c
>> monitor/qmp-cmds.c
>> qom/qom-qmp-cmds.c
>> tests/unit/test-qmp-cmds.c
>>
>> $ git ls-files | fgrep monitor.c
>> monitor/monitor.c
>> softmmu/qdev-monitor.c
>> stubs/monitor.c
>> target/arm/monitor.c
>> target/i386/monitor.c
>> target/m68k/monitor.c
>> target/mips/sysemu/monitor.c
>> target/nios2/monitor.c
>> target/ppc/monitor.c
>> target/riscv/monitor.c
>> target/sh4/monitor.c
>> target/sparc/monitor.c
>> target/xtensa/monitor.c
>> tests/unit/test-util-filemonitor.c
>>
>> Do you rather 'cpu-qmp-cmds.c'?
>>
>> Or is your SUBSYSTEM the $target here?
>> Because , target/arm/arm-qmp-cmds.c sounds redundant.
> 
> IIUC the SUBSYSTEM is "target" so maybe you meant target-qmp-cmds.c?

FTR Markus suggested ${target}-qmp-cmds.c on IRC, I'll respin renamed.