| Message ID | 20221220111122.8966-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | target: Restrict 'qapi-commands-machine.h' to system emulation | expand |
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.
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.
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?
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.
[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