Re: [PATCH 04/16] KVM: arm64: Introduce kvm_share_hyp()

On Sunday 17 Oct 2021 at 11:41:21 (+0100), Marc Zyngier wrote:

Not directly related to this code, but it looks to me that
kvm_host_owns_hyp_mappings() really ought to check for
is_kernel_in_hyp_mode() on its own. VHE really deals with its own
mappings, and create_hyp_mappings() already has a check to do nothing
on VHE.

Sure, I'll stick a patch at the beginning of the series.

quoted

+
+	return pkvm_share_hyp(kvm_kaddr_to_phys(from), kvm_kaddr_to_phys(to));
+}
+
 /**
  * create_hyp_mappings - duplicate a kernel virtual address range in Hyp mode
  * @from:	The virtual kernel start address of the range

@@ -316,12 +327,8 @@ int create_hyp_mappings(void *from, void *to, enum kvm_pgtable_prot prot)
 	if (is_kernel_in_hyp_mode())
 		return 0;
 
-	if (!kvm_host_owns_hyp_mappings()) {
-		if (WARN_ON(prot != PAGE_HYP))
-			return -EPERM;
-		return pkvm_share_hyp(kvm_kaddr_to_phys(from),
-				      kvm_kaddr_to_phys(to));
-	}
+	if (WARN_ON(!kvm_host_owns_hyp_mappings()))
+		return -EPERM;

Do we really need this? Can't we just verify that all the code paths
to create_hyp_mappings() check for kvm_host_owns_hyp_mappings()?

At the very least, make this a VM_BUG_ON() so that this is limited to
debug. Yes, I'm quickly developing a WARN_ON()-phobia.

Right, that _is_ purely debug. It's just that folks are used to being
able to just call create_hyp_mappings() for anything, so I wanted to
make sure we have an easy way to catch future changes that would
unknowingly break pKVM, but no objection to make this VM_BUG_ON().

Cheers,
Quentin

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