| Message ID | 20220525013147.2215355-2-nobuhiro1.iwamatsu@toshiba.co.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Visconti5 IOMMU driver | expand |
Hi Nobuhiro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on joro-iommu/next]
[also build test WARNING on arm-perf/for-next/perf soc/for-next linus/master v5.18 next-20220524]
[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/intel-lab-lkp/linux/commits/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: nios2-allyesconfig (https://download.01.org/0day-ci/archive/20220525/202205251205.PHq3cWj3-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.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/intel-lab-lkp/linux/commit/69bb4f3c2ef0bb1f65922bc72bb31109897a6393
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
git checkout 69bb4f3c2ef0bb1f65922bc72bb31109897a6393
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/iommu/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/iommu/visconti-atu.c:47:29: error: field 'iommu' has incomplete type
47 | struct iommu_device iommu;
| ^~~~~
drivers/iommu/visconti-atu.c:62:29: error: field 'io_domain' has incomplete type
62 | struct iommu_domain io_domain;
| ^~~~~~~~~
In file included from include/linux/bits.h:22,
from include/linux/ratelimit_types.h:5,
from include/linux/ratelimit.h:5,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/iommu/visconti-atu.c:12:
drivers/iommu/visconti-atu.c: In function 'to_atu_domain':
include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
drivers/iommu/visconti-atu.c:70:16: note: in expansion of macro 'container_of'
70 | return container_of(domain, struct visconti_atu_domain, io_domain);
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_enable_entry':
>> drivers/iommu/visconti-atu.c:102:52: warning: right shift count >= width of type [-Wshift-count-overflow]
102 | (atu->iova[num] >> 32) & ATU_BGADDR_MASK);
| ^~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_attach_device':
drivers/iommu/visconti-atu.c:121:43: error: implicit declaration of function 'dev_iommu_priv_get' [-Werror=implicit-function-declaration]
121 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:121:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/iommu/visconti-atu.c: In function 'visconti_atu_detach_device':
drivers/iommu/visconti-atu.c:150:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
150 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:196:41: warning: 'struct iommu_iotlb_gather' declared inside parameter list will not be visible outside of this definition or declaration
196 | struct iommu_iotlb_gather *iotlb_gather)
| ^~~~~~~~~~~~~~~~~~
In file included from include/linux/printk.h:555,
from include/asm-generic/bug.h:22,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/current.h:5,
from ./arch/nios2/include/generated/asm/current.h:1,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/iommu/visconti-atu.c:12:
drivers/iommu/visconti-atu.c: In function 'visconti_atu_iova_to_phys':
>> drivers/iommu/visconti-atu.c:251:27: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'dma_addr_t' {aka 'unsigned int'} [-Wformat=]
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:29: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:9: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/visconti-atu.c:251:9: note: in expansion of macro 'dev_dbg'
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ^~~~~~~
drivers/iommu/visconti-atu.c:251:45: note: format string is defined here
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ~~~^
| |
| long long unsigned int
| %x
In file included from include/linux/printk.h:555,
from include/asm-generic/bug.h:22,
from ./arch/nios2/include/generated/asm/bug.h:1,
from include/linux/bug.h:5,
from include/linux/thread_info.h:13,
from include/asm-generic/current.h:5,
from ./arch/nios2/include/generated/asm/current.h:1,
from include/linux/sched.h:12,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/iommu/visconti-atu.c:12:
>> drivers/iommu/visconti-atu.c:251:27: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'phys_addr_t' {aka 'unsigned int'} [-Wformat=]
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:134:29: note: in definition of macro '__dynamic_func_call'
134 | func(&id, ##__VA_ARGS__); \
| ^~~~~~~~~~~
include/linux/dynamic_debug.h:166:9: note: in expansion of macro '_dynamic_func_call'
166 | _dynamic_func_call(fmt,__dynamic_dev_dbg, \
| ^~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:9: note: in expansion of macro 'dynamic_dev_dbg'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~
include/linux/dev_printk.h:155:30: note: in expansion of macro 'dev_fmt'
155 | dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/iommu/visconti-atu.c:251:9: note: in expansion of macro 'dev_dbg'
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ^~~~~~~
drivers/iommu/visconti-atu.c:251:53: note: format string is defined here
251 | dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
| ~~~^
| |
| long long unsigned int
| %x
drivers/iommu/visconti-atu.c: In function 'visconti_atu_of_xlate':
drivers/iommu/visconti-atu.c:262:17: error: implicit declaration of function 'dev_iommu_priv_set' [-Werror=implicit-function-declaration]
262 | dev_iommu_priv_set(dev, platform_get_drvdata(pdev));
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_domain_alloc':
drivers/iommu/visconti-atu.c:273:21: error: 'IOMMU_DOMAIN_UNMANAGED' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:273:21: note: each undeclared identifier is reported only once for each function it appears in
drivers/iommu/visconti-atu.c:273:55: error: 'IOMMU_DOMAIN_DMA' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_probe_device':
drivers/iommu/visconti-atu.c:298:39: error: implicit declaration of function 'dev_iommu_fwspec_get' [-Werror=implicit-function-declaration]
298 | struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
| ^~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:298:39: warning: initialization of 'struct iommu_fwspec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/iommu/visconti-atu.c:301:30: error: invalid use of undefined type 'struct iommu_fwspec'
301 | if (!fwspec || fwspec->ops != &visconti_atu_ops)
| ^~
drivers/iommu/visconti-atu.c:304:13: warning: assignment to 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
304 | atu = dev_iommu_priv_get(dev);
| ^
drivers/iommu/visconti-atu.c: In function 'visconti_atu_release_device':
drivers/iommu/visconti-atu.c:310:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
310 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:315:9: error: implicit declaration of function 'iommu_fwspec_free' [-Werror=implicit-function-declaration]
315 | iommu_fwspec_free(dev);
| ^~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:318:21: error: variable 'visconti_atu_ops' has initializer but incomplete type
318 | static const struct iommu_ops visconti_atu_ops = {
| ^~~~~~~~~
drivers/iommu/visconti-atu.c:319:10: error: 'const struct iommu_ops' has no member named 'domain_alloc'
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:319:25: warning: excess elements in struct initializer
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:319:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:320:10: error: 'const struct iommu_ops' has no member named 'probe_device'
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: warning: excess elements in struct initializer
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:321:10: error: 'const struct iommu_ops' has no member named 'release_device'
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: warning: excess elements in struct initializer
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:322:10: error: 'const struct iommu_ops' has no member named 'device_group'
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:322:25: error: 'generic_device_group' undeclared here (not in a function)
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:322:25: warning: excess elements in struct initializer
drivers/iommu/visconti-atu.c:322:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:323:10: error: 'const struct iommu_ops' has no member named 'of_xlate'
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~
drivers/iommu/visconti-atu.c:323:21: warning: excess elements in struct initializer
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:323:21: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:324:10: error: 'const struct iommu_ops' has no member named 'pgsize_bitmap'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:41:33: warning: excess elements in struct initializer
41 | #define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:324:26: note: in expansion of macro 'ATU_IOMMU_PGSIZE_BITMAP'
vim +102 drivers/iommu/visconti-atu.c
83
84 static void visconti_atu_enable_entry(struct visconti_atu_device *atu,
85 int num)
86 {
87 dev_dbg(atu->dev, "enable ATU: %d\n", atu->enable_entry);
88
89 visconti_atu_write(atu, ATU_AT_EN, 0);
90 if (atu->enable_entry & BIT(num)) {
91 visconti_atu_write(atu,
92 ATU_AT_REG(num, ATU_AT_BLADDR),
93 atu->iova[num]);
94 visconti_atu_write(atu,
95 ATU_AT_REG(num, ATU_AT_ELADDR),
96 atu->iova[num] + atu->size[num] - 1);
97 visconti_atu_write(atu,
98 ATU_AT_REG(num, ATU_AT_BGADDR0),
99 atu->iova[num] & ATU_BGADDR_MASK);
100 visconti_atu_write(atu,
101 ATU_AT_REG(num, ATU_AT_BGADDR1),
> 102 (atu->iova[num] >> 32) & ATU_BGADDR_MASK);
103 }
104 visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry);
105 visconti_atu_write(atu, ATU_AT_EN, 1);
106 }
107
108 static void visconti_atu_disable_entry(struct visconti_atu_device *atu)
109 {
110 dev_dbg(atu->dev, "disable ATU: %d\n", atu->enable_entry);
111
112 visconti_atu_write(atu, ATU_AT_EN, 0);
113 visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry);
114 visconti_atu_write(atu, ATU_AT_EN, 1);
115 }
116
117 static int visconti_atu_attach_device(struct iommu_domain *io_domain,
118 struct device *dev)
119 {
120 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
121 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
122 int ret = 0;
123
124 if (!atu) {
125 dev_err(dev, "Cannot attach to ATU\n");
126 return -ENXIO;
127 }
128
129 mutex_lock(&domain->mutex);
130
131 if (!domain->atu) {
132 domain->atu = atu;
133 } else if (domain->atu != atu) {
134 dev_err(dev, "Can't attach ATU %s to domain on ATU %s\n",
135 dev_name(atu->dev), dev_name(domain->atu->dev));
136 ret = -EINVAL;
137 } else {
138 dev_warn(dev, "Reusing ATU context\n");
139 }
140
141 mutex_unlock(&domain->mutex);
142
143 return ret;
144 }
145
146 static void visconti_atu_detach_device(struct iommu_domain *io_domain,
147 struct device *dev)
148 {
149 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
150 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
151
152 if (domain->atu != atu)
153 return;
154
155 domain->atu = NULL;
156 }
157
158 static int visconti_atu_map(struct iommu_domain *io_domain,
159 unsigned long iova,
160 phys_addr_t paddr,
161 size_t size, int prot, gfp_t gfp)
162 {
163 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
164 struct visconti_atu_device *atu = domain->atu;
165 unsigned long flags;
166 unsigned int i;
167
168 if (!domain)
169 return -ENODEV;
170
171 spin_lock_irqsave(&atu->lock, flags);
172 for (i = 0; i < atu->num_map_entry; i++) {
173 if (!(atu->enable_entry & BIT(i))) {
174 atu->enable_entry |= BIT(i);
175 atu->iova[i] = iova;
176 atu->paddr[i] = paddr;
177 atu->size[i] = size;
178
179 visconti_atu_enable_entry(atu, i);
180 break;
181 }
182 }
183 spin_unlock_irqrestore(&atu->lock, flags);
184
185 if (i == atu->num_map_entry) {
186 dev_err(atu->dev, "map: not enough entry.\n");
187 return -ENOMEM;
188 }
189
190 return 0;
191 }
192
193 static size_t visconti_atu_unmap(struct iommu_domain *io_domain,
194 unsigned long iova,
195 size_t size,
196 struct iommu_iotlb_gather *iotlb_gather)
197 {
198 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
199 struct visconti_atu_device *atu = domain->atu;
200 size_t tmp_size = size;
201 unsigned long flags;
202 unsigned int i;
203
204 spin_lock_irqsave(&atu->lock, flags);
205
206 while (tmp_size != 0) {
207 for (i = 0; i < atu->num_map_entry; i++) {
208 if (atu->iova[i] != iova)
209 continue;
210
211 atu->enable_entry &= ~BIT(i);
212 iova += atu->size[i];
213 tmp_size -= atu->size[i];
214
215 visconti_atu_disable_entry(atu);
216
217 break;
218 }
219 if (i == atu->num_map_entry) {
220 dev_err(atu->dev, "unmap: not found entry.\n");
221 size = 0;
222 goto out;
223 }
224 }
225
226 if (!atu->num_map_entry)
227 visconti_atu_write(atu, ATU_AT_EN, 0);
228 out:
229 spin_unlock_irqrestore(&atu->lock, flags);
230 return size;
231 }
232
233 static phys_addr_t visconti_atu_iova_to_phys(struct iommu_domain *io_domain,
234 dma_addr_t iova)
235 {
236 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
237 struct visconti_atu_device *atu = domain->atu;
238 phys_addr_t paddr = 0;
239 unsigned int i;
240
241 for (i = 0; i < atu->num_map_entry; i++) {
242 if (!(atu->enable_entry & BIT(i)))
243 continue;
244 if (atu->iova[i] <= iova && iova < (atu->iova[i] + atu->size[i])) {
245 paddr = atu->paddr[i];
246 paddr += iova & (atu->size[i] - 1);
247 break;
248 }
249 }
250
> 251 dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
252
253 return paddr;
254 }
255
Hi Nobuhiro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on joro-iommu/next]
[also build test WARNING on arm-perf/for-next/perf soc/for-next linus/master v5.18 next-20220524]
[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/intel-lab-lkp/linux/commits/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220525/202205251452.kFXLqhQx-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.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/intel-lab-lkp/linux/commit/69bb4f3c2ef0bb1f65922bc72bb31109897a6393
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
git checkout 69bb4f3c2ef0bb1f65922bc72bb31109897a6393
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash drivers/iommu/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/iommu/visconti-atu.c:47:29: error: field 'iommu' has incomplete type
47 | struct iommu_device iommu;
| ^~~~~
drivers/iommu/visconti-atu.c:62:29: error: field 'io_domain' has incomplete type
62 | struct iommu_domain io_domain;
| ^~~~~~~~~
In file included from include/linux/bits.h:22,
from include/linux/ratelimit_types.h:5,
from include/linux/ratelimit.h:5,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/iommu/visconti-atu.c:12:
drivers/iommu/visconti-atu.c: In function 'to_atu_domain':
include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
drivers/iommu/visconti-atu.c:70:16: note: in expansion of macro 'container_of'
70 | return container_of(domain, struct visconti_atu_domain, io_domain);
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_attach_device':
drivers/iommu/visconti-atu.c:121:43: error: implicit declaration of function 'dev_iommu_priv_get' [-Werror=implicit-function-declaration]
121 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:121:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/iommu/visconti-atu.c: In function 'visconti_atu_detach_device':
drivers/iommu/visconti-atu.c:150:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
150 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
>> drivers/iommu/visconti-atu.c:196:41: warning: 'struct iommu_iotlb_gather' declared inside parameter list will not be visible outside of this definition or declaration
196 | struct iommu_iotlb_gather *iotlb_gather)
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_of_xlate':
drivers/iommu/visconti-atu.c:262:17: error: implicit declaration of function 'dev_iommu_priv_set' [-Werror=implicit-function-declaration]
262 | dev_iommu_priv_set(dev, platform_get_drvdata(pdev));
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_domain_alloc':
drivers/iommu/visconti-atu.c:273:21: error: 'IOMMU_DOMAIN_UNMANAGED' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:273:21: note: each undeclared identifier is reported only once for each function it appears in
drivers/iommu/visconti-atu.c:273:55: error: 'IOMMU_DOMAIN_DMA' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_probe_device':
drivers/iommu/visconti-atu.c:298:39: error: implicit declaration of function 'dev_iommu_fwspec_get' [-Werror=implicit-function-declaration]
298 | struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
| ^~~~~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:298:39: warning: initialization of 'struct iommu_fwspec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/iommu/visconti-atu.c:301:30: error: invalid use of undefined type 'struct iommu_fwspec'
301 | if (!fwspec || fwspec->ops != &visconti_atu_ops)
| ^~
>> drivers/iommu/visconti-atu.c:304:13: warning: assignment to 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
304 | atu = dev_iommu_priv_get(dev);
| ^
drivers/iommu/visconti-atu.c: In function 'visconti_atu_release_device':
drivers/iommu/visconti-atu.c:310:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
310 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:315:9: error: implicit declaration of function 'iommu_fwspec_free' [-Werror=implicit-function-declaration]
315 | iommu_fwspec_free(dev);
| ^~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:318:21: error: variable 'visconti_atu_ops' has initializer but incomplete type
318 | static const struct iommu_ops visconti_atu_ops = {
| ^~~~~~~~~
drivers/iommu/visconti-atu.c:319:10: error: 'const struct iommu_ops' has no member named 'domain_alloc'
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:319:25: warning: excess elements in struct initializer
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:319:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:320:10: error: 'const struct iommu_ops' has no member named 'probe_device'
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: warning: excess elements in struct initializer
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:321:10: error: 'const struct iommu_ops' has no member named 'release_device'
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: warning: excess elements in struct initializer
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:322:10: error: 'const struct iommu_ops' has no member named 'device_group'
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:322:25: error: 'generic_device_group' undeclared here (not in a function)
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:322:25: warning: excess elements in struct initializer
drivers/iommu/visconti-atu.c:322:25: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:323:10: error: 'const struct iommu_ops' has no member named 'of_xlate'
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~
drivers/iommu/visconti-atu.c:323:21: warning: excess elements in struct initializer
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:323:21: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c:324:10: error: 'const struct iommu_ops' has no member named 'pgsize_bitmap'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:41:33: warning: excess elements in struct initializer
41 | #define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:324:26: note: in expansion of macro 'ATU_IOMMU_PGSIZE_BITMAP'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:41:33: note: (near initialization for 'visconti_atu_ops')
41 | #define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:324:26: note: in expansion of macro 'ATU_IOMMU_PGSIZE_BITMAP'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:325:10: error: 'const struct iommu_ops' has no member named 'default_domain_ops'
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:326:18: error: 'const struct iommu_domain_ops' has no member named 'attach_dev'
326 | .attach_dev = visconti_atu_attach_device,
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:326:31: warning: excess elements in struct initializer
326 | .attach_dev = visconti_atu_attach_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:326:31: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:327:18: error: 'const struct iommu_domain_ops' has no member named 'detach_dev'
327 | .detach_dev = visconti_atu_detach_device,
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:327:31: warning: excess elements in struct initializer
327 | .detach_dev = visconti_atu_detach_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:327:31: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:328:18: error: 'const struct iommu_domain_ops' has no member named 'map'
328 | .map = visconti_atu_map,
| ^~~
drivers/iommu/visconti-atu.c:328:24: warning: excess elements in struct initializer
328 | .map = visconti_atu_map,
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:328:24: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:329:18: error: 'const struct iommu_domain_ops' has no member named 'unmap'
329 | .unmap = visconti_atu_unmap,
| ^~~~~
drivers/iommu/visconti-atu.c:329:26: warning: excess elements in struct initializer
329 | .unmap = visconti_atu_unmap,
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:329:26: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:330:18: error: 'const struct iommu_domain_ops' has no member named 'iova_to_phys'
330 | .iova_to_phys = visconti_atu_iova_to_phys,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:330:33: warning: excess elements in struct initializer
330 | .iova_to_phys = visconti_atu_iova_to_phys,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:330:33: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:331:18: error: 'const struct iommu_domain_ops' has no member named 'free'
331 | .free = visconti_atu_domain_free,
| ^~~~
drivers/iommu/visconti-atu.c:331:25: warning: excess elements in struct initializer
331 | .free = visconti_atu_domain_free,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:331:25: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:325:64: error: invalid use of undefined type 'const struct iommu_domain_ops'
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^
drivers/iommu/visconti-atu.c:325:31: warning: excess elements in struct initializer
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^
drivers/iommu/visconti-atu.c:325:31: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c: In function 'visconti_atu_probe':
drivers/iommu/visconti-atu.c:366:22: error: implicit declaration of function 'iommu_group_alloc' [-Werror=implicit-function-declaration]
366 | atu->group = iommu_group_alloc();
| ^~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:366:20: warning: assignment to 'struct iommu_group *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
366 | atu->group = iommu_group_alloc();
| ^
drivers/iommu/visconti-atu.c:380:15: error: implicit declaration of function 'iommu_device_sysfs_add' [-Werror=implicit-function-declaration]
380 | ret = iommu_device_sysfs_add(&atu->iommu, dev, NULL, dev_name(dev));
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:384:15: error: implicit declaration of function 'iommu_device_register'; did you mean 'of_device_register'? [-Werror=implicit-function-declaration]
384 | ret = iommu_device_register(&atu->iommu, &visconti_atu_ops, dev);
| ^~~~~~~~~~~~~~~~~~~~~
| of_device_register
drivers/iommu/visconti-atu.c:388:14: error: implicit declaration of function 'iommu_present'; did you mean 'pmd_present'? [-Werror=implicit-function-declaration]
388 | if (!iommu_present(&platform_bus_type))
| ^~~~~~~~~~~~~
| pmd_present
drivers/iommu/visconti-atu.c:389:17: error: implicit declaration of function 'bus_set_iommu' [-Werror=implicit-function-declaration]
389 | bus_set_iommu(&platform_bus_type, &visconti_atu_ops);
| ^~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:395:9: error: implicit declaration of function 'iommu_device_sysfs_remove' [-Werror=implicit-function-declaration]
395 | iommu_device_sysfs_remove(&atu->iommu);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_remove':
drivers/iommu/visconti-atu.c:405:9: error: implicit declaration of function 'iommu_device_unregister'; did you mean 'of_device_unregister'? [-Werror=implicit-function-declaration]
405 | iommu_device_unregister(&atu->iommu);
| ^~~~~~~~~~~~~~~~~~~~~~~
| of_device_unregister
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:318:31: error: storage size of 'visconti_atu_ops' isn't known
318 | static const struct iommu_ops visconti_atu_ops = {
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:318:31: error: storage size of 'visconti_atu_ops' isn't known
cc1: some warnings being treated as errors
vim +121 drivers/iommu/visconti-atu.c
116
117 static int visconti_atu_attach_device(struct iommu_domain *io_domain,
118 struct device *dev)
119 {
120 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
> 121 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
122 int ret = 0;
123
124 if (!atu) {
125 dev_err(dev, "Cannot attach to ATU\n");
126 return -ENXIO;
127 }
128
129 mutex_lock(&domain->mutex);
130
131 if (!domain->atu) {
132 domain->atu = atu;
133 } else if (domain->atu != atu) {
134 dev_err(dev, "Can't attach ATU %s to domain on ATU %s\n",
135 dev_name(atu->dev), dev_name(domain->atu->dev));
136 ret = -EINVAL;
137 } else {
138 dev_warn(dev, "Reusing ATU context\n");
139 }
140
141 mutex_unlock(&domain->mutex);
142
143 return ret;
144 }
145
146 static void visconti_atu_detach_device(struct iommu_domain *io_domain,
147 struct device *dev)
148 {
149 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
150 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
151
152 if (domain->atu != atu)
153 return;
154
155 domain->atu = NULL;
156 }
157
158 static int visconti_atu_map(struct iommu_domain *io_domain,
159 unsigned long iova,
160 phys_addr_t paddr,
161 size_t size, int prot, gfp_t gfp)
162 {
163 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
164 struct visconti_atu_device *atu = domain->atu;
165 unsigned long flags;
166 unsigned int i;
167
168 if (!domain)
169 return -ENODEV;
170
171 spin_lock_irqsave(&atu->lock, flags);
172 for (i = 0; i < atu->num_map_entry; i++) {
173 if (!(atu->enable_entry & BIT(i))) {
174 atu->enable_entry |= BIT(i);
175 atu->iova[i] = iova;
176 atu->paddr[i] = paddr;
177 atu->size[i] = size;
178
179 visconti_atu_enable_entry(atu, i);
180 break;
181 }
182 }
183 spin_unlock_irqrestore(&atu->lock, flags);
184
185 if (i == atu->num_map_entry) {
186 dev_err(atu->dev, "map: not enough entry.\n");
187 return -ENOMEM;
188 }
189
190 return 0;
191 }
192
193 static size_t visconti_atu_unmap(struct iommu_domain *io_domain,
194 unsigned long iova,
195 size_t size,
> 196 struct iommu_iotlb_gather *iotlb_gather)
197 {
198 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
199 struct visconti_atu_device *atu = domain->atu;
200 size_t tmp_size = size;
201 unsigned long flags;
202 unsigned int i;
203
204 spin_lock_irqsave(&atu->lock, flags);
205
206 while (tmp_size != 0) {
207 for (i = 0; i < atu->num_map_entry; i++) {
208 if (atu->iova[i] != iova)
209 continue;
210
211 atu->enable_entry &= ~BIT(i);
212 iova += atu->size[i];
213 tmp_size -= atu->size[i];
214
215 visconti_atu_disable_entry(atu);
216
217 break;
218 }
219 if (i == atu->num_map_entry) {
220 dev_err(atu->dev, "unmap: not found entry.\n");
221 size = 0;
222 goto out;
223 }
224 }
225
226 if (!atu->num_map_entry)
227 visconti_atu_write(atu, ATU_AT_EN, 0);
228 out:
229 spin_unlock_irqrestore(&atu->lock, flags);
230 return size;
231 }
232
233 static phys_addr_t visconti_atu_iova_to_phys(struct iommu_domain *io_domain,
234 dma_addr_t iova)
235 {
236 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
237 struct visconti_atu_device *atu = domain->atu;
238 phys_addr_t paddr = 0;
239 unsigned int i;
240
241 for (i = 0; i < atu->num_map_entry; i++) {
242 if (!(atu->enable_entry & BIT(i)))
243 continue;
244 if (atu->iova[i] <= iova && iova < (atu->iova[i] + atu->size[i])) {
245 paddr = atu->paddr[i];
246 paddr += iova & (atu->size[i] - 1);
247 break;
248 }
249 }
250
251 dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr);
252
253 return paddr;
254 }
255
256 static int visconti_atu_of_xlate(struct device *dev, struct of_phandle_args *args)
257 {
258 if (!dev_iommu_priv_get(dev)) {
259 struct platform_device *pdev;
260
261 pdev = of_find_device_by_node(args->np);
262 dev_iommu_priv_set(dev, platform_get_drvdata(pdev));
263 platform_device_put(pdev);
264 }
265
266 return 0;
267 }
268
269 static struct iommu_domain *visconti_atu_domain_alloc(unsigned int type)
270 {
271 struct visconti_atu_domain *domain;
272
273 if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
274 return NULL;
275
276 domain = kzalloc(sizeof(*domain), GFP_KERNEL);
277 if (!domain)
278 return NULL;
279
280 mutex_init(&domain->mutex);
281
282 domain->io_domain.geometry.aperture_start = 0;
283 domain->io_domain.geometry.aperture_end = DMA_BIT_MASK(32);
284 domain->io_domain.geometry.force_aperture = true;
285
286 return &domain->io_domain;
287 }
288
289 static void visconti_atu_domain_free(struct iommu_domain *io_domain)
290 {
291 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
292
293 kfree(domain);
294 }
295
296 static struct iommu_device *visconti_atu_probe_device(struct device *dev)
297 {
> 298 struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
299 struct visconti_atu_device *atu;
300
301 if (!fwspec || fwspec->ops != &visconti_atu_ops)
302 return ERR_PTR(-ENODEV);
303
> 304 atu = dev_iommu_priv_get(dev);
305 return &atu->iommu;
306 }
307
308 static void visconti_atu_release_device(struct device *dev)
309 {
310 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
311
312 if (!atu)
313 return;
314
315 iommu_fwspec_free(dev);
316 }
317
318 static const struct iommu_ops visconti_atu_ops = {
> 319 .domain_alloc = visconti_atu_domain_alloc,
320 .probe_device = visconti_atu_probe_device,
321 .release_device = visconti_atu_release_device,
322 .device_group = generic_device_group,
323 .of_xlate = visconti_atu_of_xlate,
324 .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
325 .default_domain_ops = &(const struct iommu_domain_ops) {
326 .attach_dev = visconti_atu_attach_device,
327 .detach_dev = visconti_atu_detach_device,
328 .map = visconti_atu_map,
329 .unmap = visconti_atu_unmap,
330 .iova_to_phys = visconti_atu_iova_to_phys,
331 .free = visconti_atu_domain_free,
332 }
333 };
334
On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > +static const struct iommu_ops visconti_atu_ops = { > + .domain_alloc = visconti_atu_domain_alloc, > + .probe_device = visconti_atu_probe_device, > + .release_device = visconti_atu_release_device, > + .device_group = generic_device_group, > + .of_xlate = visconti_atu_of_xlate, > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > + .default_domain_ops = &(const struct iommu_domain_ops) { > + .attach_dev = visconti_atu_attach_device, > + .detach_dev = visconti_atu_detach_device, The detach_dev callback is about to be deprecated. The new drivers should implement the default domain and blocking domain instead. > + .map = visconti_atu_map, > + .unmap = visconti_atu_unmap, > + .iova_to_phys = visconti_atu_iova_to_phys, > + .free = visconti_atu_domain_free, > + } > +}; Best regards, baolu
Hi Nobuhiro,
I love your patch! Yet something to improve:
[auto build test ERROR on joro-iommu/next]
[also build test ERROR on arm-perf/for-next/perf soc/for-next linus/master v5.18 next-20220524]
[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/intel-lab-lkp/linux/commits/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220525/202205251708.q7cwjpF8-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.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/intel-lab-lkp/linux/commit/69bb4f3c2ef0bb1f65922bc72bb31109897a6393
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326
git checkout 69bb4f3c2ef0bb1f65922bc72bb31109897a6393
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
Note: the linux-review/Nobuhiro-Iwamatsu/Add-Visconti5-IOMMU-driver/20220525-093326 HEAD 07739c72b066c0781c371eec7614ed876441e8dd builds fine.
It only hurts bisectability.
All errors (new ones prefixed by >>):
>> drivers/iommu/visconti-atu.c:47:29: error: field 'iommu' has incomplete type
47 | struct iommu_device iommu;
| ^~~~~
>> drivers/iommu/visconti-atu.c:62:29: error: field 'io_domain' has incomplete type
62 | struct iommu_domain io_domain;
| ^~~~~~~~~
In file included from include/linux/bits.h:22,
from include/linux/ratelimit_types.h:5,
from include/linux/ratelimit.h:5,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/dma-mapping.h:7,
from drivers/iommu/visconti-atu.c:12:
drivers/iommu/visconti-atu.c: In function 'to_atu_domain':
include/linux/compiler_types.h:293:27: error: expression in static assertion is not an integer
293 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:78:56: note: in definition of macro '__static_assert'
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~
include/linux/container_of.h:19:9: note: in expansion of macro 'static_assert'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
include/linux/container_of.h:19:23: note: in expansion of macro '__same_type'
19 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~
drivers/iommu/visconti-atu.c:70:16: note: in expansion of macro 'container_of'
70 | return container_of(domain, struct visconti_atu_domain, io_domain);
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_attach_device':
>> drivers/iommu/visconti-atu.c:121:43: error: implicit declaration of function 'dev_iommu_priv_get' [-Werror=implicit-function-declaration]
121 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:121:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
drivers/iommu/visconti-atu.c: In function 'visconti_atu_detach_device':
drivers/iommu/visconti-atu.c:150:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
150 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:196:41: warning: 'struct iommu_iotlb_gather' declared inside parameter list will not be visible outside of this definition or declaration
196 | struct iommu_iotlb_gather *iotlb_gather)
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_of_xlate':
>> drivers/iommu/visconti-atu.c:262:17: error: implicit declaration of function 'dev_iommu_priv_set' [-Werror=implicit-function-declaration]
262 | dev_iommu_priv_set(dev, platform_get_drvdata(pdev));
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_domain_alloc':
>> drivers/iommu/visconti-atu.c:273:21: error: 'IOMMU_DOMAIN_UNMANAGED' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:273:21: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/iommu/visconti-atu.c:273:55: error: 'IOMMU_DOMAIN_DMA' undeclared (first use in this function)
273 | if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_probe_device':
>> drivers/iommu/visconti-atu.c:298:39: error: implicit declaration of function 'dev_iommu_fwspec_get' [-Werror=implicit-function-declaration]
298 | struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
| ^~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:298:39: warning: initialization of 'struct iommu_fwspec *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
>> drivers/iommu/visconti-atu.c:301:30: error: invalid use of undefined type 'struct iommu_fwspec'
301 | if (!fwspec || fwspec->ops != &visconti_atu_ops)
| ^~
drivers/iommu/visconti-atu.c:304:13: warning: assignment to 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
304 | atu = dev_iommu_priv_get(dev);
| ^
drivers/iommu/visconti-atu.c: In function 'visconti_atu_release_device':
drivers/iommu/visconti-atu.c:310:43: warning: initialization of 'struct visconti_atu_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
310 | struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
| ^~~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:315:9: error: implicit declaration of function 'iommu_fwspec_free' [-Werror=implicit-function-declaration]
315 | iommu_fwspec_free(dev);
| ^~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: At top level:
>> drivers/iommu/visconti-atu.c:318:21: error: variable 'visconti_atu_ops' has initializer but incomplete type
318 | static const struct iommu_ops visconti_atu_ops = {
| ^~~~~~~~~
>> drivers/iommu/visconti-atu.c:319:10: error: 'const struct iommu_ops' has no member named 'domain_alloc'
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:319:25: warning: excess elements in struct initializer
319 | .domain_alloc = visconti_atu_domain_alloc,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:319:25: note: (near initialization for 'visconti_atu_ops')
>> drivers/iommu/visconti-atu.c:320:10: error: 'const struct iommu_ops' has no member named 'probe_device'
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: warning: excess elements in struct initializer
320 | .probe_device = visconti_atu_probe_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:320:25: note: (near initialization for 'visconti_atu_ops')
>> drivers/iommu/visconti-atu.c:321:10: error: 'const struct iommu_ops' has no member named 'release_device'
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: warning: excess elements in struct initializer
321 | .release_device = visconti_atu_release_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:321:27: note: (near initialization for 'visconti_atu_ops')
>> drivers/iommu/visconti-atu.c:322:10: error: 'const struct iommu_ops' has no member named 'device_group'
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:322:25: error: 'generic_device_group' undeclared here (not in a function)
322 | .device_group = generic_device_group,
| ^~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:322:25: warning: excess elements in struct initializer
drivers/iommu/visconti-atu.c:322:25: note: (near initialization for 'visconti_atu_ops')
>> drivers/iommu/visconti-atu.c:323:10: error: 'const struct iommu_ops' has no member named 'of_xlate'
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~
drivers/iommu/visconti-atu.c:323:21: warning: excess elements in struct initializer
323 | .of_xlate = visconti_atu_of_xlate,
| ^~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:323:21: note: (near initialization for 'visconti_atu_ops')
>> drivers/iommu/visconti-atu.c:324:10: error: 'const struct iommu_ops' has no member named 'pgsize_bitmap'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:41:33: warning: excess elements in struct initializer
41 | #define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:324:26: note: in expansion of macro 'ATU_IOMMU_PGSIZE_BITMAP'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:41:33: note: (near initialization for 'visconti_atu_ops')
41 | #define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:324:26: note: in expansion of macro 'ATU_IOMMU_PGSIZE_BITMAP'
324 | .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP,
| ^~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:325:10: error: 'const struct iommu_ops' has no member named 'default_domain_ops'
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^~~~~~~~~~~~~~~~~~
>> drivers/iommu/visconti-atu.c:326:18: error: 'const struct iommu_domain_ops' has no member named 'attach_dev'
326 | .attach_dev = visconti_atu_attach_device,
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:326:31: warning: excess elements in struct initializer
326 | .attach_dev = visconti_atu_attach_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:326:31: note: (near initialization for '(anonymous)')
>> drivers/iommu/visconti-atu.c:327:18: error: 'const struct iommu_domain_ops' has no member named 'detach_dev'
327 | .detach_dev = visconti_atu_detach_device,
| ^~~~~~~~~~
drivers/iommu/visconti-atu.c:327:31: warning: excess elements in struct initializer
327 | .detach_dev = visconti_atu_detach_device,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:327:31: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:328:18: error: 'const struct iommu_domain_ops' has no member named 'map'
328 | .map = visconti_atu_map,
| ^~~
drivers/iommu/visconti-atu.c:328:24: warning: excess elements in struct initializer
328 | .map = visconti_atu_map,
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:328:24: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:329:18: error: 'const struct iommu_domain_ops' has no member named 'unmap'
329 | .unmap = visconti_atu_unmap,
| ^~~~~
drivers/iommu/visconti-atu.c:329:26: warning: excess elements in struct initializer
329 | .unmap = visconti_atu_unmap,
| ^~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:329:26: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:330:18: error: 'const struct iommu_domain_ops' has no member named 'iova_to_phys'
330 | .iova_to_phys = visconti_atu_iova_to_phys,
| ^~~~~~~~~~~~
drivers/iommu/visconti-atu.c:330:33: warning: excess elements in struct initializer
330 | .iova_to_phys = visconti_atu_iova_to_phys,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:330:33: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:331:18: error: 'const struct iommu_domain_ops' has no member named 'free'
331 | .free = visconti_atu_domain_free,
| ^~~~
drivers/iommu/visconti-atu.c:331:25: warning: excess elements in struct initializer
331 | .free = visconti_atu_domain_free,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:331:25: note: (near initialization for '(anonymous)')
drivers/iommu/visconti-atu.c:325:64: error: invalid use of undefined type 'const struct iommu_domain_ops'
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^
drivers/iommu/visconti-atu.c:325:31: warning: excess elements in struct initializer
325 | .default_domain_ops = &(const struct iommu_domain_ops) {
| ^
drivers/iommu/visconti-atu.c:325:31: note: (near initialization for 'visconti_atu_ops')
drivers/iommu/visconti-atu.c: In function 'visconti_atu_probe':
drivers/iommu/visconti-atu.c:366:22: error: implicit declaration of function 'iommu_group_alloc' [-Werror=implicit-function-declaration]
366 | atu->group = iommu_group_alloc();
| ^~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:366:20: warning: assignment to 'struct iommu_group *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
366 | atu->group = iommu_group_alloc();
| ^
drivers/iommu/visconti-atu.c:380:15: error: implicit declaration of function 'iommu_device_sysfs_add' [-Werror=implicit-function-declaration]
380 | ret = iommu_device_sysfs_add(&atu->iommu, dev, NULL, dev_name(dev));
| ^~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:384:15: error: implicit declaration of function 'iommu_device_register'; did you mean 'of_device_register'? [-Werror=implicit-function-declaration]
384 | ret = iommu_device_register(&atu->iommu, &visconti_atu_ops, dev);
| ^~~~~~~~~~~~~~~~~~~~~
| of_device_register
drivers/iommu/visconti-atu.c:388:14: error: implicit declaration of function 'iommu_present'; did you mean 'pmd_present'? [-Werror=implicit-function-declaration]
388 | if (!iommu_present(&platform_bus_type))
| ^~~~~~~~~~~~~
| pmd_present
drivers/iommu/visconti-atu.c:389:17: error: implicit declaration of function 'bus_set_iommu' [-Werror=implicit-function-declaration]
389 | bus_set_iommu(&platform_bus_type, &visconti_atu_ops);
| ^~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:395:9: error: implicit declaration of function 'iommu_device_sysfs_remove' [-Werror=implicit-function-declaration]
395 | iommu_device_sysfs_remove(&atu->iommu);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c: In function 'visconti_atu_remove':
drivers/iommu/visconti-atu.c:405:9: error: implicit declaration of function 'iommu_device_unregister'; did you mean 'of_device_unregister'? [-Werror=implicit-function-declaration]
405 | iommu_device_unregister(&atu->iommu);
| ^~~~~~~~~~~~~~~~~~~~~~~
| of_device_unregister
drivers/iommu/visconti-atu.c: At top level:
drivers/iommu/visconti-atu.c:318:31: error: storage size of 'visconti_atu_ops' isn't known
318 | static const struct iommu_ops visconti_atu_ops = {
| ^~~~~~~~~~~~~~~~
drivers/iommu/visconti-atu.c:318:31: error: storage size of 'visconti_atu_ops' isn't known
cc1: some warnings being treated as errors
vim +/iommu +47 drivers/iommu/visconti-atu.c
43
44 struct visconti_atu_device {
45 struct device *dev;
46 void __iomem *base;
> 47 struct iommu_device iommu;
48 struct iommu_group *group;
49
50 unsigned int num_entry;
51 unsigned int num_map_entry;
52 unsigned int enable_entry;
53 unsigned long iova[ATU_MAX_IOMMU_ENTRY];
54 phys_addr_t paddr[ATU_MAX_IOMMU_ENTRY];
55 size_t size[ATU_MAX_IOMMU_ENTRY];
56
57 spinlock_t lock;
58 };
59
60 struct visconti_atu_domain {
61 struct visconti_atu_device *atu;
> 62 struct iommu_domain io_domain;
63 struct mutex mutex;
64 };
65
66 static const struct iommu_ops visconti_atu_ops;
67
68 static struct visconti_atu_domain *to_atu_domain(struct iommu_domain *domain)
69 {
70 return container_of(domain, struct visconti_atu_domain, io_domain);
71 }
72
73 static inline void visconti_atu_write(struct visconti_atu_device *atu, u32 reg,
74 u32 val)
75 {
76 writel_relaxed(val, atu->base + reg);
77 }
78
79 static inline u32 visconti_atu_read(struct visconti_atu_device *atu, u32 reg)
80 {
81 return readl_relaxed(atu->base + reg);
82 }
83
84 static void visconti_atu_enable_entry(struct visconti_atu_device *atu,
85 int num)
86 {
87 dev_dbg(atu->dev, "enable ATU: %d\n", atu->enable_entry);
88
89 visconti_atu_write(atu, ATU_AT_EN, 0);
90 if (atu->enable_entry & BIT(num)) {
91 visconti_atu_write(atu,
92 ATU_AT_REG(num, ATU_AT_BLADDR),
93 atu->iova[num]);
94 visconti_atu_write(atu,
95 ATU_AT_REG(num, ATU_AT_ELADDR),
96 atu->iova[num] + atu->size[num] - 1);
97 visconti_atu_write(atu,
98 ATU_AT_REG(num, ATU_AT_BGADDR0),
99 atu->iova[num] & ATU_BGADDR_MASK);
100 visconti_atu_write(atu,
101 ATU_AT_REG(num, ATU_AT_BGADDR1),
102 (atu->iova[num] >> 32) & ATU_BGADDR_MASK);
103 }
104 visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry);
105 visconti_atu_write(atu, ATU_AT_EN, 1);
106 }
107
108 static void visconti_atu_disable_entry(struct visconti_atu_device *atu)
109 {
110 dev_dbg(atu->dev, "disable ATU: %d\n", atu->enable_entry);
111
112 visconti_atu_write(atu, ATU_AT_EN, 0);
113 visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry);
114 visconti_atu_write(atu, ATU_AT_EN, 1);
115 }
116
117 static int visconti_atu_attach_device(struct iommu_domain *io_domain,
118 struct device *dev)
119 {
120 struct visconti_atu_domain *domain = to_atu_domain(io_domain);
> 121 struct visconti_atu_device *atu = dev_iommu_priv_get(dev);
122 int ret = 0;
123
124 if (!atu) {
125 dev_err(dev, "Cannot attach to ATU\n");
126 return -ENXIO;
127 }
128
129 mutex_lock(&domain->mutex);
130
131 if (!domain->atu) {
132 domain->atu = atu;
133 } else if (domain->atu != atu) {
134 dev_err(dev, "Can't attach ATU %s to domain on ATU %s\n",
135 dev_name(atu->dev), dev_name(domain->atu->dev));
136 ret = -EINVAL;
137 } else {
138 dev_warn(dev, "Reusing ATU context\n");
139 }
140
141 mutex_unlock(&domain->mutex);
142
143 return ret;
144 }
145
On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > +static const struct iommu_ops visconti_atu_ops = { > > + .domain_alloc = visconti_atu_domain_alloc, > > + .probe_device = visconti_atu_probe_device, > > + .release_device = visconti_atu_release_device, > > + .device_group = generic_device_group, > > + .of_xlate = visconti_atu_of_xlate, > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > + .attach_dev = visconti_atu_attach_device, > > + .detach_dev = visconti_atu_detach_device, > > The detach_dev callback is about to be deprecated. The new drivers > should implement the default domain and blocking domain instead. Yes please, new drivers need to use default_domains. It is very strange that visconti_atu_detach_device() does nothing. It is not required that a domain is fully unmapped before being destructed, I think detach should set ATU_AT_EN to 0. What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and return that from ops->def_domain_type(). Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0 Also, if I surmise how this works properly, it is not following the iommu API to halt all DMA during map/unmap operations. Should at least document this and explain why it is OK.. I'm feeling like these "special" drivers need some kind of handshake with their only users because they don't work with things like VFIO.. Jason
> From: Jason Gunthorpe > Sent: Thursday, May 26, 2022 2:27 AM > > On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > > +static const struct iommu_ops visconti_atu_ops = { > > > + .domain_alloc = visconti_atu_domain_alloc, > > > + .probe_device = visconti_atu_probe_device, > > > + .release_device = visconti_atu_release_device, > > > + .device_group = generic_device_group, > > > + .of_xlate = visconti_atu_of_xlate, > > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > > + .attach_dev = visconti_atu_attach_device, > > > + .detach_dev = visconti_atu_detach_device, > > > > The detach_dev callback is about to be deprecated. The new drivers > > should implement the default domain and blocking domain instead. > > Yes please, new drivers need to use default_domains. > > It is very strange that visconti_atu_detach_device() does nothing. It > is not required that a domain is fully unmapped before being > destructed, I think detach should set ATU_AT_EN to 0. Looks the atu is shared by all devices behind and can only serve one I/O address space. The atu registers only keep information about iova ranges without any device notation. That is probably the reason why both attach/detach() don't touch hardware. iiuc then this suggests there should be only one iommu group per atu, instead of using generic_device_group() to create one group per device. > > What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is I guess it's a blocking behavior since that register tracks which iova range register is valid. > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > return that from ops->def_domain_type(). BLOCKING should not be used as a default domain type for DMA API which needs either a DMA or IDENTITY type. > > Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0 Agree > > Also, if I surmise how this works properly, it is not following the > iommu API to halt all DMA during map/unmap operations. Should at least > document this and explain why it is OK.. > > I'm feeling like these "special" drivers need some kind of handshake > with their only users because they don't work with things like VFIO.. > > Jason
On Thu, May 26, 2022 at 12:43:25AM +0000, Tian, Kevin wrote: > iiuc then this suggests there should be only one iommu group per atu, > instead of using generic_device_group() to create one group per > device. Sounds like it > > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > > return that from ops->def_domain_type(). > > BLOCKING should not be used as a default domain type for DMA API > which needs either a DMA or IDENTITY type. New drivers should not have a NULL group->default_domain. IMHO this driver does not support the DMA API so the default_domain should be assigned to blocking and the DMA API disabled. We might need some core changes to accommodate this. The alternative would be to implement the identity domain, assuming the ATU thing can store that kind of translation. Jason
Hi, > -----Original Message----- > From: Baolu Lu <baolu.lu@linux.intel.com> > Sent: Wednesday, May 25, 2022 3:27 PM > To: iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣＴ) > <nobuhiro1.iwamatsu@toshiba.co.jp>; Joerg Roedel <joro@8bytes.org>; Will > Deacon <will@kernel.org>; Rob Herring <robh+dt@kernel.org>; Jason > Gunthorpe <jgg@nvidia.com> > Cc: baolu.lu@linux.intel.com; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; iommu@lists.linux-foundation.org; ishikawa > yuji(石川 悠司 ○ＲＤＣ□ＡＩＴＣ○ＥＡ開) > <yuji2.ishikawa@toshiba.co.jp>; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver > > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > +static const struct iommu_ops visconti_atu_ops = { > > + .domain_alloc = visconti_atu_domain_alloc, > > + .probe_device = visconti_atu_probe_device, > > + .release_device = visconti_atu_release_device, > > + .device_group = generic_device_group, > > + .of_xlate = visconti_atu_of_xlate, > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > + .attach_dev = visconti_atu_attach_device, > > + .detach_dev = visconti_atu_detach_device, > > The detach_dev callback is about to be deprecated. The new drivers should > implement the default domain and blocking domain instead. I see. I will update this with next version. > > > + .map = visconti_atu_map, > > + .unmap = visconti_atu_unmap, > > + .iova_to_phys = visconti_atu_iova_to_phys, > > + .free = visconti_atu_domain_free, > > + } > > +}; > > Best regards, > baolu Best regards, Nobuhiro
Hi, Thanks for your review. > -----Original Message----- > From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Thursday, May 26, 2022 3:27 AM > To: Baolu Lu <baolu.lu@linux.intel.com> > Cc: iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣＴ) > <nobuhiro1.iwamatsu@toshiba.co.jp>; Joerg Roedel <joro@8bytes.org>; Will > Deacon <will@kernel.org>; Rob Herring <robh+dt@kernel.org>; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > iommu@lists.linux-foundation.org; ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴ > Ｃ○ＥＡ開) <yuji2.ishikawa@toshiba.co.jp>; > linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver > > On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > > +static const struct iommu_ops visconti_atu_ops = { > > > + .domain_alloc = visconti_atu_domain_alloc, > > > + .probe_device = visconti_atu_probe_device, > > > + .release_device = visconti_atu_release_device, > > > + .device_group = generic_device_group, > > > + .of_xlate = visconti_atu_of_xlate, > > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > > + .attach_dev = visconti_atu_attach_device, > > > + .detach_dev = visconti_atu_detach_device, > > > > The detach_dev callback is about to be deprecated. The new drivers > > should implement the default domain and blocking domain instead. > > Yes please, new drivers need to use default_domains. > > It is very strange that visconti_atu_detach_device() does nothing. It is not > required that a domain is fully unmapped before being destructed, I think > detach should set ATU_AT_EN to 0. I see, I rethink implementation. > > What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > return that from ops->def_domain_type(). If ATU_AT_ENTRY_EN is 0, nothing happens. It does not work with IOMMU, it works with the memory space set in device tree. Also, I investigate about IOMMU_DOMAIN_BLOCKING. > > Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0 > > Also, if I surmise how this works properly, it is not following the iommu API to > halt all DMA during map/unmap operations. Should at least document this and > explain why it is OK.. I see, I will check DMA during map and unmap operations. > > I'm feeling like these "special" drivers need some kind of handshake with their > only users because they don't work with things like VFIO.. Since the devices that utilize this IOMMU function are fixed, I do not think that a special handshake is required. Could you you tell me where you thought you needed a handshake? Best regards, Nobuhiro > > Jason
On Mon, Jun 20, 2022 at 05:49:13AM +0000, nobuhiro1.iwamatsu@toshiba.co.jp wrote: > Hi, > > Thanks for your review. > > > -----Original Message----- > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Thursday, May 26, 2022 3:27 AM > > To: Baolu Lu <baolu.lu@linux.intel.com> > > Cc: iwamatsu nobuhiro(岩松 信洋 □ＳＷＣ◯ＡＣＴ) > > <nobuhiro1.iwamatsu@toshiba.co.jp>; Joerg Roedel <joro@8bytes.org>; Will > > Deacon <will@kernel.org>; Rob Herring <robh+dt@kernel.org>; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > > iommu@lists.linux-foundation.org; ishikawa yuji(石川 悠司 ○ＲＤＣ□ＡＩＴ > > Ｃ○ＥＡ開) <yuji2.ishikawa@toshiba.co.jp>; > > linux-arm-kernel@lists.infradead.org > > Subject: Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver > > > > On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > > > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > > > +static const struct iommu_ops visconti_atu_ops = { > > > > + .domain_alloc = visconti_atu_domain_alloc, > > > > + .probe_device = visconti_atu_probe_device, > > > > + .release_device = visconti_atu_release_device, > > > > + .device_group = generic_device_group, > > > > + .of_xlate = visconti_atu_of_xlate, > > > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > > > + .attach_dev = visconti_atu_attach_device, > > > > + .detach_dev = visconti_atu_detach_device, > > > > > > The detach_dev callback is about to be deprecated. The new drivers > > > should implement the default domain and blocking domain instead. > > > > Yes please, new drivers need to use default_domains. > > > > It is very strange that visconti_atu_detach_device() does nothing. It is not > > required that a domain is fully unmapped before being destructed, I think > > detach should set ATU_AT_EN to 0. > > I see, I rethink implementation. > > > > > What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is > > rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and > > return that from ops->def_domain_type(). > > If ATU_AT_ENTRY_EN is 0, nothing happens. It does not work with IOMMU, > it works with the memory space set in device tree. So that would be an assignment to DOMAIN_IDENTITY > > I'm feeling like these "special" drivers need some kind of handshake with their > > only users because they don't work with things like VFIO.. > > Since the devices that utilize this IOMMU function are fixed, I do > not think that a special handshake is required. Could you you tell > me where you thought you needed a handshake? In this case the iommu driver is so limited that it will not work with VFIO - it is only ment to be used with the fixed drivers that are paired with it. Ideally we'd prevent VFIO from connecting and only allow drivers that know the limitations of the IOMMU to use the unmanaged domain. Jason
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index c79a0df090c0..8a4351020b7f 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -486,4 +486,11 @@ config SPRD_IOMMU Say Y here if you want to use the multimedia devices listed above. +config VISCONTI_ATU + tristate "Toshiba Visconti5 IOMMU Support" + depends on ARCH_VISCONTI || COMPILE_TEST + select IOMMU_API + help + Support for the IOMMU API for Toshiba Visconti5 ARM SoCs. + endif # IOMMU_SUPPORT diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile index 44475a9b3eea..077189f908ea 100644 --- a/drivers/iommu/Makefile +++ b/drivers/iommu/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o obj-$(CONFIG_IOMMU_SVA) += iommu-sva-lib.o io-pgfault.o obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o obj-$(CONFIG_APPLE_DART) += apple-dart.o +obj-$(CONFIG_VISCONTI_ATU) += visconti-atu.o diff --git a/drivers/iommu/visconti-atu.c b/drivers/iommu/visconti-atu.c new file mode 100644 index 000000000000..269c912ad4c9 --- /dev/null +++ b/drivers/iommu/visconti-atu.c @@ -0,0 +1,426 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Toshiba Visconti5 IOMMU (ATU) driver + * + * (C) Copyright 2022 Toshiba Electronic Devices & Storage Corporation + * (C) Copyright 2022 Toshiba CORPORATION + * + * Author: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> + */ + +#include <linux/dma-iommu.h> +#include <linux/dma-mapping.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/sizes.h> +#include <linux/slab.h> + +/* Regsiter address */ +#define ATU_AT_EN 0x0000 +#define ATU_AT_ENTRY_EN 0x0020 + +#define ATU_AT_BLADDR 0x0030 +#define ATU_AT_ELADDR 0x0038 +#define ATU_AT_BGADDR0 0x0040 +#define ATU_AT_BGADDR1 0x0044 +#define ATU_AT_CONF 0x0048 +#define ATU_AT_REG(n, reg) (0x20 * n + reg) + +#define ATU_INT_START 0x0440 +#define ATU_INT_MASKED_STAT 0x0444 +#define ATU_INT_MASK 0x0448 +#define ATU_RP_CONF 0x0450 +#define ATU_ERR_ADDR 0x0454 +#define ATU_ERR_CLR 0x045C +#define ATU_STAT 0x0460 + +#define ATU_BGADDR_MASK GENMASK(31, 0) + +#define ATU_IOMMU_PGSIZE_BITMAP 0x7ffff000 /* SZ_1G - SZ_4K */ +#define ATU_MAX_IOMMU_ENTRY 32 + +struct visconti_atu_device { + struct device *dev; + void __iomem *base; + struct iommu_device iommu; + struct iommu_group *group; + + unsigned int num_entry; + unsigned int num_map_entry; + unsigned int enable_entry; + unsigned long iova[ATU_MAX_IOMMU_ENTRY]; + phys_addr_t paddr[ATU_MAX_IOMMU_ENTRY]; + size_t size[ATU_MAX_IOMMU_ENTRY]; + + spinlock_t lock; +}; + +struct visconti_atu_domain { + struct visconti_atu_device *atu; + struct iommu_domain io_domain; + struct mutex mutex; +}; + +static const struct iommu_ops visconti_atu_ops; + +static struct visconti_atu_domain *to_atu_domain(struct iommu_domain *domain) +{ + return container_of(domain, struct visconti_atu_domain, io_domain); +} + +static inline void visconti_atu_write(struct visconti_atu_device *atu, u32 reg, + u32 val) +{ + writel_relaxed(val, atu->base + reg); +} + +static inline u32 visconti_atu_read(struct visconti_atu_device *atu, u32 reg) +{ + return readl_relaxed(atu->base + reg); +} + +static void visconti_atu_enable_entry(struct visconti_atu_device *atu, + int num) +{ + dev_dbg(atu->dev, "enable ATU: %d\n", atu->enable_entry); + + visconti_atu_write(atu, ATU_AT_EN, 0); + if (atu->enable_entry & BIT(num)) { + visconti_atu_write(atu, + ATU_AT_REG(num, ATU_AT_BLADDR), + atu->iova[num]); + visconti_atu_write(atu, + ATU_AT_REG(num, ATU_AT_ELADDR), + atu->iova[num] + atu->size[num] - 1); + visconti_atu_write(atu, + ATU_AT_REG(num, ATU_AT_BGADDR0), + atu->iova[num] & ATU_BGADDR_MASK); + visconti_atu_write(atu, + ATU_AT_REG(num, ATU_AT_BGADDR1), + (atu->iova[num] >> 32) & ATU_BGADDR_MASK); + } + visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry); + visconti_atu_write(atu, ATU_AT_EN, 1); +} + +static void visconti_atu_disable_entry(struct visconti_atu_device *atu) +{ + dev_dbg(atu->dev, "disable ATU: %d\n", atu->enable_entry); + + visconti_atu_write(atu, ATU_AT_EN, 0); + visconti_atu_write(atu, ATU_AT_ENTRY_EN, atu->enable_entry); + visconti_atu_write(atu, ATU_AT_EN, 1); +} + +static int visconti_atu_attach_device(struct iommu_domain *io_domain, + struct device *dev) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + struct visconti_atu_device *atu = dev_iommu_priv_get(dev); + int ret = 0; + + if (!atu) { + dev_err(dev, "Cannot attach to ATU\n"); + return -ENXIO; + } + + mutex_lock(&domain->mutex); + + if (!domain->atu) { + domain->atu = atu; + } else if (domain->atu != atu) { + dev_err(dev, "Can't attach ATU %s to domain on ATU %s\n", + dev_name(atu->dev), dev_name(domain->atu->dev)); + ret = -EINVAL; + } else { + dev_warn(dev, "Reusing ATU context\n"); + } + + mutex_unlock(&domain->mutex); + + return ret; +} + +static void visconti_atu_detach_device(struct iommu_domain *io_domain, + struct device *dev) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + struct visconti_atu_device *atu = dev_iommu_priv_get(dev); + + if (domain->atu != atu) + return; + + domain->atu = NULL; +} + +static int visconti_atu_map(struct iommu_domain *io_domain, + unsigned long iova, + phys_addr_t paddr, + size_t size, int prot, gfp_t gfp) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + struct visconti_atu_device *atu = domain->atu; + unsigned long flags; + unsigned int i; + + if (!domain) + return -ENODEV; + + spin_lock_irqsave(&atu->lock, flags); + for (i = 0; i < atu->num_map_entry; i++) { + if (!(atu->enable_entry & BIT(i))) { + atu->enable_entry |= BIT(i); + atu->iova[i] = iova; + atu->paddr[i] = paddr; + atu->size[i] = size; + + visconti_atu_enable_entry(atu, i); + break; + } + } + spin_unlock_irqrestore(&atu->lock, flags); + + if (i == atu->num_map_entry) { + dev_err(atu->dev, "map: not enough entry.\n"); + return -ENOMEM; + } + + return 0; +} + +static size_t visconti_atu_unmap(struct iommu_domain *io_domain, + unsigned long iova, + size_t size, + struct iommu_iotlb_gather *iotlb_gather) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + struct visconti_atu_device *atu = domain->atu; + size_t tmp_size = size; + unsigned long flags; + unsigned int i; + + spin_lock_irqsave(&atu->lock, flags); + + while (tmp_size != 0) { + for (i = 0; i < atu->num_map_entry; i++) { + if (atu->iova[i] != iova) + continue; + + atu->enable_entry &= ~BIT(i); + iova += atu->size[i]; + tmp_size -= atu->size[i]; + + visconti_atu_disable_entry(atu); + + break; + } + if (i == atu->num_map_entry) { + dev_err(atu->dev, "unmap: not found entry.\n"); + size = 0; + goto out; + } + } + + if (!atu->num_map_entry) + visconti_atu_write(atu, ATU_AT_EN, 0); +out: + spin_unlock_irqrestore(&atu->lock, flags); + return size; +} + +static phys_addr_t visconti_atu_iova_to_phys(struct iommu_domain *io_domain, + dma_addr_t iova) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + struct visconti_atu_device *atu = domain->atu; + phys_addr_t paddr = 0; + unsigned int i; + + for (i = 0; i < atu->num_map_entry; i++) { + if (!(atu->enable_entry & BIT(i))) + continue; + if (atu->iova[i] <= iova && iova < (atu->iova[i] + atu->size[i])) { + paddr = atu->paddr[i]; + paddr += iova & (atu->size[i] - 1); + break; + } + } + + dev_dbg(atu->dev, "iova_to_phys: %llx -> %llx\n", iova, paddr); + + return paddr; +} + +static int visconti_atu_of_xlate(struct device *dev, struct of_phandle_args *args) +{ + if (!dev_iommu_priv_get(dev)) { + struct platform_device *pdev; + + pdev = of_find_device_by_node(args->np); + dev_iommu_priv_set(dev, platform_get_drvdata(pdev)); + platform_device_put(pdev); + } + + return 0; +} + +static struct iommu_domain *visconti_atu_domain_alloc(unsigned int type) +{ + struct visconti_atu_domain *domain; + + if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA) + return NULL; + + domain = kzalloc(sizeof(*domain), GFP_KERNEL); + if (!domain) + return NULL; + + mutex_init(&domain->mutex); + + domain->io_domain.geometry.aperture_start = 0; + domain->io_domain.geometry.aperture_end = DMA_BIT_MASK(32); + domain->io_domain.geometry.force_aperture = true; + + return &domain->io_domain; +} + +static void visconti_atu_domain_free(struct iommu_domain *io_domain) +{ + struct visconti_atu_domain *domain = to_atu_domain(io_domain); + + kfree(domain); +} + +static struct iommu_device *visconti_atu_probe_device(struct device *dev) +{ + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); + struct visconti_atu_device *atu; + + if (!fwspec || fwspec->ops != &visconti_atu_ops) + return ERR_PTR(-ENODEV); + + atu = dev_iommu_priv_get(dev); + return &atu->iommu; +} + +static void visconti_atu_release_device(struct device *dev) +{ + struct visconti_atu_device *atu = dev_iommu_priv_get(dev); + + if (!atu) + return; + + iommu_fwspec_free(dev); +} + +static const struct iommu_ops visconti_atu_ops = { + .domain_alloc = visconti_atu_domain_alloc, + .probe_device = visconti_atu_probe_device, + .release_device = visconti_atu_release_device, + .device_group = generic_device_group, + .of_xlate = visconti_atu_of_xlate, + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, + .default_domain_ops = &(const struct iommu_domain_ops) { + .attach_dev = visconti_atu_attach_device, + .detach_dev = visconti_atu_detach_device, + .map = visconti_atu_map, + .unmap = visconti_atu_unmap, + .iova_to_phys = visconti_atu_iova_to_phys, + .free = visconti_atu_domain_free, + } +}; + +static int visconti_atu_probe(struct platform_device *pdev) +{ + struct visconti_atu_device *atu; + struct device *dev = &pdev->dev; + struct resource *res; + u32 reserved_entry; + int ret; + + atu = devm_kzalloc(&pdev->dev, sizeof(*atu), GFP_KERNEL); + if (!atu) + return -ENOMEM; + + ret = of_property_read_u32(dev->of_node, "toshiba,max-entry", + &atu->num_entry); + if (ret < 0) { + dev_err(dev, "cannot get max-entry data\n"); + return ret; + } + + ret = of_property_read_u32(dev->of_node, "toshiba,reserved-entry", + &reserved_entry); + if (ret < 0) + reserved_entry = 0; + + if (atu->num_entry < reserved_entry) + return -EINVAL; + + atu->num_map_entry = atu->num_entry - reserved_entry; + atu->enable_entry = 0; + atu->dev = dev; + + atu->group = iommu_group_alloc(); + if (IS_ERR(atu->group)) { + ret = PTR_ERR(atu->group); + goto out; + } + + spin_lock_init(&atu->lock); + + atu->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); + if (IS_ERR(atu->base)) { + ret = PTR_ERR(atu->base); + goto out; + } + + ret = iommu_device_sysfs_add(&atu->iommu, dev, NULL, dev_name(dev)); + if (ret) + goto out; + + ret = iommu_device_register(&atu->iommu, &visconti_atu_ops, dev); + if (ret) + goto remove_sysfs; + + if (!iommu_present(&platform_bus_type)) + bus_set_iommu(&platform_bus_type, &visconti_atu_ops); + platform_set_drvdata(pdev, atu); + + return 0; + +remove_sysfs: + iommu_device_sysfs_remove(&atu->iommu); +out: + return ret; +} + +static int visconti_atu_remove(struct platform_device *pdev) +{ + struct visconti_atu_device *atu = platform_get_drvdata(pdev); + + iommu_device_sysfs_remove(&atu->iommu); + iommu_device_unregister(&atu->iommu); + + return 0; +} + +static const struct of_device_id visconti_atu_of_match[] = { + { .compatible = "toshiba,visconti-atu", }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, visconti_atu_of_match); + +static struct platform_driver visconti_atu_driver = { + .driver = { + .name = "visconti-atu", + .of_match_table = visconti_atu_of_match, + .suppress_bind_attrs = true, + }, + .probe = visconti_atu_probe, + .remove = visconti_atu_remove, +}; + +builtin_platform_driver(visconti_atu_driver);
Add IOMMU (ATU) driver can bse used for Visconti5's multimedia IPs, such as DCNN (Deep Convolutional Neural Network), VIIF(Video Input), VOIF(Video output), and others. Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp> --- drivers/iommu/Kconfig | 7 + drivers/iommu/Makefile | 1 + drivers/iommu/visconti-atu.c | 426 +++++++++++++++++++++++++++++++++++ 3 files changed, 434 insertions(+) create mode 100644 drivers/iommu/visconti-atu.c