kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: David Brazdil <dbrazdil@google.com>
Cc: kernel-team@android.com, android-kvm@google.com,
	Catalin Marinas <catalin.marinas@arm.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will@kernel.org>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v3 05/15] arm64: kvm: Build hyp-entry.S separately for VHE/nVHE
Date: Thu, 25 Jun 2020 09:12:50 +0100	[thread overview]
Message-ID: <491f3c8877897a4ac69d69fb7354c1cb@kernel.org> (raw)
In-Reply-To: <20200622102041.myve2otyoj5q7j5s@google.com>

Hi David,

On 2020-06-22 11:20, David Brazdil wrote:
> Hi Marc,
> 
>> > -	void *dst = lm_alias(__bp_harden_hyp_vecs + slot * SZ_2K);
>> > +	char *vec = has_vhe() ? __bp_harden_hyp_vecs
>> > +			      : kvm_nvhe_sym(__bp_harden_hyp_vecs);
>> 
>> If we get this construct often, then something that abstracts
>> the uggliness of the symbol duality would be nice...
> 
> Agreed, I do hope that this will end up being limited to finding the 
> address of
> the hyp-init vector once EL2 becomes self-contained. Even this vector 
> selection
> can be done in EL2 where the symbol duality does not exist.
> If we were to hide it, there is a trade off between code "elegance" and 
> clarity
> of what's happening under the hood. I was thinking we could extract 
> this
> `has_vhe() ? foo : __kvm_nvhe_foo` as a `#define foo` but I do worry 
> that
> anybody debugging this code would be cursing my name.

I would say that whoever is debugging this code better have an 
understanding
of how things are supposed to work. Given that this is only a handful of
people so far, I think your name is safe! ;-)

> It would also not work
> with other macros that take symbol names, notably kvm_ksym_ref. But 
> that can be
> rewritten to accept a pointer instead. The more verbose but less magic 
> approach
> is to have a bunch of different helpers for various situations, eg.
> __pa_symbol_nvhe. What would be your preference?

I'd be happy with the (maybe temporary) magic approach. It helps 
reasoning
about things, and makes the transition smoother. Yes, bugs could crop up
there, but given the static nature of obtaining a symbol's address, I'm
fairly confident we'll get it right. The same cannot be said about 
pointers
though.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

  reply	other threads:[~2020-06-25  8:12 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-18 12:25 [PATCH v3 00/15] Split off nVHE hyp code David Brazdil
2020-06-18 12:25 ` [PATCH v3 01/15] arm64: kvm: Fix symbol dependency in __hyp_call_panic_nvhe David Brazdil
2020-06-18 12:25 ` [PATCH v3 02/15] arm64: kvm: Move __smccc_workaround_1_smc to .rodata David Brazdil
2020-06-18 13:57   ` Marc Zyngier
2020-06-19  9:51     ` David Brazdil
2020-06-19 10:06       ` Marc Zyngier
2020-06-18 12:25 ` [PATCH v3 03/15] arm64: kvm: Add build rules for separate nVHE object files David Brazdil
2020-06-18 16:34   ` Marc Zyngier
2020-06-19 12:11     ` David Brazdil
2020-06-18 12:25 ` [PATCH v3 04/15] arm64: kvm: Handle calls to prefixed hyp functions David Brazdil
2020-06-18 16:51   ` Marc Zyngier
2020-06-18 12:25 ` [PATCH v3 05/15] arm64: kvm: Build hyp-entry.S separately for VHE/nVHE David Brazdil
2020-06-18 17:17   ` Marc Zyngier
2020-06-22 10:20     ` David Brazdil
2020-06-25  8:12       ` Marc Zyngier [this message]
2020-06-25 11:53         ` David Brazdil
2020-06-18 12:25 ` [PATCH v3 06/15] arm64: kvm: Move hyp-init.S to nVHE David Brazdil
2020-06-18 12:25 ` [PATCH v3 07/15] arm64: kvm: Split hyp/tlb.c to VHE/nVHE David Brazdil
2020-06-19  8:28   ` Marc Zyngier
2020-06-18 12:25 ` [PATCH v3 08/15] arm64: kvm: Split hyp/switch.c " David Brazdil
2020-06-18 19:52   ` kernel test robot
2020-06-25  5:03   ` kernel test robot
2020-06-25  8:16     ` Marc Zyngier
2020-06-25  8:34       ` Will Deacon
2020-06-25 18:26         ` Nick Desaulniers
2020-06-25 16:37       ` Luc Van Oostenryck
2020-06-25 17:55       ` Ard Biesheuvel
2020-06-18 12:25 ` [PATCH v3 09/15] arm64: kvm: Split hyp/debug-sr.c " David Brazdil
2020-06-18 12:25 ` [PATCH v3 10/15] arm64: kvm: Split hyp/sysreg-sr.c " David Brazdil
2020-06-18 12:25 ` [PATCH v3 11/15] arm64: kvm: Split hyp/timer-sr.c " David Brazdil
2020-06-18 12:25 ` [PATCH v3 12/15] arm64: kvm: Compile remaining hyp/ files for both VHE/nVHE David Brazdil
2020-06-18 12:25 ` [PATCH v3 13/15] arm64: kvm: Add comments around __kvm_nvhe_ symbol aliases David Brazdil
2020-06-18 12:25 ` [PATCH v3 14/15] arm64: kvm: Remove __hyp_text macro, use build rules instead David Brazdil
2020-06-18 12:25 ` [PATCH v3 15/15] arm64: kvm: Lift instrumentation restrictions on VHE 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=491f3c8877897a4ac69d69fb7354c1cb@kernel.org \
    --to=maz@kernel.org \
    --cc=android-kvm@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dbrazdil@google.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=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;
as well as URLs for NNTP newsgroup(s).