[RFC PATCH 0/5] KVM/ARM64 Add support for pinned VMIDs

On an ARM64 system with a SMMUv3 implementation that fully supports
Broadcast TLB Maintenance(BTM) feature as part of the Distributed
Virtual Memory(DVM) protocol, the CPU TLB invalidate instructions are
received by SMMUv3. This is very useful when the SMMUv3 shares the
page tables with the CPU(eg: Guest SVA use case). For this to work,
the SMMU must use the same VMID that is allocated by KVM to configure
the stage 2 translations. At present KVM VMID allocations are recycled
on rollover and may change as a result. This will create issues if we
have to share the KVM VMID with SMMU.
 
Please see the discussion here,
https://lore.kernel.org/linux-iommu/20200522101755.GA3453945@myrica/

This series proposes a way to share the VMID between KVM and IOMMU
driver by,

1. Splitting the KVM VMID space into two equal halves based on the
   command line option "kvm-arm.pinned_vmid_enable".
2. First half of the VMID space follows the normal recycle on rollover
   policy.
3. Second half of the VMID space doesn't roll over and is used to
   allocate pinned VMIDs.
4. Provides helper function to retrieve the KVM instance associated
   with a device(if it is part of a vfio group).
5. Introduces generic interfaces to get/put pinned KVM VMIDs.

Open Items:
1. I couldn't figure out a way to determine whether a platform actually
   fully supports DVM/BTM or not. Not sure we can take a call based on
   SMMUv3 BTM feature bit alone. Probably we can get it from firmware
   via IORT?
2. The current splitting of VMID space is only one way to do this and
   probably not the best. Maybe we can follow the pinned ASID method used
   in SVA code. Suggestions welcome here.
3. The detach_pasid_table() interface is not very clear to me as the current
   Qemu prototype is not  using that. This requires fixing from my side.
 
This is based on Jean-Philippe's SVA series[1] and Eric's SMMUv3 dual-stage
support series[2].

The branch with the whole vSVA + BTM solution is here,
https://github.com/hisilicon/kernel-dev/tree/5.10-rc4-2stage-v13-vsva-btm-rfc

This is lightly tested on a HiSilicon D06 platform with uacce/zip dev test tool,
./zip_sva_per -k tlb

Thanks,
Shameer

1. https://github.com/Linaro/linux-kernel-uadk/commits/uacce-devel-5.10
2. https://lore.kernel.org/linux-iommu/20201118112151.25412-1-eric.auger@redhat.com/T/

Shameer Kolothum (5):
  vfio: Add a helper to retrieve kvm instance from a dev
  KVM: Add generic infrastructure to support pinned VMIDs
  KVM: ARM64: Add support for pinned VMIDs
  iommu/arm-smmu-v3: Use pinned VMID for NESTED stage with BTM
  KVM: arm64: Make sure pinned vmid is released on VM exit

 arch/arm64/include/asm/kvm_host.h           |   2 +
 arch/arm64/kvm/Kconfig                      |   1 +
 arch/arm64/kvm/arm.c                        | 116 +++++++++++++++++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  49 ++++++++-
 drivers/vfio/vfio.c                         |  12 ++
 include/linux/kvm_host.h                    |  17 +++
 include/linux/vfio.h                        |   1 +
 virt/kvm/Kconfig                            |   2 +
 virt/kvm/kvm_main.c                         |  25 +++++
 9 files changed, 220 insertions(+), 5 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

A device that belongs to vfio_group has the kvm instance associated
with it. Retrieve it.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/vfio/vfio.c  | 12 ++++++++++++
 include/linux/vfio.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 2151bc7f87ab..d1968e4bf9f4 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c

@@ -876,6 +876,18 @@ struct vfio_device *vfio_device_get_from_dev(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(vfio_device_get_from_dev);
 
+struct kvm *vfio_kvm_get_from_dev(struct device *dev)
+{
+	struct vfio_group *group;
+
+	group = vfio_group_get_from_dev(dev);
+	if (!group)
+		return NULL;
+
+	return group->kvm;
+}
+EXPORT_SYMBOL_GPL(vfio_kvm_get_from_dev);
+
 static struct vfio_device *vfio_device_get_from_name(struct vfio_group *group,
 						     char *buf)
 {

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 38d3c6a8dc7e..8716298fee27 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h

@@ -56,6 +56,7 @@ extern void *vfio_del_group_dev(struct device *dev);
 extern struct vfio_device *vfio_device_get_from_dev(struct device *dev);
 extern void vfio_device_put(struct vfio_device *device);
 extern void *vfio_device_data(struct vfio_device *device);
+extern struct kvm *vfio_kvm_get_from_dev(struct device *dev);
 
 /**
  * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Provide generic helper functions to get/put pinned VMIDs if the arch
supports it.

Signed-off-by: Shameer Kolothum <redacted>
---
 include/linux/kvm_host.h | 17 +++++++++++++++++
 virt/kvm/Kconfig         |  2 ++
 virt/kvm/kvm_main.c      | 25 +++++++++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7f2e2a09ebbd..25856db74a08 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h

@@ -836,6 +836,8 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
 int kvm_vcpu_yield_to(struct kvm_vcpu *target);
 void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu, bool usermode_vcpu_not_eligible);
+int kvm_pinned_vmid_get(struct device *dev);
+int kvm_pinned_vmid_put(struct device *dev);
 
 void kvm_flush_remote_tlbs(struct kvm *kvm);
 void kvm_reload_remote_mmus(struct kvm *kvm);

@@ -1478,4 +1480,19 @@ static inline void kvm_handle_signal_exit(struct kvm_vcpu *vcpu)
 }
 #endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
 
+#ifdef CONFIG_HAVE_KVM_PINNED_VMID
+int kvm_arch_pinned_vmid_get(struct kvm *kvm);
+int kvm_arch_pinned_vmid_put(struct kvm *kvm);
+#else
+static inline int kvm_arch_pinned_vmid_get(struct kvm *kvm)
+{
+	return -EINVAL;
+}
+
+static inline int kvm_arch_pinned_vmid_put(struct kvm *kvm)
+{
+	return -EINVAL;
+}
+#endif
+
 #endif

diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 1c37ccd5d402..bb55616c5616 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig

@@ -63,3 +63,5 @@ config HAVE_KVM_NO_POLL
 
 config KVM_XFER_TO_GUEST_WORK
        bool
+config HAVE_KVM_PINNED_VMID
+       bool

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2541a17ff1c4..632d391f0e34 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c

@@ -51,6 +51,7 @@
 #include <linux/io.h>
 #include <linux/lockdep.h>
 #include <linux/kthread.h>
+#include <linux/vfio.h>
 
 #include <asm/processor.h>
 #include <asm/ioctl.h>

@@ -2849,6 +2850,30 @@ bool kvm_vcpu_wake_up(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_wake_up);
 
+int kvm_pinned_vmid_get(struct device *dev)
+{
+	struct kvm *kvm;
+
+	kvm = vfio_kvm_get_from_dev(dev);
+	if (!kvm)
+		return -EINVAL;
+
+	return kvm_arch_pinned_vmid_get(kvm);
+}
+EXPORT_SYMBOL_GPL(kvm_pinned_vmid_get);
+
+int kvm_pinned_vmid_put(struct device *dev)
+{
+	struct kvm *kvm;
+
+	kvm = vfio_kvm_get_from_dev(dev);
+	if (!kvm)
+		return -EINVAL;
+
+	return kvm_arch_pinned_vmid_put(kvm);
+}
+EXPORT_SYMBOL_GPL(kvm_pinned_vmid_put);
+
 #ifndef CONFIG_S390
 /*
  * Kick a sleeping VCPU, or a guest VCPU in guest mode, into host kernel mode.

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

If the SMMU supports BTM and the device belongs to NESTED domain
with shared pasid table, we need to use the VMID allocated by the
KVM for the s2 configuration. Hence, request a pinned VMID from KVM.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 49 ++++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 26bf7da1bcd0..04f83f7c8319 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -28,6 +28,7 @@
 #include <linux/pci.h>
 #include <linux/pci-ats.h>
 #include <linux/platform_device.h>
+#include <linux/kvm_host.h>
 
 #include <linux/amba/bus.h>
 

@@ -2195,6 +2196,33 @@ static void arm_smmu_bitmap_free(unsigned long *map, int idx)
 	clear_bit(idx, map);
 }
 
+static int arm_smmu_pinned_vmid_get(struct arm_smmu_domain *smmu_domain)
+{
+	struct arm_smmu_master *master;
+
+	master = list_first_entry_or_null(&smmu_domain->devices,
+					  struct arm_smmu_master, domain_head);
+	if (!master)
+		return -EINVAL;
+
+	return kvm_pinned_vmid_get(master->dev);
+}
+
+static int arm_smmu_pinned_vmid_put(struct arm_smmu_domain *smmu_domain)
+{
+	struct arm_smmu_master *master;
+
+	master = list_first_entry_or_null(&smmu_domain->devices,
+					  struct arm_smmu_master, domain_head);
+	if (!master)
+		return -EINVAL;
+
+	if (smmu_domain->s2_cfg.vmid)
+		return kvm_pinned_vmid_put(master->dev);
+
+	return 0;
+}
+
 static void arm_smmu_domain_free(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);

@@ -2215,8 +2243,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
 		mutex_unlock(&arm_smmu_asid_lock);
 	}
 	if (s2_cfg->set) {
-		if (s2_cfg->vmid)
-			arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
+		if (s2_cfg->vmid) {
+			if (!(smmu->features & ARM_SMMU_FEAT_BTM) &&
+			    smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+				arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
+		}
 	}
 
 	kfree(smmu_domain);

@@ -3199,6 +3230,17 @@ static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
 				!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
 			goto out;
 
+		if (smmu->features & ARM_SMMU_FEAT_BTM) {
+			ret = arm_smmu_pinned_vmid_get(smmu_domain);
+			if (ret < 0)
+				goto out;
+
+			if (smmu_domain->s2_cfg.vmid)
+				arm_smmu_bitmap_free(smmu->vmid_map, smmu_domain->s2_cfg.vmid);
+
+			smmu_domain->s2_cfg.vmid = (u16)ret;
+		}
+
 		smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
 		smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
 		smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;

@@ -3221,6 +3263,7 @@ static int arm_smmu_attach_pasid_table(struct iommu_domain *domain,
 static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_master *master;
 	unsigned long flags;
 

@@ -3237,6 +3280,8 @@ static void arm_smmu_detach_pasid_table(struct iommu_domain *domain)
 		arm_smmu_install_ste_for_dev(master);
 	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
+	if (smmu->features & ARM_SMMU_FEAT_BTM)
+		arm_smmu_pinned_vmid_put(smmu_domain);
 unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
 }

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

On an ARM64 system with a SMMUv3 implementation that fully supports
Broadcast TLB Maintenance(BTM) feature, the CPU TLB invalidate
instructions are received by SMMU. This is very useful when the
SMMU shares the page tables with the CPU(eg: Guest SVA use case).
For this to work, the SMMU must use the same VMID that is allocated
by KVM to configure the stage 2 translations.

At present KVM VMID allocations are recycled on rollover and may
change as a result. This will create issues if we have to share
the KVM VMID with SMMU. Hence, we spilt the KVM VMID space into
two, the first half follows the normal recycle on rollover policy
while the second half of the VMID pace is used to allocate pinned
VMIDs. This feature is enabled based on a command line option
"kvm-arm.pinned_vmid_enable".

Signed-off-by: Shameer Kolothum <redacted>
---
 arch/arm64/include/asm/kvm_host.h |   2 +
 arch/arm64/kvm/Kconfig            |   1 +
 arch/arm64/kvm/arm.c              | 104 +++++++++++++++++++++++++++++-
 3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0cd9f0f75c13..db6441c6a580 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h

@@ -25,6 +25,7 @@
 #include <asm/fpsimd.h>
 #include <asm/kvm.h>
 #include <asm/kvm_asm.h>
+#include <linux/refcount.h>
 #include <asm/thread_info.h>
 
 #define __KVM_HAVE_ARCH_INTC_INITIALIZED

@@ -65,6 +66,7 @@ struct kvm_vmid {
 	/* The VMID generation used for the virt. memory system */
 	u64    vmid_gen;
 	u32    vmid;
+	refcount_t   pinned;
 };
 
 struct kvm_s2_mmu {

diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 043756db8f6e..c5c52953e842 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig

@@ -40,6 +40,7 @@ menuconfig KVM
 	select HAVE_KVM_VCPU_RUN_PID_CHANGE
 	select TASKSTATS
 	select TASK_DELAY_ACCT
+	select HAVE_KVM_PINNED_VMID
 	help
 	  Support hosting virtualized guest machines.
 

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index c0ffb019ca8b..8955968be49f 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c

@@ -56,6 +56,19 @@ static atomic64_t kvm_vmid_gen = ATOMIC64_INIT(1);
 static u32 kvm_next_vmid;
 static DEFINE_SPINLOCK(kvm_vmid_lock);
 
+static bool kvm_pinned_vmid_enable;
+
+static int __init early_pinned_vmid_enable(char *buf)
+{
+	return strtobool(buf, &kvm_pinned_vmid_enable);
+}
+
+early_param("kvm-arm.pinned_vmid_enable", early_pinned_vmid_enable);
+
+static DEFINE_IDA(kvm_pinned_vmids);
+static u32 kvm_pinned_vmid_start;
+static u32 kvm_pinned_vmid_end;
+
 static bool vgic_present;
 
 static DEFINE_PER_CPU(unsigned char, kvm_arm_hardware_enabled);

@@ -475,6 +488,10 @@ void force_vm_exit(const cpumask_t *mask)
 static bool need_new_vmid_gen(struct kvm_vmid *vmid)
 {
 	u64 current_vmid_gen = atomic64_read(&kvm_vmid_gen);
+
+	if (refcount_read(&vmid->pinned))
+		return false;
+
 	smp_rmb(); /* Orders read of kvm_vmid_gen and kvm->arch.vmid */
 	return unlikely(READ_ONCE(vmid->vmid_gen) != current_vmid_gen);
 }

@@ -485,6 +502,8 @@ static bool need_new_vmid_gen(struct kvm_vmid *vmid)
  */
 static void update_vmid(struct kvm_vmid *vmid)
 {
+	unsigned int vmid_bits;
+
 	if (!need_new_vmid_gen(vmid))
 		return;
 

@@ -521,7 +540,12 @@ static void update_vmid(struct kvm_vmid *vmid)
 
 	vmid->vmid = kvm_next_vmid;
 	kvm_next_vmid++;
-	kvm_next_vmid &= (1 << kvm_get_vmid_bits()) - 1;
+	if (kvm_pinned_vmid_enable)
+		vmid_bits = kvm_get_vmid_bits() - 1;
+	else
+		vmid_bits = kvm_get_vmid_bits();
+
+	kvm_next_vmid &= (1 << vmid_bits) - 1;
 
 	smp_wmb();
 	WRITE_ONCE(vmid->vmid_gen, atomic64_read(&kvm_vmid_gen));

@@ -569,6 +593,71 @@ static int kvm_vcpu_first_run_init(struct kvm_vcpu *vcpu)
 	return ret;
 }
 
+int kvm_arch_pinned_vmid_get(struct kvm *kvm)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vmid *kvm_vmid;
+	int ret;
+
+	if (!kvm_pinned_vmid_enable || !atomic_read(&kvm->online_vcpus))
+		return -EINVAL;
+
+	vcpu = kvm_get_vcpu(kvm, 0);
+	if (!vcpu)
+		return -EINVAL;
+
+	kvm_vmid = &vcpu->arch.hw_mmu->vmid;
+
+	spin_lock(&kvm_vmid_lock);
+
+	if (refcount_inc_not_zero(&kvm_vmid->pinned)) {
+		spin_unlock(&kvm_vmid_lock);
+		return kvm_vmid->vmid;
+	}
+
+	ret = ida_alloc_range(&kvm_pinned_vmids, kvm_pinned_vmid_start,
+			      kvm_pinned_vmid_end, GFP_KERNEL);
+	if (ret < 0) {
+		spin_unlock(&kvm_vmid_lock);
+		return ret;
+	}
+
+	force_vm_exit(cpu_all_mask);
+	kvm_call_hyp(__kvm_flush_vm_context);
+
+	kvm_vmid->vmid = (u32)ret;
+	refcount_set(&kvm_vmid->pinned, 1);
+	spin_unlock(&kvm_vmid_lock);
+
+	return ret;
+}
+
+int kvm_arch_pinned_vmid_put(struct kvm *kvm)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vmid *kvm_vmid;
+
+	if (!kvm_pinned_vmid_enable)
+		return -EINVAL;
+
+	vcpu = kvm_get_vcpu(kvm, 0);
+	if (!vcpu)
+		return -EINVAL;
+
+	kvm_vmid = &vcpu->arch.hw_mmu->vmid;
+
+	spin_lock(&kvm_vmid_lock);
+
+	if (!refcount_read(&kvm_vmid->pinned))
+		goto out;
+
+	if (refcount_dec_and_test(&kvm_vmid->pinned))
+		ida_free(&kvm_pinned_vmids, kvm_vmid->vmid);
+out:
+	spin_unlock(&kvm_vmid_lock);
+	return 0;
+}
+
 bool kvm_arch_intc_initialized(struct kvm *kvm)
 {
 	return vgic_initialized(kvm);

@@ -1680,6 +1769,16 @@ static void check_kvm_target_cpu(void *ret)
 	*(int *)ret = kvm_target_cpu();
 }
 
+static void kvm_arm_pinned_vmid_init(void)
+{
+	unsigned int vmid_bits = kvm_get_vmid_bits();
+
+	kvm_pinned_vmid_start = (1 << (vmid_bits - 1));
+	kvm_pinned_vmid_end = (1 << vmid_bits) - 1;
+
+	kvm_info("Pinned VMID[0x%x - 0x%x] enabled\n", kvm_pinned_vmid_start, kvm_pinned_vmid_end);
+}
+
 struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr)
 {
 	struct kvm_vcpu *vcpu;

@@ -1790,6 +1889,9 @@ int kvm_arch_init(void *opaque)
 	else
 		kvm_info("Hyp mode initialized successfully\n");
 
+	if (kvm_pinned_vmid_enable)
+		kvm_arm_pinned_vmid_init();
+
 	return 0;
 
 out_hyp:

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Since the pinned VMID space is not recycled, we need to make sure that
we release the vmid back into the pool when we are done with it.

Signed-off-by: Shameer Kolothum <redacted>
---
 arch/arm64/kvm/arm.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 8955968be49f..d9900ffb88f4 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c

@@ -181,8 +181,16 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 	kvm_vgic_destroy(kvm);
 
 	for (i = 0; i < KVM_MAX_VCPUS; ++i) {
-		if (kvm->vcpus[i]) {
-			kvm_vcpu_destroy(kvm->vcpus[i]);
+		struct kvm_vcpu *vcpu = kvm->vcpus[i];
+
+		if (vcpu) {
+			struct kvm_vmid *vmid = &vcpu->arch.hw_mmu->vmid;
+
+			if (refcount_read(&vmid->pinned)) {
+				ida_free(&kvm_pinned_vmids, vmid->vmid);
+				refcount_set(&vmid->pinned, 0);
+			}
+			kvm_vcpu_destroy(vcpu);
 			kvm->vcpus[i] = NULL;
 		}
 	}

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Shameer,

On Mon, Feb 22, 2021 at 03:53:37PM +0000, Shameer Kolothum wrote:

This probably needs to hold devices_lock while using master.

That will require a TLB invalidation on the old VMID, once the STE is
rewritten.

More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without PASID
table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no?  They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}

It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and might break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.

Besides we probably want to restrict this feature to systems supporting
VMID16 on both SMMU and CPUs, or at least check that they are compatible.

Aliasing here as well: the VMID is still live but can be reallocated by
KVM and another domain might obtain it.

Thanks,
Jean

 unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
 }
-- 
2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Jean,

-----Original Message-----
From: Jean-Philippe Brucker [mailto:jean-philippe@linaro.org]
Sent: 04 March 2021 17:11
To: Shameerali Kolothum Thodi <redacted>
Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org;
kvmarm@lists.cs.columbia.edu; maz@kernel.org;
alex.williamson@redhat.com; eric.auger@redhat.com;
zhangfei.gao@linaro.org; Jonathan Cameron
[off-list ref]; Zengtao (B) [off-list ref];
linuxarm@openeuler.org
Subject: Re: [RFC PATCH 4/5] iommu/arm-smmu-v3: Use pinned VMID for
NESTED stage with BTM

Hi Shameer,

On Mon, Feb 22, 2021 at 03:53:37PM +0000, Shameer Kolothum wrote:

quoted

If the SMMU supports BTM and the device belongs to NESTED domain
with shared pasid table, we need to use the VMID allocated by the
KVM for the s2 configuration. Hence, request a pinned VMID from KVM.

Signed-off-by: Shameer Kolothum <redacted>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 49

++++++++++++++++++++-

quoted

 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

quoted

index 26bf7da1bcd0..04f83f7c8319 100644

--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c

@@ -28,6 +28,7 @@
 #include <linux/pci.h>
 #include <linux/pci-ats.h>
 #include <linux/platform_device.h>
+#include <linux/kvm_host.h>

 #include <linux/amba/bus.h>

@@ -2195,6 +2196,33 @@ static void arm_smmu_bitmap_free(unsigned

long *map, int idx)

quoted

 	clear_bit(idx, map);
 }

+static int arm_smmu_pinned_vmid_get(struct arm_smmu_domain

*smmu_domain)

quoted

+{
+	struct arm_smmu_master *master;
+
+	master = list_first_entry_or_null(&smmu_domain->devices,
+					  struct arm_smmu_master, domain_head);

This probably needs to hold devices_lock while using master.

Ok.

quoted

+	if (!master)
+		return -EINVAL;
+
+	return kvm_pinned_vmid_get(master->dev);
+}
+
+static int arm_smmu_pinned_vmid_put(struct arm_smmu_domain

*smmu_domain)

quoted

+{
+	struct arm_smmu_master *master;
+
+	master = list_first_entry_or_null(&smmu_domain->devices,
+					  struct arm_smmu_master, domain_head);
+	if (!master)
+		return -EINVAL;
+
+	if (smmu_domain->s2_cfg.vmid)
+		return kvm_pinned_vmid_put(master->dev);
+
+	return 0;
+}
+
 static void arm_smmu_domain_free(struct iommu_domain *domain)
 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);

@@ -2215,8 +2243,11 @@ static void arm_smmu_domain_free(struct

iommu_domain *domain)

quoted

 		mutex_unlock(&arm_smmu_asid_lock);
 	}
 	if (s2_cfg->set) {
-		if (s2_cfg->vmid)
-			arm_smmu_bitmap_free(smmu->vmid_map, s2_cfg->vmid);
+		if (s2_cfg->vmid) {
+			if (!(smmu->features & ARM_SMMU_FEAT_BTM) &&
+			    smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+				arm_smmu_bitmap_free(smmu->vmid_map,

s2_cfg->vmid);

quoted

+		}
 	}

 	kfree(smmu_domain);

@@ -3199,6 +3230,17 @@ static int arm_smmu_attach_pasid_table(struct

iommu_domain *domain,

quoted

 				!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
 			goto out;

+		if (smmu->features & ARM_SMMU_FEAT_BTM) {
+			ret = arm_smmu_pinned_vmid_get(smmu_domain);
+			if (ret < 0)
+				goto out;
+
+			if (smmu_domain->s2_cfg.vmid)
+				arm_smmu_bitmap_free(smmu->vmid_map,

smmu_domain->s2_cfg.vmid);

quoted

+
+			smmu_domain->s2_cfg.vmid = (u16)ret;

That will require a TLB invalidation on the old VMID, once the STE is
rewritten.

True. Will add that.

More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without PASID
table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no?  They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}

It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and might
break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.

Right, that is indeed a problem. I will take a look at this suggestion.
 

Besides we probably want to restrict this feature to systems supporting
VMID16 on both SMMU and CPUs, or at least check that they are compatible.

Yes. Ideally I would like to detect that in the KVM code and enable/disable the
VMID splitting based on that. But I am yet to figure out an easy way to do that
in KVM.

quoted

+		}
+
 		smmu_domain->s1_cfg.cdcfg.cdtab_dma = cfg->base_ptr;
 		smmu_domain->s1_cfg.s1cdmax = cfg->pasid_bits;
 		smmu_domain->s1_cfg.s1fmt = cfg->vendor_data.smmuv3.s1fmt;

@@ -3221,6 +3263,7 @@ static int arm_smmu_attach_pasid_table(struct

iommu_domain *domain,

quoted

 static void arm_smmu_detach_pasid_table(struct iommu_domain

*domain)

quoted

 {
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	struct arm_smmu_master *master;
 	unsigned long flags;

@@ -3237,6 +3280,8 @@ static void arm_smmu_detach_pasid_table(struct

iommu_domain *domain)

quoted

 		arm_smmu_install_ste_for_dev(master);
 	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);

+	if (smmu->features & ARM_SMMU_FEAT_BTM)
+		arm_smmu_pinned_vmid_put(smmu_domain);

Aliasing here as well: the VMID is still live but can be reallocated by
KVM and another domain might obtain it.

Ok. Got it.

Thanks for the review,
Shameer

Thanks,
Jean

quoted

 unlock:
 	mutex_unlock(&smmu_domain->init_mutex);
 }
--
2.17.1

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Shameer,

[+Will]

On Mon, 22 Feb 2021 15:53:36 +0000,
Shameer Kolothum [off-list ref] wrote:

On an ARM64 system with a SMMUv3 implementation that fully supports
Broadcast TLB Maintenance(BTM) feature, the CPU TLB invalidate
instructions are received by SMMU. This is very useful when the
SMMU shares the page tables with the CPU(eg: Guest SVA use case).
For this to work, the SMMU must use the same VMID that is allocated
by KVM to configure the stage 2 translations.

At present KVM VMID allocations are recycled on rollover and may
change as a result. This will create issues if we have to share
the KVM VMID with SMMU. Hence, we spilt the KVM VMID space into
two, the first half follows the normal recycle on rollover policy
while the second half of the VMID pace is used to allocate pinned
VMIDs. This feature is enabled based on a command line option
"kvm-arm.pinned_vmid_enable".

I think this is the wrong approach. Instead of shoving the notion of
pinned VMID into the current allocator, which really isn't designed
for this, it'd be a lot better if we aligned the KVM VMID allocator
with the ASID allocator, which already has support for pinning and is
in general much more efficient.

Julien Grall worked on such a series[1] a long while ago, which got
stalled because of the 32bit KVM port. Since we don't have this burden
anymore, I'd rather you look in that direction instead of wasting half
of the VMID space on potentially pinned VMIDs.

Thanks,

	M.

[1] https://patchwork.kernel.org/project/linux-arm-kernel/cover/20190724162534.7390-1-julien.grall@arm.com/


-- 
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

Hi Marc,

-----Original Message-----
From: Marc Zyngier [mailto:maz@kernel.org]
Sent: 09 March 2021 10:33
To: Shameerali Kolothum Thodi <redacted>
Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org;
kvmarm@lists.cs.columbia.edu; alex.williamson@redhat.com;
jean-philippe@linaro.org; eric.auger@redhat.com; zhangfei.gao@linaro.org;
Jonathan Cameron [off-list ref]; Zengtao (B)
[off-list ref]; linuxarm@openeuler.org; Will Deacon
[off-list ref]
Subject: Re: [RFC PATCH 3/5] KVM: ARM64: Add support for pinned VMIDs

Hi Shameer,

[+Will]

On Mon, 22 Feb 2021 15:53:36 +0000,
Shameer Kolothum [off-list ref] wrote:

quoted

On an ARM64 system with a SMMUv3 implementation that fully supports
Broadcast TLB Maintenance(BTM) feature, the CPU TLB invalidate
instructions are received by SMMU. This is very useful when the
SMMU shares the page tables with the CPU(eg: Guest SVA use case).
For this to work, the SMMU must use the same VMID that is allocated
by KVM to configure the stage 2 translations.

At present KVM VMID allocations are recycled on rollover and may
change as a result. This will create issues if we have to share
the KVM VMID with SMMU. Hence, we spilt the KVM VMID space into
two, the first half follows the normal recycle on rollover policy
while the second half of the VMID pace is used to allocate pinned
VMIDs. This feature is enabled based on a command line option
"kvm-arm.pinned_vmid_enable".

I think this is the wrong approach. Instead of shoving the notion of
pinned VMID into the current allocator, which really isn't designed
for this, it'd be a lot better if we aligned the KVM VMID allocator
with the ASID allocator, which already has support for pinning and is
in general much more efficient.

Ok. Agree that this is not efficient, but was easy to prototype something :)

Julien Grall worked on such a series[1] a long while ago, which got
stalled because of the 32bit KVM port. Since we don't have this burden
anymore, I'd rather you look in that direction instead of wasting half
of the VMID space on potentially pinned VMIDs.

Sure. I will check that and work on it.

Thanks,
Shameer

Thanks,

	M.

[1]
https://patchwork.kernel.org/project/linux-arm-kernel/cover/20190724162534
.7390-1-julien.grall@arm.com/


--
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

Hi Jean,

-----Original Message-----
From: Jean-Philippe Brucker [mailto:jean-philippe@linaro.org]
Sent: 04 March 2021 17:11
To: Shameerali Kolothum Thodi <redacted>
Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org;
kvmarm@lists.cs.columbia.edu; maz@kernel.org;
alex.williamson@redhat.com; eric.auger@redhat.com;
zhangfei.gao@linaro.org; Jonathan Cameron
[off-list ref]; Zengtao (B) [off-list ref];
linuxarm@openeuler.org
Subject: Re: [RFC PATCH 4/5] iommu/arm-smmu-v3: Use pinned VMID for
NESTED stage with BTM

[...]

quoted

 	kfree(smmu_domain);

@@ -3199,6 +3230,17 @@ static int arm_smmu_attach_pasid_table(struct

iommu_domain *domain,

quoted

 				!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB))
 			goto out;

+		if (smmu->features & ARM_SMMU_FEAT_BTM) {
+			ret = arm_smmu_pinned_vmid_get(smmu_domain);
+			if (ret < 0)
+				goto out;
+
+			if (smmu_domain->s2_cfg.vmid)
+				arm_smmu_bitmap_free(smmu->vmid_map,

smmu_domain->s2_cfg.vmid);

quoted

+
+			smmu_domain->s2_cfg.vmid = (u16)ret;

That will require a TLB invalidation on the old VMID, once the STE is
rewritten.

More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without PASID
table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no?  They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}

Now that we are trying to align the KVM VMID allocation algorithm similar to
that of the ASID allocator [1], I attempted to use that for the SMMU pinned 
VMID allocation. But the issue you have mentioned above is still valid. 

And as a solution what I have tried now is follow what pinned ASID is doing 
in SVA,
 -Use xarray for private VMIDs
 -Get pinned VMID from KVM for DOMAIN_NESTED with PASID table
 -If the new pinned VMID is in use by private, then update the private
  VMID(VMID update to a live STE).

This seems to work, but still need to run more tests with this though.  

It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and might
break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.

Another solution I have in mind is, make the new KVM VMID allocator common
between SMMUv3 and KVM. This will help to avoid all the private and shared
VMID splitting, also no need for live updates to STE VMID. One possible drawback
is less number of available KVM VMIDs but with 16 bit VMID space I am not sure
how much that is a concern.

Please let me know your thoughts.

Thanks,
Shameer

[1]. https://lore.kernel.org/kvmarm/20210616155606.2806-1-shameerali.kolothum.thodi@huawei.com/


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Shameer,

On Wed, Jul 21, 2021 at 08:54:00AM +0000, Shameerali Kolothum Thodi wrote:

quoted

More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without PASID
table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no?  They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}

Now that we are trying to align the KVM VMID allocation algorithm similar to
that of the ASID allocator [1], I attempted to use that for the SMMU pinned 
VMID allocation. But the issue you have mentioned above is still valid. 

And as a solution what I have tried now is follow what pinned ASID is doing 
in SVA,
 -Use xarray for private VMIDs
 -Get pinned VMID from KVM for DOMAIN_NESTED with PASID table
 -If the new pinned VMID is in use by private, then update the private
  VMID(VMID update to a live STE).

This seems to work, but still need to run more tests with this though.  

quoted

It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and might
break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.

Another solution I have in mind is, make the new KVM VMID allocator common
between SMMUv3 and KVM. This will help to avoid all the private and shared
VMID splitting, also no need for live updates to STE VMID. One possible drawback
is less number of available KVM VMIDs but with 16 bit VMID space I am not sure
how much that is a concern.

Yes I think that works too. In practice there shouldn't be many VMIDs on
the SMMU side, the feature's only enabled when a user wants to assign
devices with nesting translation (unlike ASIDs where each device in the
system gets a private ASID by default).

Note that you still need to pin all VMIDs used by the SMMU, otherwise
you'll have to update the STE after rollover.

The problem we have with VFIO_TYPE1_NESTING_IOMMU might be solved by the
upcoming deprecation of VFIO_*_IOMMU [2]. We need a specific sequence from
userspace:
1. Attach VFIO group to KVM (KVM_DEV_VFIO_GROUP_ADD)
2. Create nesting IOMMU domain and attach the group to it
   (VFIO_GROUP_SET_CONTAINER, VFIO_SET_IOMMU becomes
    IOMMU_IOASID_ALLOC, VFIO_DEVICE_ATTACH_IOASID)
Currently QEMU does 2 then 1, which would cause the SMMU to allocate a
separate VMID. If we wanted to extend VFIO_TYPE1_NESTING_IOMMU with PASID
tables we'd need to mandate 1-2 and may break existing users. In the new
design we can require from the start that creating a nesting IOMMU
container through /dev/iommu *must* come with a KVM context, that way
we're sure to reuse the existing VMID.

Thanks,
Jean

[2] https://lore.kernel.org/linux-iommu/BN9PR11MB5433B1E4AE5B0480369F97178C189@BN9PR11MB5433.namprd11.prod.outlook.com/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-----Original Message-----
From: Jean-Philippe Brucker [mailto:jean-philippe@linaro.org]
Sent: 22 July 2021 17:46
To: Shameerali Kolothum Thodi <redacted>
Cc: linux-arm-kernel@lists.infradead.org; iommu@lists.linux-foundation.org;
kvmarm@lists.cs.columbia.edu; maz@kernel.org;
alex.williamson@redhat.com; eric.auger@redhat.com;
zhangfei.gao@linaro.org; Jonathan Cameron
[off-list ref]; Zengtao (B) [off-list ref];
linuxarm@openeuler.org; Linuxarm [off-list ref]
Subject: [Linuxarm] Re: [RFC PATCH 4/5] iommu/arm-smmu-v3: Use pinned
VMID for NESTED stage with BTM

Hi Shameer,

On Wed, Jul 21, 2021 at 08:54:00AM +0000, Shameerali Kolothum Thodi
wrote:

quoted

quoted

More generally I think this pinned VMID set conflicts with that of
stage-2-only domains (which is the default state until a guest attaches a
PASID table). Say you have one guest using DOMAIN_NESTED without

PASID

quoted

quoted

table, just DMA to IPA using VMID 0x8000. Now another guest attaches a
PASID table and obtains the same VMID from KVM. The stage-2 translation
might use TLB entries from the other guest, no?  They'll both create
stage-2 TLB entries with {StreamWorld=NS-EL1, VMID=0x8000}

Now that we are trying to align the KVM VMID allocation algorithm similar

to

quoted

that of the ASID allocator [1], I attempted to use that for the SMMU pinned
VMID allocation. But the issue you have mentioned above is still valid.

And as a solution what I have tried now is follow what pinned ASID is doing
in SVA,
 -Use xarray for private VMIDs
 -Get pinned VMID from KVM for DOMAIN_NESTED with PASID table
 -If the new pinned VMID is in use by private, then update the private
  VMID(VMID update to a live STE).

This seems to work, but still need to run more tests with this though.

quoted

It's tempting to allocate all VMIDs through KVM instead, but that will
force a dependency on KVM to use VFIO_TYPE1_NESTING_IOMMU and

might

quoted

quoted

break
existing users of that extension (though I'm not sure there are any).
Instead we might need to restrict the SMMU VMID bitmap to match the
private VMID set in KVM.

Another solution I have in mind is, make the new KVM VMID allocator

common

quoted

between SMMUv3 and KVM. This will help to avoid all the private and

shared

quoted

VMID splitting, also no need for live updates to STE VMID. One possible

drawback

quoted

is less number of available KVM VMIDs but with 16 bit VMID space I am not

sure

quoted

how much that is a concern.

Yes I think that works too. In practice there shouldn't be many VMIDs on
the SMMU side, the feature's only enabled when a user wants to assign
devices with nesting translation (unlike ASIDs where each device in the
system gets a private ASID by default).

Ok. What about implementations that supports only stage 2? Do we
need a private VMID allocator for those or can use the same common
KVM VMID allocator?

Note that you still need to pin all VMIDs used by the SMMU, otherwise
you'll have to update the STE after rollover.

Sure.

The problem we have with VFIO_TYPE1_NESTING_IOMMU might be solved by
the
upcoming deprecation of VFIO_*_IOMMU [2]. We need a specific sequence
from
userspace:
1. Attach VFIO group to KVM (KVM_DEV_VFIO_GROUP_ADD)
2. Create nesting IOMMU domain and attach the group to it
   (VFIO_GROUP_SET_CONTAINER, VFIO_SET_IOMMU becomes
    IOMMU_IOASID_ALLOC, VFIO_DEVICE_ATTACH_IOASID)
Currently QEMU does 2 then 1, which would cause the SMMU to allocate a
separate VMID.

Yes. I have observed this with my current implementation. I have a check
to see the private S2 config VMID belongs to the same domain s2_cfg, then
skip the live update to the STE VMID.

If we wanted to extend VFIO_TYPE1_NESTING_IOMMU with
PASID
tables we'd need to mandate 1-2 and may break existing users. In the new
design we can require from the start that creating a nesting IOMMU
container through /dev/iommu *must* come with a KVM context, that way
we're sure to reuse the existing VMID.

Ok. That helps.

Thanks,
Shameer
 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel