From: Robin Murphy <robin.murphy@arm.com>
To: "Isaac J. Manjarres" <isaacm@codeaurora.org>,
iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, freedreno@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: will@kernel.org, joro@8bytes.org, pdaly@codeaurora.org,
pratikp@codeaurora.org, kernel-team@android.com
Subject: Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Date: Tue, 22 Dec 2020 19:27:41 +0000 [thread overview]
Message-ID: <4f388512-c3df-e9fc-ebd4-624ab36a6858@arm.com> (raw)
In-Reply-To: <1608597876-32367-2-git-send-email-isaacm@codeaurora.org>
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
> The io-pgtable code constructs an array of init functions for each
> page table format at compile time. This is not ideal, as this
> increases the footprint of the io-pgtable code, as well as prevents
> io-pgtable formats from being built as kernel modules.
>
> In preparation for modularizing the io-pgtable formats, switch to a
> dynamic registration scheme, where each io-pgtable format can register
> their init functions with the io-pgtable code at boot or module
> insertion time.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 34 +++++++++++++-
> drivers/iommu/io-pgtable-arm.c | 90 ++++++++++++++++++++++++++----------
> drivers/iommu/io-pgtable.c | 94 ++++++++++++++++++++++++++++++++------
> include/linux/io-pgtable.h | 51 +++++++++++++--------
> 4 files changed, 209 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index 1d92ac9..89aad2f 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -28,6 +28,7 @@
> #include <linux/iommu.h>
> #include <linux/kernel.h>
> #include <linux/kmemleak.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> @@ -835,7 +836,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> +static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> + .fmt = ARM_V7S,
> .alloc = arm_v7s_alloc_pgtable,
> .free = arm_v7s_free_pgtable,
> };
> @@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
> pr_info("self test ok\n");
> return 0;
> }
> -subsys_initcall(arm_v7s_do_selftests);
> +#else
> +static int arm_v7s_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_v7s_init(void)
> +{
> + int ret;
> +
> + ret = io_pgtable_ops_register(&io_pgtable_arm_v7s_init_fns);
> + if (ret < 0) {
> + pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not to
abbreviate "format" would be nice.
> + return ret;
> + }
> +
> + ret = arm_v7s_do_selftests();
> + if (ret < 0)
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +
> + return ret;
> +}
> +core_initcall(arm_v7s_init);
> +
> +static void __exit arm_v7s_exit(void)
> +{
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +}
> +module_exit(arm_v7s_exit);
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 87def58..ff0ea2f 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -13,6 +13,7 @@
> #include <linux/bitops.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
> - .alloc = arm_mali_lpae_alloc_pgtable,
> - .free = arm_lpae_free_pgtable,
> +static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] = {
> + {
> + .fmt = ARM_32_LPAE_S1,
> + .alloc = arm_32_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_32_LPAE_S2,
> + .alloc = arm_32_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S1,
> + .alloc = arm_64_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S2,
> + .alloc = arm_64_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_MALI_LPAE,
> + .alloc = arm_mali_lpae_alloc_pgtable,
> + .free = arm_lpae_free_pgtable,
> + },
> };
>
> #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
> @@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
> pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> return fail ? -EFAULT : 0;
> }
> -subsys_initcall(arm_lpae_do_selftests);
> +#else
> +static int __init arm_lpae_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_lpae_init(void)
> +{
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++) {
> + ret = io_pgtable_ops_register(&io_pgtable_arm_lpae_init_fns[i]);
> + if (ret < 0) {
> + pr_err("Failed to register ARM LPAE fmt: %d\n");
> + goto err_io_pgtable_register;
> + }
> + }
> +
> + ret = arm_lpae_do_selftests();
> + if (ret < 0)
> + goto err_io_pgtable_register;
> +
> + return 0;
> +
> +err_io_pgtable_register:
> + for (i = i - 1; i >= 0; i--)
Personally I find "while (i--)" a bit clearer for this kind of
unwinding, but maybe post-decrement isn't to everyone's taste.
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> + return ret;
> +}
> +core_initcall(arm_lpae_init);
> +
> +static void __exit arm_lpae_exit(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++)
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> +}
> +module_exit(arm_lpae_exit);
> diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> index 94394c8..2c6eb2e 100644
> --- a/drivers/iommu/io-pgtable.c
> +++ b/drivers/iommu/io-pgtable.c
> @@ -10,33 +10,45 @@
> #include <linux/bug.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/rwlock.h>
> +#include <linux/slab.h>
> #include <linux/types.h>
>
> -static const struct io_pgtable_init_fns *
> -io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE
> - [ARM_32_LPAE_S1] = &io_pgtable_arm_32_lpae_s1_init_fns,
> - [ARM_32_LPAE_S2] = &io_pgtable_arm_32_lpae_s2_init_fns,
> - [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
> - [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
> - [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns,
> -#endif
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S
> - [ARM_V7S] = &io_pgtable_arm_v7s_init_fns,
> -#endif
> +struct io_pgtable_init_fns_node {
> + struct io_pgtable_init_fns *fns;
> + struct list_head list;
> };
>
> +static LIST_HEAD(io_pgtable_init_fns_list);
> +static DEFINE_RWLOCK(io_pgtable_init_fns_list_lock);
> +
> +static struct io_pgtable_init_fns *io_pgtable_get_init_fns(enum io_pgtable_fmt fmt)
> +{
> + struct io_pgtable_init_fns_node *iter;
> + struct io_pgtable_init_fns *fns = NULL;
> +
> + read_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == fmt) {
> + fns = iter->fns;
> + break;
> + }
> + read_unlock(&io_pgtable_init_fns_list_lock);
> +
> + return fns;
> +}
I think it would be a lot easier to stick with a simple array indexed by
enum - that way you can just set/clear/test entries without needing to
worry about locking. Basically just remove the const and the
initialisers from the existing one ;)
(and if you think you're concerned about memory, consider that just the
list head plus lock is already half the size of the table)
Other than that, I think this all looks pretty promising - I'd suggest
sending a non-RFC after rc1 so that it gets everyone's proper attention.
Thanks,
Robin.
> +
> struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,
> struct io_pgtable_cfg *cfg,
> void *cookie)
> {
> struct io_pgtable *iop;
> - const struct io_pgtable_init_fns *fns;
> + struct io_pgtable_init_fns *fns;
>
> if (fmt >= IO_PGTABLE_NUM_FMTS)
> return NULL;
>
> - fns = io_pgtable_init_table[fmt];
> + fns = io_pgtable_get_init_fns(fmt);
> if (!fns)
> return NULL;
>
> @@ -59,12 +71,64 @@ EXPORT_SYMBOL_GPL(alloc_io_pgtable_ops);
> void free_io_pgtable_ops(struct io_pgtable_ops *ops)
> {
> struct io_pgtable *iop;
> + struct io_pgtable_init_fns *fns;
>
> if (!ops)
> return;
>
> iop = io_pgtable_ops_to_pgtable(ops);
> io_pgtable_tlb_flush_all(iop);
> - io_pgtable_init_table[iop->fmt]->free(iop);
> + fns = io_pgtable_get_init_fns(iop->fmt);
> + if (fns)
> + fns->free(iop);
> }
> EXPORT_SYMBOL_GPL(free_io_pgtable_ops);
> +
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *fns_node;
> + int ret = 0;
> +
> + if (!init_fns || init_fns->fmt >= IO_PGTABLE_NUM_FMTS ||
> + !init_fns->alloc || !init_fns->free)
> + return -EINVAL;
> +
> + fns_node = kzalloc(sizeof(*fns_node), GFP_KERNEL);
> + if (!fns_node)
> + return -ENOMEM;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == init_fns->fmt) {
> + ret = -EEXIST;
> + kfree(fns_node);
> + break;
> + }
> +
> + if (!ret) {
> + fns_node->fns = init_fns;
> + INIT_LIST_HEAD(&fns_node->list);
> + list_add_tail(&fns_node->list, &io_pgtable_init_fns_list);
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_register);
> +
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *tmp;
> +
> + if (!init_fns)
> + return;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry_safe(iter, tmp, &io_pgtable_init_fns_list, list)
> + if (iter->fns == init_fns) {
> + list_del(&iter->list);
> + kfree(iter);
> + break;
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_unregister);
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index ea727eb..45b367ce 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -163,6 +163,38 @@ struct io_pgtable_ops {
> };
>
> /**
> + * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> + * particular format.
> + *
> + * @fmt: The page table format.
> + * @alloc: Allocate a set of page tables described by cfg.
> + * @free: Free the page tables associated with iop.
> + */
> +struct io_pgtable_init_fns {
> + enum io_pgtable_fmt fmt;
> + struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> + void (*free)(struct io_pgtable *iop);
> +};
> +
> +/**
> + * io_pgtable_ops_register() - Register the page table routines for a page table
> + * format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> + * io_pgtable_ops_unregister() - Unregister the page table routines for a page
> + * table format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> * alloc_io_pgtable_ops() - Allocate a page table allocator for use by an IOMMU.
> *
> * @fmt: The page table format.
> @@ -233,23 +265,4 @@ io_pgtable_tlb_add_page(struct io_pgtable *iop,
> iop->cfg.tlb->tlb_add_page(gather, iova, granule, iop->cookie);
> }
>
> -/**
> - * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> - * particular format.
> - *
> - * @alloc: Allocate a set of page tables described by cfg.
> - * @free: Free the page tables associated with iop.
> - */
> -struct io_pgtable_init_fns {
> - struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> - void (*free)(struct io_pgtable *iop);
> -};
> -
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns;
> -
> #endif /* __IO_PGTABLE_H */
>
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: "Isaac J. Manjarres" <isaacm@codeaurora.org>,
iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, freedreno@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: pratikp@codeaurora.org, kernel-team@android.com, will@kernel.org,
pdaly@codeaurora.org
Subject: Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Date: Tue, 22 Dec 2020 19:27:41 +0000 [thread overview]
Message-ID: <4f388512-c3df-e9fc-ebd4-624ab36a6858@arm.com> (raw)
In-Reply-To: <1608597876-32367-2-git-send-email-isaacm@codeaurora.org>
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
> The io-pgtable code constructs an array of init functions for each
> page table format at compile time. This is not ideal, as this
> increases the footprint of the io-pgtable code, as well as prevents
> io-pgtable formats from being built as kernel modules.
>
> In preparation for modularizing the io-pgtable formats, switch to a
> dynamic registration scheme, where each io-pgtable format can register
> their init functions with the io-pgtable code at boot or module
> insertion time.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 34 +++++++++++++-
> drivers/iommu/io-pgtable-arm.c | 90 ++++++++++++++++++++++++++----------
> drivers/iommu/io-pgtable.c | 94 ++++++++++++++++++++++++++++++++------
> include/linux/io-pgtable.h | 51 +++++++++++++--------
> 4 files changed, 209 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index 1d92ac9..89aad2f 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -28,6 +28,7 @@
> #include <linux/iommu.h>
> #include <linux/kernel.h>
> #include <linux/kmemleak.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> @@ -835,7 +836,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> +static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> + .fmt = ARM_V7S,
> .alloc = arm_v7s_alloc_pgtable,
> .free = arm_v7s_free_pgtable,
> };
> @@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
> pr_info("self test ok\n");
> return 0;
> }
> -subsys_initcall(arm_v7s_do_selftests);
> +#else
> +static int arm_v7s_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_v7s_init(void)
> +{
> + int ret;
> +
> + ret = io_pgtable_ops_register(&io_pgtable_arm_v7s_init_fns);
> + if (ret < 0) {
> + pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not to
abbreviate "format" would be nice.
> + return ret;
> + }
> +
> + ret = arm_v7s_do_selftests();
> + if (ret < 0)
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +
> + return ret;
> +}
> +core_initcall(arm_v7s_init);
> +
> +static void __exit arm_v7s_exit(void)
> +{
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +}
> +module_exit(arm_v7s_exit);
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 87def58..ff0ea2f 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -13,6 +13,7 @@
> #include <linux/bitops.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
> - .alloc = arm_mali_lpae_alloc_pgtable,
> - .free = arm_lpae_free_pgtable,
> +static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] = {
> + {
> + .fmt = ARM_32_LPAE_S1,
> + .alloc = arm_32_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_32_LPAE_S2,
> + .alloc = arm_32_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S1,
> + .alloc = arm_64_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S2,
> + .alloc = arm_64_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_MALI_LPAE,
> + .alloc = arm_mali_lpae_alloc_pgtable,
> + .free = arm_lpae_free_pgtable,
> + },
> };
>
> #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
> @@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
> pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> return fail ? -EFAULT : 0;
> }
> -subsys_initcall(arm_lpae_do_selftests);
> +#else
> +static int __init arm_lpae_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_lpae_init(void)
> +{
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++) {
> + ret = io_pgtable_ops_register(&io_pgtable_arm_lpae_init_fns[i]);
> + if (ret < 0) {
> + pr_err("Failed to register ARM LPAE fmt: %d\n");
> + goto err_io_pgtable_register;
> + }
> + }
> +
> + ret = arm_lpae_do_selftests();
> + if (ret < 0)
> + goto err_io_pgtable_register;
> +
> + return 0;
> +
> +err_io_pgtable_register:
> + for (i = i - 1; i >= 0; i--)
Personally I find "while (i--)" a bit clearer for this kind of
unwinding, but maybe post-decrement isn't to everyone's taste.
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> + return ret;
> +}
> +core_initcall(arm_lpae_init);
> +
> +static void __exit arm_lpae_exit(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++)
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> +}
> +module_exit(arm_lpae_exit);
> diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> index 94394c8..2c6eb2e 100644
> --- a/drivers/iommu/io-pgtable.c
> +++ b/drivers/iommu/io-pgtable.c
> @@ -10,33 +10,45 @@
> #include <linux/bug.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/rwlock.h>
> +#include <linux/slab.h>
> #include <linux/types.h>
>
> -static const struct io_pgtable_init_fns *
> -io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE
> - [ARM_32_LPAE_S1] = &io_pgtable_arm_32_lpae_s1_init_fns,
> - [ARM_32_LPAE_S2] = &io_pgtable_arm_32_lpae_s2_init_fns,
> - [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
> - [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
> - [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns,
> -#endif
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S
> - [ARM_V7S] = &io_pgtable_arm_v7s_init_fns,
> -#endif
> +struct io_pgtable_init_fns_node {
> + struct io_pgtable_init_fns *fns;
> + struct list_head list;
> };
>
> +static LIST_HEAD(io_pgtable_init_fns_list);
> +static DEFINE_RWLOCK(io_pgtable_init_fns_list_lock);
> +
> +static struct io_pgtable_init_fns *io_pgtable_get_init_fns(enum io_pgtable_fmt fmt)
> +{
> + struct io_pgtable_init_fns_node *iter;
> + struct io_pgtable_init_fns *fns = NULL;
> +
> + read_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == fmt) {
> + fns = iter->fns;
> + break;
> + }
> + read_unlock(&io_pgtable_init_fns_list_lock);
> +
> + return fns;
> +}
I think it would be a lot easier to stick with a simple array indexed by
enum - that way you can just set/clear/test entries without needing to
worry about locking. Basically just remove the const and the
initialisers from the existing one ;)
(and if you think you're concerned about memory, consider that just the
list head plus lock is already half the size of the table)
Other than that, I think this all looks pretty promising - I'd suggest
sending a non-RFC after rc1 so that it gets everyone's proper attention.
Thanks,
Robin.
> +
> struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,
> struct io_pgtable_cfg *cfg,
> void *cookie)
> {
> struct io_pgtable *iop;
> - const struct io_pgtable_init_fns *fns;
> + struct io_pgtable_init_fns *fns;
>
> if (fmt >= IO_PGTABLE_NUM_FMTS)
> return NULL;
>
> - fns = io_pgtable_init_table[fmt];
> + fns = io_pgtable_get_init_fns(fmt);
> if (!fns)
> return NULL;
>
> @@ -59,12 +71,64 @@ EXPORT_SYMBOL_GPL(alloc_io_pgtable_ops);
> void free_io_pgtable_ops(struct io_pgtable_ops *ops)
> {
> struct io_pgtable *iop;
> + struct io_pgtable_init_fns *fns;
>
> if (!ops)
> return;
>
> iop = io_pgtable_ops_to_pgtable(ops);
> io_pgtable_tlb_flush_all(iop);
> - io_pgtable_init_table[iop->fmt]->free(iop);
> + fns = io_pgtable_get_init_fns(iop->fmt);
> + if (fns)
> + fns->free(iop);
> }
> EXPORT_SYMBOL_GPL(free_io_pgtable_ops);
> +
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *fns_node;
> + int ret = 0;
> +
> + if (!init_fns || init_fns->fmt >= IO_PGTABLE_NUM_FMTS ||
> + !init_fns->alloc || !init_fns->free)
> + return -EINVAL;
> +
> + fns_node = kzalloc(sizeof(*fns_node), GFP_KERNEL);
> + if (!fns_node)
> + return -ENOMEM;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == init_fns->fmt) {
> + ret = -EEXIST;
> + kfree(fns_node);
> + break;
> + }
> +
> + if (!ret) {
> + fns_node->fns = init_fns;
> + INIT_LIST_HEAD(&fns_node->list);
> + list_add_tail(&fns_node->list, &io_pgtable_init_fns_list);
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_register);
> +
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *tmp;
> +
> + if (!init_fns)
> + return;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry_safe(iter, tmp, &io_pgtable_init_fns_list, list)
> + if (iter->fns == init_fns) {
> + list_del(&iter->list);
> + kfree(iter);
> + break;
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_unregister);
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index ea727eb..45b367ce 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -163,6 +163,38 @@ struct io_pgtable_ops {
> };
>
> /**
> + * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> + * particular format.
> + *
> + * @fmt: The page table format.
> + * @alloc: Allocate a set of page tables described by cfg.
> + * @free: Free the page tables associated with iop.
> + */
> +struct io_pgtable_init_fns {
> + enum io_pgtable_fmt fmt;
> + struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> + void (*free)(struct io_pgtable *iop);
> +};
> +
> +/**
> + * io_pgtable_ops_register() - Register the page table routines for a page table
> + * format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> + * io_pgtable_ops_unregister() - Unregister the page table routines for a page
> + * table format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> * alloc_io_pgtable_ops() - Allocate a page table allocator for use by an IOMMU.
> *
> * @fmt: The page table format.
> @@ -233,23 +265,4 @@ io_pgtable_tlb_add_page(struct io_pgtable *iop,
> iop->cfg.tlb->tlb_add_page(gather, iova, granule, iop->cookie);
> }
>
> -/**
> - * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> - * particular format.
> - *
> - * @alloc: Allocate a set of page tables described by cfg.
> - * @free: Free the page tables associated with iop.
> - */
> -struct io_pgtable_init_fns {
> - struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> - void (*free)(struct io_pgtable *iop);
> -};
> -
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns;
> -
> #endif /* __IO_PGTABLE_H */
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: "Isaac J. Manjarres" <isaacm@codeaurora.org>,
iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, freedreno@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: pratikp@codeaurora.org, joro@8bytes.org, kernel-team@android.com,
will@kernel.org, pdaly@codeaurora.org
Subject: Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Date: Tue, 22 Dec 2020 19:27:41 +0000 [thread overview]
Message-ID: <4f388512-c3df-e9fc-ebd4-624ab36a6858@arm.com> (raw)
In-Reply-To: <1608597876-32367-2-git-send-email-isaacm@codeaurora.org>
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
> The io-pgtable code constructs an array of init functions for each
> page table format at compile time. This is not ideal, as this
> increases the footprint of the io-pgtable code, as well as prevents
> io-pgtable formats from being built as kernel modules.
>
> In preparation for modularizing the io-pgtable formats, switch to a
> dynamic registration scheme, where each io-pgtable format can register
> their init functions with the io-pgtable code at boot or module
> insertion time.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 34 +++++++++++++-
> drivers/iommu/io-pgtable-arm.c | 90 ++++++++++++++++++++++++++----------
> drivers/iommu/io-pgtable.c | 94 ++++++++++++++++++++++++++++++++------
> include/linux/io-pgtable.h | 51 +++++++++++++--------
> 4 files changed, 209 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index 1d92ac9..89aad2f 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -28,6 +28,7 @@
> #include <linux/iommu.h>
> #include <linux/kernel.h>
> #include <linux/kmemleak.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> @@ -835,7 +836,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> +static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> + .fmt = ARM_V7S,
> .alloc = arm_v7s_alloc_pgtable,
> .free = arm_v7s_free_pgtable,
> };
> @@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
> pr_info("self test ok\n");
> return 0;
> }
> -subsys_initcall(arm_v7s_do_selftests);
> +#else
> +static int arm_v7s_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_v7s_init(void)
> +{
> + int ret;
> +
> + ret = io_pgtable_ops_register(&io_pgtable_arm_v7s_init_fns);
> + if (ret < 0) {
> + pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not to
abbreviate "format" would be nice.
> + return ret;
> + }
> +
> + ret = arm_v7s_do_selftests();
> + if (ret < 0)
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +
> + return ret;
> +}
> +core_initcall(arm_v7s_init);
> +
> +static void __exit arm_v7s_exit(void)
> +{
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +}
> +module_exit(arm_v7s_exit);
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 87def58..ff0ea2f 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -13,6 +13,7 @@
> #include <linux/bitops.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
> - .alloc = arm_mali_lpae_alloc_pgtable,
> - .free = arm_lpae_free_pgtable,
> +static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] = {
> + {
> + .fmt = ARM_32_LPAE_S1,
> + .alloc = arm_32_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_32_LPAE_S2,
> + .alloc = arm_32_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S1,
> + .alloc = arm_64_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S2,
> + .alloc = arm_64_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_MALI_LPAE,
> + .alloc = arm_mali_lpae_alloc_pgtable,
> + .free = arm_lpae_free_pgtable,
> + },
> };
>
> #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
> @@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
> pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> return fail ? -EFAULT : 0;
> }
> -subsys_initcall(arm_lpae_do_selftests);
> +#else
> +static int __init arm_lpae_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_lpae_init(void)
> +{
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++) {
> + ret = io_pgtable_ops_register(&io_pgtable_arm_lpae_init_fns[i]);
> + if (ret < 0) {
> + pr_err("Failed to register ARM LPAE fmt: %d\n");
> + goto err_io_pgtable_register;
> + }
> + }
> +
> + ret = arm_lpae_do_selftests();
> + if (ret < 0)
> + goto err_io_pgtable_register;
> +
> + return 0;
> +
> +err_io_pgtable_register:
> + for (i = i - 1; i >= 0; i--)
Personally I find "while (i--)" a bit clearer for this kind of
unwinding, but maybe post-decrement isn't to everyone's taste.
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> + return ret;
> +}
> +core_initcall(arm_lpae_init);
> +
> +static void __exit arm_lpae_exit(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++)
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> +}
> +module_exit(arm_lpae_exit);
> diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> index 94394c8..2c6eb2e 100644
> --- a/drivers/iommu/io-pgtable.c
> +++ b/drivers/iommu/io-pgtable.c
> @@ -10,33 +10,45 @@
> #include <linux/bug.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/rwlock.h>
> +#include <linux/slab.h>
> #include <linux/types.h>
>
> -static const struct io_pgtable_init_fns *
> -io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE
> - [ARM_32_LPAE_S1] = &io_pgtable_arm_32_lpae_s1_init_fns,
> - [ARM_32_LPAE_S2] = &io_pgtable_arm_32_lpae_s2_init_fns,
> - [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
> - [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
> - [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns,
> -#endif
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S
> - [ARM_V7S] = &io_pgtable_arm_v7s_init_fns,
> -#endif
> +struct io_pgtable_init_fns_node {
> + struct io_pgtable_init_fns *fns;
> + struct list_head list;
> };
>
> +static LIST_HEAD(io_pgtable_init_fns_list);
> +static DEFINE_RWLOCK(io_pgtable_init_fns_list_lock);
> +
> +static struct io_pgtable_init_fns *io_pgtable_get_init_fns(enum io_pgtable_fmt fmt)
> +{
> + struct io_pgtable_init_fns_node *iter;
> + struct io_pgtable_init_fns *fns = NULL;
> +
> + read_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == fmt) {
> + fns = iter->fns;
> + break;
> + }
> + read_unlock(&io_pgtable_init_fns_list_lock);
> +
> + return fns;
> +}
I think it would be a lot easier to stick with a simple array indexed by
enum - that way you can just set/clear/test entries without needing to
worry about locking. Basically just remove the const and the
initialisers from the existing one ;)
(and if you think you're concerned about memory, consider that just the
list head plus lock is already half the size of the table)
Other than that, I think this all looks pretty promising - I'd suggest
sending a non-RFC after rc1 so that it gets everyone's proper attention.
Thanks,
Robin.
> +
> struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,
> struct io_pgtable_cfg *cfg,
> void *cookie)
> {
> struct io_pgtable *iop;
> - const struct io_pgtable_init_fns *fns;
> + struct io_pgtable_init_fns *fns;
>
> if (fmt >= IO_PGTABLE_NUM_FMTS)
> return NULL;
>
> - fns = io_pgtable_init_table[fmt];
> + fns = io_pgtable_get_init_fns(fmt);
> if (!fns)
> return NULL;
>
> @@ -59,12 +71,64 @@ EXPORT_SYMBOL_GPL(alloc_io_pgtable_ops);
> void free_io_pgtable_ops(struct io_pgtable_ops *ops)
> {
> struct io_pgtable *iop;
> + struct io_pgtable_init_fns *fns;
>
> if (!ops)
> return;
>
> iop = io_pgtable_ops_to_pgtable(ops);
> io_pgtable_tlb_flush_all(iop);
> - io_pgtable_init_table[iop->fmt]->free(iop);
> + fns = io_pgtable_get_init_fns(iop->fmt);
> + if (fns)
> + fns->free(iop);
> }
> EXPORT_SYMBOL_GPL(free_io_pgtable_ops);
> +
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *fns_node;
> + int ret = 0;
> +
> + if (!init_fns || init_fns->fmt >= IO_PGTABLE_NUM_FMTS ||
> + !init_fns->alloc || !init_fns->free)
> + return -EINVAL;
> +
> + fns_node = kzalloc(sizeof(*fns_node), GFP_KERNEL);
> + if (!fns_node)
> + return -ENOMEM;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == init_fns->fmt) {
> + ret = -EEXIST;
> + kfree(fns_node);
> + break;
> + }
> +
> + if (!ret) {
> + fns_node->fns = init_fns;
> + INIT_LIST_HEAD(&fns_node->list);
> + list_add_tail(&fns_node->list, &io_pgtable_init_fns_list);
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_register);
> +
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *tmp;
> +
> + if (!init_fns)
> + return;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry_safe(iter, tmp, &io_pgtable_init_fns_list, list)
> + if (iter->fns == init_fns) {
> + list_del(&iter->list);
> + kfree(iter);
> + break;
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_unregister);
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index ea727eb..45b367ce 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -163,6 +163,38 @@ struct io_pgtable_ops {
> };
>
> /**
> + * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> + * particular format.
> + *
> + * @fmt: The page table format.
> + * @alloc: Allocate a set of page tables described by cfg.
> + * @free: Free the page tables associated with iop.
> + */
> +struct io_pgtable_init_fns {
> + enum io_pgtable_fmt fmt;
> + struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> + void (*free)(struct io_pgtable *iop);
> +};
> +
> +/**
> + * io_pgtable_ops_register() - Register the page table routines for a page table
> + * format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> + * io_pgtable_ops_unregister() - Unregister the page table routines for a page
> + * table format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> * alloc_io_pgtable_ops() - Allocate a page table allocator for use by an IOMMU.
> *
> * @fmt: The page table format.
> @@ -233,23 +265,4 @@ io_pgtable_tlb_add_page(struct io_pgtable *iop,
> iop->cfg.tlb->tlb_add_page(gather, iova, granule, iop->cookie);
> }
>
> -/**
> - * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> - * particular format.
> - *
> - * @alloc: Allocate a set of page tables described by cfg.
> - * @free: Free the page tables associated with iop.
> - */
> -struct io_pgtable_init_fns {
> - struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> - void (*free)(struct io_pgtable *iop);
> -};
> -
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns;
> -
> #endif /* __IO_PGTABLE_H */
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: "Isaac J. Manjarres" <isaacm@codeaurora.org>,
iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, freedreno@lists.freedesktop.org,
dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: pratikp@codeaurora.org, joro@8bytes.org, kernel-team@android.com,
will@kernel.org, pdaly@codeaurora.org
Subject: Re: [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration
Date: Tue, 22 Dec 2020 19:27:41 +0000 [thread overview]
Message-ID: <4f388512-c3df-e9fc-ebd4-624ab36a6858@arm.com> (raw)
In-Reply-To: <1608597876-32367-2-git-send-email-isaacm@codeaurora.org>
On 2020-12-22 00:44, Isaac J. Manjarres wrote:
> The io-pgtable code constructs an array of init functions for each
> page table format at compile time. This is not ideal, as this
> increases the footprint of the io-pgtable code, as well as prevents
> io-pgtable formats from being built as kernel modules.
>
> In preparation for modularizing the io-pgtable formats, switch to a
> dynamic registration scheme, where each io-pgtable format can register
> their init functions with the io-pgtable code at boot or module
> insertion time.
>
> Signed-off-by: Isaac J. Manjarres <isaacm@codeaurora.org>
> ---
> drivers/iommu/io-pgtable-arm-v7s.c | 34 +++++++++++++-
> drivers/iommu/io-pgtable-arm.c | 90 ++++++++++++++++++++++++++----------
> drivers/iommu/io-pgtable.c | 94 ++++++++++++++++++++++++++++++++------
> include/linux/io-pgtable.h | 51 +++++++++++++--------
> 4 files changed, 209 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm-v7s.c b/drivers/iommu/io-pgtable-arm-v7s.c
> index 1d92ac9..89aad2f 100644
> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -28,6 +28,7 @@
> #include <linux/iommu.h>
> #include <linux/kernel.h>
> #include <linux/kmemleak.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> @@ -835,7 +836,8 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> +static struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns = {
> + .fmt = ARM_V7S,
> .alloc = arm_v7s_alloc_pgtable,
> .free = arm_v7s_free_pgtable,
> };
> @@ -982,5 +984,33 @@ static int __init arm_v7s_do_selftests(void)
> pr_info("self test ok\n");
> return 0;
> }
> -subsys_initcall(arm_v7s_do_selftests);
> +#else
> +static int arm_v7s_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_v7s_init(void)
> +{
> + int ret;
> +
> + ret = io_pgtable_ops_register(&io_pgtable_arm_v7s_init_fns);
> + if (ret < 0) {
> + pr_err("Failed to register ARM V7S format\n");
Super-nit: I think "v7s" should probably be lowercase there. Also
general consistency WRT to showing the error code and whether or not to
abbreviate "format" would be nice.
> + return ret;
> + }
> +
> + ret = arm_v7s_do_selftests();
> + if (ret < 0)
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +
> + return ret;
> +}
> +core_initcall(arm_v7s_init);
> +
> +static void __exit arm_v7s_exit(void)
> +{
> + io_pgtable_ops_unregister(&io_pgtable_arm_v7s_init_fns);
> +}
> +module_exit(arm_v7s_exit);
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index 87def58..ff0ea2f 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -13,6 +13,7 @@
> #include <linux/bitops.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> #include <linux/types.h>
> @@ -1043,29 +1044,32 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, void *cookie)
> return NULL;
> }
>
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns = {
> - .alloc = arm_64_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s1,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns = {
> - .alloc = arm_32_lpae_alloc_pgtable_s2,
> - .free = arm_lpae_free_pgtable,
> -};
> -
> -struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns = {
> - .alloc = arm_mali_lpae_alloc_pgtable,
> - .free = arm_lpae_free_pgtable,
> +static struct io_pgtable_init_fns io_pgtable_arm_lpae_init_fns[] = {
> + {
> + .fmt = ARM_32_LPAE_S1,
> + .alloc = arm_32_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_32_LPAE_S2,
> + .alloc = arm_32_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S1,
> + .alloc = arm_64_lpae_alloc_pgtable_s1,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_64_LPAE_S2,
> + .alloc = arm_64_lpae_alloc_pgtable_s2,
> + .free = arm_lpae_free_pgtable,
> + },
> + {
> + .fmt = ARM_MALI_LPAE,
> + .alloc = arm_mali_lpae_alloc_pgtable,
> + .free = arm_lpae_free_pgtable,
> + },
> };
>
> #ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE_SELFTEST
> @@ -1250,5 +1254,43 @@ static int __init arm_lpae_do_selftests(void)
> pr_info("selftest: completed with %d PASS %d FAIL\n", pass, fail);
> return fail ? -EFAULT : 0;
> }
> -subsys_initcall(arm_lpae_do_selftests);
> +#else
> +static int __init arm_lpae_do_selftests(void)
> +{
> + return 0;
> +}
> #endif
> +
> +static int __init arm_lpae_init(void)
> +{
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++) {
> + ret = io_pgtable_ops_register(&io_pgtable_arm_lpae_init_fns[i]);
> + if (ret < 0) {
> + pr_err("Failed to register ARM LPAE fmt: %d\n");
> + goto err_io_pgtable_register;
> + }
> + }
> +
> + ret = arm_lpae_do_selftests();
> + if (ret < 0)
> + goto err_io_pgtable_register;
> +
> + return 0;
> +
> +err_io_pgtable_register:
> + for (i = i - 1; i >= 0; i--)
Personally I find "while (i--)" a bit clearer for this kind of
unwinding, but maybe post-decrement isn't to everyone's taste.
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> + return ret;
> +}
> +core_initcall(arm_lpae_init);
> +
> +static void __exit arm_lpae_exit(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(io_pgtable_arm_lpae_init_fns); i++)
> + io_pgtable_ops_unregister(&io_pgtable_arm_lpae_init_fns[i]);
> +}
> +module_exit(arm_lpae_exit);
> diff --git a/drivers/iommu/io-pgtable.c b/drivers/iommu/io-pgtable.c
> index 94394c8..2c6eb2e 100644
> --- a/drivers/iommu/io-pgtable.c
> +++ b/drivers/iommu/io-pgtable.c
> @@ -10,33 +10,45 @@
> #include <linux/bug.h>
> #include <linux/io-pgtable.h>
> #include <linux/kernel.h>
> +#include <linux/rwlock.h>
> +#include <linux/slab.h>
> #include <linux/types.h>
>
> -static const struct io_pgtable_init_fns *
> -io_pgtable_init_table[IO_PGTABLE_NUM_FMTS] = {
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_LPAE
> - [ARM_32_LPAE_S1] = &io_pgtable_arm_32_lpae_s1_init_fns,
> - [ARM_32_LPAE_S2] = &io_pgtable_arm_32_lpae_s2_init_fns,
> - [ARM_64_LPAE_S1] = &io_pgtable_arm_64_lpae_s1_init_fns,
> - [ARM_64_LPAE_S2] = &io_pgtable_arm_64_lpae_s2_init_fns,
> - [ARM_MALI_LPAE] = &io_pgtable_arm_mali_lpae_init_fns,
> -#endif
> -#ifdef CONFIG_IOMMU_IO_PGTABLE_ARMV7S
> - [ARM_V7S] = &io_pgtable_arm_v7s_init_fns,
> -#endif
> +struct io_pgtable_init_fns_node {
> + struct io_pgtable_init_fns *fns;
> + struct list_head list;
> };
>
> +static LIST_HEAD(io_pgtable_init_fns_list);
> +static DEFINE_RWLOCK(io_pgtable_init_fns_list_lock);
> +
> +static struct io_pgtable_init_fns *io_pgtable_get_init_fns(enum io_pgtable_fmt fmt)
> +{
> + struct io_pgtable_init_fns_node *iter;
> + struct io_pgtable_init_fns *fns = NULL;
> +
> + read_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == fmt) {
> + fns = iter->fns;
> + break;
> + }
> + read_unlock(&io_pgtable_init_fns_list_lock);
> +
> + return fns;
> +}
I think it would be a lot easier to stick with a simple array indexed by
enum - that way you can just set/clear/test entries without needing to
worry about locking. Basically just remove the const and the
initialisers from the existing one ;)
(and if you think you're concerned about memory, consider that just the
list head plus lock is already half the size of the table)
Other than that, I think this all looks pretty promising - I'd suggest
sending a non-RFC after rc1 so that it gets everyone's proper attention.
Thanks,
Robin.
> +
> struct io_pgtable_ops *alloc_io_pgtable_ops(enum io_pgtable_fmt fmt,
> struct io_pgtable_cfg *cfg,
> void *cookie)
> {
> struct io_pgtable *iop;
> - const struct io_pgtable_init_fns *fns;
> + struct io_pgtable_init_fns *fns;
>
> if (fmt >= IO_PGTABLE_NUM_FMTS)
> return NULL;
>
> - fns = io_pgtable_init_table[fmt];
> + fns = io_pgtable_get_init_fns(fmt);
> if (!fns)
> return NULL;
>
> @@ -59,12 +71,64 @@ EXPORT_SYMBOL_GPL(alloc_io_pgtable_ops);
> void free_io_pgtable_ops(struct io_pgtable_ops *ops)
> {
> struct io_pgtable *iop;
> + struct io_pgtable_init_fns *fns;
>
> if (!ops)
> return;
>
> iop = io_pgtable_ops_to_pgtable(ops);
> io_pgtable_tlb_flush_all(iop);
> - io_pgtable_init_table[iop->fmt]->free(iop);
> + fns = io_pgtable_get_init_fns(iop->fmt);
> + if (fns)
> + fns->free(iop);
> }
> EXPORT_SYMBOL_GPL(free_io_pgtable_ops);
> +
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *fns_node;
> + int ret = 0;
> +
> + if (!init_fns || init_fns->fmt >= IO_PGTABLE_NUM_FMTS ||
> + !init_fns->alloc || !init_fns->free)
> + return -EINVAL;
> +
> + fns_node = kzalloc(sizeof(*fns_node), GFP_KERNEL);
> + if (!fns_node)
> + return -ENOMEM;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry(iter, &io_pgtable_init_fns_list, list)
> + if (iter->fns->fmt == init_fns->fmt) {
> + ret = -EEXIST;
> + kfree(fns_node);
> + break;
> + }
> +
> + if (!ret) {
> + fns_node->fns = init_fns;
> + INIT_LIST_HEAD(&fns_node->list);
> + list_add_tail(&fns_node->list, &io_pgtable_init_fns_list);
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_register);
> +
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns)
> +{
> + struct io_pgtable_init_fns_node *iter, *tmp;
> +
> + if (!init_fns)
> + return;
> +
> + write_lock(&io_pgtable_init_fns_list_lock);
> + list_for_each_entry_safe(iter, tmp, &io_pgtable_init_fns_list, list)
> + if (iter->fns == init_fns) {
> + list_del(&iter->list);
> + kfree(iter);
> + break;
> + }
> + write_unlock(&io_pgtable_init_fns_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(io_pgtable_ops_unregister);
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index ea727eb..45b367ce 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -163,6 +163,38 @@ struct io_pgtable_ops {
> };
>
> /**
> + * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> + * particular format.
> + *
> + * @fmt: The page table format.
> + * @alloc: Allocate a set of page tables described by cfg.
> + * @free: Free the page tables associated with iop.
> + */
> +struct io_pgtable_init_fns {
> + enum io_pgtable_fmt fmt;
> + struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> + void (*free)(struct io_pgtable *iop);
> +};
> +
> +/**
> + * io_pgtable_ops_register() - Register the page table routines for a page table
> + * format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +int io_pgtable_ops_register(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> + * io_pgtable_ops_unregister() - Unregister the page table routines for a page
> + * table format.
> + *
> + * @init_fns: The functions for allocating and freeing the page tables of
> + * a particular format.
> + */
> +void io_pgtable_ops_unregister(struct io_pgtable_init_fns *init_fns);
> +
> +/**
> * alloc_io_pgtable_ops() - Allocate a page table allocator for use by an IOMMU.
> *
> * @fmt: The page table format.
> @@ -233,23 +265,4 @@ io_pgtable_tlb_add_page(struct io_pgtable *iop,
> iop->cfg.tlb->tlb_add_page(gather, iova, granule, iop->cookie);
> }
>
> -/**
> - * struct io_pgtable_init_fns - Alloc/free a set of page tables for a
> - * particular format.
> - *
> - * @alloc: Allocate a set of page tables described by cfg.
> - * @free: Free the page tables associated with iop.
> - */
> -struct io_pgtable_init_fns {
> - struct io_pgtable *(*alloc)(struct io_pgtable_cfg *cfg, void *cookie);
> - void (*free)(struct io_pgtable *iop);
> -};
> -
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_32_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s1_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_64_lpae_s2_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_v7s_init_fns;
> -extern struct io_pgtable_init_fns io_pgtable_arm_mali_lpae_init_fns;
> -
> #endif /* __IO_PGTABLE_H */
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-12-22 19:28 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-22 0:44 [RFC PATCH v2 0/7] iommu: Permit modular builds of io-pgtable drivers Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` [PATCH v2 1/7] iommu/io-pgtable: Introduce dynamic io-pgtable fmt registration Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 19:27 ` Robin Murphy [this message]
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:54 ` isaacm
2020-12-22 19:54 ` isaacm
2020-12-22 19:54 ` isaacm
2020-12-23 13:44 ` Robin Murphy
2020-12-23 13:44 ` Robin Murphy
2020-12-23 13:44 ` Robin Murphy
2020-12-23 13:44 ` Robin Murphy
2020-12-23 15:14 ` isaacm
2020-12-23 15:14 ` isaacm
2020-12-23 15:14 ` isaacm
2020-12-23 15:38 ` Georgi Djakov
2020-12-23 15:38 ` Georgi Djakov
2020-12-23 15:38 ` Georgi Djakov
2020-12-23 15:38 ` Georgi Djakov
2020-12-23 19:17 ` kernel test robot
2020-12-23 19:17 ` kernel test robot
2020-12-23 19:17 ` kernel test robot
2020-12-23 19:17 ` kernel test robot
2020-12-23 19:17 ` kernel test robot
2020-12-22 0:44 ` [PATCH v2 2/7] iommu/io-pgtable: Add refcounting for io-pgtable format modules Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` [PATCH v2 3/7] iommu/arm-smmu: Add dependency on " Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:27 ` Robin Murphy
2020-12-22 19:49 ` isaacm
2020-12-22 19:49 ` isaacm
2020-12-22 19:49 ` isaacm
2020-12-23 13:05 ` Robin Murphy
2020-12-23 13:05 ` Robin Murphy
2020-12-23 13:05 ` Robin Murphy
2020-12-23 13:05 ` Robin Murphy
2020-12-23 15:10 ` isaacm
2020-12-23 15:10 ` isaacm
2020-12-23 15:10 ` isaacm
2020-12-22 0:44 ` [PATCH v2 4/7] iommu/arm-smmu-v3: Add dependency on io-pgtable-arm format module Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` [PATCH v2 5/7] drm/msm: " Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2021-01-18 21:16 ` Rob Clark
2021-01-18 21:16 ` Rob Clark
2021-01-18 21:16 ` Rob Clark
2021-01-18 21:16 ` Rob Clark
2021-01-18 21:39 ` Will Deacon
2021-01-18 21:39 ` Will Deacon
2021-01-18 21:39 ` Will Deacon
2021-01-18 21:39 ` Will Deacon
2021-01-19 17:19 ` Rob Clark
2021-01-19 17:19 ` Rob Clark
2021-01-19 17:19 ` Rob Clark
2021-01-19 17:19 ` Rob Clark
2020-12-22 0:44 ` [PATCH v2 6/7] drm/panfrost: " Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` [PATCH v2 7/7] iommu/io-pgtable-arm: Allow building modular io-pgtable fmts Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
2020-12-22 0:44 ` Isaac J. Manjarres
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4f388512-c3df-e9fc-ebd4-624ab36a6858@arm.com \
--to=robin.murphy@arm.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=iommu@lists.linux-foundation.org \
--cc=isaacm@codeaurora.org \
--cc=joro@8bytes.org \
--cc=kernel-team@android.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pdaly@codeaurora.org \
--cc=pratikp@codeaurora.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.