[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180409143700.w4uynkhdon36purz@lakrids.cambridge.arm.com>
Date: Mon, 9 Apr 2018 15:37:00 +0100
From: Mark Rutland <mark.rutland@....com>
To: Christoffer Dall <cdall@...nel.org>
Cc: Christoffer Dall <christoffer.dall@...aro.org>,
linux-arch@...r.kernel.org, cdall@...aro.org, arnd@...db.de,
marc.zyngier@....com, catalin.marinas@....com, yao.qi@....com,
kernel-hardening@...ts.openwall.com, will.deacon@....com,
linux-kernel@...r.kernel.org, awallis@...eaurora.org,
kvmarm@...ts.cs.columbia.edu, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCHv2 10/12] arm64/kvm: context-switch ptrauth registers
On Mon, Apr 09, 2018 at 02:58:18PM +0200, Christoffer Dall wrote:
> 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 see.
> > > 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.
Cool. I've switched to vcpu_load() locally, and will use that in v3.
Thanks,
Mark.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
