| 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
| ||
|
Date: Mon, 23 May 2022 11:07:32 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Jean-Philippe Brucker <jean-philippe@...aro.org>
Cc: baolu.lu@...ux.intel.com, Joerg Roedel <joro@...tes.org>,
Jason Gunthorpe <jgg@...dia.com>,
Christoph Hellwig <hch@...radead.org>,
Kevin Tian <kevin.tian@...el.com>,
Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
Jean-Philippe Brucker <jean-philippe@...aro.com>,
Dave Jiang <dave.jiang@...el.com>,
Vinod Koul <vkoul@...nel.org>,
Eric Auger <eric.auger@...hat.com>,
Liu Yi L <yi.l.liu@...el.com>,
Jacob jun Pan <jacob.jun.pan@...el.com>,
iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 06/10] iommu/sva: Refactoring
iommu_sva_bind/unbind_device()
On 2022/5/20 19:28, Jean-Philippe Brucker wrote:
> On Fri, May 20, 2022 at 02:38:12PM +0800, Baolu Lu wrote:
>> On 2022/5/20 00:39, Jean-Philippe Brucker wrote:
>>>> +struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm)
>>>> +{
>>>> + struct iommu_sva_domain *sva_domain;
>>>> + struct iommu_domain *domain;
>>>> + ioasid_t max_pasid = 0;
>>>> + int ret = -EINVAL;
>>>> +
>>>> + /* Allocate mm->pasid if necessary. */
>>>> + if (!dev->iommu->iommu_dev->pasids)
>>>> + return ERR_PTR(-EOPNOTSUPP);
>>>> +
>>>> + if (dev_is_pci(dev)) {
>>>> + max_pasid = pci_max_pasids(to_pci_dev(dev));
>>>> + if (max_pasid < 0)
>>>> + return ERR_PTR(max_pasid);
>>>> + } else {
>>>> + ret = device_property_read_u32(dev, "pasid-num-bits",
>>>> + &max_pasid);
>>>> + if (ret)
>>>> + return ERR_PTR(ret);
>>>> + max_pasid = (1UL << max_pasid);
>>>> + }
>>> The IOMMU driver needs this PASID width information earlier, when creating
>>> the PASID table (in .probe_device(), .attach_dev()). Since we're moving it
>>> to the IOMMU core to avoid code duplication, it should be done earlier and
>>> stored in dev->iommu
>> Yes, really. How about below changes?
>>
>> From f1382579e8a15ca49acdf758d38fd36451ea174d Mon Sep 17 00:00:00 2001
>> From: Lu Baolu<baolu.lu@...ux.intel.com>
>> Date: Mon, 28 Feb 2022 15:01:35 +0800
>> Subject: [PATCH 1/1] iommu: Add pasids field in struct dev_iommu
>>
>> Use this field to save the number of PASIDs that a device is able to
>> consume. It is a generic attribute of a device and lifting it into the
>> per-device dev_iommu struct could help to avoid the boilerplate code
>> in various IOMMU drivers.
>>
>> Signed-off-by: Lu Baolu<baolu.lu@...ux.intel.com>
>> ---
>> drivers/iommu/iommu.c | 15 +++++++++++++++
>> include/linux/iommu.h | 2 ++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index e49c5a5b8cc1..6b731171d42f 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -20,6 +20,7 @@
>> #include <linux/idr.h>
>> #include <linux/err.h>
>> #include <linux/pci.h>
>> +#include <linux/pci-ats.h>
>> #include <linux/bitops.h>
>> #include <linux/property.h>
>> #include <linux/fsl/mc.h>
>> @@ -194,6 +195,8 @@ EXPORT_SYMBOL_GPL(iommu_device_unregister);
>> static struct dev_iommu *dev_iommu_get(struct device *dev)
>> {
>> struct dev_iommu *param = dev->iommu;
>> + u32 max_pasids = 0;
>> + int ret;
>>
>> if (param)
>> return param;
>> @@ -202,6 +205,18 @@ static struct dev_iommu *dev_iommu_get(struct device
>> *dev)
>> if (!param)
>> return NULL;
>>
>> + if (dev_is_pci(dev)) {
>> + ret = pci_max_pasids(to_pci_dev(dev));
>> + if (ret > 0)
>> + max_pasids = ret;
>> + } else {
>> + ret = device_property_read_u32(dev, "pasid-num-bits",
>> + &max_pasids);
>> + if (!ret)
>> + max_pasids = (1UL << max_pasids);
>> + }
>> + param->pasids = max_pasids;
>> +
> we could also do a min() with the IOMMU PASID size here
>
>> mutex_init(¶m->lock);
>> dev->iommu = param;
>> return param;
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 45f274b2640d..d4296136ba75 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -371,6 +371,7 @@ struct iommu_fault_param {
>> * @fwspec: IOMMU fwspec data
>> * @iommu_dev: IOMMU device this device is linked to
>> * @priv: IOMMU Driver private data
>> + * @pasids: number of supported PASIDs
> 'max_pasids' to stay consistent?
Both done. How about below changes?
From 008c73b9c0ad51a4a70a18d60361a76c28a63342 Mon Sep 17 00:00:00 2001
From: Lu Baolu <baolu.lu@...ux.intel.com>
Date: Mon, 28 Feb 2022 15:01:35 +0800
Subject: [PATCH 1/1] iommu: Add max_pasids field in struct dev_iommu
Use this field to save the number of PASIDs that a device is able to
consume. It is a generic attribute of a device and lifting it into the
per-device dev_iommu struct could help to avoid the boilerplate code
in various IOMMU drivers.
Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
---
drivers/iommu/iommu.c | 26 ++++++++++++++++++++++++++
include/linux/iommu.h | 2 ++
2 files changed, 28 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 847ad47a2dfd..365d0f2b7f55 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -20,6 +20,7 @@
#include <linux/idr.h>
#include <linux/err.h>
#include <linux/pci.h>
+#include <linux/pci-ats.h>
#include <linux/bitops.h>
#include <linux/property.h>
#include <linux/fsl/mc.h>
@@ -218,6 +219,30 @@ static void dev_iommu_free(struct device *dev)
kfree(param);
}
+static u32 dev_iommu_get_max_pasids(struct device *dev)
+{
+ u32 max_pasids = dev->iommu->iommu_dev->max_pasids;
+ u32 num_bits;
+ int ret;
+
+ if (!max_pasids)
+ return 0;
+
+ if (dev_is_pci(dev)) {
+ ret = pci_max_pasids(to_pci_dev(dev));
+ if (ret < 0)
+ return 0;
+
+ return min_t(u32, max_pasids, ret);
+ }
+
+ ret = device_property_read_u32(dev, "pasid-num-bits", &num_bits);
+ if (ret)
+ return 0;
+
+ return min_t(u32, max_pasids, 1UL << num_bits);
+}
+
static int __iommu_probe_device(struct device *dev, struct list_head
*group_list)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
@@ -242,6 +267,7 @@ static int __iommu_probe_device(struct device *dev,
struct list_head *group_list
goto out_module_put;
}
+ iommu_dev->max_pasids = dev_iommu_get_max_pasids(dev);
dev->iommu->iommu_dev = iommu_dev;
group = iommu_group_get_for_dev(dev);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 03fbb1b71536..d50afb2c9a09 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -364,6 +364,7 @@ struct iommu_fault_param {
* @fwspec: IOMMU fwspec data
* @iommu_dev: IOMMU device this device is linked to
* @priv: IOMMU Driver private data
+ * @max_pasids: number of PASIDs device can consume
*
* TODO: migrate other per device data pointers under iommu_dev_data, e.g.
* struct iommu_group *iommu_group;
@@ -375,6 +376,7 @@ struct dev_iommu {
struct iommu_fwspec *fwspec;
struct iommu_device *iommu_dev;
void *priv;
+ u32 max_pasids;
};
int iommu_device_register(struct iommu_device *iommu,
Best regards,
baolu
Powered by blists - more mailing lists