From: David Brazdil <dbrazdil@google.com>
To: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
linux-kernel@vger.kernel.org, James Morse <james.morse@arm.com>,
linux-arm-kernel@lists.infradead.org,
Marc Zyngier <maz@kernel.org>, Tejun Heo <tj@kernel.org>,
Dennis Zhou <dennis@kernel.org>, Christoph Lameter <cl@linux.com>,
kernel-team@android.com, kvmarm@lists.cs.columbia.edu,
Julien Thierry <julien.thierry.kdev@gmail.com>,
Andrew Scull <ascull@google.com>
Subject: Re: [PATCH v3 04/11] kvm: arm64: Remove __hyp_this_cpu_read
Date: Mon, 21 Sep 2020 14:43:55 +0100 [thread overview]
Message-ID: <20200921134355.5lzma3qzyiexxepd@google.com> (raw)
In-Reply-To: <20200918090029.GC30834@willie-the-truck>
Hi Will,
> > +static inline unsigned long __hyp_my_cpu_offset(void)
> > +{
> > + unsigned long off;
> > +
> > + /*
> > + * We want to allow caching the value, so avoid using volatile and
> > + * instead use a fake stack read to hazard against barrier().
> > + */
>
> I don't think we need to copy/paste the comment...
>
> > + asm("mrs %0, tpidr_el2" : "=r" (off) :
> > + "Q" (*(const unsigned long *)current_stack_pointer));
>
> ... especially given that we're not preemptible at EL2 with nVHE, maybe
> we don't need to play this trick at all because we're always going to be
> on the same CPU. So we could actually just do:
>
> return read_sysreg(tpidr_el2);
>
> which is much better, and the comment should say something to that effect.
I must be misinterpreting the comment. I understood that it enables the compiler
optimizing multiple reads of TPIDR by avoiding 'asm volatile' (signaling that
the value does not change between reads). So what exactly does it do?
read_sysreg expands to 'asm volatile' but I have no problem with priotizing
readability over a micro-optimization.
> > +#if defined(__KVM_NVHE_HYPERVISOR__) || defined(__KVM_VHE_HYPERVISOR__)
> > +#define __my_cpu_offset __hyp_my_cpu_offset()
>
> Why would VHE code need to use this? Especially in light of my preemption
> comments above, shouldn't it now be using __kern_my_cpu_offset()?
During v2 review Andrew Scull pointed out we can avoid alternatives on VHE code
by using __hyp_my_cpu_offset for it as well. Obviously if __hyp_my_cpu_offset
becomes nVHE-specific, we can always move VHE back to __kern. This was just
about saving a few cycles during boot.
David
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-09-21 13:46 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 17:34 [PATCH v3 00/10] Independent per-CPU data section for nVHE David Brazdil
2020-09-16 17:34 ` [PATCH v3 01/11] kvm: arm64: Partially link nVHE hyp code, simplify HYPCOPY David Brazdil
2020-09-18 8:51 ` Will Deacon
2020-09-22 17:55 ` David Brazdil
2020-09-16 17:34 ` [PATCH v3 02/11] kvm: arm64: Move nVHE hyp namespace macros to hyp_image.h David Brazdil
2020-09-18 8:52 ` Will Deacon
2020-09-16 17:34 ` [PATCH v3 03/11] kvm: arm64: Only define __kvm_ex_table for CONFIG_KVM David Brazdil
2020-09-18 9:00 ` Will Deacon
2020-09-16 17:34 ` [PATCH v3 04/11] kvm: arm64: Remove __hyp_this_cpu_read David Brazdil
2020-09-18 9:00 ` Will Deacon
2020-09-21 13:43 ` David Brazdil [this message]
2020-09-16 17:34 ` [PATCH v3 05/11] kvm: arm64: Remove hyp_adr/ldr_this_cpu David Brazdil
2020-09-18 9:05 ` Will Deacon
2020-09-21 14:53 ` David Brazdil
2020-09-21 17:15 ` Will Deacon
2020-09-16 17:34 ` [PATCH v3 06/11] kvm: arm64: Add helpers for accessing nVHE hyp per-cpu vars David Brazdil
2020-09-18 9:24 ` Will Deacon
2020-09-16 17:34 ` [PATCH v3 07/11] kvm: arm64: Duplicate arm64_ssbd_callback_required for nVHE hyp David Brazdil
2020-09-18 11:59 ` Will Deacon
2020-09-22 18:07 ` David Brazdil
2020-09-16 17:34 ` [PATCH v3 08/11] kvm: arm64: Create separate instances of kvm_host_data for VHE/nVHE David Brazdil
2020-09-18 11:58 ` Will Deacon
2020-09-16 17:34 ` [PATCH v3 09/11] kvm: arm64: Mark hyp stack pages reserved David Brazdil
2020-09-18 12:00 ` Will Deacon
2020-09-22 18:08 ` David Brazdil
2020-09-16 17:34 ` [PATCH v3 10/11] kvm: arm64: Set up hyp percpu data for nVHE David Brazdil
2020-09-18 12:22 ` Will Deacon
2020-09-22 18:34 ` David Brazdil
2020-09-16 17:34 ` [PATCH v3 11/11] kvm: arm64: Remove unnecessary hyp mappings David Brazdil
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=20200921134355.5lzma3qzyiexxepd@google.com \
--to=dbrazdil@google.com \
--cc=ascull@google.com \
--cc=catalin.marinas@arm.com \
--cc=cl@linux.com \
--cc=dennis@kernel.org \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=tj@kernel.org \
--cc=will@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox