Hi Alex, On 06/10/2016 22:17, Alex Williamson wrote: > On Thu, 6 Oct 2016 08:45:20 +0000 > Eric Auger <eric.auger@xxxxxxxxxx> wrote: > >> We introduce a new msi-doorbell API that allows msi controllers >> to allocate and register their doorbells. This is useful when >> those doorbells are likely to be iommu mapped (typically on ARM). >> The VFIO layer will need to gather information about those doorbells: >> whether they are safe (ie. they implement irq remapping) and how >> many IOMMU pages are requested to map all of them. >> >> This patch first introduces the dedicated msi_doorbell_info struct >> and the registration/unregistration functions. >> >> A doorbell region is characterized by its physical address base, size, >> and whether it its safe (ie. it implements IRQ remapping). A doorbell >> can be per-cpu of global. We currently only care about global doorbells. > ^^ s/of/or/ OK > >> >> A function returns whether all doorbells are safe. >> >> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >> >> --- >> v12 -> v13: >> - directly select MSI_DOORBELL in ARM_SMMU and ARM_SMMU_V3 configs >> - remove prot attribute >> - move msi_doorbell_info struct definition in msi-doorbell.c >> - change the commit title >> - change proto of the registration function >> - msi_doorbell_safe now in this patch >> >> v11 -> v12: >> - rename irqchip_doorbell into msi_doorbell, irqchip_doorbell_list >> into msi_doorbell_list and irqchip_doorbell_mutex into >> msi_doorbell_mutex >> - fix style issues: align msi_doorbell struct members, kernel-doc comments >> - use kzalloc >> - use container_of in msi_doorbell_unregister_global >> - compute nb_unsafe_doorbells on registration/unregistration >> - registration simply returns NULL if allocation failed >> >> v10 -> v11: >> - remove void *chip_data argument from register/unregister function >> - remove lookup funtions since we restored the struct irq_chip >> msi_doorbell_info ops to realize this function >> - reword commit message and title >> >> Conflicts: >> kernel/irq/Makefile >> >> Conflicts: >> drivers/iommu/Kconfig >> --- >> drivers/iommu/Kconfig | 2 + >> include/linux/msi-doorbell.h | 77 ++++++++++++++++++++++++++++++++++ >> kernel/irq/Kconfig | 4 ++ >> kernel/irq/Makefile | 1 + >> kernel/irq/msi-doorbell.c | 98 ++++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 182 insertions(+) >> create mode 100644 include/linux/msi-doorbell.h >> create mode 100644 kernel/irq/msi-doorbell.c >> >> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig >> index 8ee54d7..0cc7fac 100644 >> --- a/drivers/iommu/Kconfig >> +++ b/drivers/iommu/Kconfig >> @@ -297,6 +297,7 @@ config SPAPR_TCE_IOMMU >> config ARM_SMMU >> bool "ARM Ltd. System MMU (SMMU) Support" >> depends on (ARM64 || ARM) && MMU >> + select MSI_DOORBELL >> select IOMMU_API >> select IOMMU_IO_PGTABLE_LPAE >> select ARM_DMA_USE_IOMMU if ARM >> @@ -310,6 +311,7 @@ config ARM_SMMU >> config ARM_SMMU_V3 >> bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support" >> depends on ARM64 >> + select MSI_DOORBELL >> select IOMMU_API >> select IOMMU_IO_PGTABLE_LPAE >> select GENERIC_MSI_IRQ_DOMAIN >> diff --git a/include/linux/msi-doorbell.h b/include/linux/msi-doorbell.h >> new file mode 100644 >> index 0000000..c18a382 >> --- /dev/null >> +++ b/include/linux/msi-doorbell.h >> @@ -0,0 +1,77 @@ >> +/* >> + * API to register/query MSI doorbells likely to be IOMMU mapped >> + * >> + * Copyright (C) 2016 Red Hat, Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#ifndef _LINUX_MSI_DOORBELL_H >> +#define _LINUX_MSI_DOORBELL_H >> + >> +struct msi_doorbell_info; >> + >> +#ifdef CONFIG_MSI_DOORBELL >> + >> +/** >> + * msi_doorbell_register - allocate and register a global doorbell >> + * @base: physical base address of the global doorbell >> + * @size: size of the global doorbell >> + * @prot: protection/memory attributes >> + * @safe: true is irq_remapping implemented for this doorbell >> + * @dbinfo: returned doorbell info >> + * >> + * Return: 0 on success, -ENOMEM on allocation failure >> + */ >> +int msi_doorbell_register_global(phys_addr_t base, size_t size, >> + bool safe, >> + struct msi_doorbell_info **dbinfo); >> + > > Seems like alloc/free behavior vs register/unregister. Also seems > cleaner to just return a struct msi_doorbell_info* and use PTR_ERR for > return codes. These are of course superficial changes that could be > addressed in the future. Sure > >> +/** >> + * msi_doorbell_unregister_global - unregister a global doorbell >> + * @db: doorbell info to unregister >> + * >> + * remove the doorbell descriptor from the list of registered doorbells >> + * and deallocates it >> + */ >> +void msi_doorbell_unregister_global(struct msi_doorbell_info *db); >> + >> +/** >> + * msi_doorbell_safe - return whether all registered doorbells are safe >> + * >> + * Safe doorbells are those which implement irq remapping >> + * Return: true if all doorbells are safe, false otherwise >> + */ >> +bool msi_doorbell_safe(void); >> + >> +#else >> + >> +static inline int >> +msi_doorbell_register_global(phys_addr_t base, size_t size, >> + int prot, bool safe, >> + struct msi_doorbell_info **dbinfo) >> +{ >> + *dbinfo = NULL; >> + return 0; > > If we return a struct* > > return NULL; Yep > >> +} >> + >> +static inline void >> +msi_doorbell_unregister_global(struct msi_doorbell_info *db) {} >> + >> +static inline bool msi_doorbell_safe(void) >> +{ >> + return true; >> +} > > Is it? Yes I will return false and change the safety check in vfio_iommu_type1.c Thanks Eric > >> +#endif /* CONFIG_MSI_DOORBELL */ >> + >> +#endif >> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig >> index 3bbfd6a..d4faaaa 100644 >> --- a/kernel/irq/Kconfig >> +++ b/kernel/irq/Kconfig >> @@ -72,6 +72,10 @@ config GENERIC_IRQ_IPI >> config GENERIC_MSI_IRQ >> bool >> >> +# MSI doorbell support (for doorbell IOMMU mapping) >> +config MSI_DOORBELL >> + bool >> + >> # Generic MSI hierarchical interrupt domain support >> config GENERIC_MSI_IRQ_DOMAIN >> bool >> diff --git a/kernel/irq/Makefile b/kernel/irq/Makefile >> index 1d3ee31..5b04dd1 100644 >> --- a/kernel/irq/Makefile >> +++ b/kernel/irq/Makefile >> @@ -10,3 +10,4 @@ obj-$(CONFIG_PM_SLEEP) += pm.o >> obj-$(CONFIG_GENERIC_MSI_IRQ) += msi.o >> obj-$(CONFIG_GENERIC_IRQ_IPI) += ipi.o >> obj-$(CONFIG_SMP) += affinity.o >> +obj-$(CONFIG_MSI_DOORBELL) += msi-doorbell.o >> diff --git a/kernel/irq/msi-doorbell.c b/kernel/irq/msi-doorbell.c >> new file mode 100644 >> index 0000000..60a262a >> --- /dev/null >> +++ b/kernel/irq/msi-doorbell.c >> @@ -0,0 +1,98 @@ >> +/* >> + * API to register/query MSI doorbells likely to be IOMMU mapped >> + * >> + * Copyright (C) 2016 Red Hat, Inc. >> + * Author: Eric Auger <eric.auger@xxxxxxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/slab.h> >> +#include <linux/irq.h> >> +#include <linux/msi-doorbell.h> >> + >> +/** >> + * struct msi_doorbell_info - MSI doorbell region descriptor >> + * @percpu_doorbells: per cpu doorbell base address >> + * @global_doorbell: base address of the doorbell >> + * @doorbell_is_percpu: is the doorbell per cpu or global? >> + * @safe: true if irq remapping is implemented >> + * @size: size of the doorbell >> + */ >> +struct msi_doorbell_info { >> + union { >> + phys_addr_t __percpu *percpu_doorbells; >> + phys_addr_t global_doorbell; >> + }; >> + bool doorbell_is_percpu; >> + bool safe; >> + size_t size; >> +}; >> + >> +struct msi_doorbell { >> + struct msi_doorbell_info info; >> + struct list_head next; >> +}; >> + >> +/* list of registered MSI doorbells */ >> +static LIST_HEAD(msi_doorbell_list); >> + >> +/* counts the number of unsafe registered doorbells */ >> +static uint nb_unsafe_doorbells; >> + >> +/* protects the list and nb__unsafe_doorbells */ > > Extra underscore > >> +static DEFINE_MUTEX(msi_doorbell_mutex); >> + >> +int msi_doorbell_register_global(phys_addr_t base, size_t size, bool safe, >> + struct msi_doorbell_info **dbinfo) >> +{ >> + struct msi_doorbell *db; >> + >> + db = kzalloc(sizeof(*db), GFP_KERNEL); >> + if (!db) >> + return -ENOMEM; >> + >> + db->info.global_doorbell = base; >> + db->info.size = size; >> + db->info.safe = safe; >> + >> + mutex_lock(&msi_doorbell_mutex); >> + list_add(&db->next, &msi_doorbell_list); >> + if (!db->info.safe) >> + nb_unsafe_doorbells++; >> + mutex_unlock(&msi_doorbell_mutex); >> + *dbinfo = &db->info; >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(msi_doorbell_register_global); >> + >> +void msi_doorbell_unregister_global(struct msi_doorbell_info *dbinfo) >> +{ >> + struct msi_doorbell *db; >> + >> + db = container_of(dbinfo, struct msi_doorbell, info); >> + >> + mutex_lock(&msi_doorbell_mutex); >> + list_del(&db->next); >> + if (!db->info.safe) >> + nb_unsafe_doorbells--; >> + mutex_unlock(&msi_doorbell_mutex); >> + kfree(db); >> +} >> +EXPORT_SYMBOL_GPL(msi_doorbell_unregister_global); >> + >> +bool msi_doorbell_safe(void) >> +{ >> + return !nb_unsafe_doorbells; >> +} >> +EXPORT_SYMBOL_GPL(msi_doorbell_safe); > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
