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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dave,

On 5/28/19 3:41 PM, Dave Martin wrote:
On Wed, Apr 24, 2019 at 02:41:21PM +0100, Dave Martin wrote:
On Wed, Apr 24, 2019 at 12:32:22PM +0530, Amit Daniel Kachhap wrote:
Hi,

On 4/23/19 9:16 PM, Dave Martin wrote:

[...]

diff --git a/arm/kvm-cpu.c b/arm/kvm-cpu.c
index 7780251..acd1d5f 100644
--- a/arm/kvm-cpu.c
+++ b/arm/kvm-cpu.c
@@ -68,6 +68,18 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
  		vcpu_init.features[0] |= (1UL << KVM_ARM_VCPU_PSCI_0_2);
  	}
+	/* Check Pointer Authentication command line arguments. */
+	if (kvm->cfg.arch.enable_ptrauth && kvm->cfg.arch.disable_ptrauth)
+		die("Both enable-ptrauth and disable-ptrauth option cannot be present");

Preferably, print the leading dashes, the same as the user would see
on the command line (e.g., --enable-ptrauth, --disable-ptrauth).

For brevity, we could write something like:

		die("--enable-ptrauth conflicts with --disable-ptrauth");

[...]

@@ -106,8 +118,12 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned long cpu_id)
  			die("Unable to find matching target");
  	}
-	if (err || target->init(vcpu))
-		die("Unable to initialise vcpu");
+	if (err || target->init(vcpu)) {
+		if (kvm->cfg.arch.enable_ptrauth)
+			die("Unable to initialise vcpu with pointer authentication feature");

We don't special-case this error message for any other feature yet:
there are a variety of reasons why we might have failed, so suggesting
that the failure is something to do with ptrauth may be misleading to
the user.

If we want to be more informative, we could do something like the
following:

	bool supported;

	supported = kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
		    kvm__supports_extension(kvm, KVM_CAP_ARM_PTRAUTH_GENERIC);

	if (kvm->cfg.arch.enable_ptrauth && !supported)
		die("--enable-ptrauth not supported on this host");

	if (supported && !kvm->cfg.arch.disable_ptrauth)
		vcpu_init.features[0] |= ARM_VCPU_PTRAUTH_FEATURE;

	/* ... */

	if (err || target->init(vcpu))
		die("Unable to initialise vcpu");

We don't do this for any other feature today, but since it helps the
user to understand what went wrong it's probably a good idea.
Yes this is more clear. As Mark has picked the core guest ptrauth patches. I
will post this changes as standalone.

Sounds good.  (I also need to do that separately for SVE...)

Were you planning to repost this?

Alternatively, I can fix up the diagnostic messages discussed here and
post it together with the SVE support.  I'll do that locally for now,
but let me know what you plan to do.  I'd like to get the SVE support
posted soon so that people can test it.
I will clean up the print messages as you suggested and repost it shortly.

Thanks,
Amit Daniel

Cheers
---Dave

_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux