From: Robin Murphy <robin.murphy@arm.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: jgg@nvidia.com, joro@8bytes.org, will@kernel.org,
marcan@marcan.st, sven@svenpeter.dev, robdclark@gmail.com,
m.szyprowski@samsung.com, krzysztof.kozlowski@linaro.org,
baolu.lu@linux.intel.com, agross@kernel.org,
bjorn.andersson@linaro.org, matthias.bgg@gmail.com,
heiko@sntech.de, orsonzhai@gmail.com, baolin.wang7@gmail.com,
zhang.lyra@gmail.com, wens@csie.org, jernej.skrabec@gmail.com,
samuel@sholland.org, jean-philippe@linaro.org,
alex.williamson@redhat.com, suravee.suthikulpanit@amd.com,
alyssa@rosenzweig.io, alim.akhtar@samsung.com,
dwmw2@infradead.org, yong.wu@mediatek.com,
mjrosato@linux.ibm.com, gerald.schaefer@linux.ibm.com,
thierry.reding@gmail.com, vdumpa@nvidia.com,
jonathanh@nvidia.com, cohuck@redhat.com,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-mediatek@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-s390@vger.kernel.org,
linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org,
virtualization@lists.linux-foundation.org, kvm@vger.kernel.org
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: jgg@nvidia.com, joro@8bytes.org, will@kernel.org,
marcan@marcan.st, sven@svenpeter.dev, robdclark@gmail.com,
m.szyprowski@samsung.com, krzysztof.kozlowski@linaro.org,
baolu.lu@linux.intel.com, agross@kernel.org,
bjorn.andersson@linaro.org, matthias.bgg@gmail.com,
heiko@sntech.de, orsonzhai@gmail.com, baolin.wang7@gmail.com,
zhang.lyra@gmail.com, wens@csie.org, jernej.skrabec@gmail.com,
samuel@sholland.org, jean-philippe@linaro.org,
alex.williamson@redhat.com, suravee.suthikulpanit@amd.com,
alyssa@rosenzweig.io, alim.akhtar@samsung.com,
dwmw2@infradead.org, yong.wu@mediatek.com,
mjrosato@linux.ibm.com, gerald.schaefer@linux.ibm.com,
thierry.reding@gmail.com, vdumpa@nvidia.com,
jonathanh@nvidia.com, cohuck@redhat.com,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-mediatek@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-s390@vger.kernel.org,
linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org,
virtualization@lists.linux-foundation.org, kvm@vger.kernel.org
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: cohuck@redhat.com, heiko@sntech.de, linux-tegra@vger.kernel.org,
thierry.reding@gmail.com, alim.akhtar@samsung.com,
will@kernel.org, alyssa@rosenzweig.io, jean-philippe@linaro.org,
linux-samsung-soc@vger.kernel.org, kvm@vger.kernel.org,
samuel@sholland.org, zhang.lyra@gmail.com,
jernej.skrabec@gmail.com, jonathanh@nvidia.com,
linux-rockchip@lists.infradead.org, wens@csie.org,
agross@kernel.org, linux-arm-kernel@lists.infradead.org,
jgg@nvidia.com, orsonzhai@gmail.com,
gerald.schaefer@linux.ibm.com, linux-sunxi@lists.linux.dev,
linux-arm-msm@vger.kernel.org, alex.williamson@redhat.com,
iommu@lists.linux-foundation.org,
linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
virtualization@lists.linux-foundation.org,
linux-s390@vger.kernel.org, marcan@marcan.st,
linux-kernel@vger.kernel.org, krzysztof.kozlowski@linaro.org,
baolin.wang7@gmail.com, dwmw2@infradead.org
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
_______________________________________________
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: Nicolin Chen <nicolinc@nvidia.com>
Cc: jgg@nvidia.com, joro@8bytes.org, will@kernel.org,
marcan@marcan.st, sven@svenpeter.dev, robdclark@gmail.com,
m.szyprowski@samsung.com, krzysztof.kozlowski@linaro.org,
baolu.lu@linux.intel.com, agross@kernel.org,
bjorn.andersson@linaro.org, matthias.bgg@gmail.com,
heiko@sntech.de, orsonzhai@gmail.com, baolin.wang7@gmail.com,
zhang.lyra@gmail.com, wens@csie.org, jernej.skrabec@gmail.com,
samuel@sholland.org, jean-philippe@linaro.org,
alex.williamson@redhat.com, suravee.suthikulpanit@amd.com,
alyssa@rosenzweig.io, alim.akhtar@samsung.com,
dwmw2@infradead.org, yong.wu@mediatek.com,
mjrosato@linux.ibm.com, gerald.schaefer@linux.ibm.com,
thierry.reding@gmail.com, vdumpa@nvidia.com,
jonathanh@nvidia.com, cohuck@redhat.com,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-mediatek@lists.infradead.org,
linux-rockchip@lists.infradead.org, linux-s390@vger.kernel.org,
linux-sunxi@lists.linux.dev, linux-tegra@vger.kernel.org,
virtualization@lists.linux-foundation.org, kvm@vger.kernel.org
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: cohuck@redhat.com, heiko@sntech.de, mjrosato@linux.ibm.com,
bjorn.andersson@linaro.org, linux-tegra@vger.kernel.org,
thierry.reding@gmail.com, alim.akhtar@samsung.com,
will@kernel.org, alyssa@rosenzweig.io, m.szyprowski@samsung.com,
jean-philippe@linaro.org, linux-samsung-soc@vger.kernel.org,
kvm@vger.kernel.org, samuel@sholland.org, zhang.lyra@gmail.com,
joro@8bytes.org, robdclark@gmail.com, jernej.skrabec@gmail.com,
jonathanh@nvidia.com, linux-rockchip@lists.infradead.org,
wens@csie.org, agross@kernel.org,
linux-arm-kernel@lists.infradead.org, jgg@nvidia.com,
orsonzhai@gmail.com, gerald.schaefer@linux.ibm.com,
linux-sunxi@lists.linux.dev, sven@svenpeter.dev,
linux-arm-msm@vger.kernel.org, iommu@lists.linux-foundation.org,
linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
virtualization@lists.linux-foundation.org, yong.wu@mediatek.com,
linux-s390@vger.kernel.org, marcan@marcan.st, vdumpa@nvidia.com,
linux-kernel@vger.kernel.org, krzysztof.kozlowski@linaro.org,
suravee.suthikulpanit@amd.com, baolin.wang7@gmail.com,
dwmw2@infradead.org, baolu.lu@linux.intel.com
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
WARNING: multiple messages have this Message-ID (diff)
From: Robin Murphy <robin.murphy@arm.com>
To: Nicolin Chen <nicolinc@nvidia.com>
Cc: cohuck@redhat.com, heiko@sntech.de, mjrosato@linux.ibm.com,
bjorn.andersson@linaro.org, linux-tegra@vger.kernel.org,
thierry.reding@gmail.com, alim.akhtar@samsung.com,
will@kernel.org, alyssa@rosenzweig.io, m.szyprowski@samsung.com,
jean-philippe@linaro.org, linux-samsung-soc@vger.kernel.org,
kvm@vger.kernel.org, samuel@sholland.org, zhang.lyra@gmail.com,
joro@8bytes.org, robdclark@gmail.com, jernej.skrabec@gmail.com,
jonathanh@nvidia.com, linux-rockchip@lists.infradead.org,
wens@csie.org, agross@kernel.org,
linux-arm-kernel@lists.infradead.org, jgg@nvidia.com,
orsonzhai@gmail.com, gerald.schaefer@linux.ibm.com,
linux-sunxi@lists.linux.dev, sven@svenpeter.dev,
linux-arm-msm@vger.kernel.org, alex.williamson@redhat.com,
iommu@lists.linux-foundation.org,
linux-mediatek@lists.infradead.org, matthias.bgg@gmail.com,
virtualization@lists.linux-foundation.org, yong.wu@mediatek.com,
linux-s390@vger.kernel.org, marcan@marcan.st,
linux-kernel@vger.kernel.org, krzysztof.kozlowski@linaro.org,
suravee.suthikulpanit@amd.com, baolin.wang7@gmail.com,
dwmw2@infradead.org, baolu.lu@linux.intel.com
Subject: Re: [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain
Date: Mon, 6 Jun 2022 18:50:33 +0100 [thread overview]
Message-ID: <6575de6d-94ba-c427-5b1e-967750ddff23@arm.com> (raw)
In-Reply-To: <Yp4wiJZWxoCLY8tm@Asurada-Nvidia>
On 2022-06-06 17:51, Nicolin Chen wrote:
> Hi Robin,
>
> On Mon, Jun 06, 2022 at 03:33:42PM +0100, Robin Murphy wrote:
>> On 2022-06-06 07:19, Nicolin Chen wrote:
>>> The core code should not call an iommu driver op with a struct device
>>> parameter unless it knows that the dev_iommu_priv_get() for that struct
>>> device was setup by the same driver. Otherwise in a mixed driver system
>>> the iommu_priv could be casted to the wrong type.
>>
>> We don't have mixed-driver systems, and there are plenty more
>> significant problems than this one to solve before we can (but thanks
>> for pointing it out - I hadn't got as far as auditing the public
>> interfaces yet). Once domains are allocated via a particular device's
>> IOMMU instance in the first place, there will be ample opportunity for
>> the core to stash suitable identifying information in the domain for
>> itself. TBH even the current code could do it without needing the
>> weirdly invasive changes here.
>
> Do you have an alternative and less invasive solution in mind?
>
>>> Store the iommu_ops pointer in the iommu_domain and use it as a check to
>>> validate that the struct device is correct before invoking any domain op
>>> that accepts a struct device.
>>
>> In fact this even describes exactly that - "Store the iommu_ops pointer
>> in the iommu_domain", vs. the "Store the iommu_ops pointer in the
>> iommu_domain_ops" which the patch is actually doing :/
>
> Will fix that.
Well, as before I'd prefer to make the code match the commit message -
if I really need to spell it out, see below - since I can't imagine that
we should ever have need to identify a set of iommu_domain_ops in
isolation, therefore I think it's considerably clearer to use the
iommu_domain itself. However, either way we really don't need this yet,
so we may as well just go ahead and remove the redundant test from VFIO
anyway, and I can add some form of this patch to my dev branch for now.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cde2e1d6ab9b..72990edc9314 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1902,6 +1902,7 @@ static struct iommu_domain
*__iommu_domain_alloc(struct device *dev,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = ops->pgsize_bitmap;
+ domain->owner = ops;
if (!domain->ops)
domain->ops = ops->default_domain_ops;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 6f64cbbc6721..79e557207f53 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -89,6 +89,7 @@ struct iommu_domain_geometry {
struct iommu_domain {
unsigned type;
+ const struct iommu_ops *owner; /* Who allocated this domain */
const struct iommu_domain_ops *ops;
unsigned long pgsize_bitmap; /* Bitmap of page sizes in use */
iommu_fault_handler_t handler;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-06-06 17:50 UTC|newest]
Thread overview: 138+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-06 6:19 [PATCH 0/5] Simplify vfio_iommu_type1 attach/detach routine Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` [PATCH 1/5] iommu: Return -EMEDIUMTYPE for incompatible domain and device/group Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-06 6:19 ` Nicolin Chen
2022-06-07 3:23 ` Baolu Lu
2022-06-07 3:23 ` Baolu Lu
2022-06-07 3:23 ` Baolu Lu
2022-06-07 3:23 ` Baolu Lu
2022-06-07 3:23 ` Baolu Lu
2022-06-07 4:03 ` Nicolin Chen
2022-06-07 4:03 ` Nicolin Chen
2022-06-07 4:03 ` Nicolin Chen
2022-06-07 4:03 ` Nicolin Chen via iommu
2022-06-07 4:03 ` Nicolin Chen
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 7:49 ` Tian, Kevin
2022-06-08 17:38 ` Nicolin Chen
2022-06-08 17:38 ` Nicolin Chen
2022-06-08 17:38 ` Nicolin Chen
2022-06-08 17:38 ` Nicolin Chen
2022-06-08 17:38 ` Nicolin Chen via iommu
2022-06-06 6:19 ` [PATCH 2/5] iommu: Ensure device has the same iommu_ops as the domain Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-06 14:33 ` Robin Murphy
2022-06-06 14:33 ` Robin Murphy
2022-06-06 14:33 ` Robin Murphy
2022-06-06 14:33 ` Robin Murphy
2022-06-06 14:33 ` Robin Murphy
2022-06-06 14:33 ` Robin Murphy
2022-06-06 16:51 ` Nicolin Chen
2022-06-06 16:51 ` Nicolin Chen
2022-06-06 16:51 ` Nicolin Chen
2022-06-06 16:51 ` Nicolin Chen
2022-06-06 16:51 ` Nicolin Chen via iommu
2022-06-06 17:50 ` Robin Murphy [this message]
2022-06-06 17:50 ` Robin Murphy
2022-06-06 17:50 ` Robin Murphy
2022-06-06 17:50 ` Robin Murphy
2022-06-06 17:50 ` Robin Murphy
2022-06-06 17:50 ` Robin Murphy
2022-06-06 18:28 ` Nicolin Chen
2022-06-06 18:28 ` Nicolin Chen
2022-06-06 18:28 ` Nicolin Chen
2022-06-06 18:28 ` Nicolin Chen via iommu
2022-06-06 18:28 ` Nicolin Chen
2022-06-06 18:52 ` Jason Gunthorpe
2022-06-06 18:52 ` Jason Gunthorpe
2022-06-06 18:52 ` Jason Gunthorpe
2022-06-06 18:52 ` Jason Gunthorpe
2022-06-06 18:52 ` Jason Gunthorpe via iommu
2022-06-06 6:19 ` [PATCH 3/5] vfio/iommu_type1: Prefer to reuse domains vs match enforced cache coherency Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 8:28 ` Tian, Kevin
2022-06-08 11:17 ` Jason Gunthorpe
2022-06-08 11:17 ` Jason Gunthorpe
2022-06-08 11:17 ` Jason Gunthorpe
2022-06-08 11:17 ` Jason Gunthorpe via iommu
2022-06-08 11:17 ` Jason Gunthorpe
2022-06-08 23:48 ` Tian, Kevin
2022-06-08 23:48 ` Tian, Kevin
2022-06-08 23:48 ` Tian, Kevin
2022-06-08 23:48 ` Tian, Kevin
2022-06-08 23:48 ` Tian, Kevin
2022-06-08 23:48 ` Tian, Kevin
2022-06-14 20:45 ` Nicolin Chen
2022-06-14 20:45 ` Nicolin Chen
2022-06-14 20:45 ` Nicolin Chen
2022-06-14 20:45 ` Nicolin Chen via iommu
2022-06-14 20:45 ` Nicolin Chen
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 7:35 ` Tian, Kevin
2022-06-15 23:12 ` Nicolin Chen
2022-06-15 23:12 ` Nicolin Chen
2022-06-15 23:12 ` Nicolin Chen via iommu
2022-06-15 23:12 ` Nicolin Chen
2022-06-06 6:19 ` [PATCH 4/5] vfio/iommu_type1: Clean up update_dirty_scope in detach_group() Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 8:35 ` Tian, Kevin
2022-06-08 17:46 ` Nicolin Chen
2022-06-08 17:46 ` Nicolin Chen
2022-06-08 17:46 ` Nicolin Chen
2022-06-08 17:46 ` Nicolin Chen
2022-06-08 17:46 ` Nicolin Chen via iommu
2022-06-06 6:19 ` [PATCH 5/5] vfio/iommu_type1: Simplify group attachment Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen
2022-06-06 6:19 ` Nicolin Chen via iommu
2022-06-07 7:44 ` [PATCH 0/5] Simplify vfio_iommu_type1 attach/detach routine Baolu Lu
2022-06-07 7:44 ` Baolu Lu
2022-06-07 7:44 ` Baolu Lu
2022-06-07 7:44 ` Baolu Lu
2022-06-07 7:44 ` Baolu Lu
2022-06-07 11:58 ` Jason Gunthorpe
2022-06-07 11:58 ` Jason Gunthorpe
2022-06-07 11:58 ` Jason Gunthorpe
2022-06-07 11:58 ` Jason Gunthorpe
2022-06-07 11:58 ` Jason Gunthorpe via iommu
2022-06-07 12:42 ` Baolu Lu
2022-06-07 12:42 ` Baolu Lu
2022-06-07 12:42 ` Baolu Lu
2022-06-07 12:42 ` Baolu Lu
2022-06-07 12:42 ` Baolu Lu
2022-06-10 16:39 [PATCH 5/5] vfio/iommu_type1: Simplify group attachment kernel test robot
2022-06-13 13:42 ` Dan Carpenter
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=6575de6d-94ba-c427-5b1e-967750ddff23@arm.com \
--to=robin.murphy@arm.com \
--cc=agross@kernel.org \
--cc=alex.williamson@redhat.com \
--cc=alim.akhtar@samsung.com \
--cc=alyssa@rosenzweig.io \
--cc=baolin.wang7@gmail.com \
--cc=baolu.lu@linux.intel.com \
--cc=bjorn.andersson@linaro.org \
--cc=cohuck@redhat.com \
--cc=dwmw2@infradead.org \
--cc=gerald.schaefer@linux.ibm.com \
--cc=heiko@sntech.de \
--cc=iommu@lists.linux-foundation.org \
--cc=jean-philippe@linaro.org \
--cc=jernej.skrabec@gmail.com \
--cc=jgg@nvidia.com \
--cc=jonathanh@nvidia.com \
--cc=joro@8bytes.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=kvm@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=linux-s390@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=linux-tegra@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=marcan@marcan.st \
--cc=matthias.bgg@gmail.com \
--cc=mjrosato@linux.ibm.com \
--cc=nicolinc@nvidia.com \
--cc=orsonzhai@gmail.com \
--cc=robdclark@gmail.com \
--cc=samuel@sholland.org \
--cc=suravee.suthikulpanit@amd.com \
--cc=sven@svenpeter.dev \
--cc=thierry.reding@gmail.com \
--cc=vdumpa@nvidia.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=wens@csie.org \
--cc=will@kernel.org \
--cc=yong.wu@mediatek.com \
--cc=zhang.lyra@gmail.com \
/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.