| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <857004e1-6ab1-f825-7796-9c0b557e7cc8@redhat.com>
Date: Fri, 24 Mar 2023 16:28:26 +0100
From: Eric Auger <eric.auger@...hat.com>
To: Nicolin Chen <nicolinc@...dia.com>, jgg@...dia.com,
robin.murphy@....com, will@...nel.org
Cc: kevin.tian@...el.com, baolu.lu@...ux.intel.com, joro@...tes.org,
shameerali.kolothum.thodi@...wei.com, jean-philippe@...aro.org,
linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 11/14] iommu/arm-smmu-v3: Add
arm_smmu_domain_alloc_user
Hi Nicolin,
On 3/9/23 11:53, Nicolin Chen wrote:
> The arm_smmu_domain_alloc_user callback function is used for userspace to
> allocate iommu_domains, such as standalone stage-1 domain, nested stage-1
> domain, and nested stage-2 domain. The input user_data is in the type of
> struct iommu_hwpt_arm_smmuv3 that contains the configurations of a nested
> stage-1 or a nested stage-2 iommu_domain. A NULL user_data will just opt
> in a standalone stage-1 domain allocation.
>
> Add a constitutive function __arm_smmu_domain_alloc to support that.
>
> Since ops->domain_alloc_user has a valid dev pointer, the master pointer
> is available when calling __arm_smmu_domain_alloc() in this case, meaning
> that arm_smmu_domain_finalise() can be done at the allocation stage. This
> allows IOMMUFD to initialize the hw_pagetable for the domain.
>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 95 ++++++++++++++-------
> 1 file changed, 65 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 2d29f7320570..5ff74edfbd68 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2053,36 +2053,6 @@ static void *arm_smmu_hw_info(struct device *dev, u32 *length)
> return info;
> }
>
> -static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
> -{
> - struct arm_smmu_domain *smmu_domain;
> -
> - if (type == IOMMU_DOMAIN_SVA)
> - return arm_smmu_sva_domain_alloc();
> -
> - if (type != IOMMU_DOMAIN_UNMANAGED &&
> - type != IOMMU_DOMAIN_DMA &&
> - type != IOMMU_DOMAIN_DMA_FQ &&
> - type != IOMMU_DOMAIN_IDENTITY)
> - return NULL;
> -
> - /*
> - * Allocate the domain and initialise some of its data structures.
> - * We can't really do anything meaningful until we've added a
> - * master.
> - */
> - smmu_domain = kzalloc(sizeof(*smmu_domain), GFP_KERNEL);
> - if (!smmu_domain)
> - return NULL;
> -
> - mutex_init(&smmu_domain->init_mutex);
> - INIT_LIST_HEAD(&smmu_domain->devices);
> - spin_lock_init(&smmu_domain->devices_lock);
> - INIT_LIST_HEAD(&smmu_domain->mmu_notifiers);
> -
> - return &smmu_domain->domain;
> -}
> -
> static struct iommu_domain *arm_smmu_get_unmanaged_domain(struct device *dev)
> {
> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> @@ -2893,10 +2863,75 @@ static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
> arm_smmu_sva_remove_dev_pasid(domain, dev, pasid);
> }
>
> +static struct iommu_domain *
> +__arm_smmu_domain_alloc(unsigned type,
> + struct arm_smmu_domain *s2,
> + struct arm_smmu_master *master,
> + const struct iommu_hwpt_arm_smmuv3 *user_cfg)
> +{
> + struct arm_smmu_domain *smmu_domain;
> + struct iommu_domain *domain;
> + int ret = 0;
> +
> + if (type == IOMMU_DOMAIN_SVA)
> + return arm_smmu_sva_domain_alloc();
> +
> + if (type != IOMMU_DOMAIN_UNMANAGED &&
> + type != IOMMU_DOMAIN_DMA &&
> + type != IOMMU_DOMAIN_DMA_FQ &&
> + type != IOMMU_DOMAIN_IDENTITY)
> + return NULL;
> +
> + /*
> + * Allocate the domain and initialise some of its data structures.
> + * We can't really finalise the domain unless a master is given.
> + */
> + smmu_domain = kzalloc(sizeof(*smmu_domain), GFP_KERNEL);
> + if (!smmu_domain)
> + return NULL;
> + domain = &smmu_domain->domain;
> +
> + domain->type = type;
> + domain->ops = arm_smmu_ops.default_domain_ops;
Compared to the original code, that's something new. Please can you
explain why this is added in this patch?
> +
> + mutex_init(&smmu_domain->init_mutex);
> + INIT_LIST_HEAD(&smmu_domain->devices);
> + spin_lock_init(&smmu_domain->devices_lock);
> + INIT_LIST_HEAD(&smmu_domain->mmu_notifiers);
> +
> + if (master) {
> + smmu_domain->smmu = master->smmu;
> + ret = arm_smmu_domain_finalise(domain, master, user_cfg);
> + if (ret) {
> + kfree(smmu_domain);
> + return NULL;
> + }
> + }
> +
> + return domain;
> +}
> +
> +static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
> +{
> + return __arm_smmu_domain_alloc(type, NULL, NULL, NULL);
> +}
> +
> +static struct iommu_domain *
> +arm_smmu_domain_alloc_user(struct device *dev, struct iommu_domain *parent,
> + const void *user_data)
> +{
> + const struct iommu_hwpt_arm_smmuv3 *user_cfg = user_data;
> + struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> + unsigned type = IOMMU_DOMAIN_UNMANAGED;
is there any guarantee that master is non null? Don't we want to check?
> +
> + return __arm_smmu_domain_alloc(type, NULL, master, user_cfg);
> +}
> +
> static struct iommu_ops arm_smmu_ops = {
> .capable = arm_smmu_capable,
> .hw_info = arm_smmu_hw_info,
> .domain_alloc = arm_smmu_domain_alloc,
> + .domain_alloc_user = arm_smmu_domain_alloc_user,
> .get_unmanaged_domain = arm_smmu_get_unmanaged_domain,
> .probe_device = arm_smmu_probe_device,
> .release_device = arm_smmu_release_device,
Thanks
Eric
Powered by blists - more mailing lists