Re: [PATCH v4 30/34] KVM: arm64: Add kvm_pgtable_stage2_find_range()

On Thursday 11 Mar 2021 at 19:04:07 (+0000), Will Deacon wrote:

quoted

On Wed, Mar 10, 2021 at 05:57:47PM +0000, Quentin Perret wrote:

quoted

+	for (level = pgt->start_level; level < KVM_PGTABLE_MAX_LEVELS; level++) {
+		granule = kvm_granule_size(level);
+		start = ALIGN_DOWN(addr, granule);
+		end = start + granule;
+
+		if (!kvm_level_support_block_mappings(level))
+			continue;
+
+		if (start < range->start || range->end < end)
+			continue;
+
+		/*
+		 * Check the presence of existing mappings with incompatible
+		 * permissions within the current block range, and try one level
+		 * deeper if one is found.
+		 */
+		ret = kvm_pgtable_walk(pgt, start, granule, &check_perm_walker);
+		if (ret != -EEXIST)
+			break;
+	}

Can you write this as a:

	do {
		...
	} while (level < KVM_PGTABLE_MAX_LEVELS && ret == -EEXIST);

loop?

I tried it but found it a little less pretty -- the pre-assignment of
level and the increment at the end make it really feel like a for loop
to me:

--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c

@@ -1098,26 +1098,23 @@ int kvm_pgtable_stage2_find_range(struct kvm_pgtable *pgt, u64 addr,
                return ret;
        attr &= KVM_PTE_LEAF_S2_COMPAT_MASK;
 
-       for (level = pgt->start_level; level < KVM_PGTABLE_MAX_LEVELS; level++) {
+       ret = -EEXIST;
+       level = pgt->start_level;
+       do {
                granule = kvm_granule_size(level);
                start = ALIGN_DOWN(addr, granule);
                end = start + granule;
 
-               if (!kvm_level_support_block_mappings(level))
-                       continue;
-
-               if (start < range->start || range->end < end)
-                       continue;