On Wed, Feb 20, 2019 at 04:19:18PM +0000, Mark Rutland wrote:

On Mon, Feb 18, 2019 at 07:52:27PM +0000, Dave Martin wrote:

quoted

-static bool __hyp_text __hyp_switch_fpsimd(struct kvm_vcpu *vcpu)
+/* Check for an FPSIMD/SVE trap and handle as appropriate */
+static bool __hyp_text __hyp_handle_fpsimd(struct kvm_vcpu *vcpu)
 {
-	struct user_fpsimd_state *host_fpsimd = vcpu->arch.host_fpsimd_state;
+	bool vhe, sve_guest, sve_host;
+	u8 trap_class;

Nit: elsewhere in kvm, this gets called hsr_ec. Can we use the same name
here?

Done.

(This name is not used in hyp/switch.c yet, and I was sort of trying to
conform to the naming of kvm_vcpu_trap_get_class().  But hsr_ec is used
elsewhere, and more self-describing, so it makes sense.)

quoted

 
-	if (has_vhe())
-		write_sysreg(read_sysreg(cpacr_el1) | CPACR_EL1_FPEN,
-			     cpacr_el1);
-	else
+	if (!system_supports_fpsimd())
+		return false;
+
+	if (system_supports_sve()) {
+		sve_guest = vcpu_has_sve(vcpu);
+		sve_host = vcpu->arch.flags & KVM_ARM64_HOST_SVE_IN_USE;
+		vhe = true;
+	} else {
+		sve_guest = false;
+		sve_host = false;
+		vhe = has_vhe();
+	}
+
+	trap_class = kvm_vcpu_trap_get_class(vcpu);
+	if (trap_class != ESR_ELx_EC_FP_ASIMD &&
+	    (!sve_guest || trap_class != ESR_ELx_EC_SVE))
+		return false;

This is somewhat painful to decipher, but I couldn't come up with
something that was both succint and legible.

Maybe it's worth duplicating the SVE check, i.e.

	if (hsr_ec != ESR_ELx_EC_FP_ASIMD &&
	    hsr_ec != ESR_ELx_EC_SVE)
		return false;

	if (!sve_guest && hsr_ec == ESR_ELx_EC_SVE)
		return false;

... ?

There were previously some static_key checks in here that I didn't want
to duplicate, which is part of the reason for trying to merge related
if-conditions together explicitly.

This is no longer relevant now that the static_key checks have been
hoisted out explicitly.  Something along the lines of your version
would definitely be clearer.  gcc also seems to merge the conditions
competently, so I'll pick up your suggestion for now.

Thanks
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel