Re: [PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
On 2021-07-24 06:23, Lu Baolu wrote:
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy
---
drivers/iommu/intel/iommu.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e2add5a0caef..77d322272743 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct
dmar_domain *domain)
int iommu_id;
/* si_domain and vm domain should not get here. */
- if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+ if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
return NULL;
for_each_domain_iommu(iommu_id, domain)
@@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct
dmar_domain *domain,
pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) <<
VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
if (domain_use_first_level(domain)) {
pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API)
pteval |= DMA_FL_PTE_ACCESS;
}
if (cmpxchg64(>val, 0ULL, pteval))
@@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *domain,
unsigned long iov_pfn,
if (domain_use_first_level(domain)) {
attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API) {
attr |= DMA_FL_PTE_ACCESS;
if (prot & DMA_PTE_WRITE)
attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
switch (type) {
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(0);
if (!dmar_domain) {
@@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct
device *dev)
static void intel_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- if (domain && domain->type == IOMMU_DOMAIN_DMA)
- iommu_setup_dma_ops(dev, 0, U64_MAX);
- else
- set_dma_ops(dev, NULL);
+ set_dma_ops(dev, NULL);
Is it reasonable to remove above line? The idea is that vendor iommu
driver should not override the dma_ops if device doesn't have a DMA
domain.
As the commit message implies, that's exactly how iommu_setup_dma_ops()
has always behaved anyway. There should be no functional change here.
Robin.
+ iommu_setup_dma_ops(dev, 0, U64_MAX);
}
static void intel_iommu_get_resv_regions(struct device *device,
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
Hi Robin,
On 2021/7/22 2:20, Robin Murphy wrote:
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy
---
drivers/iommu/intel/iommu.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e2add5a0caef..77d322272743 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain
*domain)
int iommu_id;
/* si_domain and vm domain should not get here. */
- if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+ if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
return NULL;
for_each_domain_iommu(iommu_id, domain)
@@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain
*domain,
pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) <<
VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
if (domain_use_first_level(domain)) {
pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
+ if (domain->domain.type &
__IOMMU_DOMAIN_DMA_API)
pteval |= DMA_FL_PTE_ACCESS;
}
if (cmpxchg64(>val, 0ULL, pteval))
@@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned
long iov_pfn,
if (domain_use_first_level(domain)) {
attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API) {
attr |= DMA_FL_PTE_ACCESS;
if (prot & DMA_PTE_WRITE)
attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
switch (type) {
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(0);
if (!dmar_domain) {
@@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct device
*dev)
static void intel_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- if (domain && domain->type == IOMMU_DOMAIN_DMA)
- iommu_setup_dma_ops(dev, 0, U64_MAX);
- else
- set_dma_ops(dev, NULL);
+ set_dma_ops(dev, NULL);
Is it reasonable to remove above line? The idea is that vendor iommu
driver should not override the dma_ops if device doesn't have a DMA
domain.
+ iommu_setup_dma_ops(dev, 0, U64_MAX);
}
static void intel_iommu_get_resv_regions(struct device *device,
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
Hi Robin,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on rockchip/for-next linus/master v5.14-rc2
next-20210722]
[cannot apply to sunxi/sunxi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: x86_64-randconfig-a015-20210722 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project
9625ca5b602616b2f5584e8a49ba93c52c141e40)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://github.com/0day-ci/linux/commit/c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
git checkout c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir
ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
>> drivers/iommu/intel/iommu.c:604:38: error: use of undeclared identifier
>> '__IOMMU_DOMAIN_DMA'
if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
^
1 error generated.
vim +/__IOMMU_DOMAIN_DMA +604 drivers/iommu/intel/iommu.c
597
598 /* This functionin only returns single iommu in a domain */
599 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
600 {
601 int iommu_id;
602
603 /* si_domain and vm domain should not get here. */
> 604 if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
605 return NULL;
606
607 for_each_domain_iommu(iommu_id, domain)
608 break;
609
610 if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
611 return NULL;
612
613 return g_iommus[iommu_id];
614 }
615
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
On 2021-07-22 17:44, kernel test robot wrote:
Hi Robin,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on rockchip/for-next linus/master v5.14-rc2
next-20210722]
[cannot apply to sunxi/sunxi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
git checkout c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross
O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/bug.h:17,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from drivers/iommu/intel/iommu.c:18:
drivers/iommu/intel/iommu.c: In function 'domain_get_iommu':
drivers/iommu/intel/iommu.c:604:38: error: '__IOMMU_DOMAIN_DMA' undeclared
(first use in this function); did you mean 'IOMMU_DOMAIN_DMA'?
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition);\
| ^
drivers/iommu/intel/iommu.c:604:38: note: each undeclared identifier is
reported only once for each function it appears in
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition);\
| ^
vim +604 drivers/iommu/intel/iommu.c
597
598 /* This functionin only returns single iommu in a domain */
599 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
600 {
601 int iommu_id;
602
603 /* si_domain and vm domain should not get here. */
> 604 if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
Bleh, of course that should be __IOMMU_DOMAIN_DMA_API like the other two
instances. I'll fix this locally ready for v2.
Thanks,
Robin.
605 return NULL;
606
607 for_each_domain_iommu(iommu_id, domain)
608 break;
609
610 if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
611 return NULL;
612
613 return g_iommus[iommu_id];
614 }
615
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
Hi Robin,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on rockchip/for-next linus/master v5.14-rc2
next-20210722]
[cannot apply to sunxi/sunxi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Robin-Murphy/iommu-Refactor-DMA-domain-strictness/20210722-022514
git checkout c05e0e1856b394eff1167c00f7bbd6ac7cc9dea6
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross
O=build_dir ARCH=ia64 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/bug.h:17,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/preempt.h:5,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/wait.h:9,
from include/linux/wait_bit.h:8,
from include/linux/fs.h:6,
from include/linux/debugfs.h:15,
from drivers/iommu/intel/iommu.c:18:
drivers/iommu/intel/iommu.c: In function 'domain_get_iommu':
>> drivers/iommu/intel/iommu.c:604:38: error: '__IOMMU_DOMAIN_DMA' undeclared
>> (first use in this function); did you mean 'IOMMU_DOMAIN_DMA'?
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition);\
| ^
drivers/iommu/intel/iommu.c:604:38: note: each undeclared identifier is
reported only once for each function it appears in
604 | if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
| ^~
include/asm-generic/bug.h:121:25: note: in definition of macro 'WARN_ON'
121 | int __ret_warn_on = !!(condition);\
| ^
vim +604 drivers/iommu/intel/iommu.c
597
598 /* This functionin only returns single iommu in a domain */
599 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
600 {
601 int iommu_id;
602
603 /* si_domain and vm domain should not get here. */
> 604 if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
605 return NULL;
606
607 for_each_domain_iommu(iommu_id, domain)
608 break;
609
610 if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
611 return NULL;
612
613 return g_iommus[iommu_id];
614 }
615
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 17/23] iommu/vt-d: Prepare for multiple DMA domain types
In preparation for the strict vs. non-strict decision for DMA domains to
be expressed in the domain type, make sure we expose our flush queue
awareness by accepting the new domain type, and test the specific
feature flag where we want to identify DMA domains in general. The DMA
ops setup can simply be made unconditional, since iommu-dma already
knows not to touch identity domains.
Signed-off-by: Robin Murphy
---
drivers/iommu/intel/iommu.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index e2add5a0caef..77d322272743 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -601,7 +601,7 @@ struct intel_iommu *domain_get_iommu(struct dmar_domain
*domain)
int iommu_id;
/* si_domain and vm domain should not get here. */
- if (WARN_ON(domain->domain.type != IOMMU_DOMAIN_DMA))
+ if (WARN_ON(!(domain->domain.type & __IOMMU_DOMAIN_DMA)))
return NULL;
for_each_domain_iommu(iommu_id, domain)
@@ -1035,7 +1035,7 @@ static struct dma_pte *pfn_to_dma_pte(struct dmar_domain
*domain,
pteval = ((uint64_t)virt_to_dma_pfn(tmp_page) <<
VTD_PAGE_SHIFT) | DMA_PTE_READ | DMA_PTE_WRITE;
if (domain_use_first_level(domain)) {
pteval |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA)
+ if (domain->domain.type &
__IOMMU_DOMAIN_DMA_API)
pteval |= DMA_FL_PTE_ACCESS;
}
if (cmpxchg64(>val, 0ULL, pteval))
@@ -2346,7 +2346,7 @@ __domain_mapping(struct dmar_domain *domain, unsigned
long iov_pfn,
if (domain_use_first_level(domain)) {
attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
- if (domain->domain.type == IOMMU_DOMAIN_DMA) {
+ if (domain->domain.type & __IOMMU_DOMAIN_DMA_API) {
attr |= DMA_FL_PTE_ACCESS;
if (prot & DMA_PTE_WRITE)
attr |= DMA_FL_PTE_DIRTY;
@@ -4528,6 +4528,7 @@ static struct iommu_domain
*intel_iommu_domain_alloc(unsigned type)
switch (type) {
case IOMMU_DOMAIN_DMA:
+ case IOMMU_DOMAIN_DMA_FQ:
case IOMMU_DOMAIN_UNMANAGED:
dmar_domain = alloc_domain(0);
if (!dmar_domain) {
@@ -5164,12 +5165,8 @@ static void intel_iommu_release_device(struct device
*dev)
static void intel_iommu_probe_finalize(struct device *dev)
{
- struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
-
- if (domain && domain->type == IOMMU_DOMAIN_DMA)
- iommu_setup_dma_ops(dev, 0, U64_MAX);
- else
- set_dma_ops(dev, NULL);
+ set_dma_ops(dev, NULL);
+ iommu_setup_dma_ops(dev, 0, U64_MAX);
}
static void intel_iommu_get_resv_regions(struct device *device,
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
