On 13/04/18 17:27, Punit Agrawal wrote:
Hi Suzuki,
I haven't had a chance to look at the code but noticed one issue below.
Suzuki K Poulose <suzuki.poulose@xxxxxxx> writes:
Now that we can manage the stage2 page table per VM, switch the
configuration details to per VM instance. We keep track of the
IPA bits, number of page table levels and the VTCR bits (which
depends on the IPA and the number of levels). While at it, remove
unused pgd_lock field from kvm_arch for arm64.
Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
Cc: Christoffer Dall <cdall@xxxxxxxxxx>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
---
arch/arm64/include/asm/kvm_host.h | 14 ++++++++++++--
arch/arm64/include/asm/kvm_mmu.h | 11 +++++++++--
arch/arm64/include/asm/stage2_pgtable.h | 1 -
arch/arm64/kvm/hyp/switch.c | 3 +--
virt/kvm/arm/mmu.c | 4 ++++
5 files changed, 26 insertions(+), 7 deletions(-)
[...]
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index bb458bf..e86d7f4 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -136,9 +136,10 @@ static inline unsigned long __kern_hyp_va(unsigned long v)
*/
#define KVM_PHYS_SHIFT (40)
-#define kvm_phys_shift(kvm) KVM_PHYS_SHIFT
+#define kvm_phys_shift(kvm) (kvm->arch.phys_shift)
#define kvm_phys_size(kvm) (_AC(1, ULL) << kvm_phys_shift(kvm))
#define kvm_phys_mask(kvm) (kvm_phys_size(kvm) - _AC(1, ULL))
+#define kvm_stage2_levels(kvm) (kvm->arch.s2_levels)
static inline bool kvm_page_empty(void *ptr)
{
[...]
diff --git a/arch/arm64/include/asm/stage2_pgtable.h b/arch/arm64/include/asm/stage2_pgtable.h
index 33e8ebb..9b75b83 100644
--- a/arch/arm64/include/asm/stage2_pgtable.h
+++ b/arch/arm64/include/asm/stage2_pgtable.h
@@ -44,7 +44,6 @@
*/
#define __s2_pgd_ptrs(pa, lvls) (1 << ((pa) - pt_levels_pgdir_shift((lvls))))
-#define kvm_stage2_levels(kvm) stage2_pt_levels(kvm_phys_shift(kvm))
#define stage2_pgdir_shift(kvm) \
pt_levels_pgdir_shift(kvm_stage2_levels(kvm))
#define stage2_pgdir_size(kvm) (_AC(1, UL) << stage2_pgdir_shift((kvm)))
[...]
diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 7a264c6..746f38e 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -753,6 +753,10 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm)
return -EINVAL;
}
+ /* Make sure we have the stage2 configured for this VM */
+ if (WARN_ON(!kvm_stage2_levels(kvm)))
+ return -EINVAL;
+
This hunk breaks the 32-bit build as kvm_stag2_levels() isn't defined on
arm.
Thanks for spotting. I have fixed this locally in my next version to check
for the kvm_phys_shift(), as I plan to delay the levels selection to the
actual allocation of the table, giving us a fall back to increase the level
if we are unable to allocate contiguous pages (e.g, 16 * 64K pages with say 46bit
IPA).
Cheers
Suzuki
