Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
Hi,
Replying again, as there was some issue with mailer last time.
On 3/21/2017 11:47 PM, Will Deacon wrote:
On Tue, Mar 21, 2017 at 05:46:29PM +, Robin Murphy wrote:
On 21/03/17 17:21, Will Deacon wrote:
On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
@@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device
*dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain = __iommu_domain_alloc(dev->bus,
-IOMMU_DOMAIN_DMA);
+ group->default_domain =
+ __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
It would be good to have a fall-back here if we are talking to an IOMMU
driver that uses default domains, but does not support identity-mapped
domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
category. A dev_warn() also makes sense in case allocating a identity
domain fails.
Sure, something like the diff below?
Will
--->8
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 42a842e3f95f..f787626a745d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1027,10 +1027,19 @@ struct iommu_group *iommu_group_get_for_dev(struct
device *dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain =
- __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ struct iommu_domain *dom;
+
+ dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ if (!dom) {
+ dev_warn(dev,
+"failed to allocate default IOMMU domain of type
%u; falling back to IOMMU_DOMAIN_DMA",
+iommu_def_domain_type);
Conversely, that's going to be noisy if iommu_def_domain_type was
IOMMU_DOMAIN_DMA to begin with. I think it makes sense to warn if the
user asked for a specific default domain type on the command line and
that didn't work, but maybe not to bother otherwise. Plus, if they asked
for passthrough, then not allocating a default domain at all is probably
closer to the desired result than installing a DMA ops domain would be.
You're right -- I'll hack this about to check if the default type isn't
DOMAIN_DMA before warning about the allocation failure.
if some master devices want 'IDENTITY_DOMAIN' as default (because
those devices do not want any iommu resources to be used/dma_ops to be
set) and some 'DMA_DOMAIN' as default, then should the default be
'DMA_DOMAIN' and then masters needing IDENTITY_DOMAIN explicitly do an
detach_dev later. This [1] was adding the support for detach_dev
of the default DMA_DOMAINs.
[1] https://patchwork.codeaurora.org/patch/164933/
Regards,
Sricharan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
Hi,
On 3/21/2017 11:47 PM, Will Deacon wrote:
On Tue, Mar 21, 2017 at 05:46:29PM +, Robin Murphy wrote:
On 21/03/17 17:21, Will Deacon wrote:
On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
@@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device
*dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain = __iommu_domain_alloc(dev->bus,
-IOMMU_DOMAIN_DMA);
+ group->default_domain =
+ __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
It would be good to have a fall-back here if we are talking to an IOMMU
driver that uses default domains, but does not support identity-mapped
domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
category. A dev_warn() also makes sense in case allocating a identity
domain fails.
Sure, something like the diff below?
Will
--->8
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 42a842e3f95f..f787626a745d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1027,10 +1027,19 @@ struct iommu_group *iommu_group_get_for_dev(struct
device *dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain =
- __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ struct iommu_domain *dom;
+
+ dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ if (!dom) {
+ dev_warn(dev,
+"failed to allocate default IOMMU domain of type
%u; falling back to IOMMU_DOMAIN_DMA",
+iommu_def_domain_type);
Conversely, that's going to be noisy if iommu_def_domain_type was
IOMMU_DOMAIN_DMA to begin with. I think it makes sense to warn if the
user asked for a specific default domain type on the command line and
that didn't work, but maybe not to bother otherwise. Plus, if they asked
for passthrough, then not allocating a default domain at all is probably
closer to the desired result than installing a DMA ops domain would be.
You're right -- I'll hack this about to check if the default type isn't
DOMAIN_DMA before warning about the allocation failure.
if some master devices want 'IDENTITY_DOMAIN' as default (because those
devices do not want any iommu resources to be used/dma_ops to be set)
and some 'DMA_DOMAIN' as default, then should the default be
'DMA_DOMAIN' and then masters needing IDENTITY_DOMAIN explicitly do an
detach_dev later. This [1] was adding the support for detach_dev
of the default DMA_DOMAINs.
[1] https://patchwork.codeaurora.org/patch/164933/
Regards,
Sricharan
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
On Tue, Mar 21, 2017 at 05:21:37PM +, Will Deacon wrote:
> On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
> > On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
> > > @@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct
> > > device *dev)
> > >* IOMMU driver.
> > >*/
> > > if (!group->default_domain) {
> > > - group->default_domain = __iommu_domain_alloc(dev->bus,
> > > - IOMMU_DOMAIN_DMA);
> > > + group->default_domain =
> > > + __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> >
> > It would be good to have a fall-back here if we are talking to an IOMMU
> > driver that uses default domains, but does not support identity-mapped
> > domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
> > category. A dev_warn() also makes sense in case allocating a identity
> > domain fails.
>
> Sure, something like the diff below?
Yes, this looks good.
Thanks,
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
On Tue, Mar 21, 2017 at 05:46:29PM +, Robin Murphy wrote:
> On 21/03/17 17:21, Will Deacon wrote:
> > On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
> >> On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
> >>> @@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct
> >>> device *dev)
> >>>* IOMMU driver.
> >>>*/
> >>> if (!group->default_domain) {
> >>> - group->default_domain = __iommu_domain_alloc(dev->bus,
> >>> - IOMMU_DOMAIN_DMA);
> >>> + group->default_domain =
> >>> + __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> >>
> >> It would be good to have a fall-back here if we are talking to an IOMMU
> >> driver that uses default domains, but does not support identity-mapped
> >> domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
> >> category. A dev_warn() also makes sense in case allocating a identity
> >> domain fails.
> >
> > Sure, something like the diff below?
> >
> > Will
> >
> > --->8
> >
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 42a842e3f95f..f787626a745d 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1027,10 +1027,19 @@ struct iommu_group *iommu_group_get_for_dev(struct
> > device *dev)
> > * IOMMU driver.
> > */
> > if (!group->default_domain) {
> > - group->default_domain =
> > - __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> > + struct iommu_domain *dom;
> > +
> > + dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> > + if (!dom) {
> > + dev_warn(dev,
> > +"failed to allocate default IOMMU domain of
> > type %u; falling back to IOMMU_DOMAIN_DMA",
> > +iommu_def_domain_type);
>
> Conversely, that's going to be noisy if iommu_def_domain_type was
> IOMMU_DOMAIN_DMA to begin with. I think it makes sense to warn if the
> user asked for a specific default domain type on the command line and
> that didn't work, but maybe not to bother otherwise. Plus, if they asked
> for passthrough, then not allocating a default domain at all is probably
> closer to the desired result than installing a DMA ops domain would be.
You're right -- I'll hack this about to check if the default type isn't
DOMAIN_DMA before warning about the allocation failure.
Cheers,
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
On 21/03/17 17:21, Will Deacon wrote:
> On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
>> On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
>>> @@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct
>>> device *dev)
>>> * IOMMU driver.
>>> */
>>> if (!group->default_domain) {
>>> - group->default_domain = __iommu_domain_alloc(dev->bus,
>>> -IOMMU_DOMAIN_DMA);
>>> + group->default_domain =
>>> + __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
>>
>> It would be good to have a fall-back here if we are talking to an IOMMU
>> driver that uses default domains, but does not support identity-mapped
>> domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
>> category. A dev_warn() also makes sense in case allocating a identity
>> domain fails.
>
> Sure, something like the diff below?
>
> Will
>
> --->8
>
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 42a842e3f95f..f787626a745d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1027,10 +1027,19 @@ struct iommu_group *iommu_group_get_for_dev(struct
> device *dev)
>* IOMMU driver.
>*/
> if (!group->default_domain) {
> - group->default_domain =
> - __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> + struct iommu_domain *dom;
> +
> + dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> + if (!dom) {
> + dev_warn(dev,
> + "failed to allocate default IOMMU domain of
> type %u; falling back to IOMMU_DOMAIN_DMA",
> + iommu_def_domain_type);
Conversely, that's going to be noisy if iommu_def_domain_type was
IOMMU_DOMAIN_DMA to begin with. I think it makes sense to warn if the
user asked for a specific default domain type on the command line and
that didn't work, but maybe not to bother otherwise. Plus, if they asked
for passthrough, then not allocating a default domain at all is probably
closer to the desired result than installing a DMA ops domain would be.
Robin.
> + dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
> + }
> +
> + group->default_domain = dom;
> if (!group->domain)
> - group->domain = group->default_domain;
> + group->domain = dom;
> }
>
> ret = iommu_group_add_device(group, dev);
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
On Tue, Mar 21, 2017 at 04:45:27PM +0100, Joerg Roedel wrote:
> On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
> > @@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct
> > device *dev)
> > * IOMMU driver.
> > */
> > if (!group->default_domain) {
> > - group->default_domain = __iommu_domain_alloc(dev->bus,
> > -IOMMU_DOMAIN_DMA);
> > + group->default_domain =
> > + __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
>
> It would be good to have a fall-back here if we are talking to an IOMMU
> driver that uses default domains, but does not support identity-mapped
> domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
> category. A dev_warn() also makes sense in case allocating a identity
> domain fails.
Sure, something like the diff below?
Will
--->8
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 42a842e3f95f..f787626a745d 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1027,10 +1027,19 @@ struct iommu_group *iommu_group_get_for_dev(struct
device *dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain =
- __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ struct iommu_domain *dom;
+
+ dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
+ if (!dom) {
+ dev_warn(dev,
+"failed to allocate default IOMMU domain of
type %u; falling back to IOMMU_DOMAIN_DMA",
+iommu_def_domain_type);
+ dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
+ }
+
+ group->default_domain = dom;
if (!group->domain)
- group->domain = group->default_domain;
+ group->domain = dom;
}
ret = iommu_group_add_device(group, dev);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
Hi Will,
On Fri, Mar 10, 2017 at 08:49:36PM +, Will Deacon wrote:
> @@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct
> device *dev)
>* IOMMU driver.
>*/
> if (!group->default_domain) {
> - group->default_domain = __iommu_domain_alloc(dev->bus,
> - IOMMU_DOMAIN_DMA);
> + group->default_domain =
> + __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
It would be good to have a fall-back here if we are talking to an IOMMU
driver that uses default domains, but does not support identity-mapped
domains (yet). Exynos and Rockchip IOMMU drivers seem to fall into this
category. A dev_warn() also makes sense in case allocating a identity
domain fails.
Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 5/5] iommu: Allow default domain type to be set on the kernel command line
The IOMMU core currently initialises the default domain for each group
to IOMMU_DOMAIN_DMA, under the assumption that devices will use
IOMMU-backed DMA ops by default. However, in some cases it is desirable
for the DMA ops to bypass the IOMMU for performance reasons, reserving
use of translation for subsystems such as VFIO that require it for
enforcing device isolation.
Rather than modify each IOMMU driver to provide different semantics for
DMA domains, instead we introduce a command line parameter that can be
used to change the type of the default domain. Passthrough can then be
specified using "iommu.passthrough=1" on the kernel command line.
Signed-off-by: Will Deacon
---
Documentation/admin-guide/kernel-parameters.txt | 6 ++
drivers/iommu/iommu.c | 17 +++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index 986e44387dad..243895f6872e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1635,6 +1635,12 @@
nobypass[PPC/POWERNV]
Disable IOMMU bypass, using IOMMU for PCI devices.
+ iommu.passthrough=
+ [ARM64] Configure DMA to bypass the IOMMU by default.
+ Format: { "0" | "1" }
+ 0 - Use IOMMU translation for DMA.
+ 1 - Bypass the IOMMU for DMA.
+ unset - Use IOMMU translation for DMA.
io7=[HW] IO7 for Marvel based alpha systems
See comment before marvel_specify_io7 in
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8ea14f41a979..42a842e3f95f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -36,6 +36,7 @@
static struct kset *iommu_group_kset;
static DEFINE_IDA(iommu_group_ida);
+static unsigned int iommu_def_domain_type = IOMMU_DOMAIN_DMA;
struct iommu_callback_data {
const struct iommu_ops *ops;
@@ -111,6 +112,18 @@ static int __iommu_attach_group(struct iommu_domain
*domain,
static void __iommu_detach_group(struct iommu_domain *domain,
struct iommu_group *group);
+static int __init iommu_set_def_domain_type(char *str)
+{
+ bool pt;
+
+ if (!str || strtobool(str, &pt))
+ return -EINVAL;
+
+ iommu_def_domain_type = pt ? IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_DMA;
+ return 0;
+}
+early_param("iommu.passthrough", iommu_set_def_domain_type);
+
static ssize_t iommu_group_attr_show(struct kobject *kobj,
struct attribute *__attr, char *buf)
{
@@ -1014,8 +1027,8 @@ struct iommu_group *iommu_group_get_for_dev(struct device
*dev)
* IOMMU driver.
*/
if (!group->default_domain) {
- group->default_domain = __iommu_domain_alloc(dev->bus,
-IOMMU_DOMAIN_DMA);
+ group->default_domain =
+ __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
if (!group->domain)
group->domain = group->default_domain;
}
--
2.1.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
