linux-kernel - Re: [PATCH] arm64: KVM: Skip PSTATE.PAN reest at EL2 in non-VHE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 12 Sep 2017 15:19:05 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Dongjiu Geng <gengdongjiu@...wei.com>
Cc:     <christoffer.dall@...aro.org>, <vladimir.murzin@....com>,
        <james.morse@....com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: KVM: Skip PSTATE.PAN reest at EL2 in non-VHE

On Mon, Sep 11 2017 at  7:16:52 pm BST, Dongjiu Geng <gengdongjiu@...wei.com> wrote:
> PSTATE.PAN disables reading and/or writing to a userspace virtual
> address from EL1 in non-VHE or from EL2 in VHE. In non-VHE, there is
> no any userspace mapping at EL2, so no need to reest the PSTATE.PAN.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@...wei.com>
> Signed-off-by: Haibin Zhang <zhanghaibin7@...wei.com>
> ---
>  arch/arm64/kvm/hyp/entry.S | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/entry.S b/arch/arm64/kvm/hyp/entry.S
> index 12ee62d6d410..86a7549b1b4c 100644
> --- a/arch/arm64/kvm/hyp/entry.S
> +++ b/arch/arm64/kvm/hyp/entry.S
> @@ -96,8 +96,12 @@ ENTRY(__guest_exit)
>  
>  	add	x1, x1, #VCPU_CONTEXT
>  
> -	ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> +	b	2f			// skip PAN at EL2 in non-VHE
> +alternative_else_nop_endif
>  
> +	ALTERNATIVE(nop, SET_PSTATE_PAN(1), ARM64_HAS_PAN, CONFIG_ARM64_PAN)
> +2:
>  	// Store the guest regs x2 and x3
>  	stp	x2, x3,   [x1, #CPU_XREG_OFFSET(2)]

Aside from Vladimir's comment about why this may not be an important
change in practice (both features are v8.1, and expected to be
implemented at the same time as VHE), I'm not sure this brings us
much.

We're just trading a write to PSTATE (which will have no effect other
than storing a bit in PSTATE) for a branch, and I'm not sure what is the
worse. Your patch definitely makes the code less readable, and I value
ease of maintenance very much.

Do you have any data coming from a non-VHE, PAN-enabled system that
shows a measurable, significant performance improvement with this patch?
Because that would be the only reason why I'd consider such a change.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.