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/
>
> 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.
> +/**
> + * 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;
> +}
> +
> +static inline void
> +msi_doorbell_unregister_global(struct msi_doorbell_info *db) {}
> +
> +static inline bool msi_doorbell_safe(void)
> +{
> + return true;
> +}
Is it?
> +#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);
--
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
