Re: [Qemu-devel] [Qemu-arm] [PATCH v2 18/23] target/arm: Move CPU state dumping routines to helper.c

From: Peter Maydell <peter.maydell@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: qemu-arm <qemu-arm@nongnu.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"QEMU Developers" <qemu-devel@nongnu.org>,
	"Robert Bradford" <robert.bradford@intel.com>,
	"Samuel Ortiz" <sameo@linux.intel.com>
Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v2 18/23] target/arm: Move CPU state dumping routines to helper.c
Date: Mon, 17 Jun 2019 15:52:05 +0100	[thread overview]
Message-ID: <CAFEAcA8t7EYC6PK24oHcsGsD7PazuqCzGcLuwiMmh2C4dKJF2A@mail.gmail.com> (raw)
In-Reply-To: <87h88o9svq.fsf@zen.linaroharston>

On Mon, 17 Jun 2019 at 15:41, Alex Bennée <alex.bennee@linaro.org> wrote:
> Hmm so helper is a mix of non-TCG and TCG bits whereas helper-a64.c is
> basically just TCG helpers. It makes me wonder if we are breaking a
> convention here as helper.c is traditionally only TCG helpers.

The original convention was:
 * op_helper.c has functions which need access to things which
   TCG puts in specific registers and which are accessed from
   C via gcc "register variables" (most notably the CPU env pointer)
 * helper.c has functions which don't need access to the register
   variables (a mishmash of stuff, usually including interrupt/exception
   handling)
 * some targets further split out some parts of the C code into other
   foo-helper.c files
This distinction became entirely moot when we reworked the TCG code
to no longer use register variables (but instead pass in the env
pointer and so on directly as a function argument). You can still
see the comments at the top of target/i386/{op_helper,helper}.c that
claim this is what the files are for, though :-)

So these days there is really no fixed convention, except that
when we've added new things we've sometimes put them in their
own .c file. helper-a64.c in particular is on its own because it's
code that's only compiled into aarch64-softmmu, not arm-softmmu.

> It feels like there should be different file that is unambiguously used
> for both TCG and KVM based workloads where things like the cpu dump code
> can live.

Some sort of split like this seems like a good idea. I don't
know if any of the other target archs have already got a
good convention/naming scheme we could copy?

thanks
-- PMM