[PATCH v13 04/15] genirq/msi: Introduce the MSI doorbell API
Auger Eric
eric.auger at redhat.com
Fri Oct 7 10:13:34 PDT 2016
Hi Alex,
On 06/10/2016 22:17, Alex Williamson wrote:
> On Thu, 6 Oct 2016 08:45:20 +0000
> Eric Auger <eric.auger at redhat.com> 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 at redhat.com>
>>
>> ---
>> 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 at redhat.com>
>> + *
>> + * 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
More information about the linux-arm-kernel
mailing list