Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication

From: Dave Martin <Dave.Martin@arm.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: Christoffer Dall <cdall@kernel.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Kristina Martsenko <kristina.martsenko@arm.com>,
	Zhang Lei <zhang.lei@jp.fujitsu.com>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH kvmtool v3 5/9] KVM: arm/arm64: Add a vcpu feature for pointer authentication
Date: Mon, 3 Jun 2019 12:23:03 +0100	[thread overview]
Message-ID: <20190603112302.GN28398@e103592.cambridge.arm.com> (raw)
In-Reply-To: <20190531180416.3e87f5ad@donnerap.cambridge.arm.com>

On Fri, May 31, 2019 at 06:04:16PM +0100, Andre Przywara wrote:
> On Thu, 30 May 2019 16:13:10 +0100
> Dave Martin <Dave.Martin@arm.com> wrote:
> 
> > From: Amit Daniel Kachhap <amit.kachhap@arm.com>
> > 
> > This patch adds a runtime capabality for KVM tool to enable Arm64 8.3
> > Pointer Authentication in guest kernel. Two vcpu features
> > KVM_ARM_VCPU_PTRAUTH_[ADDRESS/GENERIC] are supplied together to enable
> > Pointer Authentication in KVM guest after checking the capability.
> > 
> > Command line options --enable-ptrauth and --disable-ptrauth are added
> > to use this feature. However, if those options are not provided then
> > also this feature is enabled if host supports this capability.
> 
> I don't really get the purpose of two options, I think that's quite
> confusing. Should the first one either be dropped at all or called
> something with "force"?
> 
> I guess the idea is to fail if pointer auth isn't available, but the
> option is supplied?
> 
> Or maybe have one option with parameters?
> --ptrauth[,=enable,=disable]

So, I was following two principles here, either or both of which may be
bogus:

1) There should be a way to determine whether KVM turns a given feature
on or off (instead of magically defaulting to something).

2) To a first approaximation, kvmtool should allow each major KVM ABI
feature to be exercised.

3) By default, kvmtool should offer the maximum feature set possible to
the guest.

(3) is well established, but (1) and (2) may be open to question?

If we hold to both principles, it makes sense to have options
functionally equivalent to what I suggested (where KVM provides the
control in the first place), but there may be more convenient ways
to respell the options.

If we really can't decide, maybe it's better to drop the options
altogether until we have a real use case.

I've found the options very useful for testing and debugging on the SVE
side, but I can't comment on ptrauth.  Maybe someone else has a view?

> > The macros defined in the headers are not in sync and should be replaced
> > from the upstream.
> 
> This is no longer true, I guess?

Ah yes, that comment can go.

Cheers
---Dave
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm