[PATCH] iommu: add include/uapi/linux/iommu.h to MAINTAINERS file
When include/uapi/linux/iommu.h was created it was never added to the file list in MAINTAINERS. Cc: Joerg Roedel Signed-off-by: Jerry Snitselaar --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index e1897ed32930..061648b6e393 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8954,6 +8954,7 @@ F:drivers/iommu/ F: include/linux/iommu.h F: include/linux/iova.h F: include/linux/of_iommu.h +F: include/uapi/linux/iommu.h IO_URING M: Jens Axboe -- 2.24.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa CMA
On Fri, Jun 05, 2020 at 06:04:31AM +, Song Bao Hua (Barry Song) wrote: > > > > -Original Message- > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > > Sent: Thursday, June 4, 2020 11:37 PM > > To: kbu...@lists.01.org; Song Bao Hua (Barry Song) > > ; h...@lst.de; m.szyprow...@samsung.com; > > robin.mur...@arm.com; catalin.mari...@arm.com > > Cc: l...@intel.com; Dan Carpenter ; > > kbuild-...@lists.01.org; iommu@lists.linux-foundation.org; > > linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org; Linuxarm > > ; Jonathan Cameron > > ; John Garry > > Subject: Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa > > CMA > > > > Hi Barry, > > > > url: > > https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CM > > A-for-ARM-server/20200603-104821 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git > > for-next/core > > config: x86_64-randconfig-m001-20200603 (attached as .config) > > compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot > > Reported-by: Dan Carpenter > > Dan, thanks! Good catch! > as this patch has not been in mainline yet, is it correct to add these > "reported-by" in patch v2? These are just an automated email from the zero day bot. I look over the Smatch warnings and then forward them on. regards, dan carpenter ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/mediatek: Use totalram_pages to setup enable_4GB
On 04.06.20 17:27, David Hildenbrand wrote: > On 04.06.20 17:06, Christoph Hellwig wrote: >> On Thu, Jun 04, 2020 at 01:32:40PM +0200, David Hildenbrand wrote: >>> Just a thought: If memory hotplug is applicable as well, you might >>> either want to always assume data->enable_4GB, or handle memory hotplug >>> events from the memory notifier, when new memory gets onlined (not sure >>> how tricky that is). >> >> We probably want a highest_pfn_possible() or similar API instead of >> having drivers poking into random VM internals. > > Well, memory notifiers are a reasonable api used accross the kernel to > get notified when new memory is onlined to the buddy that could be used > for allocations. > > highest_pfn_possible() would have to default to something linked to > MAX_PHYSMEM_BITS whenever memory hotplug is configured, I am not sure > how helpful that is (IOW, you can just default to enable_4GB=true in > that case instead in most cases). Correction: At least on x86-64 we have max_possible_pfn, which will consult the ACPI SRAT table to figure out the maximum possible PFN. (Without SRAT, max_possible_pfn will point at the end of initial boot memory and not consider hotplug memory - something that e.g., newer QEMU versions work around by creating SRAT tables if memory hotplug might be possible, even if there is no actual NUMA configuration). pci-swiotlb.c similarly relies on that to figure out if there are any !DMA addresses to handle. -- Thanks, David / dhildenb ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa CMA
Hi Barry,
I love your patch! Perhaps something to improve:
[auto build test WARNING on arm64/for-next/core]
[also build test WARNING on linus/master v5.7]
[cannot apply to next-20200605]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CMA-for-ARM-server/20200603-104821
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
for-next/core
config: i386-randconfig-r016-20200605 (attached as .config)
compiler: gcc-4.9 (Ubuntu 4.9.3-13ubuntu2) 4.9.3
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/printk.h:326:0,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:19,
from arch/x86/include/asm/bug.h:83,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from include/linux/memblock.h:13,
from kernel/dma/contiguous.c:21:
kernel/dma/contiguous.c: In function 'dma_pernuma_cma_reserve':
>> include/linux/dynamic_debug.h:82:16: warning: format '%llu' expects argument
>> of type 'long long unsigned int', but argument 4 has type 'unsigned int'
>> [-Wformat=]
static struct _ddebug __aligned(8) ^
include/linux/dynamic_debug.h:123:2: note: in expansion of macro
'DEFINE_DYNAMIC_DEBUG_METADATA'
DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); ^
include/linux/dynamic_debug.h:143:2: note: in expansion of macro
'__dynamic_func_call'
__dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
^
include/linux/dynamic_debug.h:153:2: note: in expansion of macro
'_dynamic_func_call'
_dynamic_func_call(fmt, __dynamic_pr_debug, ^
include/linux/printk.h:330:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^
kernel/dma/contiguous.c:128:3: note: in expansion of macro 'pr_debug'
pr_debug("%s: reserved %llu MiB on node %dn", __func__,
^
vim +82 include/linux/dynamic_debug.h
923abb9d797ba0 Gal Pressman 2019-05-01 75
923abb9d797ba0 Gal Pressman 2019-05-01 76 extern __printf(3, 4)
923abb9d797ba0 Gal Pressman 2019-05-01 77 void __dynamic_ibdev_dbg(struct
_ddebug *descriptor,
923abb9d797ba0 Gal Pressman 2019-05-01 78 const
struct ib_device *ibdev,
923abb9d797ba0 Gal Pressman 2019-05-01 79 const
char *fmt, ...);
923abb9d797ba0 Gal Pressman 2019-05-01 80
2bdde670beedf7 Rasmus Villemoes 2019-03-07 81 #define
DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)\
c0d2af63786394 Joe Perches 2012-10-18 @82 static struct _ddebug
__aligned(8) \
07613b0b5ef857 Jason Baron 2011-10-04 83
__attribute__((section("__verbose"))) name = { \
07613b0b5ef857 Jason Baron 2011-10-04 84 .modname =
KBUILD_MODNAME, \
07613b0b5ef857 Jason Baron 2011-10-04 85 .function =
__func__, \
07613b0b5ef857 Jason Baron 2011-10-04 86 .filename =
__FILE__, \
07613b0b5ef857 Jason Baron 2011-10-04 87 .format =
(fmt),\
07613b0b5ef857 Jason Baron 2011-10-04 88 .lineno =
__LINE__, \
07613b0b5ef857 Jason Baron 2011-10-04 89 .flags =
_DPRINTK_FLAGS_DEFAULT,\
2bdde670beedf7 Rasmus Villemoes 2019-03-07 90
_DPRINTK_KEY_INIT \
07613b0b5ef857 Jason Baron 2011-10-04 91 }
07613b0b5ef857 Jason Baron 2011-10-04 92
:: The code at line 82 was first introduced by commit
:: c0d2af637863940b1a4fb208224ca7acb905c39f dynamic_debug: Remove
unnecessary __used
:: TO: Joe Perches
:: CC: Greg Kroah-Hartman
---
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 v3 09/13] device core: Introduce multiple dma pfn offsets
Hi Christoph,
a question arouse, is there a real value to dealing with PFNs (as opposed to
real addresses) in the core DMA code/structures? I see that in some cases it
eases interacting with mm, but the overwhelming usage of say,
dev->dma_pfn_offset, involves shifting it.
Hi Jim,
On Thu, 2020-06-04 at 14:01 -0400, Jim Quinlan wrote:
> Hi Nicolas,
[...]
> > I understand the need for dev to be around, devm_*() is key. But also it's
> > important to keep the functions on purpose. And if of_dma_get_range() starts
> > setting ranges it calls, for the very least, for a function rename. Although
> > I'd rather split the parsing and setting of ranges as mentioned earlier.
> > That
> > said, I get that's a more drastic move.
>
> I agree with you. I could do this from device.c:
>
> of_dma_get_num_ranges(..., &num_ranges); /* new function */
> r = devm_kcalloc(dev, num_ranges + 1, sizeof(*r), GFP_KERNEL);
> of_dma_get_range(np, &dma_addr, &paddr, &size, r, num_ranges);
>
> The problem here is that there could be four ranges, all with
> offset=0. My current code would optimize this case out but the above
> would have us holding useless memory and looping through the four
> ranges on every dma <=> phys conversion only to add 0.
Point taken. Ultimately it's setting the device's dma ranges in
of_dma_get_range() that was really bothering me, so if we have to pass the
device pointer for allocations, be it.
> > Talking about drastic moves. How about getting rid of the concept of
> > dma_pfn_offset for drivers altogether. Let them provide
> > dma_pfn_offset_regions
> > (even when there is only one). I feel it's conceptually nicer, as you'd be
> > dealing only in one currency, so to speak, and you'd centralize the bus DMA
> > ranges setter function which is always easier to maintain.
> Do you agree that we have to somehow hang this info on the struct
> device structure? Because in the dma2phys() and phys2dma() all you
> have is the dev parameter. I don't see how this can be done w/o
> involving dev.
Sorry I didn't make myself clear here. What bothers me is having two functions
setting the same device parameter trough different means, I'd be happy to get
rid of attach_uniform_dma_pfn_offset(), and always use the same function to set
a device's bus dma regions. Something the likes of this comes to mind:
dma_attach_pfn_offset_region(struct device *dev, struct dma_pfn_offset_regions
*r)
We could maybe use some helper macros for the linear case. But that's the gist
of it.
Also, it goes hand in hand with the comment below. Why having a special case
for non sparse DMA offsets in struct dma_pfn_offset_regions? The way I see it,
in this case, code simplicity is more interesting than a small optimization.
> > I'd go as far as not creating a special case for uniform offsets. Let just
> > set
> > cpu_end and dma_end to -1 so we always get a match. It's slightly more
> > compute
> > heavy, but I don't think it's worth the optimization.
> Well, there are two subcases here. One where we do know the bounds
> and one where we do not. I suppose for the latter I could have the
> drivers calling it with begin=0 and end=~(dma_addr_t)0. Let me give
> this some thought...
>
> > Just my two cents :)
>
> Worth much more than $0.02 IMO :-)
BTW, would you consider renaming the DMA offset struct to something simpler
like, struct bus_dma_region? It complements 'dev->bus_dma_limit' better IMO.
> BTW, I tried putting the "if (dev->dma_pfn_offset_map)" clause inside
> the inline functions but the problem is that it slows the fastpath;
> consider the following code from dma-direct.h
>
> if (dev->dma_pfn_offset_map) {
> unsigned long dma_pfn_offset =
dma_pfn_offset_from_phys_addr(dev, paddr);
>
> dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> }
> return dev_addr;
>
> becomes
>
> unsigned long dma_pfn_offset = dma_pfn_offset_from_phys_addr(dev,
paddr);
>
> dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> return dev_addr;
>
> So those configurations that have no dma_pfn_offsets are doing an
> unnecessary shift and add.
Fair enough. Still not a huge difference, but I see the value being the most
common case.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] iommu/amd: Parse supported address sizes from IVRS
The IVRS ACPI table specifies maximum address sizes for I/O virtual
addresses that can be handled by the IOMMUs in the system. Parse that
data from the IVRS header so that it can be considered in limiting the
I/O aperture.
Based on prior work by Marius Hillenbrand.
Link: https://www.amd.com/system/files/TechDocs/48882_IOMMU_3.05_PUB.pdf
Signed-off-by: Sebastian Ott
---
drivers/iommu/amd_iommu.c | 1 +
drivers/iommu/amd_iommu_init.c | 26 ++
drivers/iommu/amd_iommu_types.h | 3 +++
3 files changed, 30 insertions(+)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 2883ac389abb..fb4a44550c4a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -88,6 +88,7 @@ const struct iommu_ops amd_iommu_ops;
static ATOMIC_NOTIFIER_HEAD(ppr_notifier);
int amd_iommu_max_glx_val = -1;
+u8 amd_iommu_ivrs_va_size;
/*
* general struct to manage commands send to an IOMMU
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 5b81fd16f5fa..3b07218e7000 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -41,6 +42,7 @@
* definitions for the ACPI scanning code
*/
#define IVRS_HEADER_LENGTH 48
+#define IVRS_HEADER_IVINFO_OFFSET 36
#define ACPI_IVHD_TYPE_MAX_SUPPORTED 0x40
#define ACPI_IVMD_TYPE_ALL 0x20
@@ -2492,6 +2494,27 @@ static void __init free_dma_resources(void)
free_unity_maps();
}
+static void __init get_ivrs_ivinfo(struct acpi_table_header *ivrs)
+{
+ u32 *ivinfo = (u32 *)((u8 *)ivrs + IVRS_HEADER_IVINFO_OFFSET);
+ u8 va_size = FIELD_GET(GENMASK(21, 15), *ivinfo);
+ u8 valid_va_sizes[] = {32, 40, 48, 64};
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(valid_va_sizes); i++) {
+ if (va_size == valid_va_sizes[i]) {
+ amd_iommu_ivrs_va_size = va_size;
+ break;
+ }
+ }
+
+ if (!amd_iommu_ivrs_va_size) {
+ pr_warn("Invalid virtual address size %u in IVRS header, use
most restrictive %u\n",
+ va_size, valid_va_sizes[0]);
+ amd_iommu_ivrs_va_size = valid_va_sizes[0];
+ }
+}
+
/*
* This is the hardware init function for AMD IOMMU in the system.
* This function is called either from amd_iommu_init or from the interrupt
@@ -2546,6 +2569,9 @@ static int __init early_amd_iommu_init(void)
if (ret)
goto out;
+ get_ivrs_ivinfo(ivrs_base);
+ DUMP_printk("IVRS vasize=%d\n", amd_iommu_ivrs_va_size);
+
amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base);
DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 7a8fdec138bd..8141b2874170 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -755,6 +755,9 @@ extern bool amd_iommu_force_isolation;
/* Max levels of glxval supported */
extern int amd_iommu_max_glx_val;
+/* Maximum virtual address size supported */
+extern u8 amd_iommu_ivrs_va_size;
+
/*
* This function flushes all internal caches of
* the IOMMU used by this driver.
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] iommu/amd: I/O VA address limits
The IVRS ACPI table specifies maximum address sizes for I/O virtual addresses that can be handled by the IOMMUs in the system. Parse that data from the IVRS header to provide aperture information for DMA mappings and users of the iommu API. Sebastian Ott (3): iommu/amd: Parse supported address sizes from IVRS iommu/amd: Restrict aperture for domains to conform with IVRS iommu/amd: Actually enforce geometry aperture drivers/iommu/amd_iommu.c | 16 +++- drivers/iommu/amd_iommu_init.c | 26 ++ drivers/iommu/amd_iommu_types.h | 3 +++ 3 files changed, 40 insertions(+), 5 deletions(-) -- 2.17.1 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] iommu/amd: Actually enforce geometry aperture
Add a check to enforce that I/O virtual addresses picked by iommu API users stay within the domains geometry aperture. Signed-off-by: Sebastian Ott --- drivers/iommu/amd_iommu.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index d2e79e27778e..6485e2081706 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -2618,6 +2618,11 @@ static int amd_iommu_map(struct iommu_domain *dom, unsigned long iova, if (pgtable.mode == PAGE_MODE_NONE) return -EINVAL; + if (dom->geometry.force_aperture && + (iova < dom->geometry.aperture_start || +iova + page_size - 1 > dom->geometry.aperture_end)) + return -EINVAL; + if (iommu_prot & IOMMU_READ) prot |= IOMMU_PROT_IR; if (iommu_prot & IOMMU_WRITE) -- 2.17.1 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/amd: Restrict aperture for domains to conform with IVRS
The IVRS ACPI table specifies maximum address sizes for I/O virtual
addresses. When allocating new protection domains that perform
translation, propagate these limits as the domain's geometry / aperture.
Based on prior work by Marius Hillenbrand.
Signed-off-by: Sebastian Ott
---
drivers/iommu/amd_iommu.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index fb4a44550c4a..d2e79e27778e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2460,6 +2460,7 @@ static struct protection_domain
*protection_domain_alloc(void)
static struct iommu_domain *amd_iommu_domain_alloc(unsigned type)
{
+ dma_addr_t max_va = DMA_BIT_MASK(amd_iommu_ivrs_va_size);
struct protection_domain *pdomain;
u64 *pt_root, root;
@@ -2477,11 +2478,6 @@ static struct iommu_domain
*amd_iommu_domain_alloc(unsigned type)
root = amd_iommu_domain_encode_pgtable(pt_root,
PAGE_MODE_3_LEVEL);
atomic64_set(&pdomain->pt_root, root);
-
- pdomain->domain.geometry.aperture_start = 0;
- pdomain->domain.geometry.aperture_end = ~0ULL;
- pdomain->domain.geometry.force_aperture = true;
-
break;
case IOMMU_DOMAIN_DMA:
pdomain = dma_ops_domain_alloc();
@@ -2501,6 +2497,10 @@ static struct iommu_domain
*amd_iommu_domain_alloc(unsigned type)
return NULL;
}
+ pdomain->domain.geometry.aperture_start = 0;
+ pdomain->domain.geometry.aperture_end = max_va;
+ pdomain->domain.geometry.force_aperture = true;
+
return &pdomain->domain;
}
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/13] device core: Introduce multiple dma pfn offsets
On Fri, Jun 5, 2020 at 1:27 PM Nicolas Saenz Julienne
wrote:
>
> Hi Christoph,
> a question arouse, is there a real value to dealing with PFNs (as opposed to
> real addresses) in the core DMA code/structures? I see that in some cases it
> eases interacting with mm, but the overwhelming usage of say,
> dev->dma_pfn_offset, involves shifting it.
>
> Hi Jim,
> On Thu, 2020-06-04 at 14:01 -0400, Jim Quinlan wrote:
> > Hi Nicolas,
>
> [...]
>
> > > I understand the need for dev to be around, devm_*() is key. But also it's
> > > important to keep the functions on purpose. And if of_dma_get_range()
> > > starts
> > > setting ranges it calls, for the very least, for a function rename.
> > > Although
> > > I'd rather split the parsing and setting of ranges as mentioned earlier.
> > > That
> > > said, I get that's a more drastic move.
> >
> > I agree with you. I could do this from device.c:
> >
> > of_dma_get_num_ranges(..., &num_ranges); /* new function */
> > r = devm_kcalloc(dev, num_ranges + 1, sizeof(*r), GFP_KERNEL);
> > of_dma_get_range(np, &dma_addr, &paddr, &size, r, num_ranges);
> >
> > The problem here is that there could be four ranges, all with
> > offset=0. My current code would optimize this case out but the above
> > would have us holding useless memory and looping through the four
> > ranges on every dma <=> phys conversion only to add 0.
>
> Point taken. Ultimately it's setting the device's dma ranges in
> of_dma_get_range() that was really bothering me, so if we have to pass the
> device pointer for allocations, be it.
>
> > > Talking about drastic moves. How about getting rid of the concept of
> > > dma_pfn_offset for drivers altogether. Let them provide
> > > dma_pfn_offset_regions
> > > (even when there is only one). I feel it's conceptually nicer, as you'd be
> > > dealing only in one currency, so to speak, and you'd centralize the bus
> > > DMA
> > > ranges setter function which is always easier to maintain.
> > Do you agree that we have to somehow hang this info on the struct
> > device structure? Because in the dma2phys() and phys2dma() all you
> > have is the dev parameter. I don't see how this can be done w/o
> > involving dev.
>
> Sorry I didn't make myself clear here. What bothers me is having two functions
> setting the same device parameter trough different means, I'd be happy to get
> rid of attach_uniform_dma_pfn_offset(), and always use the same function to
> set
> a device's bus dma regions. Something the likes of this comes to mind:
>
> dma_attach_pfn_offset_region(struct device *dev, struct
> dma_pfn_offset_regions *r)
>
> We could maybe use some helper macros for the linear case. But that's the gist
> of it.
>
> Also, it goes hand in hand with the comment below. Why having a special case
> for non sparse DMA offsets in struct dma_pfn_offset_regions? The way I see it,
> in this case, code simplicity is more interesting than a small optimization.
I've removed the special case and also need for 'dev' in
of_dma_get_range(). v4 is comming...
>
> > > I'd go as far as not creating a special case for uniform offsets. Let just
> > > set
> > > cpu_end and dma_end to -1 so we always get a match. It's slightly more
> > > compute
> > > heavy, but I don't think it's worth the optimization.
> > Well, there are two subcases here. One where we do know the bounds
> > and one where we do not. I suppose for the latter I could have the
> > drivers calling it with begin=0 and end=~(dma_addr_t)0. Let me give
> > this some thought...
> >
> > > Just my two cents :)
> >
> > Worth much more than $0.02 IMO :-)
>
> BTW, would you consider renaming the DMA offset struct to something simpler
> like, struct bus_dma_region? It complements 'dev->bus_dma_limit' better IMO.
Will do
Thanks,
Jim
>
> > BTW, I tried putting the "if (dev->dma_pfn_offset_map)" clause inside
> > the inline functions but the problem is that it slows the fastpath;
> > consider the following code from dma-direct.h
> >
> > if (dev->dma_pfn_offset_map) {
> > unsigned long dma_pfn_offset =
> dma_pfn_offset_from_phys_addr(dev, paddr);
> >
> > dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> > }
> > return dev_addr;
> >
> > becomes
> >
> > unsigned long dma_pfn_offset = dma_pfn_offset_from_phys_addr(dev,
> paddr);
> >
> > dev_addr -= ((dma_addr_t)dma_pfn_offset << PAGE_SHIFT);
> > return dev_addr;
> >
> > So those configurations that have no dma_pfn_offsets are doing an
> > unnecessary shift and add.
>
> Fair enough. Still not a huge difference, but I see the value being the most
> common case.
>
> Regards,
> Nicolas
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 08/12] device core: Introduce multiple dma pfn offsets
The new field in struct device 'dma_pfn_offset_map' is used to facilitate
the use of single or multiple pfn offsets between cpu addrs and dma addrs.
It subsumes the role of dev->dma_pfn_offset -- a uniform offset.
The function of_dma_get_range() has been modified to take two additional
arguments: the "map", which is an array that holds the information
regarding the pfn offset regions, and map_size, which is the size in bytes
of the map array.
of_dma_configure() is the typical manner to set pfn offsets but there are a
number of ad hoc assignments to dev->dma_pfn_offset in the kernel driver
code. These cases now invoke the function
dma_attach_uniform_pfn_offset(dev, pfn_offset).
Signed-off-by: Jim Quinlan
---
arch/arm/include/asm/dma-mapping.h| 9 +--
arch/arm/mach-keystone/keystone.c | 9 ++-
arch/sh/drivers/pci/pcie-sh7786.c | 3 +-
arch/sh/kernel/dma-coherent.c | 14 ++--
arch/x86/pci/sta2x11-fixup.c | 7 +-
drivers/acpi/arm64/iort.c | 5 +-
drivers/gpu/drm/sun4i/sun4i_backend.c | 5 +-
drivers/iommu/io-pgtable-arm.c| 2 +-
.../platform/sunxi/sun4i-csi/sun4i_csi.c | 5 +-
.../platform/sunxi/sun6i-csi/sun6i_csi.c | 4 +-
drivers/of/address.c | 72 ---
drivers/of/device.c | 19 +++--
drivers/of/of_private.h | 11 +--
drivers/of/unittest.c | 8 ++-
drivers/remoteproc/remoteproc_core.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_hw.c| 7 +-
drivers/usb/core/message.c| 4 +-
drivers/usb/core/usb.c| 2 +-
include/linux/device.h| 4 +-
include/linux/dma-direct.h| 16 -
include/linux/dma-mapping.h | 38 ++
kernel/dma/coherent.c | 11 +--
kernel/dma/mapping.c | 38 ++
23 files changed, 235 insertions(+), 60 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..f1e72f99468b 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,9 @@ static inline const struct dma_map_ops
*get_arch_dma_ops(struct bus_type *bus)
#ifndef __arch_pfn_to_dma
static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
{
- if (dev)
- pfn -= dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn -= dma_pfn_offset_from_phys_addr(dev, PFN_PHYS(pfn));
+
return (dma_addr_t)__pfn_to_bus(pfn);
}
@@ -44,8 +45,8 @@ static inline unsigned long dma_to_pfn(struct device *dev,
dma_addr_t addr)
{
unsigned long pfn = __bus_to_pfn(addr);
- if (dev)
- pfn += dev->dma_pfn_offset;
+ if (dev && dev->dma_pfn_offset_map)
+ pfn += dma_pfn_offset_from_dma_addr(dev, addr);
return pfn;
}
diff --git a/arch/arm/mach-keystone/keystone.c
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..dfb095b31534 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -8,6 +8,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -38,9 +39,11 @@ static int keystone_platform_notifier(struct notifier_block
*nb,
return NOTIFY_BAD;
if (!dev->of_node) {
- dev->dma_pfn_offset = keystone_dma_pfn_offset;
- dev_err(dev, "set dma_pfn_offset%08lx\n",
- dev->dma_pfn_offset);
+ int ret = dma_attach_uniform_pfn_offset
+ (dev, keystone_dma_pfn_offset);
+
+ dev_err(dev, "set dma_pfn_offset%08lx%s\n",
+ dev->dma_pfn_offset, ret ? " failed" : "");
}
return NOTIFY_OK;
}
diff --git a/arch/sh/drivers/pci/pcie-sh7786.c
b/arch/sh/drivers/pci/pcie-sh7786.c
index e0b568aaa701..3e63c6b6e070 100644
--- a/arch/sh/drivers/pci/pcie-sh7786.c
+++ b/arch/sh/drivers/pci/pcie-sh7786.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -487,7 +488,7 @@ int pcibios_map_platform_irq(const struct pci_dev *pdev, u8
slot, u8 pin)
void pcibios_bus_add_device(struct pci_dev *pdev)
{
- pdev->dev.dma_pfn_offset = dma_pfn_offset;
+ dma_attach_uniform_pfn_offset(&pdev->dev, dma_pfn_offset);
}
static int __init sh7786_pcie_core_init(void)
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
index d4811691b93c..f4a092e74910 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -14,6 +14,7 @@ void *arch_dma_alloc(struct device *dev, size_t size,
dma_addr_t *dma_handle,
{
void *ret, *ret_nocache;
int order = get_order(size);
+ phys_addr_t phys;
gfp |= __GFP_Z
[PATCH v4 00/12] PCI: brcmstb: enable PCIe for STB chips
v4:
Commit "device core: Introduce multiple dma pfn offsets"
-- of_dma_get_range() does not take a dev param but instead
takes two "out" params: map and map_size. We do this so
that the code that parses dma-ranges is separate from
the code that modifies 'dev'. (Nicolas)
-- the separate case of having a single pfn offset has
been removed and is now processed by going through the
map array. (Nicolas)
-- move attach_uniform_dma_pfn_offset() from of/address.c to
dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
-- devm_kcalloc => devm_kzalloc (DanC)
-- add/fix assignment to dev->dma_pfn_offset_map for func
attach_uniform_dma_pfn_offset() (DanC, Nicolas)
-- s/struct dma_pfn_offset_region/struct bus_dma_region/ (Nicolas)
-- s/attach_uniform_dma_pfn_offset/dma_attach_uniform_pfn_offset/
-- s/attach_dma_pfn_offset_map/dma_attach_pfn_offset_map/
-- More use of PFN_{PHYS,DOWN,UP}. (AndyS)
Commit "of: Include a dev param in of_dma_get_range()"
-- this commit was sqaushed with "device core: Introduce ..."
v3:
Commit "device core: Introduce multiple dma pfn offsets"
Commit "arm: dma-mapping: Invoke dma offset func if needed"
-- The above two commits have been squashed. More importantly,
the code has been modified so that the functionality for
multiple pfn offsets subsumes the use of dev->dma_pfn_offset.
In fact, dma_pfn_offset is removed and supplanted by
dma_pfn_offset_map, which is a pointer to an array. The
more common case of a uniform offset is now handled as
a map with a single entry, while cases requiring multiple
pfn offsets use a map with multiple entries. Code paths
that used to do this:
dev->dma_pfn_offset = mydrivers_pfn_offset;
have been changed to do this:
attach_uniform_dma_pfn_offset(dev, pfn_offset);
Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
-- Add if/then clause for required props: resets, reset-names (RobH)
-- Change compatible list from const to enum (RobH)
-- Change list of u32-tuples to u64 (RobH)
Commit "of: Include a dev param in of_dma_get_range()"
-- modify of/unittests.c to add NULL param in of_dma_get_range() call.
Commit "device core: Add ability to handle multiple dma offsets"
-- align comment in device.h (AndyS).
-- s/cpu_beg/cpu_start/ and s/dma_beg/dma_start/ in struct
dma_pfn_offset_region (AndyS).
v2:
Commit: "device core: Add ability to handle multiple dma offsets"
o Added helper func attach_dma_pfn_offset_map() in address.c (Chistoph)
o Helpers funcs added to __phys_to_dma() & __dma_to_phys() (Christoph)
o Added warning when multiple offsets are needed and !DMA_PFN_OFFSET_MAP
o dev->dma_pfn_map => dev->dma_pfn_offset_map
o s/frm/from/ for dma_pfn_offset_frm_{phys,dma}_addr() (Christoph)
o In device.h: s/const void */const struct dma_pfn_offset_region */
o removed 'unlikely' from unlikely(dev->dma_pfn_offset_map) since
guarded by CONFIG_DMA_PFN_OFFSET_MAP (Christoph)
o Since dev->dma_pfn_offset is copied in usb/core/{usb,message}.c, now
dev->dma_pfn_offset_map is copied as well.
o Merged two of the DMA commits into one (Christoph).
Commit "arm: dma-mapping: Invoke dma offset func if needed":
o Use helper functions instead of #if CONFIG_DMA_PFN_OFFSET
Other commits' changes:
o Removed need for carrying of_id var in priv (Nicolas)
o Commit message rewordings (Bjorn)
o Commit log messages filled to 75 chars (Bjorn)
o devm_reset_control_get_shared())
=> devm_reset_control_get_optional_shared (Philipp)
o Add call to reset_control_assert() in PCIe remove routines (Philipp)
v1:
This patchset expands the usefulness of the Broadcom Settop Box PCIe
controller by building upon the PCIe driver used currently by the
Raspbery Pi. Other forms of this patchset were submitted by me years
ago and not accepted; the major sticking point was the code required
for the DMA remapping needed for the PCIe driver to work [1].
There have been many changes to the DMA and OF subsystems since that
time, making a cleaner and less intrusive patchset possible. This
patchset implements a generalization of "dev->dma_pfn_offset", except
that instead of a single scalar offset it provides for multiple
offsets via a function which depends upon the "dma-ranges" property of
the PCIe host controller. This is required for proper functionality
of the BrcmSTB PCIe controller and possibly some other devices.
[1]
https://lore.kernel.org/linux-arm-kernel/1516058925-46522-5-git-send-email-jim2101...@gmail.com/
Jim Quinlan (12):
PCI: brcmstb: PCIE_BRCMSTB depends on ARCH_BRCMSTB
ata: ahci_brcm: Fix use of BCM7216 reset controller
dt-bindings: PCI: Add bindings for more Brcmstb chips
PCI: brcmstb: Add bcm7278 register info
PCI: brcmstb: Add suspend and resume pm_ops
PCI: brcmstb: Add bcm7278 PERST support
PCI: brcmstb: Add control of rescal reset
device core: Introduce
Re: [PATCH 0/2] Introduce PCI_FIXUP_IOMMU
On Thu, Jun 04, 2020 at 09:33:07PM +0800, Zhangfei Gao wrote: > On 2020/6/2 上午1:41, Bjorn Helgaas wrote: > > On Thu, May 28, 2020 at 09:33:44AM +0200, Joerg Roedel wrote: > > > On Wed, May 27, 2020 at 01:18:42PM -0500, Bjorn Helgaas wrote: > > > > Is this slowdown significant? We already iterate over every device > > > > when applying PCI_FIXUP_FINAL quirks, so if we used the existing > > > > PCI_FIXUP_FINAL, we wouldn't be adding a new loop. We would only be > > > > adding two more iterations to the loop in pci_do_fixups() that tries > > > > to match quirks against the current device. I doubt that would be a > > > > measurable slowdown. > > > I don't know how significant it is, but I remember people complaining > > > about adding new PCI quirks because it takes too long for them to run > > > them all. That was in the discussion about the quirk disabling ATS on > > > AMD Stoney systems. > > > > > > So it probably depends on how many PCI devices are in the system whether > > > it causes any measureable slowdown. > > I found this [1] from Paul Menzel, which was a slowdown caused by > > quirk_usb_early_handoff(). I think the real problem is individual > > quirks that take a long time. > > > > The PCI_FIXUP_IOMMU things we're talking about should be fast, and of > > course, they're only run for matching devices anyway. So I'd rather > > keep them as PCI_FIXUP_FINAL than add a whole new phase. > > > Thanks Bjorn for taking time for this. > If so, it would be much simpler. > > +++ b/drivers/iommu/iommu.c > @@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct > fwnode_handle *iommu_fwnode, > fwspec->iommu_fwnode = iommu_fwnode; > fwspec->ops = ops; > dev_iommu_fwspec_set(dev, fwspec); > + > + if (dev_is_pci(dev)) > + pci_fixup_device(pci_fixup_final, to_pci_dev(dev)); > + > > Then pci_fixup_final will be called twice, the first in pci_bus_add_device. > Here in iommu_fwspec_init is the second time, specifically for iommu_fwspec. > Will send this when 5.8-rc1 is open. Wait, this whole fixup approach seems wrong to me. No matter how you do the fixup, it's still a fixup, which means it requires ongoing maintenance. Surely we don't want to have to add the Vendor/Device ID for every new AMBA device that comes along, do we? Bjorn ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [kbuild-all] Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa CMA
On Fri, Jun 05, 2020 at 11:57:51AM +0300, Dan Carpenter wrote: > On Fri, Jun 05, 2020 at 06:04:31AM +, Song Bao Hua (Barry Song) wrote: > > > > > > > -Original Message- > > > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > > > Sent: Thursday, June 4, 2020 11:37 PM > > > To: kbu...@lists.01.org; Song Bao Hua (Barry Song) > > > ; h...@lst.de; m.szyprow...@samsung.com; > > > robin.mur...@arm.com; catalin.mari...@arm.com > > > Cc: l...@intel.com; Dan Carpenter ; > > > kbuild-...@lists.01.org; iommu@lists.linux-foundation.org; > > > linux-arm-ker...@lists.infradead.org; linux-ker...@vger.kernel.org; > > > Linuxarm > > > ; Jonathan Cameron > > > ; John Garry > > > Subject: Re: [PATCH 1/3] dma-direct: provide the ability to reserve > > > per-numa > > > CMA > > > > > > Hi Barry, > > > > > > url: > > > https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CM > > > A-for-ARM-server/20200603-104821 > > > base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git > > > for-next/core > > > config: x86_64-randconfig-m001-20200603 (attached as .config) > > > compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot > > > Reported-by: Dan Carpenter > > > > Dan, thanks! Good catch! > > as this patch has not been in mainline yet, is it correct to add these > > "reported-by" in patch v2? Hi Barry, we provides the suggestion here, but you may decide to add or not as appropriate for your situation. For the patch still under development, it is not that necessary to add i think. > > These are just an automated email from the zero day bot. I look over > the Smatch warnings and then forward them on. > > regards, > dan carpenter > ___ > kbuild-all mailing list -- kbuild-...@lists.01.org > To unsubscribe send an email to kbuild-all-le...@lists.01.org ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
