Messages in this thread Patch in this message |  | | Date | Mon, 26 Jul 2021 14:13:06 +0100 | From | Quentin Perret <> | Subject | Re: [PATCH v2 04/16] KVM: arm64: Optimize host memory aborts |
| |
On Monday 26 Jul 2021 at 11:35:10 (+0100), Marc Zyngier wrote: > On Mon, 26 Jul 2021 10:28:53 +0100, > Quentin Perret <qperret@google.com> wrote: > > +static int host_stage2_find_range(u64 addr, struct kvm_mem_range *range) > > nit: I find 'find_range' a bit odd. We already have found a > range. We're just trying to narrow it down to something that fits in a > single block mapping. How about 'host_stage2_adjust_range'?
Ack.
> > +{ > > + u64 granule, start, end; > > + kvm_pte_t pte; > > + u32 level; > > + int ret; > > + > > + ret = kvm_pgtable_get_leaf(&host_kvm.pgt, addr, &pte, &level); > > + if (ret) > > + return ret; > > + > > + if (kvm_pte_valid(pte)) > > + return -EAGAIN; > > + > > + if (pte) > > + return -EPERM; > > + > > + do { > > + granule = kvm_granule_size(level); > > + start = ALIGN_DOWN(addr, granule); > > + end = start + granule; > > + level++; > > + } while ((level < KVM_PGTABLE_MAX_LEVELS) && > > + (!kvm_level_supports_block_mapping(level) || > > + start < range->start || range->end < end)); > > + > > This expression does my head in. You are trying to find the largest > block mapping that entirely fits in range, right? Can we just express > that directly (with a global negation for the purpose of the loop)? > > do { > [...] > } while (level < KVM_PGTABLE_MAX_LEVELS && > !(kvm_level_supports_block_mapping(level) && > start >= range->start && > end <= range->end)); > > I personally find this much more readable, because it expresses the > condition we are looking for rather than a lot of conditions forcing > us to continue. > > You could also use a kvm_mem_range for the iteration, and add a helper > that checks for the inclusion.
Something like this (untested)?
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c index 75273166d2c5..07d228163090 100644 --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c @@ -234,9 +234,15 @@ static inline int __host_stage2_idmap(u64 start, u64 end, __ret; \ })
+static inline bool range_included(struct kvm_mem_range *child, + struct kvm_mem_range *parent) +{ + return parent->start <= child->start && child->end <= parent->end; +} + static int host_stage2_find_range(u64 addr, struct kvm_mem_range *range) { - u64 granule, start, end; + struct kvm_mem_range cur; kvm_pte_t pte; u32 level; int ret; @@ -252,16 +258,15 @@ static int host_stage2_find_range(u64 addr, struct kvm_mem_range *range) return -EPERM;
do { - granule = kvm_granule_size(level); - start = ALIGN_DOWN(addr, granule); - end = start + granule; + u64 granule = kvm_granule_size(level); + cur.start = ALIGN_DOWN(addr, granule); + cur.end = cur.start + granule; level++; } while ((level < KVM_PGTABLE_MAX_LEVELS) && - (!kvm_level_supports_block_mapping(level) || - start < range->start || range->end < end)); + !(kvm_level_supports_block_mapping(level) && + range_included(&cur, parent)));
- range->start = start; - range->end = end; + *range = cur;
return 0; }
|  |