All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	qemu-arm <qemu-arm@nongnu.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Patch Tracking <patches@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 5/7] target-arm: Add isread parameter to CPAccessFns
Date: Fri, 05 Feb 2016 16:17:57 +0000	[thread overview]
Message-ID: <87egcrnnbe.fsf@linaro.org> (raw)
In-Reply-To: <CAFEAcA_RJs_DaBa2YqP3z+-ykMntPULEWAYO8LnQz8Qzs+dyXA@mail.gmail.com>


Peter Maydell <peter.maydell@linaro.org> writes:

> On 5 February 2016 at 14:20, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> -typedef CPAccessResult CPAccessFn(CPUARMState *env, const ARMCPRegInfo *opaque);
>>> +typedef CPAccessResult CPAccessFn(CPUARMState *env,
>>> +                                  const ARMCPRegInfo *opaque,
>>> +                                  bool isread);
>>
>> I guess my only comment here is we've extended the call for every access
>> check with another parameter (and associated TCG activity) for something
>> only one handler currently cares about.
>>
>> Is there an argument for an rwaccessfn() that we use for just those
>> registers that care about the detail? I know system registers are hardly
>> a fast path priority but I'm concerned about knock on effects on
>> performance. Have you done any measurements?
>
> I haven't measured, no, but since there are only 3 arguments the
> third argument is going to be in a register on any host architecture
> we care about, which means the overhead is just going to be a single
> "load constant 0 or 1 into register before the call". I think that's
> going to be lost in the noise compared to actually having to make
> the function call at all, the work the function call does, and then
> the second function call later to do the read or write.

I was thinking of knock on effects on spilling other registers in the
TCG code. I guess this depends on how complex the code is around system
register access.

>
> thanks
> -- PMM


--
Alex Bennée

  reply	other threads:[~2016-02-05 16:18 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-03 13:38 [Qemu-devel] [PATCH 0/7] Fix some more EL3 things and enable EL3 for AArch64 Peter Maydell
2016-02-03 13:38 ` [Qemu-devel] [PATCH 1/7] target-arm: Fix typo in comment in arm_is_secure_below_el3() Peter Maydell
2016-02-05  9:52   ` [Qemu-devel] [Qemu-arm] " Alex Bennée
2016-02-06 11:49   ` [Qemu-devel] " Edgar E. Iglesias
2016-02-06 18:24   ` [Qemu-devel] [Qemu-arm] " Sergey Fedorov
2016-02-03 13:38 ` [Qemu-devel] [PATCH 2/7] target-arm: Implement MDCR_EL3 and SDCR Peter Maydell
2016-02-05 11:13   ` Alex Bennée
2016-02-05 11:28     ` Peter Maydell
2016-02-06 12:04   ` Edgar E. Iglesias
2016-02-06 18:42   ` Sergey Fedorov
2016-02-08 13:11     ` Peter Maydell
2016-02-03 13:38 ` [Qemu-devel] [PATCH 3/7] target-arm: Use access_trap_aa32s_el1() for SCR and MVBAR Peter Maydell
2016-02-05 13:43   ` [Qemu-devel] [Qemu-arm] " Alex Bennée
2016-02-06 12:17   ` [Qemu-devel] " Edgar E. Iglesias
2016-02-06 13:48     ` Peter Maydell
2016-02-06 16:03       ` Edgar E. Iglesias
2016-02-06 16:10   ` Edgar E. Iglesias
2016-02-03 13:38 ` [Qemu-devel] [PATCH 4/7] target-arm: Update arm_generate_debug_exceptions() to handle EL2/EL3 Peter Maydell
2016-02-05 14:09   ` Alex Bennée
2016-02-05 15:55     ` Peter Maydell
2016-02-06 18:43   ` Sergey Fedorov
2016-02-03 13:38 ` [Qemu-devel] [PATCH 5/7] target-arm: Add isread parameter to CPAccessFns Peter Maydell
2016-02-05 14:20   ` Alex Bennée
2016-02-05 14:29     ` Peter Maydell
2016-02-05 16:17       ` Alex Bennée [this message]
2016-02-05 16:27         ` Peter Maydell
2016-02-05 16:43           ` Alex Bennée
2016-02-06 16:16   ` Edgar E. Iglesias
2016-02-06 18:52   ` Sergey Fedorov
2016-02-03 13:38 ` [Qemu-devel] [PATCH 6/7] target-arm: Implement NSACR trapping behaviour Peter Maydell
2016-02-05 16:07   ` [Qemu-devel] [Qemu-arm] " Alex Bennée
2016-02-05 16:22     ` Peter Maydell
2016-02-06 16:42   ` [Qemu-devel] " Edgar E. Iglesias
2016-02-03 13:38 ` [Qemu-devel] [PATCH 7/7] target-arm: Enable EL3 for Cortex-A53 and Cortex-A57 Peter Maydell
2016-02-05 16:08   ` Alex Bennée
2016-02-06 16:43   ` Edgar E. Iglesias
2016-02-06 18:55   ` Sergey Fedorov
2016-02-08 13:18 ` [Qemu-devel] [Qemu-arm] [PATCH 0/7] Fix some more EL3 things and enable EL3 for AArch64 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87egcrnnbe.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=edgar.iglesias@gmail.com \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.