On Wed, Feb 21, 2018 at 05:52:41PM +0000, Marc Zyngier wrote:

On 21/02/18 17:39, Andrew Jones wrote:

quoted

On Thu, Feb 15, 2018 at 10:03:02PM +0100, Christoffer Dall wrote:

quoted

The debug save/restore functions can be improved by using the has_vhe()
static key instead of the instruction alternative.  Using the static key
uses the same paradigm as we're going to use elsewhere, it makes the
code more readable, and it generates slightly better code (no
stack setups and function calls unless necessary).

We also use a static key on the restore path, because it will be
marginally faster than loading a value from memory.

Finally, we don't have to conditionally clear the debug dirty flag if
it's set, we can just clear it.

Reviewed-by: Marc Zyngier <redacted>
Signed-off-by: Christoffer Dall <redacted>
---

Notes:
    Changes since v1:
     - Change dot to comma in comment
     - Rename __debug_restore_spe to __debug_restore_spe_nvhe

 arch/arm64/kvm/hyp/debug-sr.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Maybe after this series is merged, if there are any hyp_alternate_select's
left, we can replace all the remaining ones with has_vhe() and then just
completely remove hyp_alternate_select.

Note that older compilers (such as GCC 4.8) will generate horrible code
with static keys, as they do not support "asm goto". Not that I want to
preserve the home brew hyp_alternate_select mechanism, but I just want
to make it plain that some distros will definitely suffer from the
transition.

Yeah, I've seen that. I even wrote some patches to try and deal with
it once, because RHEL currently has gcc 4.8, and static keys are now
used in kernel hot paths for kpti, and I knew this series was coming.
My patches didn't seem like something usptream would care about, so
I never posted them. Indeed, I see here[*] that at least x86 is saying
that at some point (soon?) asm-goto will be a hard requirement.

I just checked Christoffer's branch. The only hyp_alternate_select,
that couldn't be changed to a has_vhe is __check_arm_834220, but
that one can just be changed to
cpus_have_const_cap(ARM64_WORKAROUND_834220), since it's just acting
as a boolean anyway.

Thanks,
drew

[*] https://lkml.org/lkml/2018/2/20/51