[PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Jing Zhang]

Add a selftest to verify the support for pointer authentication in KVM
guest.

Signed-off-by: Jing Zhang <jingzhangos@google.com>
---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/aarch64/pauth_test.c        | 143 ++++++++++++++++++
 .../selftests/kvm/include/aarch64/processor.h |   2 +
 3 files changed, 146 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/aarch64/pauth_test.c

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index c692cc86e7da..9bac5aecd66d 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -143,6 +143,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
+TEST_GEN_PROGS_aarch64 += aarch64/pauth_test
 TEST_GEN_PROGS_aarch64 += aarch64/psci_test
 TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
 TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
diff --git a/tools/testing/selftests/kvm/aarch64/pauth_test.c b/tools/testing/selftests/kvm/aarch64/pauth_test.c
new file mode 100644
index 000000000000..d5f982da8891
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/pauth_test.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * pauth_test - Test for KVM guest pointer authentication.
+ *
+ * Copyright (c) 2023 Google LLC.
+ *
+ */
+
+#define _GNU_SOURCE
+
+#include <sched.h>
+
+#include "kvm_util.h"
+#include "processor.h"
+#include "test_util.h"
+
+enum uc_args {
+	WAIT_MIGRATION,
+	PASS,
+	FAIL,
+	FAIL_KVM,
+	FAIL_INSTR,
+};
+
+static noinline void pac_corruptor(void)
+{
+	__asm__ __volatile__(
+		"paciasp\n"
+		"eor lr, lr, #1 << 53\n"
+	);
+
+	/* Migrate guest to another physical CPU before authentication */
+	GUEST_SYNC(WAIT_MIGRATION);
+	__asm__ __volatile__("autiasp\n");
+}
+
+static void guest_code(void)
+{
+	uint64_t sctlr = read_sysreg(sctlr_el1);
+
+	/* Enable PAuth */
+	sctlr |= SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB;
+	write_sysreg(sctlr, sctlr_el1);
+	isb();
+
+	pac_corruptor();
+
+	/* Shouldn't be here unless the pac_corruptor didn't do its work */
+	GUEST_SYNC(FAIL);
+	GUEST_DONE();
+}
+
+/* Guest will get an unknown exception if KVM doesn't support guest PAuth */
+static void guest_unknown_handler(struct ex_regs *regs)
+{
+	GUEST_SYNC(FAIL_KVM);
+	GUEST_DONE();
+}
+
+/* Guest will get a FPAC exception if KVM support guest PAuth */
+static void guest_fpac_handler(struct ex_regs *regs)
+{
+	GUEST_SYNC(PASS);
+	GUEST_DONE();
+}
+
+/* Guest will get an instruction abort exception if the PAuth instructions have
+ * no effect (or PAuth not enabled in guest), which would cause guest to fetch
+ * an invalid instruction due to the corrupted LR.
+ */
+static void guest_iabt_handler(struct ex_regs *regs)
+{
+	GUEST_SYNC(FAIL_INSTR);
+	GUEST_DONE();
+}
+
+int main(void)
+{
+	struct kvm_vcpu_init init;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct ucall uc;
+	cpu_set_t cpu_mask;
+
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_ADDRESS));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_GENERIC));
+
+	vm = vm_create(1);
+
+	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
+	init.features[0] |= ((1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS) |
+			     (1 << KVM_ARM_VCPU_PTRAUTH_GENERIC));
+
+	vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
+
+	vm_init_descriptor_tables(vm);
+	vcpu_init_descriptor_tables(vcpu);
+
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_UNKNOWN, guest_unknown_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_FPAC, guest_fpac_handler);
+	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
+				ESR_EC_IABT, guest_iabt_handler);
+
+	while (1) {
+		vcpu_run(vcpu);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			break;
+		case UCALL_SYNC:
+			switch (uc.args[1]) {
+			case PASS:
+				/* KVM guest PAuth works! */
+				break;
+			case WAIT_MIGRATION:
+				sched_getaffinity(0, sizeof(cpu_mask), &cpu_mask);
+				CPU_CLR(sched_getcpu(), &cpu_mask);
+				sched_setaffinity(0, sizeof(cpu_mask), &cpu_mask);
+				break;
+			case FAIL:
+				TEST_FAIL("Guest corruptor code doesn't work!\n");
+				break;
+			case FAIL_KVM:
+				TEST_FAIL("KVM doesn't support guest PAuth!\n");
+				break;
+			case FAIL_INSTR:
+				TEST_FAIL("Guest PAuth instructions don't work!\n");
+				break;
+			}
+			break;
+		case UCALL_DONE:
+			goto done;
+		default:
+			TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
+		}
+	}
+
+done:
+	kvm_vm_free(vm);
+}
diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
index cb537253a6b9..f8d541af9c06 100644
--- a/tools/testing/selftests/kvm/include/aarch64/processor.h
+++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
@@ -104,7 +104,9 @@ enum {
 #define ESR_EC_SHIFT		26
 #define ESR_EC_MASK		(ESR_EC_NUM - 1)
 
+#define ESR_EC_UNKNOWN		0x00
 #define ESR_EC_SVC64		0x15
+#define ESR_EC_FPAC		0x1c
 #define ESR_EC_IABT		0x21
 #define ESR_EC_DABT		0x25
 #define ESR_EC_HW_BP_CURRENT	0x31

base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef
-- 
2.41.0.487.g6d72f3e995-goog

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Marc Zyngier]

Hi Jing,

Thanks for getting the ball rolling on that front.

On Wed, 26 Jul 2023 05:46:51 +0100,
Jing Zhang <jingzhangos@google.com> wrote:
> 
> Add a selftest to verify the support for pointer authentication in KVM
> guest.

I guess it'd be worth describing *what* you are testing.

> 
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/aarch64/pauth_test.c        | 143 ++++++++++++++++++
>  .../selftests/kvm/include/aarch64/processor.h |   2 +
>  3 files changed, 146 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/aarch64/pauth_test.c
> 
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index c692cc86e7da..9bac5aecd66d 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -143,6 +143,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
>  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
>  TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
>  TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
> +TEST_GEN_PROGS_aarch64 += aarch64/pauth_test
>  TEST_GEN_PROGS_aarch64 += aarch64/psci_test
>  TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
>  TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
> diff --git a/tools/testing/selftests/kvm/aarch64/pauth_test.c b/tools/testing/selftests/kvm/aarch64/pauth_test.c
> new file mode 100644
> index 000000000000..d5f982da8891
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/pauth_test.c
> @@ -0,0 +1,143 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * pauth_test - Test for KVM guest pointer authentication.
> + *
> + * Copyright (c) 2023 Google LLC.
> + *
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <sched.h>
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +#include "test_util.h"
> +
> +enum uc_args {
> +	WAIT_MIGRATION,
> +	PASS,
> +	FAIL,
> +	FAIL_KVM,
> +	FAIL_INSTR,
> +};
> +
> +static noinline void pac_corruptor(void)
> +{
> +	__asm__ __volatile__(
> +		"paciasp\n"
> +		"eor lr, lr, #1 << 53\n"

Why bit 53? This looks pretty odd. Given that you don't compute the
size of the PAC field (think FEAT_D128...), this isn't a safe bet...

> +	);
> +
> +	/* Migrate guest to another physical CPU before authentication */
> +	GUEST_SYNC(WAIT_MIGRATION);
> +	__asm__ __volatile__("autiasp\n");

If the test was compiled with PAuth enabled, you shouldn't need to add
the paciasp/authiasp instructions. On the other hand, the whole
"corrupt LR" is extremely fragile -- the compiler doesn't know you've
messed with it, and it is free to reuse it.

If you want a reliable test, you must write this entirely in
assembly.

> +}
> +
> +static void guest_code(void)
> +{
> +	uint64_t sctlr = read_sysreg(sctlr_el1);
> +
> +	/* Enable PAuth */
> +	sctlr |= SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB;
> +	write_sysreg(sctlr, sctlr_el1);
> +	isb();

Out of curiosity, where are the keys set up? Because part of the
validation would be that for a given set of keys and authentication
algorithm, we obtain the same results.

> +
> +	pac_corruptor();
> +
> +	/* Shouldn't be here unless the pac_corruptor didn't do its work */
> +	GUEST_SYNC(FAIL);
> +	GUEST_DONE();
> +}
> +
> +/* Guest will get an unknown exception if KVM doesn't support guest PAuth */
> +static void guest_unknown_handler(struct ex_regs *regs)
> +{
> +	GUEST_SYNC(FAIL_KVM);
> +	GUEST_DONE();
> +}
> +
> +/* Guest will get a FPAC exception if KVM support guest PAuth */
> +static void guest_fpac_handler(struct ex_regs *regs)
> +{
> +	GUEST_SYNC(PASS);
> +	GUEST_DONE();
> +}
> +
> +/* Guest will get an instruction abort exception if the PAuth instructions have
> + * no effect (or PAuth not enabled in guest), which would cause guest to fetch
> + * an invalid instruction due to the corrupted LR.
> + */
> +static void guest_iabt_handler(struct ex_regs *regs)
> +{
> +	GUEST_SYNC(FAIL_INSTR);
> +	GUEST_DONE();
> +}
> +
> +int main(void)
> +{
> +	struct kvm_vcpu_init init;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm;
> +	struct ucall uc;
> +	cpu_set_t cpu_mask;
> +
> +	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_ADDRESS));
> +	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_GENERIC));
> +
> +	vm = vm_create(1);
> +
> +	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
> +	init.features[0] |= ((1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS) |
> +			     (1 << KVM_ARM_VCPU_PTRAUTH_GENERIC));
> +
> +	vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
> +
> +	vm_init_descriptor_tables(vm);
> +	vcpu_init_descriptor_tables(vcpu);
> +
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_EC_UNKNOWN, guest_unknown_handler);
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_EC_FPAC, guest_fpac_handler);
> +	vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> +				ESR_EC_IABT, guest_iabt_handler);
> +
> +	while (1) {
> +		vcpu_run(vcpu);
> +
> +		switch (get_ucall(vcpu, &uc)) {
> +		case UCALL_ABORT:
> +			REPORT_GUEST_ASSERT(uc);
> +			break;
> +		case UCALL_SYNC:
> +			switch (uc.args[1]) {
> +			case PASS:
> +				/* KVM guest PAuth works! */
> +				break;
> +			case WAIT_MIGRATION:
> +				sched_getaffinity(0, sizeof(cpu_mask), &cpu_mask);
> +				CPU_CLR(sched_getcpu(), &cpu_mask);
> +				sched_setaffinity(0, sizeof(cpu_mask), &cpu_mask);
> +				break;
> +			case FAIL:
> +				TEST_FAIL("Guest corruptor code doesn't work!\n");
> +				break;
> +			case FAIL_KVM:
> +				TEST_FAIL("KVM doesn't support guest PAuth!\n");

Why is that a hard failure? The vast majority of the HW out there
doesn't support PAuth...

> +				break;
> +			case FAIL_INSTR:
> +				TEST_FAIL("Guest PAuth instructions don't work!\n");
> +				break;
> +			}
> +			break;
> +		case UCALL_DONE:
> +			goto done;

Why a goto instead of a break?

> +		default:
> +			TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
> +		}
> +	}
> +
> +done:
> +	kvm_vm_free(vm);
> +}
> diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
> index cb537253a6b9..f8d541af9c06 100644
> --- a/tools/testing/selftests/kvm/include/aarch64/processor.h
> +++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
> @@ -104,7 +104,9 @@ enum {
>  #define ESR_EC_SHIFT		26
>  #define ESR_EC_MASK		(ESR_EC_NUM - 1)
>  
> +#define ESR_EC_UNKNOWN		0x00
>  #define ESR_EC_SVC64		0x15
> +#define ESR_EC_FPAC		0x1c
>  #define ESR_EC_IABT		0x21
>  #define ESR_EC_DABT		0x25
>  #define ESR_EC_HW_BP_CURRENT	0x31
> 

Although that's a good start, PAuth instructions that are in the NOP
space are not that interesting, because we already have tons of code
using it. What I'd really love to see is a test exercising some of the
non-NOP stuff, including use of the generic auth keys and the
PACGA/XPAC* instructions.

As I mentioned above, another thing I'd like to see is a set of
reference results for a given set of keys and architected algorithm
(QARMA3, QARMA5) so that we can compare between implementations
(excluding the IMPDEF implementations, of course).

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Jing Zhang]

Hi Marc,

On Wed, Jul 26, 2023 at 12:01 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Hi Jing,
>
> Thanks for getting the ball rolling on that front.
>
> On Wed, 26 Jul 2023 05:46:51 +0100,
> Jing Zhang <jingzhangos@google.com> wrote:
> >
> > Add a selftest to verify the support for pointer authentication in KVM
> > guest.
>
> I guess it'd be worth describing *what* you are testing.
Sure, I will add test details.
>
> >
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
> > ---
> >  tools/testing/selftests/kvm/Makefile          |   1 +
> >  .../selftests/kvm/aarch64/pauth_test.c        | 143 ++++++++++++++++++
> >  .../selftests/kvm/include/aarch64/processor.h |   2 +
> >  3 files changed, 146 insertions(+)
> >  create mode 100644 tools/testing/selftests/kvm/aarch64/pauth_test.c
> >
> > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> > index c692cc86e7da..9bac5aecd66d 100644
> > --- a/tools/testing/selftests/kvm/Makefile
> > +++ b/tools/testing/selftests/kvm/Makefile
> > @@ -143,6 +143,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
> >  TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
> >  TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
> >  TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
> > +TEST_GEN_PROGS_aarch64 += aarch64/pauth_test
> >  TEST_GEN_PROGS_aarch64 += aarch64/psci_test
> >  TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
> >  TEST_GEN_PROGS_aarch64 += aarch64/vcpu_width_config
> > diff --git a/tools/testing/selftests/kvm/aarch64/pauth_test.c b/tools/testing/selftests/kvm/aarch64/pauth_test.c
> > new file mode 100644
> > index 000000000000..d5f982da8891
> > --- /dev/null
> > +++ b/tools/testing/selftests/kvm/aarch64/pauth_test.c
> > @@ -0,0 +1,143 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * pauth_test - Test for KVM guest pointer authentication.
> > + *
> > + * Copyright (c) 2023 Google LLC.
> > + *
> > + */
> > +
> > +#define _GNU_SOURCE
> > +
> > +#include <sched.h>
> > +
> > +#include "kvm_util.h"
> > +#include "processor.h"
> > +#include "test_util.h"
> > +
> > +enum uc_args {
> > +     WAIT_MIGRATION,
> > +     PASS,
> > +     FAIL,
> > +     FAIL_KVM,
> > +     FAIL_INSTR,
> > +};
> > +
> > +static noinline void pac_corruptor(void)
> > +{
> > +     __asm__ __volatile__(
> > +             "paciasp\n"
> > +             "eor lr, lr, #1 << 53\n"
>
> Why bit 53? This looks pretty odd. Given that you don't compute the
> size of the PAC field (think FEAT_D128...), this isn't a safe bet...
>
It is kind of arbitrarily chosen, but surely belongs to PAC since the
VM was created with mode VM_MODE_P40V48_4K by default. I will change
the VM creation call to explicitly specify the mode.
> > +     );
> > +
> > +     /* Migrate guest to another physical CPU before authentication */
> > +     GUEST_SYNC(WAIT_MIGRATION);
> > +     __asm__ __volatile__("autiasp\n");
>
> If the test was compiled with PAuth enabled, you shouldn't need to add
> the paciasp/authiasp instructions. On the other hand, the whole
> "corrupt LR" is extremely fragile -- the compiler doesn't know you've
> messed with it, and it is free to reuse it.
>
> If you want a reliable test, you must write this entirely in
> assembly.
Right. I didn't take care of the LR when I added the GUEST_SYNC call.
I guess it would work too if I save the LR on stack before the call to
GUEST_SYNC and restore it later before the authentication instruction.
>
> > +}
> > +
> > +static void guest_code(void)
> > +{
> > +     uint64_t sctlr = read_sysreg(sctlr_el1);
> > +
> > +     /* Enable PAuth */
> > +     sctlr |= SCTLR_ELx_ENIA | SCTLR_ELx_ENIB | SCTLR_ELx_ENDA | SCTLR_ELx_ENDB;
> > +     write_sysreg(sctlr, sctlr_el1);
> > +     isb();
>
> Out of curiosity, where are the keys set up? Because part of the
> validation would be that for a given set of keys and authentication
> algorithm, we obtain the same results.
I just used whatever in the key system registers. I will set up the
keys explicitly with given values. This then can also verify that KVM
saves/restores the keys correctly.
>
> > +
> > +     pac_corruptor();
> > +
> > +     /* Shouldn't be here unless the pac_corruptor didn't do its work */
> > +     GUEST_SYNC(FAIL);
> > +     GUEST_DONE();
> > +}
> > +
> > +/* Guest will get an unknown exception if KVM doesn't support guest PAuth */
> > +static void guest_unknown_handler(struct ex_regs *regs)
> > +{
> > +     GUEST_SYNC(FAIL_KVM);
> > +     GUEST_DONE();
> > +}
> > +
> > +/* Guest will get a FPAC exception if KVM support guest PAuth */
> > +static void guest_fpac_handler(struct ex_regs *regs)
> > +{
> > +     GUEST_SYNC(PASS);
> > +     GUEST_DONE();
> > +}
> > +
> > +/* Guest will get an instruction abort exception if the PAuth instructions have
> > + * no effect (or PAuth not enabled in guest), which would cause guest to fetch
> > + * an invalid instruction due to the corrupted LR.
> > + */
> > +static void guest_iabt_handler(struct ex_regs *regs)
> > +{
> > +     GUEST_SYNC(FAIL_INSTR);
> > +     GUEST_DONE();
> > +}
> > +
> > +int main(void)
> > +{
> > +     struct kvm_vcpu_init init;
> > +     struct kvm_vcpu *vcpu;
> > +     struct kvm_vm *vm;
> > +     struct ucall uc;
> > +     cpu_set_t cpu_mask;
> > +
> > +     TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_ADDRESS));
> > +     TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PTRAUTH_GENERIC));
> > +
> > +     vm = vm_create(1);
> > +
> > +     vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
> > +     init.features[0] |= ((1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS) |
> > +                          (1 << KVM_ARM_VCPU_PTRAUTH_GENERIC));
> > +
> > +     vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);
> > +
> > +     vm_init_descriptor_tables(vm);
> > +     vcpu_init_descriptor_tables(vcpu);
> > +
> > +     vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> > +                             ESR_EC_UNKNOWN, guest_unknown_handler);
> > +     vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> > +                             ESR_EC_FPAC, guest_fpac_handler);
> > +     vm_install_sync_handler(vm, VECTOR_SYNC_CURRENT,
> > +                             ESR_EC_IABT, guest_iabt_handler);
> > +
> > +     while (1) {
> > +             vcpu_run(vcpu);
> > +
> > +             switch (get_ucall(vcpu, &uc)) {
> > +             case UCALL_ABORT:
> > +                     REPORT_GUEST_ASSERT(uc);
> > +                     break;
> > +             case UCALL_SYNC:
> > +                     switch (uc.args[1]) {
> > +                     case PASS:
> > +                             /* KVM guest PAuth works! */
> > +                             break;
> > +                     case WAIT_MIGRATION:
> > +                             sched_getaffinity(0, sizeof(cpu_mask), &cpu_mask);
> > +                             CPU_CLR(sched_getcpu(), &cpu_mask);
> > +                             sched_setaffinity(0, sizeof(cpu_mask), &cpu_mask);
> > +                             break;
> > +                     case FAIL:
> > +                             TEST_FAIL("Guest corruptor code doesn't work!\n");
> > +                             break;
> > +                     case FAIL_KVM:
> > +                             TEST_FAIL("KVM doesn't support guest PAuth!\n");
>
> Why is that a hard failure? The vast majority of the HW out there
> doesn't support PAuth...
Since previous TEST_REQUIRES have passed, KVM should be able to
support guest PAuth. The test will be skipped on those HW without
PAuth.
>
> > +                             break;
> > +                     case FAIL_INSTR:
> > +                             TEST_FAIL("Guest PAuth instructions don't work!\n");
> > +                             break;
> > +                     }
> > +                     break;
> > +             case UCALL_DONE:
> > +                     goto done;
>
> Why a goto instead of a break?
To jump out of the while(1) loop.
>
> > +             default:
> > +                     TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
> > +             }
> > +     }
> > +
> > +done:
> > +     kvm_vm_free(vm);
> > +}
> > diff --git a/tools/testing/selftests/kvm/include/aarch64/processor.h b/tools/testing/selftests/kvm/include/aarch64/processor.h
> > index cb537253a6b9..f8d541af9c06 100644
> > --- a/tools/testing/selftests/kvm/include/aarch64/processor.h
> > +++ b/tools/testing/selftests/kvm/include/aarch64/processor.h
> > @@ -104,7 +104,9 @@ enum {
> >  #define ESR_EC_SHIFT         26
> >  #define ESR_EC_MASK          (ESR_EC_NUM - 1)
> >
> > +#define ESR_EC_UNKNOWN               0x00
> >  #define ESR_EC_SVC64         0x15
> > +#define ESR_EC_FPAC          0x1c
> >  #define ESR_EC_IABT          0x21
> >  #define ESR_EC_DABT          0x25
> >  #define ESR_EC_HW_BP_CURRENT 0x31
> >
>
> Although that's a good start, PAuth instructions that are in the NOP
> space are not that interesting, because we already have tons of code
> using it. What I'd really love to see is a test exercising some of the
> non-NOP stuff, including use of the generic auth keys and the
> PACGA/XPAC* instructions.
Sure, I will add more tests for non-NOP stuff.
>
> As I mentioned above, another thing I'd like to see is a set of
> reference results for a given set of keys and architected algorithm
> (QARMA3, QARMA5) so that we can compare between implementations
> (excluding the IMPDEF implementations, of course).
Sure. Will do.
>
> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

Thanks,
Jing

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Oliver Upton]

Hi Jing,

Nothing serious, but when you're replying on a thread can you add a
leading and trailing line of whitespace between the quotation and your
reply? Otherwise threads get really dense and hard to read.

On Wed, Aug 02, 2023 at 10:19:30AM -0700, Jing Zhang wrote:
> > > +                     case FAIL_KVM:
> > > +                             TEST_FAIL("KVM doesn't support guest PAuth!\n");
> >
> > Why is that a hard failure? The vast majority of the HW out there
> > doesn't support PAuth...
> Since previous TEST_REQUIRES have passed, KVM should be able to
> support guest PAuth. The test will be skipped on those HW without
> PAuth.

So then what is the purpose of this failure mode? The only case where
this would happen is if KVM is if KVM screwed up the emulation somehow,
took a trap on a PAC instruction or register and reflected that back
into the guest as an UNDEF.

That's a perfectly valid thing to test for, but the naming and failure
messages should indicate what actually happened.

> > As I mentioned above, another thing I'd like to see is a set of
> > reference results for a given set of keys and architected algorithm
> > (QARMA3, QARMA5) so that we can compare between implementations
> > (excluding the IMPDEF implementations, of course).
> Sure. Will do.

I was initially hesitant towards testing PAC like this since it is
entirely a hardware issue besides KVM context switching, but you could
spin this off as a way to test if vCPU save/restore works correctly by
priming the vCPU from userspace.

Marc, is there something else here you're interested in exercising I
may've missed?

-- 
Thanks,
Oliver

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Jing Zhang]

Hi Oliver,

On Wed, Aug 2, 2023 at 1:17 PM Oliver Upton <oliver.upton@linux.dev> wrote:
>
> Hi Jing,
>
> Nothing serious, but when you're replying on a thread can you add a
> leading and trailing line of whitespace between the quotation and your
> reply? Otherwise threads get really dense and hard to read.

Will do. Thanks for the suggestion.

>
> On Wed, Aug 02, 2023 at 10:19:30AM -0700, Jing Zhang wrote:
> > > > +                     case FAIL_KVM:
> > > > +                             TEST_FAIL("KVM doesn't support guest PAuth!\n");
> > >
> > > Why is that a hard failure? The vast majority of the HW out there
> > > doesn't support PAuth...
> > Since previous TEST_REQUIRES have passed, KVM should be able to
> > support guest PAuth. The test will be skipped on those HW without
> > PAuth.
>
> So then what is the purpose of this failure mode? The only case where
> this would happen is if KVM is if KVM screwed up the emulation somehow,
> took a trap on a PAC instruction or register and reflected that back
> into the guest as an UNDEF.

You are right about the purpose of this test case.

>
> That's a perfectly valid thing to test for, but the naming and failure
> messages should indicate what actually happened.

Sure. Will use more sensible messages.

>
> > > As I mentioned above, another thing I'd like to see is a set of
> > > reference results for a given set of keys and architected algorithm
> > > (QARMA3, QARMA5) so that we can compare between implementations
> > > (excluding the IMPDEF implementations, of course).
> > Sure. Will do.
>
> I was initially hesitant towards testing PAC like this since it is
> entirely a hardware issue besides KVM context switching, but you could
> spin this off as a way to test if vCPU save/restore works correctly by
> priming the vCPU from userspace.
>
> Marc, is there something else here you're interested in exercising I
> may've missed?
>
> --
> Thanks,
> Oliver

Thanks,
Jing

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Sean Christopherson]

On Tue, Jul 25, 2023, Jing Zhang wrote:
> Add a selftest to verify the support for pointer authentication in KVM
> guest.
> 
> Signed-off-by: Jing Zhang <jingzhangos@google.com>

...

> +	/* Shouldn't be here unless the pac_corruptor didn't do its work */
> +	GUEST_SYNC(FAIL);

FYI, I'm fast tracking guest printf support along with a pile of GUEST_ASSERT()
changes through kvm-x86/selftests[*].  At a glance, this test can probably use
e.g. GUEST_FAIL() to make things easier to debug.

[*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc@google.com

Re: [PATCH v1] KVM: arm64: selftests: Test pointer authentication support in KVM guest

[Jing Zhang]

On Wed, Aug 2, 2023 at 3:13 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Tue, Jul 25, 2023, Jing Zhang wrote:
> > Add a selftest to verify the support for pointer authentication in KVM
> > guest.
> >
> > Signed-off-by: Jing Zhang <jingzhangos@google.com>
>
> ...
>
> > +     /* Shouldn't be here unless the pac_corruptor didn't do its work */
> > +     GUEST_SYNC(FAIL);
>
> FYI, I'm fast tracking guest printf support along with a pile of GUEST_ASSERT()
> changes through kvm-x86/selftests[*].  At a glance, this test can probably use
> e.g. GUEST_FAIL() to make things easier to debug.
>
> [*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc@google.com

Thanks for the heads up, Sean!

Jing