Re: [PATCH 04/16] KVM: arm64: Add MMIO checking infrastructure
From: Marc Zyngier <maz@kernel.org>
Date: 2021-07-22 18:05:06
Also in:
kvm, kvmarm, lkml
On Tue, 20 Jul 2021 16:49:45 +0100, Quentin Perret [off-list ref] wrote:
On Tuesday 20 Jul 2021 at 14:15:56 (+0100), Marc Zyngier wrote:quoted
On Tue, 20 Jul 2021 12:13:20 +0100, Quentin Perret [off-list ref] wrote:quoted
On Thursday 15 Jul 2021 at 17:31:47 (+0100), Marc Zyngier wrote:quoted
+struct s2_walk_data { + kvm_pte_t pteval; + u32 level; +}; + +static int s2_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep, + enum kvm_pgtable_walk_flags flag, void * const arg) +{ + struct s2_walk_data *data = arg; + + data->level = level; + data->pteval = *ptep; + return 0; +} + +/* Assumes mmu_lock taken */ +static bool __check_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa) +{ + struct s2_walk_data data; + struct kvm_pgtable_walker walker = { + .cb = s2_walker, + .flags = KVM_PGTABLE_WALK_LEAF, + .arg = &data, + }; + + kvm_pgtable_walk(vcpu->arch.hw_mmu->pgt, ALIGN_DOWN(ipa, PAGE_SIZE), + PAGE_SIZE, &walker); + + /* Must be a PAGE_SIZE mapping with our annotation */ + return (BIT(ARM64_HW_PGTABLE_LEVEL_SHIFT(data.level)) == PAGE_SIZE && + data.pteval == MMIO_NOTE);Nit: you could do this check in the walker directly and check the return value of kvm_pgtable_walk() instead. That would allow to get rid of struct s2_walk_data. Also, though the compiler might be able to optimize, maybe simplify the level check to level == (KVM_PGTABLE_MAX_LEVELS - 1)?Yup, all good points. I guess I could do the same in my other series that parses the userspace PT to extract the level.Well, actually, let me take that back. I think something like you have would be useful, but in pgtable.c directly and re-usable for stage-1 and stage-2 walks. Maybe something like the below (totally untested)? I could use such a walker in several places as well in the memory ownership series: - following the idea of [1], I could remove the kvm_pgtable_stage2_find_range() function entirely; - [2] defines 2 custom walkers that do nothing but walk host stage-2 and hyp stage-1 page-tables to check permissions and such -- they could be removed/re-implemented easily as well. And you seem to need something similar here, so clearly there is a need. WDYT?
So FWIW, I've now pushed out an updated series for the THP changes[1], and you will find a similar patch at the base of the branch. Please have a look and let me know what you think! Thanks, M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/mmu/mapping-levels -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel