LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Christoffer Dall <cdall@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>,
	linux-arch@vger.kernel.org, cdall@linaro.org, arnd@arndb.de,
	marc.zyngier@arm.com, catalin.marinas@arm.com, yao.qi@arm.com,
	kernel-hardening@lists.openwall.com, will.deacon@arm.com,
	linux-kernel@vger.kernel.org, awallis@codeaurora.org,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2 10/12] arm64/kvm: context-switch ptrauth registers
Date: Mon, 9 Apr 2018 14:58:18 +0200
Message-ID: <20180409125818.GE10904@cbox> (raw)
In-Reply-To: <20180309142838.uvcv3mhvqqlprktt@lakrids.cambridge.arm.com>

Hi Mark,

[Sorry for late reply]

On Fri, Mar 09, 2018 at 02:28:38PM +0000, Mark Rutland wrote:
> On Tue, Feb 06, 2018 at 01:38:47PM +0100, Christoffer Dall wrote:
> > On Mon, Nov 27, 2017 at 04:38:04PM +0000, Mark Rutland wrote:
> > > When pointer authentication is supported, a guest may wish to use it.
> > > This patch adds the necessary KVM infrastructure for this to work, with
> > > a semi-lazy context switch of the pointer auth state.
> > > 
> > > When we schedule a vcpu, 
> > 
> > That's not quite what the code does, the code only does this when we
> > schedule back a preempted or blocked vcpu thread.
> 
> Does that only leave the case of the vCPU being scheduled for the first
> time? Or am I missing something else?
> 
> [...]

In the current patch, you're only calling kvm_arm_vcpu_ptrauth_disable()
from kvm_arch_sched_in() which is only called on the preempt notifier
patch, which leaves out every time we enter the guest from userspace and
therefore also the initial run of the vCPU (assuming there's no
preemption in the kernel prior to running the first time).

vcpu_load() takes care of all the cases.

> 

[...]

> > 
> > I still find this decision to begin trapping again quite arbitrary, and
> > would at least prefer this to be in vcpu_load (which would make the
> > behavior match the commit text as well).
> 
> Sure, done.
> 
> > My expectation would be that if a guest is running software with pointer
> > authentication enabled, then it's likely to either keep using the
> > feature, or not use it at all, so I would make this a one-time flag.
> 
> I think it's likely that some applications will use ptrauth while others
> do not. Even if the gust OS supports ptrauth, KVM may repeatedly preempt
> an application that doesn't use it, and we'd win in that case.
> 
> There are also some rarer cases, like kexec in a guest from a
> ptrauth-aware kernel to a ptrauth-oblivious one.
> 
> I don't have strong feelings either way, and I have no data.
> 

I think your intuition sounds sane, and let's reset the flag on every
vcpu_load, and we can always revisit when we have hardware and data if
someone reports a performance issue.

Thanks,
-Christoffer

  reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-27 16:37 [PATCHv2 00/12] ARMv8.3 pointer authentication userspace support Mark Rutland
2017-11-27 16:37 ` [PATCHv2 01/12] asm-generic: mm_hooks: allow hooks to be overridden individually Mark Rutland
2017-11-27 16:37 ` [PATCHv2 02/12] arm64: add pointer authentication register bits Mark Rutland
2017-11-27 16:37 ` [PATCHv2 03/12] arm64/cpufeature: add ARMv8.3 id_aa64isar1 bits Mark Rutland
2017-11-27 16:37 ` [PATCHv2 04/12] arm64/cpufeature: detect pointer authentication Mark Rutland
2017-11-27 16:37 ` [PATCHv2 05/12] arm64: Don't trap host pointer auth use to EL2 Mark Rutland
2018-02-06 12:39   ` Christoffer Dall
2018-02-12 16:00     ` Mark Rutland
2017-11-27 16:38 ` [PATCHv2 06/12] arm64: add basic pointer authentication support Mark Rutland
2018-05-22 19:06   ` Adam Wallis
2017-11-27 16:38 ` [PATCHv2 07/12] arm64: expose user PAC bit positions via ptrace Mark Rutland
2017-11-27 16:38 ` [PATCHv2 08/12] arm64: perf: strip PAC when unwinding userspace Mark Rutland
2017-11-27 16:38 ` [PATCHv2 09/12] arm64/kvm: preserve host HCR_EL2 value Mark Rutland
2018-02-06 12:39   ` Christoffer Dall
2018-04-09 14:57     ` Mark Rutland
2018-04-09 19:03       ` Christoffer Dall
2017-11-27 16:38 ` [PATCHv2 10/12] arm64/kvm: context-switch ptrauth registers Mark Rutland
2018-02-06 12:38   ` Christoffer Dall
2018-03-09 14:28     ` Mark Rutland
2018-04-09 12:58       ` Christoffer Dall [this message]
2018-04-09 14:37         ` Mark Rutland
2017-11-27 16:38 ` [PATCHv2 11/12] arm64: enable pointer authentication Mark Rutland
2017-11-27 16:38 ` [PATCHv2 12/12] arm64: docs: document " Mark Rutland
2017-11-28 15:07   ` Andrew Jones
2017-12-04 12:39     ` Mark Rutland
2017-12-04 12:49       ` Andrew Jones

Reply instructions:

You may reply publically 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=20180409125818.GE10904@cbox \
    --to=cdall@kernel.org \
    --cc=arnd@arndb.de \
    --cc=awallis@codeaurora.org \
    --cc=catalin.marinas@arm.com \
    --cc=cdall@linaro.org \
    --cc=christoffer.dall@linaro.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=will.deacon@arm.com \
    --cc=yao.qi@arm.com \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git