* [RFC PATCH 1/3] arch/arm64: Introduce a capability to tell whether 16-bit VMID is available
2021-05-06 16:52 [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameer Kolothum
@ 2021-05-06 16:52 ` Shameer Kolothum
2021-05-06 16:52 ` [RFC PATCH 2/3] kvm/arm: Introduce a new vmid allocator for KVM Shameer Kolothum
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Shameer Kolothum @ 2021-05-06 16:52 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: maz, will, catalin.marinas, james.morse, julien.thierry.kdev,
suzuki.poulose, jean-philippe, linuxarm
From: Julien Grall <julien.grall@arm.com>
At the moment, the function kvm_get_vmid_bits() is looking up for the
sanitized value of ID_AA64MMFR1_EL1 and extract the information
regarding the number of VMID bits supported.
This is fine as the function is mainly used during VMID roll-over. New
use in a follow-up patch will require the function to be called a every
context switch so we want the function to be more efficient.
A new capability is introduced to tell whether 16-bit VMID is
available.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/kvm_mmu.h | 4 +---
arch/arm64/kernel/cpufeature.c | 9 +++++++++
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index c40f2490cd7b..acb92da5c254 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -67,7 +67,8 @@
#define ARM64_HAS_LDAPR 59
#define ARM64_KVM_PROTECTED_MODE 60
#define ARM64_WORKAROUND_NVIDIA_CARMEL_CNP 61
+#define ARM64_HAS_16BIT_VMID 62
-#define ARM64_NCAPS 62
+#define ARM64_NCAPS 63
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 90873851f677..c3080966ef83 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -213,9 +213,7 @@ void kvm_toggle_cache(struct kvm_vcpu *vcpu, bool was_enabled);
static inline unsigned int kvm_get_vmid_bits(void)
{
- int reg = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
-
- return get_vmid_bits(reg);
+ return cpus_have_const_cap(ARM64_HAS_16BIT_VMID) ? 16 : 8;
}
/*
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index e5281e1c8f1d..ff956fb2f712 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2203,6 +2203,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.matches = has_cpuid_feature,
.min_field_value = 1,
},
+ {
+ .capability = ARM64_HAS_16BIT_VMID,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .sys_reg = SYS_ID_AA64MMFR1_EL1,
+ .field_pos = ID_AA64MMFR1_VMIDBITS_SHIFT,
+ .sign = FTR_UNSIGNED,
+ .min_field_value = ID_AA64MMFR1_VMIDBITS_16,
+ .matches = has_cpuid_feature,
+ },
{},
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 2/3] kvm/arm: Introduce a new vmid allocator for KVM
2021-05-06 16:52 [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameer Kolothum
2021-05-06 16:52 ` [RFC PATCH 1/3] arch/arm64: Introduce a capability to tell whether 16-bit VMID is available Shameer Kolothum
@ 2021-05-06 16:52 ` Shameer Kolothum
2021-05-06 16:52 ` [RFC PATCH 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one Shameer Kolothum
2021-06-04 8:13 ` [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameerali Kolothum Thodi
3 siblings, 0 replies; 9+ messages in thread
From: Shameer Kolothum @ 2021-05-06 16:52 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: maz, will, catalin.marinas, james.morse, julien.thierry.kdev,
suzuki.poulose, jean-philippe, linuxarm
This is based on arm64 asid allocator algorithm and
has duplicated most of the code here.
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
arch/arm64/include/asm/kvm_host.h | 6 +
arch/arm64/kvm/vmid.c | 285 ++++++++++++++++++++++++++++++
2 files changed, 291 insertions(+)
create mode 100644 arch/arm64/kvm/vmid.c
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 3d10e6527f7d..9d476f1f34af 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -670,6 +670,12 @@ int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
int kvm_arm_pvtime_has_attr(struct kvm_vcpu *vcpu,
struct kvm_device_attr *attr);
+int kvm_arm_vmid_alloc_init(void);
+void kvm_arm_vmid_alloc_free(void);
+void kvm_arm_update_vmid(atomic64_t *id, refcount_t *pinned);
+unsigned long kvm_arm_pinned_vmid_get(atomic64_t *id, refcount_t *pinned);
+void kvm_arm_pinned_vmid_put(atomic64_t *id, refcount_t *pinned);
+
static inline void kvm_arm_pvtime_vcpu_init(struct kvm_vcpu_arch *vcpu_arch)
{
vcpu_arch->steal.base = GPA_INVALID;
diff --git a/arch/arm64/kvm/vmid.c b/arch/arm64/kvm/vmid.c
new file mode 100644
index 000000000000..14854c786e00
--- /dev/null
+++ b/arch/arm64/kvm/vmid.c
@@ -0,0 +1,285 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * VMID allocator.
+ *
+ * Based on arch/arm64/mm/context.c
+ *
+ * Copyright (C) 2002-2003 Deep Blue Solutions Ltd, all rights reserved.
+ * Copyright (C) 2012 ARM Ltd.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+
+#include <asm/kvm_asm.h>
+#include <asm/kvm_mmu.h>
+
+static u32 vmid_bits;
+static DEFINE_RAW_SPINLOCK(cpu_vmid_lock);
+
+static atomic64_t vmid_generation;
+static unsigned long *vmid_map;
+
+static DEFINE_PER_CPU(atomic64_t, active_vmids);
+static DEFINE_PER_CPU(u64, reserved_vmids);
+static cpumask_t tlb_flush_pending;
+
+static unsigned long max_pinned_vmids;
+static unsigned long nr_pinned_vmids;
+static unsigned long *pinned_vmid_map;
+
+#define VMID_MASK (~GENMASK(vmid_bits - 1, 0))
+#define VMID_FIRST_VERSION (1UL << vmid_bits)
+
+#define NUM_USER_VMIDS VMID_FIRST_VERSION
+#define vmid2idx(vmid) ((vmid) & ~VMID_MASK)
+#define idx2vmid(idx) vmid2idx(idx)
+
+#define vmid_gen_match(vmid) \
+ (!(((vmid) ^ atomic64_read(&vmid_generation)) >> vmid_bits))
+
+static void flush_context(void)
+{
+ int cpu;
+ u64 vmid;
+
+ if (pinned_vmid_map)
+ bitmap_copy(vmid_map, pinned_vmid_map, NUM_USER_VMIDS);
+ else
+ bitmap_clear(vmid_map, 0, NUM_USER_VMIDS);
+
+ for_each_possible_cpu(cpu) {
+ vmid = atomic64_xchg_relaxed(&per_cpu(active_vmids, cpu), 0);
+ /*
+ * If this CPU has already been through a
+ * rollover, but hasn't run another task in
+ * the meantime, we must preserve its reserved
+ * VMID, as this is the only trace we have of
+ * the process it is still running.
+ */
+ if (vmid == 0)
+ vmid = per_cpu(reserved_vmids, cpu);
+ __set_bit(vmid2idx(vmid), vmid_map);
+ per_cpu(reserved_vmids, cpu) = vmid;
+ }
+
+ /*
+ * Queue a TLB invalidation for each CPU to perform on next
+ * context-switch
+ */
+ cpumask_setall(&tlb_flush_pending);
+}
+
+static bool check_update_reserved_vmid(u64 vmid, u64 newvmid)
+{
+ int cpu;
+ bool hit = false;
+
+ /*
+ * Iterate over the set of reserved VMIDs looking for a match.
+ * If we find one, then we can update our mm to use newvmid
+ * (i.e. the same VMID in the current generation) but we can't
+ * exit the loop early, since we need to ensure that all copies
+ * of the old VMID are updated to reflect the mm. Failure to do
+ * so could result in us missing the reserved VMID in a future
+ * generation.
+ */
+ for_each_possible_cpu(cpu) {
+ if (per_cpu(reserved_vmids, cpu) == vmid) {
+ hit = true;
+ per_cpu(reserved_vmids, cpu) = newvmid;
+ }
+ }
+
+ return hit;
+}
+
+static u64 new_vmid(atomic64_t *id, refcount_t *pinned)
+{
+ static u32 cur_idx = 1;
+ u64 vmid = atomic64_read(id);
+ u64 generation = atomic64_read(&vmid_generation);
+
+ if (vmid != 0) {
+ u64 newvmid = generation | (vmid & ~VMID_MASK);
+
+ /*
+ * If our current VMID was active during a rollover, we
+ * can continue to use it and this was just a false alarm.
+ */
+ if (check_update_reserved_vmid(vmid, newvmid))
+ return newvmid;
+
+ /*
+ * If it is pinned, we can keep using it. Note that reserved
+ * takes priority, because even if it is also pinned, we need to
+ * update the generation into the reserved_vmids.
+ */
+ if (pinned && refcount_read(pinned))
+ return newvmid;
+
+ /*
+ * We had a valid VMID in a previous life, so try to re-use
+ * it if possible.
+ */
+ if (!__test_and_set_bit(vmid2idx(vmid), vmid_map))
+ return newvmid;
+ }
+
+ /*
+ * Allocate a free VMID. If we can't find one, take a note of the
+ * currently active VMIDs and mark the TLBs as requiring flushes. We
+ * always count from VMID #2 (index 1), as we use VMID #0 for host.
+ */
+ vmid = find_next_zero_bit(vmid_map, NUM_USER_VMIDS, cur_idx);
+ if (vmid != NUM_USER_VMIDS)
+ goto set_vmid;
+
+ /* We're out of VMIDs, so increment the global generation count */
+ generation = atomic64_add_return_relaxed(VMID_FIRST_VERSION,
+ &vmid_generation);
+ flush_context();
+
+ /* We have more VMIDs than CPUs, so this will always succeed */
+ vmid = find_next_zero_bit(vmid_map, NUM_USER_VMIDS, 1);
+
+set_vmid:
+ __set_bit(vmid, vmid_map);
+ cur_idx = vmid;
+ return idx2vmid(vmid) | generation;
+}
+
+void kvm_arm_update_vmid(atomic64_t *id, refcount_t *pinned)
+{
+ unsigned long flags;
+ unsigned int cpu;
+ u64 vmid, old_active_vmid;
+
+ vmid = atomic64_read(id);
+
+ /*
+ * The memory ordering here is subtle.
+ * If our active_vmids is non-zero and the VMID matches the current
+ * generation, then we update the active_vmids entry with a relaxed
+ * cmpxchg. Racing with a concurrent rollover means that either:
+ *
+ * - We get a zero back from the cmpxchg and end up waiting on the
+ * lock. Taking the lock synchronises with the rollover and so
+ * we are forced to see the updated generation.
+ *
+ * - We get a valid VMID back from the cmpxchg, which means the
+ * relaxed xchg in flush_context will treat us as reserved
+ * because atomic RmWs are totally ordered for a given location.
+ */
+ old_active_vmid = atomic64_read(this_cpu_ptr(&active_vmids));
+ if (old_active_vmid && vmid_gen_match(vmid) &&
+ atomic64_cmpxchg_relaxed(this_cpu_ptr(&active_vmids),
+ old_active_vmid, vmid))
+ return;
+
+ raw_spin_lock_irqsave(&cpu_vmid_lock, flags);
+ /* Check that our VMID belongs to the current generation. */
+ vmid = atomic64_read(id);
+ if (!vmid_gen_match(vmid)) {
+ vmid = new_vmid(id, pinned);
+ atomic64_set(id, vmid);
+ }
+
+ cpu = smp_processor_id();
+ if (cpumask_test_and_clear_cpu(cpu, &tlb_flush_pending))
+ kvm_call_hyp(__kvm_tlb_flush_local_all);
+
+ atomic64_set(this_cpu_ptr(&active_vmids), vmid);
+ raw_spin_unlock_irqrestore(&cpu_vmid_lock, flags);
+}
+
+unsigned long kvm_arm_pinned_vmid_get(atomic64_t *id, refcount_t *pinned)
+{
+ unsigned long flags;
+ u64 vmid;
+
+ if (!pinned_vmid_map)
+ return 0;
+
+ raw_spin_lock_irqsave(&cpu_vmid_lock, flags);
+
+ vmid = atomic64_read(id);
+
+ if (refcount_inc_not_zero(pinned))
+ goto out_unlock;
+
+ if (nr_pinned_vmids >= max_pinned_vmids) {
+ vmid = 0;
+ goto out_unlock;
+ }
+
+ if (!vmid_gen_match(vmid)) {
+ /*
+ * We went through one or more rollover since that VMID was
+ * used. Ensure that it is still valid, or generate a new one.
+ */
+ vmid = new_vmid(id, pinned);
+ atomic64_set(id, vmid);
+ }
+
+ nr_pinned_vmids++;
+ __set_bit(vmid2idx(vmid), pinned_vmid_map);
+ refcount_set(pinned, 1);
+
+out_unlock:
+ raw_spin_unlock_irqrestore(&cpu_vmid_lock, flags);
+
+ vmid &= ~VMID_MASK;
+
+ return vmid;
+}
+
+void kvm_arm_pinned_vmid_put(atomic64_t *id, refcount_t *pinned)
+{
+ unsigned long flags;
+ u64 vmid = atomic64_read(id);
+
+ if (!pinned_vmid_map)
+ return;
+
+ raw_spin_lock_irqsave(&cpu_vmid_lock, flags);
+
+ if (refcount_dec_and_test(pinned)) {
+ __clear_bit(vmid2idx(vmid), pinned_vmid_map);
+ nr_pinned_vmids--;
+ }
+
+ raw_spin_unlock_irqrestore(&cpu_vmid_lock, flags);
+}
+
+/*
+ * Initialize the VMID allocator
+ */
+int kvm_arm_vmid_alloc_init(void)
+{
+ vmid_bits = kvm_get_vmid_bits();
+
+ /*
+ * Expect allocation after rollover to fail if we don't have at least
+ * one more VMID than CPUs. VMID #0 is always reserved.
+ */
+ WARN_ON(NUM_USER_VMIDS - 1 <= num_possible_cpus());
+ atomic64_set(&vmid_generation, VMID_FIRST_VERSION);
+ vmid_map = kcalloc(BITS_TO_LONGS(NUM_USER_VMIDS),
+ sizeof(*vmid_map), GFP_KERNEL);
+ if (!vmid_map)
+ return -ENOMEM;
+
+ pinned_vmid_map = kcalloc(BITS_TO_LONGS(NUM_USER_VMIDS),
+ sizeof(*pinned_vmid_map), GFP_KERNEL);
+ nr_pinned_vmids = 0;
+ max_pinned_vmids = NUM_USER_VMIDS - num_possible_cpus() - 2;
+
+ return 0;
+}
+
+void kvm_arm_vmid_alloc_free(void)
+{
+ kfree(vmid_map);
+ kfree(pinned_vmid_map);
+}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [RFC PATCH 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one
2021-05-06 16:52 [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameer Kolothum
2021-05-06 16:52 ` [RFC PATCH 1/3] arch/arm64: Introduce a capability to tell whether 16-bit VMID is available Shameer Kolothum
2021-05-06 16:52 ` [RFC PATCH 2/3] kvm/arm: Introduce a new vmid allocator for KVM Shameer Kolothum
@ 2021-05-06 16:52 ` Shameer Kolothum
2021-06-04 8:13 ` [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameerali Kolothum Thodi
3 siblings, 0 replies; 9+ messages in thread
From: Shameer Kolothum @ 2021-05-06 16:52 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: maz, will, catalin.marinas, james.morse, julien.thierry.kdev,
suzuki.poulose, jean-philippe, linuxarm
From: Julien Grall <julien.grall@arm.com>
At the moment, the VMID algorithm will send an SGI to all the CPUs to
force an exit and then broadcast a full TLB flush and I-Cache
invalidation.
This patch use the new VMID allocator. The
benefits are:
- CPUs are not forced to exit at roll-over. Instead the VMID will be
marked reserved and the context will be flushed at next exit. This
will reduce the IPIs traffic.
- Context invalidation is now per-CPU rather than broadcasted.
- Catalin has a formal model of the ASID allocator.
With the new algo, the code is now adapted:
- The function __kvm_flush_vm_context() has been renamed to
__kvm_tlb_flush_local_all() and now only flushing the current CPU
context.
- The call to update_vmid() will be done with preemption disabled
as the new algo requires to store information per-CPU.
- The TLBs associated to EL1 will be flushed when booting a CPU to
deal with stale information. This was previously done on the
allocation of the first VMID of a new generation.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
arch/arm64/include/asm/kvm_asm.h | 4 +-
arch/arm64/include/asm/kvm_host.h | 5 +-
arch/arm64/include/asm/kvm_mmu.h | 3 +-
arch/arm64/kvm/Makefile | 2 +-
arch/arm64/kvm/arm.c | 115 ++++++++---------------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 +-
arch/arm64/kvm/hyp/nvhe/tlb.c | 10 +--
arch/arm64/kvm/hyp/vhe/tlb.c | 10 +--
arch/arm64/kvm/mmu.c | 1 -
9 files changed, 51 insertions(+), 105 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index a7ab84f781f7..29697c5ab2c2 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -44,7 +44,7 @@
#define __KVM_HOST_SMCCC_FUNC___kvm_hyp_init 0
#define __KVM_HOST_SMCCC_FUNC___kvm_vcpu_run 1
-#define __KVM_HOST_SMCCC_FUNC___kvm_flush_vm_context 2
+#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_local_all 2
#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa 3
#define __KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid 4
#define __KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context 5
@@ -182,7 +182,7 @@ DECLARE_KVM_NVHE_SYM(__per_cpu_end);
DECLARE_KVM_HYP_SYM(__bp_harden_hyp_vecs);
#define __bp_harden_hyp_vecs CHOOSE_HYP_SYM(__bp_harden_hyp_vecs)
-extern void __kvm_flush_vm_context(void);
+extern void __kvm_tlb_flush_local_all(void);
extern void __kvm_flush_cpu_context(struct kvm_s2_mmu *mmu);
extern void __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu, phys_addr_t ipa,
int level);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 9d476f1f34af..c06370f387cb 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -70,9 +70,7 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu);
struct kvm_vmid {
- /* The VMID generation used for the virt. memory system */
- u64 vmid_gen;
- u32 vmid;
+ atomic64_t id;
};
struct kvm_s2_mmu {
@@ -631,7 +629,6 @@ void kvm_arm_resume_guest(struct kvm *kvm);
ret; \
})
-void force_vm_exit(const cpumask_t *mask);
void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot);
int handle_exit(struct kvm_vcpu *vcpu, int exception_index);
diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index c3080966ef83..43e83df87e3a 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -252,7 +252,8 @@ static __always_inline u64 kvm_get_vttbr(struct kvm_s2_mmu *mmu)
u64 cnp = system_supports_cnp() ? VTTBR_CNP_BIT : 0;
baddr = mmu->pgd_phys;
- vmid_field = (u64)vmid->vmid << VTTBR_VMID_SHIFT;
+ vmid_field = atomic64_read(&vmid->id) << VTTBR_VMID_SHIFT;
+ vmid_field &= VTTBR_VMID_MASK(kvm_get_vmid_bits());
return kvm_phys_to_vttbr(baddr) | vmid_field | cnp;
}
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index 589921392cb1..717c4cbf557a 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -16,7 +16,7 @@ kvm-y := $(KVM)/kvm_main.o $(KVM)/coalesced_mmio.o $(KVM)/eventfd.o \
inject_fault.o va_layout.o handle_exit.o \
guest.o debug.o reset.o sys_regs.o \
vgic-sys-reg-v3.o fpsimd.o pmu.o \
- arch_timer.o trng.o\
+ arch_timer.o trng.o vmid.o \
vgic/vgic.o vgic/vgic-init.o \
vgic/vgic-irqfd.o vgic/vgic-v2.o \
vgic/vgic-v3.o vgic/vgic-v4.o \
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7f06ba76698d..4f58db358f72 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -55,11 +55,6 @@ static DEFINE_PER_CPU(unsigned long, kvm_arm_hyp_stack_page);
unsigned long kvm_arm_hyp_percpu_base[NR_CPUS];
DECLARE_KVM_NVHE_PER_CPU(struct kvm_nvhe_init_params, kvm_init_params);
-/* The VMID used in the VTTBR */
-static atomic64_t kvm_vmid_gen = ATOMIC64_INIT(1);
-static u32 kvm_next_vmid;
-static DEFINE_SPINLOCK(kvm_vmid_lock);
-
static bool vgic_present;
static DEFINE_PER_CPU(unsigned char, kvm_arm_hardware_enabled);
@@ -486,85 +481,13 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu)
return vcpu_mode_priv(vcpu);
}
-/* Just ensure a guest exit from a particular CPU */
-static void exit_vm_noop(void *info)
-{
-}
-
-void force_vm_exit(const cpumask_t *mask)
-{
- preempt_disable();
- smp_call_function_many(mask, exit_vm_noop, NULL, true);
- preempt_enable();
-}
-
-/**
- * need_new_vmid_gen - check that the VMID is still valid
- * @vmid: The VMID to check
- *
- * return true if there is a new generation of VMIDs being used
- *
- * The hardware supports a limited set of values with the value zero reserved
- * for the host, so we check if an assigned value belongs to a previous
- * generation, which requires us to assign a new value. If we're the first to
- * use a VMID for the new generation, we must flush necessary caches and TLBs
- * on all CPUs.
- */
-static bool need_new_vmid_gen(struct kvm_vmid *vmid)
-{
- u64 current_vmid_gen = atomic64_read(&kvm_vmid_gen);
- smp_rmb(); /* Orders read of kvm_vmid_gen and kvm->arch.vmid */
- return unlikely(READ_ONCE(vmid->vmid_gen) != current_vmid_gen);
-}
-
/**
* update_vmid - Update the vmid with a valid VMID for the current generation
* @vmid: The stage-2 VMID information struct
*/
static void update_vmid(struct kvm_vmid *vmid)
{
- if (!need_new_vmid_gen(vmid))
- return;
-
- spin_lock(&kvm_vmid_lock);
-
- /*
- * We need to re-check the vmid_gen here to ensure that if another vcpu
- * already allocated a valid vmid for this vm, then this vcpu should
- * use the same vmid.
- */
- if (!need_new_vmid_gen(vmid)) {
- spin_unlock(&kvm_vmid_lock);
- return;
- }
-
- /* First user of a new VMID generation? */
- if (unlikely(kvm_next_vmid == 0)) {
- atomic64_inc(&kvm_vmid_gen);
- kvm_next_vmid = 1;
-
- /*
- * On SMP we know no other CPUs can use this CPU's or each
- * other's VMID after force_vm_exit returns since the
- * kvm_vmid_lock blocks them from reentry to the guest.
- */
- force_vm_exit(cpu_all_mask);
- /*
- * Now broadcast TLB + ICACHE invalidation over the inner
- * shareable domain to make sure all data structures are
- * clean.
- */
- kvm_call_hyp(__kvm_flush_vm_context);
- }
-
- vmid->vmid = kvm_next_vmid;
- kvm_next_vmid++;
- kvm_next_vmid &= (1 << kvm_get_vmid_bits()) - 1;
-
- smp_wmb();
- WRITE_ONCE(vmid->vmid_gen, atomic64_read(&kvm_vmid_gen));
-
- spin_unlock(&kvm_vmid_lock);
+ kvm_arm_update_vmid(&vmid->id, NULL);
}
static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
@@ -728,8 +651,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
*/
cond_resched();
- update_vmid(&vcpu->arch.hw_mmu->vmid);
-
check_vcpu_requests(vcpu);
/*
@@ -739,6 +660,15 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
*/
preempt_disable();
+ /*
+ * The VMID allocator only tracks active VMIDs per
+ * physical CPU, and therefore the VMID allocated may not be
+ * preserved on VMID roll-over if the task was preempted,
+ * making a thread's VMID inactive. So we need to call
+ * update_vttbr in non-premptible context.
+ */
+ update_vmid(&vcpu->arch.hw_mmu->vmid);
+
kvm_pmu_flush_hwstate(vcpu);
local_irq_disable();
@@ -777,8 +707,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
*/
smp_store_mb(vcpu->mode, IN_GUEST_MODE);
- if (ret <= 0 || need_new_vmid_gen(&vcpu->arch.hw_mmu->vmid) ||
- kvm_request_pending(vcpu)) {
+ if (ret <= 0 || kvm_request_pending(vcpu)) {
vcpu->mode = OUTSIDE_GUEST_MODE;
isb(); /* Ensure work in x_flush_hwstate is committed */
kvm_pmu_sync_hwstate(vcpu);
@@ -1460,6 +1389,8 @@ static void cpu_hyp_reset(void)
{
if (!is_kernel_in_hyp_mode())
__hyp_reset_vectors();
+
+ kvm_call_hyp(__kvm_tlb_flush_local_all);
}
/*
@@ -1635,9 +1566,26 @@ static bool init_psci_relay(void)
static int init_common_resources(void)
{
+ int err;
+
+ /*
+ * Initialize the VMID allocator telling it to allocate a single
+ * VMID per VM.
+ */
+ err = kvm_arm_vmid_alloc_init();
+ if (err) {
+ kvm_err("Failed to initialize VMID allocator.\n");
+ return err;
+ }
+
return kvm_set_ipa_limit();
}
+static void free_common_resources(void)
+{
+ kvm_arm_vmid_alloc_free();
+}
+
static int init_subsystems(void)
{
int err = 0;
@@ -1918,7 +1866,7 @@ int kvm_arch_init(void *opaque)
err = kvm_arm_init_sve();
if (err)
- return err;
+ goto out_err;
if (!in_hyp_mode) {
err = init_hyp_mode();
@@ -1952,6 +1900,7 @@ int kvm_arch_init(void *opaque)
if (!in_hyp_mode)
teardown_hyp_mode();
out_err:
+ free_common_resources();
return err;
}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 936328207bde..62027448d534 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -25,9 +25,9 @@ static void handle___kvm_vcpu_run(struct kvm_cpu_context *host_ctxt)
cpu_reg(host_ctxt, 1) = __kvm_vcpu_run(kern_hyp_va(vcpu));
}
-static void handle___kvm_flush_vm_context(struct kvm_cpu_context *host_ctxt)
+static void handle___kvm_tlb_flush_local_all(struct kvm_cpu_context *host_ctxt)
{
- __kvm_flush_vm_context();
+ __kvm_tlb_flush_local_all();
}
static void handle___kvm_tlb_flush_vmid_ipa(struct kvm_cpu_context *host_ctxt)
@@ -112,7 +112,7 @@ typedef void (*hcall_t)(struct kvm_cpu_context *);
static const hcall_t host_hcall[] = {
HANDLE_FUNC(__kvm_vcpu_run),
- HANDLE_FUNC(__kvm_flush_vm_context),
+ HANDLE_FUNC(__kvm_tlb_flush_local_all),
HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa),
HANDLE_FUNC(__kvm_tlb_flush_vmid),
HANDLE_FUNC(__kvm_flush_cpu_context),
diff --git a/arch/arm64/kvm/hyp/nvhe/tlb.c b/arch/arm64/kvm/hyp/nvhe/tlb.c
index 229b06748c20..3f1fc5125e9e 100644
--- a/arch/arm64/kvm/hyp/nvhe/tlb.c
+++ b/arch/arm64/kvm/hyp/nvhe/tlb.c
@@ -138,10 +138,10 @@ void __kvm_flush_cpu_context(struct kvm_s2_mmu *mmu)
__tlb_switch_to_host(&cxt);
}
-void __kvm_flush_vm_context(void)
+void __kvm_tlb_flush_local_all(void)
{
- dsb(ishst);
- __tlbi(alle1is);
+ dsb(nshst);
+ __tlbi(alle1);
/*
* VIPT and PIPT caches are not affected by VMID, so no maintenance
@@ -153,7 +153,7 @@ void __kvm_flush_vm_context(void)
*
*/
if (icache_is_vpipt())
- asm volatile("ic ialluis");
+ asm volatile("ic iallu" : : );
- dsb(ish);
+ dsb(nsh);
}
diff --git a/arch/arm64/kvm/hyp/vhe/tlb.c b/arch/arm64/kvm/hyp/vhe/tlb.c
index 66f17349f0c3..89f229e77b7d 100644
--- a/arch/arm64/kvm/hyp/vhe/tlb.c
+++ b/arch/arm64/kvm/hyp/vhe/tlb.c
@@ -142,10 +142,10 @@ void __kvm_flush_cpu_context(struct kvm_s2_mmu *mmu)
__tlb_switch_to_host(&cxt);
}
-void __kvm_flush_vm_context(void)
+void __kvm_tlb_flush_local_all(void)
{
- dsb(ishst);
- __tlbi(alle1is);
+ dsb(nshst);
+ __tlbi(alle1);
/*
* VIPT and PIPT caches are not affected by VMID, so no maintenance
@@ -157,7 +157,7 @@ void __kvm_flush_vm_context(void)
*
*/
if (icache_is_vpipt())
- asm volatile("ic ialluis");
+ asm volatile("ic iallu" : : );
- dsb(ish);
+ dsb(nsh);
}
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 8711894db8c2..4933fc9a13fb 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -390,7 +390,6 @@ int kvm_init_stage2_mmu(struct kvm *kvm, struct kvm_s2_mmu *mmu)
mmu->kvm = kvm;
mmu->pgt = pgt;
mmu->pgd_phys = __pa(pgt->pgd);
- mmu->vmid.vmid_gen = 0;
return 0;
out_destroy_pgtable:
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach)
2021-05-06 16:52 [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameer Kolothum
` (2 preceding siblings ...)
2021-05-06 16:52 ` [RFC PATCH 3/3] kvm/arm: Align the VMID allocation with the arm64 ASID one Shameer Kolothum
@ 2021-06-04 8:13 ` Shameerali Kolothum Thodi
2021-06-04 13:54 ` Marc Zyngier
3 siblings, 1 reply; 9+ messages in thread
From: Shameerali Kolothum Thodi @ 2021-06-04 8:13 UTC (permalink / raw)
To: linux-arm-kernel, kvmarm, linux-kernel
Cc: maz, will, catalin.marinas, james.morse, julien.thierry.kdev,
suzuki.poulose, jean-philippe, Linuxarm
Hi,
> -----Original Message-----
> From: Shameerali Kolothum Thodi
> Sent: 06 May 2021 17:52
> To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> linux-kernel@vger.kernel.org
> Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; julien.thierry.kdev@gmail.com;
> suzuki.poulose@arm.com; jean-philippe@linaro.org; Linuxarm
> <linuxarm@huawei.com>
> Subject: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> approach)
>
> This is based on a suggestion from Will [0] to try out the asid
> based kvm vmid solution as a separate VMID allocator instead of
> the shared lib approach attempted in v4[1].
>
> The idea is to compare both the approaches and see whether the
> shared lib solution with callbacks make sense or not.
A gentle ping on this. Please take a look and let me know.
Thanks,
Shameer
> Though we are not yet using the pinned vmids yet, patch #2 has
> code for pinned vmid support. This is just to help the comparison.
>
> Test Setup/Results
> ----------------
> The measurement was made with maxcpus set to 8 and with the
> number of VMID limited to 4-bit. The test involves running
> concurrently 40 guests with 2 vCPUs. Each guest will then
> execute hackbench 5 times before exiting.
>
> The performance difference between the current algo and the
> new one are(avg. of 10 runs):
> - 1.9% less entry/exit from the guest
> - 0.5% faster
> This is more or less comparable to v4 numbers.
>
> For the complete series, please see,
> https://github.com/hisilicon/kernel-dev/tree/private-v5.12-rc7-vmid-2nd-rfc
>
> and for the shared asid lib v4 solution,
> https://github.com/hisilicon/kernel-dev/tree/private-v5.12-rc7-asid-v4
>
> As you can see there are of course code duplication with this
> approach but may be this one is more easy to maintain considering
> the complexity involved.
>
> Please take a look and let me know your feedback.
>
> Thanks,
> Shameer
>
>
> [0] https://lore.kernel.org/lkml/20210422160846.GB2214@willie-the-truck/
> [1]
> https://lore.kernel.org/lkml/20210414112312.13704-1-shameerali.kolothum.t
> hodi@huawei.com/
>
> Julien Grall (2):
> arch/arm64: Introduce a capability to tell whether 16-bit VMID is
> available
> kvm/arm: Align the VMID allocation with the arm64 ASID one
>
> Shameer Kolothum (1):
> kvm/arm: Introduce a new vmid allocator for KVM
>
> arch/arm64/include/asm/cpucaps.h | 3 +-
> arch/arm64/include/asm/kvm_asm.h | 4 +-
> arch/arm64/include/asm/kvm_host.h | 11 +-
> arch/arm64/include/asm/kvm_mmu.h | 7 +-
> arch/arm64/kernel/cpufeature.c | 9 +
> arch/arm64/kvm/Makefile | 2 +-
> arch/arm64/kvm/arm.c | 115 ++++--------
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 6 +-
> arch/arm64/kvm/hyp/nvhe/tlb.c | 10 +-
> arch/arm64/kvm/hyp/vhe/tlb.c | 10 +-
> arch/arm64/kvm/mmu.c | 1 -
> arch/arm64/kvm/vmid.c | 285
> +++++++++++++++++++++++++++++
> 12 files changed, 354 insertions(+), 109 deletions(-)
> create mode 100644 arch/arm64/kvm/vmid.c
>
> --
> 2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach)
2021-06-04 8:13 ` [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach) Shameerali Kolothum Thodi
@ 2021-06-04 13:54 ` Marc Zyngier
2021-06-04 14:51 ` Shameerali Kolothum Thodi
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2021-06-04 13:54 UTC (permalink / raw)
To: Shameerali Kolothum Thodi
Cc: linux-arm-kernel, kvmarm, linux-kernel, will, catalin.marinas,
james.morse, julien.thierry.kdev, suzuki.poulose, jean-philippe,
Alexandru Elisei, Linuxarm
On Fri, 04 Jun 2021 09:13:10 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Shameerali Kolothum Thodi
> > Sent: 06 May 2021 17:52
> > To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> > linux-kernel@vger.kernel.org
> > Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> > james.morse@arm.com; julien.thierry.kdev@gmail.com;
> > suzuki.poulose@arm.com; jean-philippe@linaro.org; Linuxarm
> > <linuxarm@huawei.com>
> > Subject: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> > approach)
> >
> > This is based on a suggestion from Will [0] to try out the asid
> > based kvm vmid solution as a separate VMID allocator instead of
> > the shared lib approach attempted in v4[1].
> >
> > The idea is to compare both the approaches and see whether the
> > shared lib solution with callbacks make sense or not.
>
> A gentle ping on this. Please take a look and let me know.
I had a look and I don't overly dislike it. I'd like to see the code
without the pinned stuff though, at least to ease the reviewing. I
haven't tested it in anger, but I have pushed the rebased code at [1]
as it really didn't apply to 5.13-rc4.
One thing I'm a bit worried about is that we so far relied on VMID 0
never being allocated to a guest, which is now crucial for protected
KVM. I can't really convince myself that this can never happen with
this. Plus, I've found this nugget:
<quote
max_pinned_vmids = NUM_USER_VMIDS - num_possible_cpus() - 2;
</quote>
What is this "- 2"? My hunch is that it should really be "- 1" as VMID
0 is reserved, and we have no equivalent of KPTI for S2.
M.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/mmu/vmid
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach)
2021-06-04 13:54 ` Marc Zyngier
@ 2021-06-04 14:51 ` Shameerali Kolothum Thodi
2021-06-04 15:27 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Shameerali Kolothum Thodi @ 2021-06-04 14:51 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-arm-kernel, kvmarm, linux-kernel, will, catalin.marinas,
james.morse, julien.thierry.kdev, suzuki.poulose, jean-philippe,
Alexandru Elisei, Linuxarm
Hi Marc,
> -----Original Message-----
> From: Marc Zyngier [mailto:maz@kernel.org]
> Sent: 04 June 2021 14:55
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> linux-kernel@vger.kernel.org; will@kernel.org; catalin.marinas@arm.com;
> james.morse@arm.com; julien.thierry.kdev@gmail.com;
> suzuki.poulose@arm.com; jean-philippe@linaro.org; Alexandru Elisei
> <Alexandru.Elisei@arm.com>; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> approach)
>
> On Fri, 04 Jun 2021 09:13:10 +0100,
> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> wrote:
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Shameerali Kolothum Thodi
> > > Sent: 06 May 2021 17:52
> > > To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> > > linux-kernel@vger.kernel.org
> > > Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> > > james.morse@arm.com; julien.thierry.kdev@gmail.com;
> > > suzuki.poulose@arm.com; jean-philippe@linaro.org; Linuxarm
> > > <linuxarm@huawei.com>
> > > Subject: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> > > approach)
> > >
> > > This is based on a suggestion from Will [0] to try out the asid
> > > based kvm vmid solution as a separate VMID allocator instead of
> > > the shared lib approach attempted in v4[1].
> > >
> > > The idea is to compare both the approaches and see whether the
> > > shared lib solution with callbacks make sense or not.
> >
> > A gentle ping on this. Please take a look and let me know.
>
> I had a look and I don't overly dislike it. I'd like to see the code
> without the pinned stuff though, at least to ease the reviewing. I
> haven't tested it in anger, but I have pushed the rebased code at [1]
> as it really didn't apply to 5.13-rc4.
Thanks for taking a look and the rebase. I will remove the pinned stuff
in the next revision as that was added just to compare against the previous
version.
>
> One thing I'm a bit worried about is that we so far relied on VMID 0
> never being allocated to a guest, which is now crucial for protected
> KVM. I can't really convince myself that this can never happen with
> this.
Hmm..not sure I quite follow that. As per the current logic vmid 0 is
reserved and is not allocated to Guest.
> Plus, I've found this nugget:
>
> <quote
> max_pinned_vmids = NUM_USER_VMIDS - num_possible_cpus() - 2;
> </quote>
>
> What is this "- 2"? My hunch is that it should really be "- 1" as VMID
> 0 is reserved, and we have no equivalent of KPTI for S2.
I think this is more related to the "pinned vmid" stuff and was borrowed from
the asid_update_limit() fn in arch/arm64/mm/context.c. But I missed the
comment that explains the reason behind it. It says,
---x---
/*
* There must always be an ASID available after rollover. Ensure that,
* even if all CPUs have a reserved ASID and the maximum number of ASIDs
* are pinned, there still is at least one empty slot in the ASID map.
*/
max_pinned_asids = num_available_asids - num_possible_cpus() - 2;
---x---
So this is to make sure we will have at least one VMID available after rollover
in case we have pinned the max number of VMIDs. I will include that comment
to make it clear.
Thanks,
Shameer
>
> M.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h
> =kvm-arm64/mmu/vmid
>
> --
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach)
2021-06-04 14:51 ` Shameerali Kolothum Thodi
@ 2021-06-04 15:27 ` Marc Zyngier
2021-06-07 8:48 ` Jean-Philippe Brucker
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2021-06-04 15:27 UTC (permalink / raw)
To: Shameerali Kolothum Thodi
Cc: linux-arm-kernel, kvmarm, linux-kernel, will, catalin.marinas,
james.morse, julien.thierry.kdev, suzuki.poulose, jean-philippe,
Alexandru Elisei, Linuxarm
On Fri, 04 Jun 2021 15:51:29 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
>
> Hi Marc,
>
> > -----Original Message-----
> > From: Marc Zyngier [mailto:maz@kernel.org]
> > Sent: 04 June 2021 14:55
> > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > Cc: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> > linux-kernel@vger.kernel.org; will@kernel.org; catalin.marinas@arm.com;
> > james.morse@arm.com; julien.thierry.kdev@gmail.com;
> > suzuki.poulose@arm.com; jean-philippe@linaro.org; Alexandru Elisei
> > <Alexandru.Elisei@arm.com>; Linuxarm <linuxarm@huawei.com>
> > Subject: Re: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> > approach)
> >
> > On Fri, 04 Jun 2021 09:13:10 +0100,
> > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > wrote:
> > >
> > > Hi,
> > >
> > > > -----Original Message-----
> > > > From: Shameerali Kolothum Thodi
> > > > Sent: 06 May 2021 17:52
> > > > To: linux-arm-kernel@lists.infradead.org; kvmarm@lists.cs.columbia.edu;
> > > > linux-kernel@vger.kernel.org
> > > > Cc: maz@kernel.org; will@kernel.org; catalin.marinas@arm.com;
> > > > james.morse@arm.com; julien.thierry.kdev@gmail.com;
> > > > suzuki.poulose@arm.com; jean-philippe@linaro.org; Linuxarm
> > > > <linuxarm@huawei.com>
> > > > Subject: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd
> > > > approach)
> > > >
> > > > This is based on a suggestion from Will [0] to try out the asid
> > > > based kvm vmid solution as a separate VMID allocator instead of
> > > > the shared lib approach attempted in v4[1].
> > > >
> > > > The idea is to compare both the approaches and see whether the
> > > > shared lib solution with callbacks make sense or not.
> > >
> > > A gentle ping on this. Please take a look and let me know.
> >
> > I had a look and I don't overly dislike it. I'd like to see the code
> > without the pinned stuff though, at least to ease the reviewing. I
> > haven't tested it in anger, but I have pushed the rebased code at [1]
> > as it really didn't apply to 5.13-rc4.
>
> Thanks for taking a look and the rebase. I will remove the pinned stuff
> in the next revision as that was added just to compare against the previous
> version.
>
> >
> > One thing I'm a bit worried about is that we so far relied on VMID 0
> > never being allocated to a guest, which is now crucial for protected
> > KVM. I can't really convince myself that this can never happen with
> > this.
>
> Hmm..not sure I quite follow that. As per the current logic vmid 0 is
> reserved and is not allocated to Guest.
And that's the bit I'm struggling to validate here. I guess it works
because cur_idx is set to 1 in new_vmid().
>
> > Plus, I've found this nugget:
> >
> > <quote
> > max_pinned_vmids = NUM_USER_VMIDS - num_possible_cpus() - 2;
> > </quote>
> >
> > What is this "- 2"? My hunch is that it should really be "- 1" as VMID
> > 0 is reserved, and we have no equivalent of KPTI for S2.
>
> I think this is more related to the "pinned vmid" stuff and was borrowed from
> the asid_update_limit() fn in arch/arm64/mm/context.c. But I missed the
> comment that explains the reason behind it. It says,
>
> ---x---
> /*
> * There must always be an ASID available after rollover. Ensure that,
> * even if all CPUs have a reserved ASID and the maximum number of ASIDs
> * are pinned, there still is at least one empty slot in the ASID map.
> */
> max_pinned_asids = num_available_asids - num_possible_cpus() - 2;
> ---x---
>
> So this is to make sure we will have at least one VMID available
> after rollover in case we have pinned the max number of VMIDs. I
> will include that comment to make it clear.
That doesn't really explain the -2. Or is that that we have one for
the extra empty slot, and one for the reserved?
Jean-Philippe?
Thanks,
M.
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH 0/3] kvm/arm: New VMID allocator based on asid(2nd approach)
2021-06-04 15:27 ` Marc Zyngier
@ 2021-06-07 8:48 ` Jean-Philippe Brucker
0 siblings, 0 replies; 9+ messages in thread
From: Jean-Philippe Brucker @ 2021-06-07 8:48 UTC (permalink / raw)
To: Marc Zyngier
Cc: Shameerali Kolothum Thodi, linux-arm-kernel, kvmarm,
linux-kernel, will, catalin.marinas, james.morse,
julien.thierry.kdev, suzuki.poulose, Alexandru Elisei, Linuxarm
On Fri, Jun 04, 2021 at 04:27:39PM +0100, Marc Zyngier wrote:
> > > Plus, I've found this nugget:
> > >
> > > <quote
> > > max_pinned_vmids = NUM_USER_VMIDS - num_possible_cpus() - 2;
> > > </quote>
> > >
> > > What is this "- 2"? My hunch is that it should really be "- 1" as VMID
> > > 0 is reserved, and we have no equivalent of KPTI for S2.
> >
> > I think this is more related to the "pinned vmid" stuff and was borrowed from
> > the asid_update_limit() fn in arch/arm64/mm/context.c. But I missed the
> > comment that explains the reason behind it. It says,
> >
> > ---x---
> > /*
> > * There must always be an ASID available after rollover. Ensure that,
> > * even if all CPUs have a reserved ASID and the maximum number of ASIDs
> > * are pinned, there still is at least one empty slot in the ASID map.
> > */
> > max_pinned_asids = num_available_asids - num_possible_cpus() - 2;
> > ---x---
> >
> > So this is to make sure we will have at least one VMID available
> > after rollover in case we have pinned the max number of VMIDs. I
> > will include that comment to make it clear.
>
> That doesn't really explain the -2. Or is that that we have one for
> the extra empty slot, and one for the reserved?
>
> Jean-Philippe?
Yes, -2 is for ASID#0 and the extra empty slot. A comment higher in
asids_update_limit() hints at that, but it could definitely be clearer
/*
* Expect allocation after rollover to fail if we don't have at least
* one more ASID than CPUs. ASID #0 is reserved for init_mm.
*/
Thanks,
Jean
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread