[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 0/4] Move target- and device specific code fr
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v4 0/4] Move target- and device specific code from monitor |
Date: |
Thu, 10 Sep 2015 09:18:38 -0700 |
On Thu, Sep 10, 2015 at 8:38 AM, Denis V. Lunev <address@hidden> wrote:
> The monivation of this set is simple. Recently we have proposed patch
> to monitor.c with specific x86 APIC HMP commands. The patchset was denied
> with the main motivation "No more arch specific code in monitor.c"
> This patchset is the first step to move arch specific code from
> monitor.c targets.
>
> So, monitor.c already contains a lot of generic code, as well as the target
> specifics code and eventually monitor.c volume will only grow. This trend
> leads
> to a variety of fouling code ifdeffery(and combinations thereof),
> poor readability, and entanglement of architecture of the project.
> If someone wants to improve processing logic commands at the monitor,
> it isn't necessarily must differentiate amongst the implementation of some ARM
> or x86_64 specific commands, because the project already has separation of
> target specific code on directories.
>
> The presented solution is not the best, but it is quite simple
> (PATCH doesn't add more code!) and decides the above mentioned issue.
> Subsequently it will not prevent the introduction of more advanced mechanism
> that can more effectively resolve the issue.
>
> There is a issue with the placement of code for multiple architectures
> (isn't for everyone), but this code is very small. This patch is a step
> towards
> solving the issue associated with maintaining the purity of the code and
> structure of the project, which solves not all, but doing a little better
> than it is.
>
> Changes from v3:
> - removed weak symbol, because it does not work on all platforms
> - qemu-monitor-info.texi moved to patch 4
>
> Changes from v2:
> - target_monitor_defs provides a target-specific array monitor_defs[]
> This call is added through stub to all archs as a weak symbol. It should
> be redefined on each specific architecture
> - fix end of table info (information about the system state)
> - ported to current HEAD (iothreads & skeys commands descriptions moved)
>
> Changes from v1:
> - ported to new head
>
> Signed-off-by: Pavel Butsykin <address@hidden>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Paolo Bonzini <address@hidden>
> CC: Peter Maydell <address@hidden>
>
I need this!
This is a piece of the puzzle for multi arch. I'll add it to my review
queue, but can I take a CC on this in future incase it spins again
before I get a chance?
Regards,
Peter
> Pavel Butsykin (4):
> hmp-commands-info: move info_cmds content out of monitor.c
> monitor: remove target-specific code from monitor.c
> hmp-commands.hx: fix end of table info
> monitor: added generation of documentation for hmp-commands-info.hx
>
> .gitignore | 1 +
> Makefile | 10 +-
> Makefile.target | 5 +-
> hmp-commands-info.hx | 749 +++++++++++++++++++++++
> hmp-commands.hx | 120 ----
> include/monitor/monitor-common.h | 45 ++
> include/qemu/typedefs.h | 1 +
> monitor.c | 1249
> +-------------------------------------
> qemu-doc.texi | 2 +
> stubs/Makefile.objs | 1 +
> stubs/target-monitor-defs.c | 9 +
> target-i386/Makefile.objs | 2 +-
> target-i386/monitor.c | 494 +++++++++++++++
> target-ppc/Makefile.objs | 2 +-
> target-ppc/monitor.c | 255 ++++++++
> target-sh4/Makefile.objs | 1 +
> target-sh4/monitor.c | 52 ++
> target-sparc/Makefile.objs | 2 +-
> target-sparc/monitor.c | 158 +++++
> target-xtensa/Makefile.objs | 1 +
> target-xtensa/monitor.c | 34 ++
> 21 files changed, 1827 insertions(+), 1366 deletions(-)
> create mode 100644 hmp-commands-info.hx
> create mode 100644 include/monitor/monitor-common.h
> create mode 100644 stubs/target-monitor-defs.c
> create mode 100644 target-i386/monitor.c
> create mode 100644 target-ppc/monitor.c
> create mode 100644 target-sh4/monitor.c
> create mode 100644 target-sparc/monitor.c
> create mode 100644 target-xtensa/monitor.c
>
> --
> 2.1.4
>
>