On 2024/8/17 9:54, JunBin Li wrote:
virtcca inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IADD42
--------------------------------
virtCCA feature
What is virtCCA? How it works? and why do you want to introduce this feature? Please give some detail information about what, why, how.
Signed-off-by: JunBin Li <lijunbin4@huawei.com> --- arch/arm64/kvm/mmu.c | 23 +++ arch/arm64/kvm/tmi.c | 24 ++++ arch/arm64/kvm/virtcca_cvm.c | 231 ++++++++++++++++++++++++++++++- drivers/iommu/io-pgtable-arm.c | 81 +++++++++++ drivers/iommu/iommu.c | 11 ++ drivers/pci/access.c | 33 +++++ drivers/pci/msi/msi.c | 85 ++++++++++-- drivers/pci/msi/msi.h | 29 +++- drivers/vfio/group.c | 2 +- drivers/vfio/pci/vfio_pci_core.c | 16 ++- drivers/vfio/pci/vfio_pci_rdwr.c | 61 ++++++++ drivers/vfio/vfio_iommu_type1.c | 82 +++++++++++ drivers/vfio/vfio_main.c | 18 +++ include/linux/iommu.h | 5 + include/linux/pci.h | 4 + include/linux/vfio.h | 5 + include/uapi/linux/vfio.h | 3 + virt/kvm/vfio.c | 127 ++++++++++++++++- virt/kvm/vfio.h | 11 ++ 19 files changed, 836 insertions(+), 15 deletions(-)
How can I review this? It's a big patch for review!
+u64 iova_to_pa(void *addr) +{ + uint64_t pa, par_el1; + + asm volatile( + "AT S1E1W, %0\n" + ::"r"((uint64_t)(addr)) + ); + isb(); + asm volatile( + "mrs %0, par_el1\n" + : "=r"(par_el1) + ); + + pa = ((uint64_t)(addr) & (PAGE_SIZE - 1)) | + (par_el1 & ULL(0x000ffffffffff000)); + + if (par_el1 & UL(1 << 0)) + return (uint64_t)(addr); + else + return pa; +} +EXPORT_SYMBOL(iova_to_pa);
Why not use some functions in IOVA subsystem? and why use some specfic registers? does it rely on specific hardware?
+ u64 tmi_version(void) { struct arm_smccc_res res; diff --git a/arch/arm64/kvm/virtcca_cvm.c b/arch/arm64/kvm/virtcca_cvm.c index 367bbf4fa3..75c1d2de3f 100644 --- a/arch/arm64/kvm/virtcca_cvm.c +++ b/arch/arm64/kvm/virtcca_cvm.c @@ -4,6 +4,7 @@ */ #include <linux/kvm_host.h> #include <linux/kvm.h> +#include <linux/vfio.h> #include <asm/kvm_tmi.h> #include <asm/kvm_pgtable.h> #include <asm/kvm_emulate.h> @@ -12,6 +13,7 @@ #include <linux/arm-smccc.h> #include <kvm/arm_hypercalls.h> #include <kvm/arm_psci.h> +#include "../virt/kvm/vfio.h"
This is not the right way to include the head file. it shows your architecture is messed up.
static int arm_lpae_map_pages(struct io_pgtable_ops *ops, unsigned long iova, phys_addr_t paddr, size_t pgsize, size_t pgcount, int iommu_prot, gfp_t gfp, size_t *mapped) @@ -522,12 +578,24 @@ static int arm_lpae_map_pages(struct io_pgtable_ops *ops, unsigned long iova, return 0;
prot = arm_lpae_prot_to_pte(data, iommu_prot); + +#ifdef CONFIG_HISI_VIRTCCA_HOST + ret = arm_lpae_cvm_map_unmap_pages(data, iova, paddr, pgsize * pgcount, true, mapped); + struct kvm *kvm = arm_smmu_domain_get_kvm(data); + + if (kvm && kvm_is_virtcca_cvm(kvm)) + goto out; +#endif + ret = __arm_lpae_map(data, iova, paddr, pgsize, pgcount, prot, lvl, ptep, gfp, mapped); /* * Synchronise all PTE updates for the new mapping before there's * a chance for anything to kick off a table walk for the new iova. */ +#ifdef CONFIG_HISI_VIRTCCA_HOST +out: +#endif wmb();
return ret; @@ -708,6 +776,19 @@ static size_t arm_lpae_unmap_pages(struct io_pgtable_ops *ops, unsigned long iov if (WARN_ON(iaext)) return 0;
+#ifdef CONFIG_HISI_VIRTCCA_HOST + int ret; + + ret = arm_lpae_cvm_map_unmap_pages(data, iova, 0, pgsize * pgcount, false, NULL); + if (ret) + pr_err("%s %d failed to unmap pages, iova %lx, size %lx\n", + __func__, __LINE__, iova, pgsize); + struct kvm *kvm = arm_smmu_domain_get_kvm(data); + + if (kvm && kvm_is_virtcca_cvm(kvm)) + return 0; +#endif
So virtcca and normal vm will not be available at the same time, I mean in a single kernel image, right?
+ return __arm_lpae_unmap(data, gather, iova, pgsize, pgcount, data->start_level, ptep); } diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 28f63ad432..07a084c9d9 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -592,6 +592,17 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list return ret; }
+#ifdef CONFIG_HISI_VIRTCCA_HOST +struct iommu_domain *iommu_group_get_domain(struct iommu_group *iommu_group) +{ + if (iommu_group) + return iommu_group->domain; + + return NULL; +} +EXPORT_SYMBOL_GPL(iommu_group_get_domain);
I think we already have some interface like this.
+#endif + int iommu_probe_device(struct device *dev) { const struct iommu_ops *ops; diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 6554a2e89d..1e616d7768 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -7,6 +7,10 @@
#include "pci.h"
+#ifdef CONFIG_HISI_VIRTCCA_HOST +#include <asm/kvm_tmi.h> +#endif + /* * This interrupt-safe spinlock protects all accesses to PCI * configuration space. @@ -86,6 +90,19 @@ int pci_generic_config_read(struct pci_bus *bus, unsigned int devfn, if (!addr) return PCIBIOS_DEVICE_NOT_FOUND;
+#ifdef CONFIG_HISI_VIRTCCA_HOST + if (is_cc_dev((bus->number << 8) | devfn)) { + if (size == 1) + *val = tmi_mmio_read(iova_to_pa(addr), 8, ((bus->number << 8) | devfn)); + else if (size == 2) + *val = tmi_mmio_read(iova_to_pa(addr), 16, ((bus->number << 8) | devfn)); + else + *val = tmi_mmio_read(iova_to_pa(addr), 32, ((bus->number << 8) | devfn)); + + return PCIBIOS_SUCCESSFUL; + }
why? if the hardware has no isolation on the device, how can you make sure it's a CC device?
+#endif + if (size == 1) *val = readb(addr); else if (size == 2) @@ -106,6 +123,22 @@ int pci_generic_config_write(struct pci_bus *bus, unsigned int devfn, if (!addr) return PCIBIOS_DEVICE_NOT_FOUND;
+#ifdef CONFIG_HISI_VIRTCCA_HOST + if (is_cc_dev((bus->number << 8) | devfn)) { + if (size == 1) + WARN_ON(tmi_mmio_write(iova_to_pa(addr), val, + 8, ((bus->number << 8) | devfn))); + else if (size == 2) + WARN_ON(tmi_mmio_write(iova_to_pa(addr), val, + 16, ((bus->number << 8) | devfn))); + else + WARN_ON(tmi_mmio_write(iova_to_pa(addr), val, + 32, ((bus->number << 8) | devfn))); + + return PCIBIOS_SUCCESSFUL; + } +#endif + if (size == 1) writeb(val, addr); else if (size == 2) diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c index 161c3ac171..c48ff83f65 100644 --- a/drivers/pci/msi/msi.c +++ b/drivers/pci/msi/msi.c @@ -159,9 +159,23 @@ void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg) if (WARN_ON_ONCE(entry->pci.msi_attrib.is_virtual)) return;
- msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR); - msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR); - msg->data = readl(base + PCI_MSIX_ENTRY_DATA); +#ifdef CONFIG_HISI_VIRTCCA_HOST + u64 pbase = iova_to_pa(base); + + if (virtcca_is_available() && dev != NULL && is_cc_dev(pci_dev_id(dev))) { + msg->address_lo = tmi_mmio_read(pbase + PCI_MSIX_ENTRY_LOWER_ADDR, + 32, pci_dev_id(dev)); + msg->address_hi = tmi_mmio_read(pbase + PCI_MSIX_ENTRY_UPPER_ADDR, + 32, pci_dev_id(dev)); + msg->data = tmi_mmio_read(pbase + PCI_MSIX_ENTRY_DATA, 32, pci_dev_id(dev)); + } else { +#endif + msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR); + msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR); + msg->data = readl(base + PCI_MSIX_ENTRY_DATA); +#ifdef CONFIG_HISI_VIRTCCA_HOST + } +#endif
You just ignore everything and hack your code to make it work, I can't review you code anymore. NACK for the whole solution. Thanks Hanjun