Re: A problem of Intel IOMMU hardware ？
> On Mar 16, 2021, at 8:16 PM, Longpeng (Mike, Cloud Infrastructure Service > Product Dept.) wrote: > > Hi guys, > > We find the Intel iommu cache (i.e. iotlb) maybe works wrong in a special > situation, it would cause DMA fails or get wrong data. > > The reproducer (based on Alex's vfio testsuite[1]) is in attachment, it can > reproduce the problem with high probability (~50%). I saw Lu replied, and he is much more knowledgable than I am (I was just intrigued by your email). However, if I were you I would try also to remove some “optimizations” to look for the root-cause (e.g., use domain specific invalidations instead of page-specific). The first thing that comes to my mind is the invalidation hint (ih) in iommu_flush_iotlb_psi(). I would remove it to see whether you get the failure without it. signature.asc Description: Message signed with OpenPGP ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: A problem of Intel IOMMU hardware ？
Hi Longpeng,
On 3/17/21 11:16 AM, Longpeng (Mike, Cloud Infrastructure Service
Product Dept.) wrote:
Hi guys,
We find the Intel iommu cache (i.e. iotlb) maybe works wrong in a special
situation, it would cause DMA fails or get wrong data.
The reproducer (based on Alex's vfio testsuite[1]) is in attachment, it can
reproduce the problem with high probability (~50%).
The machine we used is:
processor : 47
vendor_id : GenuineIntel
cpu family : 6
model : 85
model name : Intel(R) Xeon(R) Gold 6146 CPU @ 3.20GHz
stepping: 4
microcode : 0x269
And the iommu capability reported is:
ver 1:0 cap 8d2078c106f0466 ecap f020df
(caching mode = 0 , page-selective invalidation = 1)
(The problem is also on 'Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz' and
'Intel(R) Xeon(R) Platinum 8378A CPU @ 3.00GHz')
We run the reproducer on Linux 4.18 and it works as follow:
Step 1. alloc 4G *2M-hugetlb* memory (N.B. no problem with 4K-page mapping)
I don't understand 2M-hugetlb here means exactly. The IOMMU hardware
supports both 2M and 1G super page. The mapping physical memory is 4G.
Why couldn't it use 1G super page?
Step 2. DMA Map 4G memory
Step 3.
while (1) {
{UNMAP, 0x0, 0xa}, (a)
{UNMAP, 0xc, 0xbff4},
Have these two ranges been mapped before? Does the IOMMU driver
complains when you trying to unmap a range which has never been
mapped? The IOMMU driver implicitly assumes that mapping and
unmapping are paired.
{MAP, 0x0, 0xc000}, - (b)
use GDB to pause at here, and then DMA read IOVA=0,
IOVA 0 seems to be a special one. Have you verified with other addresses
than IOVA 0?
sometimes DMA success (as expected),
but sometimes DMA error (report not-present).
{UNMAP, 0x0, 0xc000}, - (c)
{MAP, 0x0, 0xa},
{MAP, 0xc, 0xbff4},
}
The DMA read operations sholud success between (b) and (c), it should NOT report
not-present at least!
After analysis the problem, we think maybe it's caused by the Intel iommu iotlb.
It seems the DMA Remapping hardware still uses the IOTLB or other caches of (a).
When do DMA unmap at (a), the iotlb will be flush:
intel_iommu_unmap
domain_unmap
iommu_flush_iotlb_psi
When do DMA map at (b), no need to flush the iotlb according to the capability
of this iommu:
intel_iommu_map
domain_pfn_mapping
domain_mapping
__mapping_notify_one
if (cap_caching_mode(iommu->cap)) // FALSE
iommu_flush_iotlb_psi
That's true. The iotlb flushing is not needed in case of PTE been
changed from non-present to present unless caching mode.
But the problem will disappear if we FORCE flush here. So we suspect the iommu
hardware.
Do you have any suggestion ?
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
A problem of Intel IOMMU hardware ？
Hi guys,
We find the Intel iommu cache (i.e. iotlb) maybe works wrong in a special
situation, it would cause DMA fails or get wrong data.
The reproducer (based on Alex's vfio testsuite[1]) is in attachment, it can
reproduce the problem with high probability (~50%).
The machine we used is:
processor : 47
vendor_id : GenuineIntel
cpu family : 6
model : 85
model name : Intel(R) Xeon(R) Gold 6146 CPU @ 3.20GHz
stepping: 4
microcode : 0x269
And the iommu capability reported is:
ver 1:0 cap 8d2078c106f0466 ecap f020df
(caching mode = 0 , page-selective invalidation = 1)
(The problem is also on 'Intel(R) Xeon(R) Silver 4114 CPU @ 2.20GHz' and
'Intel(R) Xeon(R) Platinum 8378A CPU @ 3.00GHz')
We run the reproducer on Linux 4.18 and it works as follow:
Step 1. alloc 4G *2M-hugetlb* memory (N.B. no problem with 4K-page mapping)
Step 2. DMA Map 4G memory
Step 3.
while (1) {
{UNMAP, 0x0, 0xa}, (a)
{UNMAP, 0xc, 0xbff4},
{MAP, 0x0, 0xc000}, - (b)
use GDB to pause at here, and then DMA read IOVA=0,
sometimes DMA success (as expected),
but sometimes DMA error (report not-present).
{UNMAP, 0x0, 0xc000}, - (c)
{MAP, 0x0, 0xa},
{MAP, 0xc, 0xbff4},
}
The DMA read operations sholud success between (b) and (c), it should NOT report
not-present at least!
After analysis the problem, we think maybe it's caused by the Intel iommu iotlb.
It seems the DMA Remapping hardware still uses the IOTLB or other caches of (a).
When do DMA unmap at (a), the iotlb will be flush:
intel_iommu_unmap
domain_unmap
iommu_flush_iotlb_psi
When do DMA map at (b), no need to flush the iotlb according to the capability
of this iommu:
intel_iommu_map
domain_pfn_mapping
domain_mapping
__mapping_notify_one
if (cap_caching_mode(iommu->cap)) // FALSE
iommu_flush_iotlb_psi
But the problem will disappear if we FORCE flush here. So we suspect the iommu
hardware.
Do you have any suggestion ?
/*
* VFIO API definition
*
* Copyright (C) 2012 Red Hat, Inc. All rights reserved.
* Author: Alex Williamson
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
#ifndef _UAPIVFIO_H
#define _UAPIVFIO_H
#include
#include
#define VFIO_API_VERSION0
/* Kernel & User level defines for VFIO IOCTLs. */
/* Extensions */
#define VFIO_TYPE1_IOMMU1
/*
* The IOCTL interface is designed for extensibility by embedding the
* structure length (argsz) and flags into structures passed between
* kernel and userspace. We therefore use the _IO() macro for these
* defines to avoid implicitly embedding a size into the ioctl request.
* As structure fields are added, argsz will increase to match and flag
* bits will be defined to indicate additional fields with valid data.
* It's *always* the caller's responsibility to indicate the size of
* the structure passed by setting argsz appropriately.
*/
#define VFIO_TYPE (';')
#define VFIO_BASE 100
/* IOCTLs for VFIO file descriptor (/dev/vfio/vfio) */
/**
* VFIO_GET_API_VERSION - _IO(VFIO_TYPE, VFIO_BASE + 0)
*
* Report the version of the VFIO API. This allows us to bump the entire
* API version should we later need to add or change features in incompatible
* ways.
* Return: VFIO_API_VERSION
* Availability: Always
*/
#define VFIO_GET_API_VERSION_IO(VFIO_TYPE, VFIO_BASE + 0)
/**
* VFIO_CHECK_EXTENSION - _IOW(VFIO_TYPE, VFIO_BASE + 1, __u32)
*
* Check whether an extension is supported.
* Return: 0 if not supported, 1 (or some other positive integer) if supported.
* Availability: Always
*/
#define VFIO_CHECK_EXTENSION_IO(VFIO_TYPE, VFIO_BASE + 1)
/**
* VFIO_SET_IOMMU - _IOW(VFIO_TYPE, VFIO_BASE + 2, __s32)
*
* Set the iommu to the given type. The type must be supported by an
* iommu driver as verified by calling CHECK_EXTENSION using the same
* type. A group must be set to this file descriptor before this
* ioctl is available. The IOMMU interfaces enabled by this call are
* specific to the value set.
* Return: 0 on success, -errno on failure
* Availability: When VFIO group attached
*/
#define VFIO_SET_IOMMU _IO(VFIO_TYPE, VFIO_BASE + 2)
/* IOCTLs for GROUP file descriptors (/dev/vfio/$GROUP) */
/**
* VFIO_GROUP_GET_STATUS - _IOR(VFIO_TYPE, VFIO_BASE + 3,
* struct vfio_group_status)
*
* Retrieve information about the group. Fills in provided
* struct vfio_group_info. Caller sets argsz.
Re: [PATCH v2 04/11] iommu/arm-smmu-v3: Split block descriptor when start dirty log
On 21-03-16 19:39:47, Keqian Zhu wrote: > Hi Yi, > > On 2021/3/16 17:17, Yi Sun wrote: > > On 21-03-10 17:06:07, Keqian Zhu wrote: > >> From: jiangkunkun > >> > >> Block descriptor is not a proper granule for dirty log tracking. > >> Take an extreme example, if DMA writes one byte, under 1G mapping, > >> the dirty amount reported to userspace is 1G, but under 4K mapping, > >> the dirty amount is just 4K. > >> > >> This adds a new interface named start_dirty_log in iommu layer and > >> arm smmuv3 implements it, which splits block descriptor to an span > >> of page descriptors. Other types of IOMMU will perform architecture > >> specific actions to start dirty log. > >> > >> To allow code reuse, the split_block operation is realized as an > >> iommu_ops too. We flush all iotlbs after the whole procedure is > >> completed to ease the pressure of iommu, as we will hanle a huge > >> range of mapping in general. > >> > >> Spliting block does not simultaneously work with other pgtable ops, > >> as the only designed user is vfio, which always hold a lock, so race > >> condition is not considered in the pgtable ops. > >> > >> Co-developed-by: Keqian Zhu > >> Signed-off-by: Kunkun Jiang > >> --- > >> > >> changelog: > >> > >> v2: > >> - Change the return type of split_block(). size_t -> int. > >> - Change commit message to properly describe race condition. (Robin) > >> - Change commit message to properly describe the need of split block. > >> - Add a new interface named start_dirty_log(). (Sun Yi) > >> - Change commit message to explain the realtionship of split_block() and > >> start_dirty_log(). > >> > >> --- > >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 52 + > >> drivers/iommu/io-pgtable-arm.c | 122 > >> drivers/iommu/iommu.c | 48 > >> include/linux/io-pgtable.h | 2 + > >> include/linux/iommu.h | 24 > >> 5 files changed, 248 insertions(+) > >> > > Could you please split iommu common interface to a separate patch? > > This may make review and comments easier. > Yup, good suggestion. > > > > > IMHO, I think the start/stop interfaces could be merged into one, e.g: > > int iommu_domain_set_hwdbm(struct iommu_domain *domain, bool enable, > >unsigned long iova, size_t size, > >int prot); > Looks good, this reduces some code. but I have a concern that this causes > loss of flexibility, > as we must pass same arguments when start|stop dirty log. What's your opinion > about this? > Per current design, start/stop interfaces have similar arguments. So I think it is ok for now. For future extension, we may think to define a structure to pass these arguments. > > > > Same comments to patch 5. > OK. Thanks. > > > > > BRs, > > Yi Sun > > > >> -- > >> 2.19.1 > > . > Thanks, > Keqian BRs, Yi Sun ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] iommu/vt-d: Fix lockdep splat in intel_pasid_get_entry()
The pasid_lock is used to synchronize different threads from modifying a
same pasid directory entry at the same time. It causes below lockdep splat.
[ 83.296538]
[ 83.296538] WARNING: possible irq lock inversion dependency detected
[ 83.296539] 5.12.0-rc3+ #25 Tainted: GW
[ 83.296539]
[ 83.296540] bash/780 just changed the state of lock:
[ 83.296540] 82b29c98 (device_domain_lock){..-.}-{2:2}, at:
iommu_flush_dev_iotlb.part.0+0x32/0x110
[ 83.296547] but this lock took another, SOFTIRQ-unsafe lock in the past:
[ 83.296547] (pasid_lock){+.+.}-{2:2}
[ 83.296548]
and interrupts could create inverse lock ordering between them.
[ 83.296549] other info that might help us debug this:
[ 83.296549] Chain exists of:
device_domain_lock --> &iommu->lock --> pasid_lock
[ 83.296551] Possible interrupt unsafe locking scenario:
[ 83.296551]CPU0CPU1
[ 83.296552]
[ 83.296552] lock(pasid_lock);
[ 83.296553]local_irq_disable();
[ 83.296553]lock(device_domain_lock);
[ 83.296554]lock(&iommu->lock);
[ 83.296554]
[ 83.296554] lock(device_domain_lock);
[ 83.296555]
*** DEADLOCK ***
Fix it by replacing the pasid_lock with an atomic exchange operation.
Reported-and-tested-by: Dave Jiang
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/pasid.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 9fb3d3e80408..1ddcb8295f72 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -24,7 +24,6 @@
/*
* Intel IOMMU system wide PASID name space:
*/
-static DEFINE_SPINLOCK(pasid_lock);
u32 intel_pasid_max_id = PASID_MAX;
int vcmd_alloc_pasid(struct intel_iommu *iommu, u32 *pasid)
@@ -259,19 +258,18 @@ struct pasid_entry *intel_pasid_get_entry(struct device
*dev, u32 pasid)
dir_index = pasid >> PASID_PDE_SHIFT;
index = pasid & PASID_PTE_MASK;
- spin_lock(&pasid_lock);
entries = get_pasid_table_from_pde(&dir[dir_index]);
if (!entries) {
entries = alloc_pgtable_page(info->iommu->node);
- if (!entries) {
- spin_unlock(&pasid_lock);
+ if (!entries)
return NULL;
- }
- WRITE_ONCE(dir[dir_index].val,
- (u64)virt_to_phys(entries) | PASID_PTE_PRESENT);
+ if (cmpxchg64(&dir[dir_index].val, 0ULL,
+ (u64)virt_to_phys(entries) | PASID_PTE_PRESENT)) {
+ free_pgtable_page(entries);
+ entries = get_pasid_table_from_pde(&dir[dir_index]);
+ }
}
- spin_unlock(&pasid_lock);
return &entries[index];
}
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/14] swiotlb: move orig addr and size validation into swiotlb_bounce
On Mon, Mar 01, 2021 at 08:44:25AM +0100, Christoph Hellwig wrote: > Move the code to find and validate the original buffer address and size > from the callers into swiotlb_bounce. This means a tiny bit of extra > work in the swiotlb_map path, but avoids code duplication and a leads to > a better code structure. Reviewed-by: Konrad Rzeszutek Wilk ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/14] swiotlb: remove the alloc_size parameter to swiotlb_tbl_unmap_single
On Mon, Mar 01, 2021 at 08:44:24AM +0100, Christoph Hellwig wrote: > Now that swiotlb remembers the allocation size there is no need to pass > it back to swiotlb_tbl_unmap_single. Reviewed-by: Konrad Rzeszutek Wilk ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5] iommu/tegra-smmu: Add pagetable mappings to debugfs
16.03.2021 14:16, Thierry Reding пишет: >> +seq_puts(s, "}\n"); >> +seq_printf(s, "Total PDE count: %u\n", pde_count); >> +seq_printf(s, "Total PTE count: %llu\n", pte_count); > Some of the above looks like it wouldn't be very easily consumed by > scripts. Is that something we want to do? Or is this targetted primarily > at human consumption? Output should be parsable using a simple regex. Could you please clarify what exactly isn't easy? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5] iommu/tegra-smmu: Add pagetable mappings to debugfs
15.03.2021 23:36, Nicolin Chen пишет:
> +static int tegra_smmu_mappings_show(struct seq_file *s, void *data)
> +{
> + struct tegra_smmu_group_debug *group_debug = s->private;
> + const struct tegra_smmu_swgroup *group;
> + struct tegra_smmu_as *as;
> + struct tegra_smmu *smmu;
> + unsigned int pd_index;
> + unsigned int pt_index;
> + unsigned long flags;
> + u64 pte_count = 0;
> + u32 pde_count = 0;
> + u32 val, ptb_reg;
> + u32 *pd;
> +
> + if (!group_debug || !group_debug->priv || !group_debug->group)
> + return 0;
I'm also now curious how difficult would be to read out the actual h/w
state, i.e. check whether ASID is enabled and then dynamically map the
pointed pages instead of using pages allocated by driver. This will show
us the real h/w state. For example this may show mappings left after
bootloader or after reboot/kexec, which could be handy to see.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5] iommu/tegra-smmu: Add pagetable mappings to debugfs
15.03.2021 23:36, Nicolin Chen пишет:
> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int
> pt_index)
> +{
> + return ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT |
> +((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT;
> +}
Looking at this again, I'm now wondering whether will be better to
replace dma_addr_t with u32 everywhere since SMMU only supports 32bits
for IOVA.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] ACPI: Add driver for the VIOT table
The ACPI Virtual I/O Translation Table describes topology of
para-virtual platforms. For now it describes the relation between
virtio-iommu and the endpoints it manages. Supporting that requires
three steps:
(1) acpi_viot_init(): parse the VIOT table, build a list of endpoints
and vIOMMUs.
(2) acpi_viot_set_iommu_ops(): when the vIOMMU driver is loaded and the
device probed, register it to the VIOT driver. This step is required
because unlike similar drivers, VIOT doesn't create the vIOMMU
device.
(3) acpi_viot_dma_setup(): when the endpoint is initialized, find the
vIOMMU and register the endpoint's DMA ops.
If step (3) happens before step (2), it is deferred until the IOMMU is
initialized, then retried.
Signed-off-by: Jean-Philippe Brucker
---
drivers/acpi/Kconfig | 3 +
drivers/iommu/Kconfig| 1 +
drivers/acpi/Makefile| 2 +
include/linux/acpi_viot.h| 26 +++
drivers/acpi/bus.c | 2 +
drivers/acpi/scan.c | 6 +
drivers/acpi/viot.c | 406 +++
drivers/iommu/virtio-iommu.c | 3 +
MAINTAINERS | 8 +
9 files changed, 457 insertions(+)
create mode 100644 include/linux/acpi_viot.h
create mode 100644 drivers/acpi/viot.c
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index eedec61e3476..3758c6940ed7 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -526,6 +526,9 @@ endif
source "drivers/acpi/pmic/Kconfig"
+config ACPI_VIOT
+ bool
+
endif # ACPI
config X86_PM_TIMER
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 192ef8f61310..2819b5c8ec30 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -403,6 +403,7 @@ config VIRTIO_IOMMU
depends on ARM64
select IOMMU_API
select INTERVAL_TREE
+ select ACPI_VIOT if ACPI
help
Para-virtualised IOMMU driver with virtio.
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 700b41adf2db..a6e644c48987 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -118,3 +118,5 @@ video-objs += acpi_video.o video_detect.o
obj-y += dptf/
obj-$(CONFIG_ARM64)+= arm64/
+
+obj-$(CONFIG_ACPI_VIOT)+= viot.o
diff --git a/include/linux/acpi_viot.h b/include/linux/acpi_viot.h
new file mode 100644
index ..1f5837595488
--- /dev/null
+++ b/include/linux/acpi_viot.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ACPI_VIOT_H__
+#define __ACPI_VIOT_H__
+
+#include
+
+#ifdef CONFIG_ACPI_VIOT
+void __init acpi_viot_init(void);
+int acpi_viot_dma_setup(struct device *dev, enum dev_dma_attr attr);
+int acpi_viot_set_iommu_ops(struct device *dev, struct iommu_ops *ops);
+#else
+static inline void acpi_viot_init(void) {}
+static inline int acpi_viot_dma_setup(struct device *dev,
+ enum dev_dma_attr attr)
+{
+ return 0;
+}
+static inline int acpi_viot_set_iommu_ops(struct device *dev,
+ struct iommu_ops *ops)
+{
+ return -ENODEV;
+}
+#endif
+
+#endif /* __ACPI_VIOT_H__ */
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index be7da23fad76..f9a965c6617e 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -27,6 +27,7 @@
#include
#endif
#include
+#include
#include
#include
#include
@@ -1338,6 +1339,7 @@ static int __init acpi_init(void)
pci_mmcfg_late_init();
acpi_iort_init();
+ acpi_viot_init();
acpi_scan_init();
acpi_ec_init();
acpi_debugfs_init();
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a184529d8fa4..4af01fddd94c 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1506,12 +1507,17 @@ int acpi_dma_configure_id(struct device *dev, enum
dev_dma_attr attr,
{
const struct iommu_ops *iommu;
u64 dma_addr = 0, size = 0;
+ int ret;
if (attr == DEV_DMA_NOT_SUPPORTED) {
set_dma_ops(dev, &dma_dummy_ops);
return 0;
}
+ ret = acpi_viot_dma_setup(dev, attr);
+ if (ret)
+ return ret > 0 ? 0 : ret;
+
iort_dma_setup(dev, &dma_addr, &size);
iommu = iort_iommu_configure_id(dev, input_id);
diff --git a/drivers/acpi/viot.c b/drivers/acpi/viot.c
new file mode 100644
index ..57a092e8551b
--- /dev/null
+++ b/drivers/acpi/viot.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtual I/O topology
+ */
+#define pr_fmt(fmt) "ACPI: VIOT: " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct viot_dev_id {
+ unsigned inttype;
+#define VIOT_DEV_TYPE_PCI 1
+#define VIOT_DEV_TYPE_MMIO 2
+ union {
+ /*
[PATCH 3/3] iommu/virtio: Enable x86 support
With the VIOT support in place, x86 platforms can now use the virtio-iommu. The arm64 Kconfig selects IOMMU_DMA, while x86 IOMMU drivers select it themselves. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 2819b5c8ec30..ccca83ef2f06 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -400,8 +400,9 @@ config HYPERV_IOMMU config VIRTIO_IOMMU tristate "Virtio IOMMU driver" depends on VIRTIO - depends on ARM64 + depends on (ARM64 || X86) select IOMMU_API + select IOMMU_DMA if X86 select INTERVAL_TREE select ACPI_VIOT if ACPI help -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] Add support for ACPI VIOT
Add a driver for the ACPI VIOT table, which enables virtio-iommu on non-devicetree platforms, including x86. This series depends on the ACPICA changes of patch 1, which will be included in next release [1] and pulled into Linux. The Virtual I/O Translation table (VIOT) describes the topology of para-virtual I/O translation devices and the endpoints they manage. It was recently approved for inclusion into the ACPI standard [2]. A provisional version of the specification can be found at [3]. After discussing non-devicetree support for virtio-iommu at length [4][5][6] we concluded that it should use this new ACPI table. And for platforms that don't implement either devicetree or ACPI, a structure that uses roughly the same format [6] can be built into the device. [1] https://github.com/acpica/acpica/pull/666 [2] https://lore.kernel.org/linux-iommu/20210218233943.gh702...@redhat.com/ [3] https://jpbrucker.net/virtio-iommu/viot/viot-v9.pdf [4] https://lore.kernel.org/linux-iommu/20191122105000.800410-1-jean-phili...@linaro.org/ [5] https://lore.kernel.org/linux-iommu/20200228172537.377327-1-jean-phili...@linaro.org/ [6] https://lore.kernel.org/linux-iommu/20200821131540.2801801-1-jean-phili...@linaro.org/ Jean-Philippe Brucker (3): ACPICA: iASL: Add definitions for the VIOT table ACPI: Add driver for the VIOT table iommu/virtio: Enable x86 support drivers/acpi/Kconfig | 3 + drivers/iommu/Kconfig| 4 +- drivers/acpi/Makefile| 2 + include/acpi/actbl3.h| 67 ++ include/linux/acpi_viot.h| 26 +++ drivers/acpi/bus.c | 2 + drivers/acpi/scan.c | 6 + drivers/acpi/viot.c | 406 +++ drivers/iommu/virtio-iommu.c | 3 + MAINTAINERS | 8 + 10 files changed, 526 insertions(+), 1 deletion(-) create mode 100644 include/linux/acpi_viot.h create mode 100644 drivers/acpi/viot.c -- 2.30.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] ACPICA: iASL: Add definitions for the VIOT table
Just here for reference, don't merge!
The actual commits will be pulled from the next ACPICA release.
I squashed the three relevant commits:
ACPICA commit fc4e33319c1ee08f20f5c44853dd8426643f6dfd
ACPICA commit 2197e354fb5dcafaddd2016ffeb0620e5bc3d5e2
ACPICA commit 856a96fdf4b51b2b8da17529df0255e6f51f1b5b
Link: https://github.com/acpica/acpica/commit/fc4e3331
Link: https://github.com/acpica/acpica/commit/2197e354
Link: https://github.com/acpica/acpica/commit/856a96fd
Signed-off-by: Bob Moore
Signed-off-by: Jean-Philippe Brucker
---
include/acpi/actbl3.h | 67 +++
1 file changed, 67 insertions(+)
diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
index df5f4b27f3aa..09d15898e9a8 100644
--- a/include/acpi/actbl3.h
+++ b/include/acpi/actbl3.h
@@ -33,6 +33,7 @@
#define ACPI_SIG_TCPA "TCPA" /* Trusted Computing Platform Alliance
table */
#define ACPI_SIG_TPM2 "TPM2" /* Trusted Platform Module 2.0 H/W
interface table */
#define ACPI_SIG_UEFI "UEFI" /* Uefi Boot Optimization Table */
+#define ACPI_SIG_VIOT "VIOT" /* Virtual I/O Translation Table */
#define ACPI_SIG_WAET "WAET" /* Windows ACPI Emulated devices Table
*/
#define ACPI_SIG_WDAT "WDAT" /* Watchdog Action Table */
#define ACPI_SIG_WDDT "WDDT" /* Watchdog Timer Description Table */
@@ -483,6 +484,72 @@ struct acpi_table_uefi {
u16 data_offset;/* Offset of remaining data in table */
};
+/***
+ *
+ * VIOT - Virtual I/O Translation Table
+ *Version 1
+ *
+
**/
+
+struct acpi_table_viot {
+ struct acpi_table_header header;/* Common ACPI table header */
+ u16 node_count;
+ u16 node_offset;
+ u8 reserved[8];
+};
+
+/* VIOT subtable header */
+
+struct acpi_viot_header {
+ u8 type;
+ u8 reserved;
+ u16 length;
+};
+
+/* Values for Type field above */
+
+enum acpi_viot_node_type {
+ ACPI_VIOT_NODE_PCI_RANGE = 0x01,
+ ACPI_VIOT_NODE_MMIO = 0x02,
+ ACPI_VIOT_NODE_VIRTIO_IOMMU_PCI = 0x03,
+ ACPI_VIOT_NODE_VIRTIO_IOMMU_MMIO = 0x04,
+ ACPI_VIOT_RESERVED = 0x05
+};
+
+/* VIOT subtables */
+
+struct acpi_viot_pci_range {
+ struct acpi_viot_header header;
+ u32 endpoint_start;
+ u16 segment_start;
+ u16 segment_end;
+ u16 bdf_start;
+ u16 bdf_end;
+ u16 output_node;
+ u8 reserved[6];
+};
+
+struct acpi_viot_mmio {
+ struct acpi_viot_header header;
+ u32 endpoint;
+ u64 base_address;
+ u16 output_node;
+ u8 reserved[6];
+};
+
+struct acpi_viot_virtio_iommu_pci {
+ struct acpi_viot_header header;
+ u16 segment;
+ u16 bdf;
+ u8 reserved[8];
+};
+
+struct acpi_viot_virtio_iommu_mmio {
+ struct acpi_viot_header header;
+ u8 reserved[4];
+ u64 base_address;
+};
+
/***
*
* WAET - Windows ACPI Emulated devices Table
--
2.30.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v13 00/15] SMMUv3 Nested Stage Setup (IOMMU part)
> Hi Krishna, > On 3/15/21 7:04 PM, Krishna Reddy wrote: > > Tested-by: Krishna Reddy > > > >> 1) pass the guest stage 1 configuration > > > > Validated Nested SMMUv3 translations for NVMe PCIe device from Guest VM > along with patch series "v11 SMMUv3 Nested Stage Setup (VFIO part)" and > QEMU patch series "vSMMUv3/pSMMUv3 2 stage VFIO integration" from > v5.2.0-2stage-rfcv8. > > NVMe PCIe device is functional with 2-stage translations and no issues > observed. > Thank you very much for your testing efforts. For your info, there are more > recent kernel series: > [PATCH v14 00/13] SMMUv3 Nested Stage Setup (IOMMU part) (Feb 23) [PATCH > v12 00/13] SMMUv3 Nested Stage Setup (VFIO part) (Feb 23) > > working along with QEMU RFC > [RFC v8 00/28] vSMMUv3/pSMMUv3 2 stage VFIO integration (Feb 25) > > If you have cycles to test with those, this would be higly appreciated. Thanks Eric for the latest patches. Will validate and update. Feel free to reach out me for validating future patch sets as necessary. -KR ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] iommu/io-pgtable-arm: Add IOMMU_LLC page protection flag
On Tue, Mar 16, 2021 at 10:04 AM Rob Clark wrote: > > On Wed, Feb 3, 2021 at 2:14 PM Rob Clark wrote: > > > > On Wed, Feb 3, 2021 at 1:46 PM Will Deacon wrote: > > > > > > On Tue, Feb 02, 2021 at 11:56:27AM +0530, Sai Prakash Ranjan wrote: > > > > On 2021-02-01 23:50, Jordan Crouse wrote: > > > > > On Mon, Feb 01, 2021 at 08:20:44AM -0800, Rob Clark wrote: > > > > > > On Mon, Feb 1, 2021 at 3:16 AM Will Deacon wrote: > > > > > > > On Fri, Jan 29, 2021 at 03:12:59PM +0530, Sai Prakash Ranjan > > > > > > > wrote: > > > > > > > > On 2021-01-29 14:35, Will Deacon wrote: > > > > > > > > > On Mon, Jan 11, 2021 at 07:45:04PM +0530, Sai Prakash Ranjan > > > > > > > > > wrote: > > > > > > > > > > +#define IOMMU_LLC(1 << 6) > > > > > > > > > > > > > > > > > > On reflection, I'm a bit worried about exposing this because > > > > > > > > > I think it > > > > > > > > > will > > > > > > > > > introduce a mismatched virtual alias with the CPU (we don't > > > > > > > > > even have a > > > > > > > > > MAIR > > > > > > > > > set up for this memory type). Now, we also have that issue > > > > > > > > > for the PTW, > > > > > > > > > but > > > > > > > > > since we always use cache maintenance (i.e. the streaming > > > > > > > > > API) for > > > > > > > > > publishing the page-tables to a non-coheren walker, it works > > > > > > > > > out. > > > > > > > > > However, > > > > > > > > > if somebody expects IOMMU_LLC to be coherent with a DMA API > > > > > > > > > coherent > > > > > > > > > allocation, then they're potentially in for a nasty surprise > > > > > > > > > due to the > > > > > > > > > mismatched outer-cacheability attributes. > > > > > > > > > > > > > > > > > > > > > > > > > Can't we add the syscached memory type similar to what is done > > > > > > > > on android? > > > > > > > > > > > > > > Maybe. How does the GPU driver map these things on the CPU side? > > > > > > > > > > > > Currently we use writecombine mappings for everything, although > > > > > > there > > > > > > are some cases that we'd like to use cached (but have not merged > > > > > > patches that would give userspace a way to flush/invalidate) > > > > > > > > > > > > > > > > LLC/system cache doesn't have a relationship with the CPU cache. Its > > > > > just a > > > > > little accelerator that sits on the connection from the GPU to DDR and > > > > > caches > > > > > accesses. The hint that Sai is suggesting is used to mark the buffers > > > > > as > > > > > 'no-write-allocate' to prevent GPU write operations from being cached > > > > > in > > > > > the LLC > > > > > which a) isn't interesting and b) takes up cache space for read > > > > > operations. > > > > > > > > > > Its easiest to think of the LLC as a bonus accelerator that has no > > > > > cost > > > > > for > > > > > us to use outside of the unfortunate per buffer hint. > > > > > > > > > > We do have to worry about the CPU cache w.r.t I/O coherency (which is > > > > > a > > > > > different hint) and in that case we have all of concerns that Will > > > > > identified. > > > > > > > > > > > > > For mismatched outer cacheability attributes which Will mentioned, I was > > > > referring to [1] in android kernel. > > > > > > I've lost track of the conversation here :/ > > > > > > When the GPU has a buffer mapped with IOMMU_LLC, is the buffer also mapped > > > into the CPU and with what attributes? Rob said "writecombine for > > > everything" -- does that mean ioremap_wc() / MEMREMAP_WC? > > > > Currently userspace asks for everything WC, so pgprot_writecombine() > > > > The kernel doesn't enforce this, but so far provides no UAPI to do > > anything useful with non-coherent cached mappings (although there is > > interest to support this) > > > > btw, I'm looking at a benchmark (gl_driver2_off) where (after some > other in-flight optimizations land) we end up bottlenecked on writing > to WC cmdstream buffers. I assume in the current state, WC goes all > the way to main memory rather than just to system cache? > oh, I guess this (mentioned earlier in thread) is what I really want for this benchmark: https://android-review.googlesource.com/c/kernel/common/+/1549097/3 > BR, > -R > > > BR, > > -R > > > > > Finally, we need to be careful when we use the word "hint" as "allocation > > > hint" has a specific meaning in the architecture, and if we only mismatch > > > on > > > those then we're actually ok. But I think IOMMU_LLC is more than just a > > > hint, since it actually drives eviction policy (i.e. it enables > > > writeback). > > > > > > Sorry for the pedantry, but I just want to make sure we're all talking > > > about the same things! > > > > > > Cheers, > > > > > > Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] iommu/io-pgtable-arm: Add IOMMU_LLC page protection flag
On Wed, Feb 3, 2021 at 2:14 PM Rob Clark wrote: > > On Wed, Feb 3, 2021 at 1:46 PM Will Deacon wrote: > > > > On Tue, Feb 02, 2021 at 11:56:27AM +0530, Sai Prakash Ranjan wrote: > > > On 2021-02-01 23:50, Jordan Crouse wrote: > > > > On Mon, Feb 01, 2021 at 08:20:44AM -0800, Rob Clark wrote: > > > > > On Mon, Feb 1, 2021 at 3:16 AM Will Deacon wrote: > > > > > > On Fri, Jan 29, 2021 at 03:12:59PM +0530, Sai Prakash Ranjan wrote: > > > > > > > On 2021-01-29 14:35, Will Deacon wrote: > > > > > > > > On Mon, Jan 11, 2021 at 07:45:04PM +0530, Sai Prakash Ranjan > > > > > > > > wrote: > > > > > > > > > +#define IOMMU_LLC(1 << 6) > > > > > > > > > > > > > > > > On reflection, I'm a bit worried about exposing this because I > > > > > > > > think it > > > > > > > > will > > > > > > > > introduce a mismatched virtual alias with the CPU (we don't > > > > > > > > even have a > > > > > > > > MAIR > > > > > > > > set up for this memory type). Now, we also have that issue for > > > > > > > > the PTW, > > > > > > > > but > > > > > > > > since we always use cache maintenance (i.e. the streaming API) > > > > > > > > for > > > > > > > > publishing the page-tables to a non-coheren walker, it works > > > > > > > > out. > > > > > > > > However, > > > > > > > > if somebody expects IOMMU_LLC to be coherent with a DMA API > > > > > > > > coherent > > > > > > > > allocation, then they're potentially in for a nasty surprise > > > > > > > > due to the > > > > > > > > mismatched outer-cacheability attributes. > > > > > > > > > > > > > > > > > > > > > > Can't we add the syscached memory type similar to what is done on > > > > > > > android? > > > > > > > > > > > > Maybe. How does the GPU driver map these things on the CPU side? > > > > > > > > > > Currently we use writecombine mappings for everything, although there > > > > > are some cases that we'd like to use cached (but have not merged > > > > > patches that would give userspace a way to flush/invalidate) > > > > > > > > > > > > > LLC/system cache doesn't have a relationship with the CPU cache. Its > > > > just a > > > > little accelerator that sits on the connection from the GPU to DDR and > > > > caches > > > > accesses. The hint that Sai is suggesting is used to mark the buffers as > > > > 'no-write-allocate' to prevent GPU write operations from being cached in > > > > the LLC > > > > which a) isn't interesting and b) takes up cache space for read > > > > operations. > > > > > > > > Its easiest to think of the LLC as a bonus accelerator that has no cost > > > > for > > > > us to use outside of the unfortunate per buffer hint. > > > > > > > > We do have to worry about the CPU cache w.r.t I/O coherency (which is a > > > > different hint) and in that case we have all of concerns that Will > > > > identified. > > > > > > > > > > For mismatched outer cacheability attributes which Will mentioned, I was > > > referring to [1] in android kernel. > > > > I've lost track of the conversation here :/ > > > > When the GPU has a buffer mapped with IOMMU_LLC, is the buffer also mapped > > into the CPU and with what attributes? Rob said "writecombine for > > everything" -- does that mean ioremap_wc() / MEMREMAP_WC? > > Currently userspace asks for everything WC, so pgprot_writecombine() > > The kernel doesn't enforce this, but so far provides no UAPI to do > anything useful with non-coherent cached mappings (although there is > interest to support this) > btw, I'm looking at a benchmark (gl_driver2_off) where (after some other in-flight optimizations land) we end up bottlenecked on writing to WC cmdstream buffers. I assume in the current state, WC goes all the way to main memory rather than just to system cache? BR, -R > BR, > -R > > > Finally, we need to be careful when we use the word "hint" as "allocation > > hint" has a specific meaning in the architecture, and if we only mismatch on > > those then we're actually ok. But I think IOMMU_LLC is more than just a > > hint, since it actually drives eviction policy (i.e. it enables writeback). > > > > Sorry for the pedantry, but I just want to make sure we're all talking > > about the same things! > > > > Cheers, > > > > Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 18/18] iommu: remove iommu_domain_{get,set}_attr
Remove the now unused iommu attr infrastructure.
Signed-off-by: Christoph Hellwig
---
drivers/iommu/iommu.c | 26 --
include/linux/iommu.h | 36
2 files changed, 62 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index afd21b37e319ee..d8df26d13c7371 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2662,32 +2662,6 @@ static int __init iommu_init(void)
}
core_initcall(iommu_init);
-int iommu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- if (!domain->ops->domain_get_attr)
- return -EINVAL;
- return domain->ops->domain_get_attr(domain, attr, data);
-}
-EXPORT_SYMBOL_GPL(iommu_domain_get_attr);
-
-int iommu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- int ret = 0;
-
- switch (attr) {
- default:
- if (domain->ops->domain_set_attr == NULL)
- return -EINVAL;
-
- ret = domain->ops->domain_set_attr(domain, attr, data);
- }
-
- return ret;
-}
-EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
-
int iommu_enable_nesting(struct iommu_domain *domain)
{
if (domain->type != IOMMU_DOMAIN_UNMANAGED)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a53d74f2007b14..9319333ca0b827 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -96,20 +96,6 @@ enum iommu_cap {
IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */
};
-/*
- * Following constraints are specifc to FSL_PAMUV1:
- * -aperture must be power of 2, and naturally aligned
- * -number of windows must be power of 2, and address space size
- * of each window is determined by aperture size / # of windows
- * -the actual size of the mapped region of a window must be power
- * of 2 starting with 4KB and physical address must be naturally
- * aligned.
- */
-
-enum iommu_attr {
- DOMAIN_ATTR_MAX,
-};
-
/* These are the possible reserved region types */
enum iommu_resv_type {
/* Memory regions which must be mapped 1:1 at all times */
@@ -191,8 +177,6 @@ struct iommu_iotlb_gather {
* @probe_finalize: Do final setup work after the device is added to an IOMMU
* group and attached to the groups domain
* @device_group: find iommu group for a particular device
- * @domain_get_attr: Query domain attributes
- * @domain_set_attr: Change domain attributes
* @enable_nesting: Enable nesting
* @set_pgtable_quirks: Set io page table quirks (IO_PGTABLE_QUIRK_*)
* @get_resv_regions: Request list of reserved regions for a device
@@ -243,10 +227,6 @@ struct iommu_ops {
void (*release_device)(struct device *dev);
void (*probe_finalize)(struct device *dev);
struct iommu_group *(*device_group)(struct device *dev);
- int (*domain_get_attr)(struct iommu_domain *domain,
- enum iommu_attr attr, void *data);
- int (*domain_set_attr)(struct iommu_domain *domain,
- enum iommu_attr attr, void *data);
int (*enable_nesting)(struct iommu_domain *domain);
int (*set_pgtable_quirks)(struct iommu_domain *domain,
unsigned long quirks);
@@ -493,10 +473,6 @@ extern int iommu_page_response(struct device *dev,
extern int iommu_group_id(struct iommu_group *group);
extern struct iommu_domain *iommu_group_default_domain(struct iommu_group *);
-extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr,
-void *data);
-extern int iommu_domain_set_attr(struct iommu_domain *domain, enum iommu_attr,
-void *data);
int iommu_enable_nesting(struct iommu_domain *domain);
int iommu_set_pgtable_quirks(struct iommu_domain *domain,
unsigned long quirks);
@@ -869,18 +845,6 @@ static inline int iommu_group_id(struct iommu_group *group)
return -ENODEV;
}
-static inline int iommu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- return -EINVAL;
-}
-
-static inline int iommu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- return -EINVAL;
-}
-
static inline int iommu_set_pgtable_quirks(struct iommu_domain *domain,
unsigned long quirks)
{
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 17/18] iommu: remove DOMAIN_ATTR_IO_PGTABLE_CFG
Use an explicit set_pgtable_quirks method instead that just passes
the actual quirk bitmask instead.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 5 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 64 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 2 +-
drivers/iommu/iommu.c | 11 +
include/linux/io-pgtable.h | 4 --
include/linux/iommu.h | 12 -
6 files changed, 35 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0f184c3dd9d9ec..4a0b14dad93e2e 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -188,10 +188,7 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
void adreno_set_llc_attributes(struct iommu_domain *iommu)
{
- struct io_pgtable_domain_attr pgtbl_cfg;
-
- pgtbl_cfg.quirks = IO_PGTABLE_QUIRK_ARM_OUTER_WBWA;
- iommu_domain_set_attr(iommu, DOMAIN_ATTR_IO_PGTABLE_CFG, &pgtbl_cfg);
+ iommu_set_pgtable_quirks(iommu, IO_PGTABLE_QUIRK_ARM_OUTER_WBWA);
}
struct msm_gem_address_space *
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 3dde22b1f8ffb0..b5e747eeed1362 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -770,8 +770,8 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
goto out_clear_smmu;
}
- if (smmu_domain->pgtbl_cfg.quirks)
- pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
+ if (smmu_domain->pgtbl_quirks)
+ pgtbl_cfg.quirks |= smmu_domain->pgtbl_quirks;
pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
if (!pgtbl_ops) {
@@ -1484,29 +1484,6 @@ static struct iommu_group *arm_smmu_device_group(struct
device *dev)
return group;
}
-static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
-
- switch(domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_IO_PGTABLE_CFG: {
- struct io_pgtable_domain_attr *pgtbl_cfg = data;
- *pgtbl_cfg = smmu_domain->pgtbl_cfg;
-
- return 0;
- }
- default:
- return -ENODEV;
- }
- break;
- default:
- return -EINVAL;
- }
-}
-
static int arm_smmu_enable_nesting(struct iommu_domain *domain)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
@@ -1522,37 +1499,19 @@ static int arm_smmu_enable_nesting(struct iommu_domain
*domain)
return ret;
}
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
+static int arm_smmu_set_pgtable_quirks(struct iommu_domain *domain,
+ unsigned long quirks)
{
- int ret = 0;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = 0;
mutex_lock(&smmu_domain->init_mutex);
-
- switch(domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_IO_PGTABLE_CFG: {
- struct io_pgtable_domain_attr *pgtbl_cfg = data;
-
- if (smmu_domain->smmu) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- smmu_domain->pgtbl_cfg = *pgtbl_cfg;
- break;
- }
- default:
- ret = -ENODEV;
- }
- break;
- default:
- ret = -EINVAL;
- }
-out_unlock:
+ if (smmu_domain->smmu)
+ ret = -EPERM;
+ else
+ smmu_domain->pgtbl_quirks = quirks;
mutex_unlock(&smmu_domain->init_mutex);
+
return ret;
}
@@ -1611,9 +1570,8 @@ static struct iommu_ops arm_smmu_ops = {
.probe_device = arm_smmu_probe_device,
.release_device = arm_smmu_release_device,
.device_group = arm_smmu_device_group,
- .domain_get_attr= arm_smmu_domain_get_attr,
- .domain_set_attr= arm_smmu_domain_set_attr,
.enable_nesting = arm_smmu_enable_nesting,
+ .set_pgtable_quirks = arm_smmu_set_pgtable_quirks,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers
[PATCH 16/18] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
From: Robin Murphy
Instead make the global iommu_dma_strict paramete in iommu.c canonical by
exporting helpers to get and set it and use those directly in the drivers.
This make sure that the iommu.strict parameter also works for the AMD and
Intel IOMMU drivers on x86. As those default to lazy flushing a new
IOMMU_CMD_LINE_STRICT is used to turn the value into a tristate to
represent the default if not overriden by an explicit parameter.
Signed-off-by: Robin Murphy .
[ported on top of the other iommu_attr changes and added a few small
missing bits]
Signed-off-by: Christoph Hellwig
---
drivers/iommu/amd/iommu.c | 23 +---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 50 +---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 -
drivers/iommu/arm/arm-smmu/arm-smmu.c | 27 +
drivers/iommu/dma-iommu.c | 9 +--
drivers/iommu/intel/iommu.c | 64 -
drivers/iommu/iommu.c | 27 ++---
include/linux/iommu.h | 4 +-
8 files changed, 40 insertions(+), 165 deletions(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40d0..ce6393d2224d86 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1771,26 +1771,6 @@ static struct iommu_group *amd_iommu_device_group(struct
device *dev)
return acpihid_device_group(dev);
}
-static int amd_iommu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- return -ENODEV;
- case IOMMU_DOMAIN_DMA:
- switch (attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = !amd_iommu_unmap_flush;
- return 0;
- default:
- return -ENODEV;
- }
- break;
- default:
- return -EINVAL;
- }
-}
-
/*
*
* The next functions belong to the dma_ops mapping/unmapping code.
@@ -1855,7 +1835,7 @@ int __init amd_iommu_init_dma_ops(void)
pr_info("IO/TLB flush on unmap enabled\n");
else
pr_info("Lazy IO/TLB flushing enabled\n");
-
+ iommu_set_dma_strict(amd_iommu_unmap_flush);
return 0;
}
@@ -2257,7 +2237,6 @@ const struct iommu_ops amd_iommu_ops = {
.release_device = amd_iommu_release_device,
.probe_finalize = amd_iommu_probe_finalize,
.device_group = amd_iommu_device_group,
- .domain_get_attr = amd_iommu_domain_get_attr,
.get_resv_regions = amd_iommu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
.is_attach_deferred = amd_iommu_is_attach_deferred,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index f1e38526d5bd40..996dfdf9d375dd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2017,7 +2017,7 @@ static int arm_smmu_domain_finalise(struct iommu_domain
*domain,
.iommu_dev = smmu->dev,
};
- if (smmu_domain->non_strict)
+ if (!iommu_get_dma_strict())
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
pgtbl_ops = alloc_io_pgtable_ops(fmt, &pgtbl_cfg, smmu_domain);
@@ -2449,52 +2449,6 @@ static struct iommu_group *arm_smmu_device_group(struct
device *dev)
return group;
}
-static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
-
- switch (domain->type) {
- case IOMMU_DOMAIN_DMA:
- switch (attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = smmu_domain->non_strict;
- return 0;
- default:
- return -ENODEV;
- }
- break;
- default:
- return -EINVAL;
- }
-}
-
-static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
- enum iommu_attr attr, void *data)
-{
- int ret = 0;
- struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
-
- mutex_lock(&smmu_domain->init_mutex);
-
- switch (domain->type) {
- case IOMMU_DOMAIN_DMA:
- switch(attr) {
- case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- smmu_domain->non_strict = *(int *)data;
- break;
- default:
- ret = -ENODEV;
- }
- break;
- default:
- ret = -EINVAL;
- }
-
- mutex_unlock(&smmu_domain->
[PATCH 15/18] iommu: remove iommu_set_cmd_line_dma_api and iommu_cmd_line_dma_api
Don't obsfucate the trivial bit flag check.
Signed-off-by: Christoph Hellwig
---
drivers/iommu/iommu.c | 23 +--
1 file changed, 5 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 58d1d11a8d5c10..052cef11ae30df 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -70,16 +70,6 @@ static const char * const iommu_group_resv_type_string[] = {
#define IOMMU_CMD_LINE_DMA_API BIT(0)
-static void iommu_set_cmd_line_dma_api(void)
-{
- iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
-}
-
-static bool iommu_cmd_line_dma_api(void)
-{
- return !!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API);
-}
-
static int iommu_alloc_default_domain(struct iommu_group *group,
struct device *dev);
static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
@@ -130,9 +120,7 @@ static const char *iommu_domain_type_str(unsigned int t)
static int __init iommu_subsys_init(void)
{
- bool cmd_line = iommu_cmd_line_dma_api();
-
- if (!cmd_line) {
+ if (!(iommu_cmd_line & IOMMU_CMD_LINE_DMA_API)) {
if (IS_ENABLED(CONFIG_IOMMU_DEFAULT_PASSTHROUGH))
iommu_set_default_passthrough(false);
else
@@ -146,7 +134,8 @@ static int __init iommu_subsys_init(void)
pr_info("Default domain type: %s %s\n",
iommu_domain_type_str(iommu_def_domain_type),
- cmd_line ? "(set via kernel command line)" : "");
+ (iommu_cmd_line & IOMMU_CMD_LINE_DMA_API) ?
+ "(set via kernel command line)" : "");
return 0;
}
@@ -2757,16 +2746,14 @@ EXPORT_SYMBOL_GPL(iommu_alloc_resv_region);
void iommu_set_default_passthrough(bool cmd_line)
{
if (cmd_line)
- iommu_set_cmd_line_dma_api();
-
+ iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
iommu_def_domain_type = IOMMU_DOMAIN_IDENTITY;
}
void iommu_set_default_translated(bool cmd_line)
{
if (cmd_line)
- iommu_set_cmd_line_dma_api();
-
+ iommu_cmd_line |= IOMMU_CMD_LINE_DMA_API;
iommu_def_domain_type = IOMMU_DOMAIN_DMA;
}
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 14/18] iommu: remove DOMAIN_ATTR_NESTING
Use an explicit enable_nesting method instead.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 43 -
drivers/iommu/arm/arm-smmu/arm-smmu.c | 30 +++---
drivers/iommu/intel/iommu.c | 31 +--
drivers/iommu/iommu.c | 10 +
drivers/vfio/vfio_iommu_type1.c | 5 +--
include/linux/iommu.h | 4 +-
6 files changed, 55 insertions(+), 68 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8594b4a8304375..f1e38526d5bd40 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2455,15 +2455,6 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_NESTING:
- *(int *)data = (smmu_domain->stage ==
ARM_SMMU_DOMAIN_NESTED);
- return 0;
- default:
- return -ENODEV;
- }
- break;
case IOMMU_DOMAIN_DMA:
switch (attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
@@ -2487,23 +2478,6 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
mutex_lock(&smmu_domain->init_mutex);
switch (domain->type) {
- case IOMMU_DOMAIN_UNMANAGED:
- switch (attr) {
- case DOMAIN_ATTR_NESTING:
- if (smmu_domain->smmu) {
- ret = -EPERM;
- goto out_unlock;
- }
-
- if (*(int *)data)
- smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
- else
- smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
- break;
- default:
- ret = -ENODEV;
- }
- break;
case IOMMU_DOMAIN_DMA:
switch(attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
@@ -2517,11 +2491,25 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
ret = -EINVAL;
}
-out_unlock:
mutex_unlock(&smmu_domain->init_mutex);
return ret;
}
+static int arm_smmu_enable_nesting(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = 0;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ if (smmu_domain->smmu)
+ ret = -EPERM;
+ else
+ smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
+ mutex_unlock(&smmu_domain->init_mutex);
+
+ return ret;
+}
+
static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
{
return iommu_fwspec_add_ids(dev, args->args, 1);
@@ -2621,6 +2609,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group = arm_smmu_device_group,
.domain_get_attr= arm_smmu_domain_get_attr,
.domain_set_attr= arm_smmu_domain_set_attr,
+ .enable_nesting = arm_smmu_enable_nesting,
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = generic_iommu_put_resv_regions,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index d8c6bfde6a6158..0aa6d667274970 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1489,9 +1489,6 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
switch(domain->type) {
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
- case DOMAIN_ATTR_NESTING:
- *(int *)data = (smmu_domain->stage ==
ARM_SMMU_DOMAIN_NESTED);
- return 0;
case DOMAIN_ATTR_IO_PGTABLE_CFG: {
struct io_pgtable_domain_attr *pgtbl_cfg = data;
*pgtbl_cfg = smmu_domain->pgtbl_cfg;
@@ -1519,6 +1516,21 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
}
}
+static int arm_smmu_enable_nesting(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ int ret = 0;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ if (smmu_domain->smmu)
+ ret = -EPERM;
+ else
+ smmu_domain->stage = ARM_SMMU_DOMAIN_NESTED;
+ mutex_unlock(&smmu_domain->init_mutex);
+
+ return ret;
+}
+
static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
enum iommu_att
[PATCH 13/18] iommu: remove DOMAIN_ATTR_GEOMETRY
The geometry information can be trivially queried from the iommu_domain
struture.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/iommu.c | 20 +++-
drivers/vfio/vfio_iommu_type1.c | 26 --
drivers/vhost/vdpa.c| 10 +++---
include/linux/iommu.h | 1 -
4 files changed, 18 insertions(+), 39 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9a4cda390993e6..23daaea7883b75 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2667,23 +2667,9 @@ core_initcall(iommu_init);
int iommu_domain_get_attr(struct iommu_domain *domain,
enum iommu_attr attr, void *data)
{
- struct iommu_domain_geometry *geometry;
- int ret = 0;
-
- switch (attr) {
- case DOMAIN_ATTR_GEOMETRY:
- geometry = data;
- *geometry = domain->geometry;
-
- break;
- default:
- if (!domain->ops->domain_get_attr)
- return -EINVAL;
-
- ret = domain->ops->domain_get_attr(domain, attr, data);
- }
-
- return ret;
+ if (!domain->ops->domain_get_attr)
+ return -EINVAL;
+ return domain->ops->domain_get_attr(domain, attr, data);
}
EXPORT_SYMBOL_GPL(iommu_domain_get_attr);
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 4bb162c1d649b3..c8e57f22f421c5 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -2252,7 +2252,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
int ret;
bool resv_msi, msi_remap;
phys_addr_t resv_msi_base = 0;
- struct iommu_domain_geometry geo;
+ struct iommu_domain_geometry *geo;
LIST_HEAD(iova_copy);
LIST_HEAD(group_resv_regions);
@@ -2333,10 +2333,9 @@ static int vfio_iommu_type1_attach_group(void
*iommu_data,
goto out_domain;
/* Get aperture info */
- iommu_domain_get_attr(domain->domain, DOMAIN_ATTR_GEOMETRY, &geo);
-
- if (vfio_iommu_aper_conflict(iommu, geo.aperture_start,
-geo.aperture_end)) {
+ geo = &domain->domain->geometry;
+ if (vfio_iommu_aper_conflict(iommu, geo->aperture_start,
+geo->aperture_end)) {
ret = -EINVAL;
goto out_detach;
}
@@ -2359,8 +2358,8 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
if (ret)
goto out_detach;
- ret = vfio_iommu_aper_resize(&iova_copy, geo.aperture_start,
-geo.aperture_end);
+ ret = vfio_iommu_aper_resize(&iova_copy, geo->aperture_start,
+geo->aperture_end);
if (ret)
goto out_detach;
@@ -2493,7 +2492,6 @@ static void vfio_iommu_aper_expand(struct vfio_iommu
*iommu,
struct list_head *iova_copy)
{
struct vfio_domain *domain;
- struct iommu_domain_geometry geo;
struct vfio_iova *node;
dma_addr_t start = 0;
dma_addr_t end = (dma_addr_t)~0;
@@ -2502,12 +2500,12 @@ static void vfio_iommu_aper_expand(struct vfio_iommu
*iommu,
return;
list_for_each_entry(domain, &iommu->domain_list, next) {
- iommu_domain_get_attr(domain->domain, DOMAIN_ATTR_GEOMETRY,
- &geo);
- if (geo.aperture_start > start)
- start = geo.aperture_start;
- if (geo.aperture_end < end)
- end = geo.aperture_end;
+ struct iommu_domain_geometry *geo = &domain->domain->geometry;
+
+ if (geo->aperture_start > start)
+ start = geo->aperture_start;
+ if (geo->aperture_end < end)
+ end = geo->aperture_end;
}
/* Modify aperture limits. The new aper is either same or bigger */
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index ef688c8c0e0e6f..25824fab433d0a 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -826,18 +826,14 @@ static void vhost_vdpa_free_domain(struct vhost_vdpa *v)
static void vhost_vdpa_set_iova_range(struct vhost_vdpa *v)
{
struct vdpa_iova_range *range = &v->range;
- struct iommu_domain_geometry geo;
struct vdpa_device *vdpa = v->vdpa;
const struct vdpa_config_ops *ops = vdpa->config;
if (ops->get_iova_range) {
*range = ops->get_iova_range(vdpa);
- } else if (v->domain &&
- !iommu_domain_get_attr(v->domain,
- DOMAIN_ATTR_GEOMETRY, &geo) &&
- geo.force_aperture) {
- range->first = geo.aperture_start;
- range->last = geo.aperture_end;
+ } else if (v->domain && v
[PATCH 12/18] iommu: remove DOMAIN_ATTR_PAGING
DOMAIN_ATTR_PAGING is never used.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/iommu.c | 5 -
include/linux/iommu.h | 1 -
2 files changed, 6 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b212bf0261820b..9a4cda390993e6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2668,7 +2668,6 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
enum iommu_attr attr, void *data)
{
struct iommu_domain_geometry *geometry;
- bool *paging;
int ret = 0;
switch (attr) {
@@ -2676,10 +2675,6 @@ int iommu_domain_get_attr(struct iommu_domain *domain,
geometry = data;
*geometry = domain->geometry;
- break;
- case DOMAIN_ATTR_PAGING:
- paging = data;
- *paging = (domain->pgsize_bitmap != 0UL);
break;
default:
if (!domain->ops->domain_get_attr)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 840864844027dc..180ff4bd7fa7ef 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -108,7 +108,6 @@ enum iommu_cap {
enum iommu_attr {
DOMAIN_ATTR_GEOMETRY,
- DOMAIN_ATTR_PAGING,
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
DOMAIN_ATTR_IO_PGTABLE_CFG,
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 11/18] iommu/fsl_pamu: remove the snoop_id field
The snoop_id is always set to ~(u32)0.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 5 ++---
drivers/iommu/fsl_pamu_domain.h | 1 -
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 21c6d9e79eddf9..701fc3f187a100 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -97,12 +97,12 @@ static int pamu_set_liodn(struct fsl_dma_domain
*dma_domain, struct device *dev,
goto out_unlock;
ret = pamu_config_ppaace(liodn, geom->aperture_start,
geom->aperture_end - 1, omi_index, 0,
-dma_domain->snoop_id, dma_domain->stash_id, 0);
+~(u32)0, dma_domain->stash_id, 0);
if (ret)
goto out_unlock;
ret = pamu_config_ppaace(liodn, geom->aperture_start,
geom->aperture_end - 1, ~(u32)0,
-0, dma_domain->snoop_id, dma_domain->stash_id,
+0, ~(u32)0, dma_domain->stash_id,
PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE);
out_unlock:
spin_unlock_irqrestore(&iommu_lock, flags);
@@ -210,7 +210,6 @@ static struct iommu_domain *fsl_pamu_domain_alloc(unsigned
type)
return NULL;
dma_domain->stash_id = ~(u32)0;
- dma_domain->snoop_id = ~(u32)0;
INIT_LIST_HEAD(&dma_domain->devices);
spin_lock_init(&dma_domain->domain_lock);
diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h
index 5f4ed253f61b31..95ac1b3cab3b69 100644
--- a/drivers/iommu/fsl_pamu_domain.h
+++ b/drivers/iommu/fsl_pamu_domain.h
@@ -13,7 +13,6 @@ struct fsl_dma_domain {
/* list of devices associated with the domain */
struct list_headdevices;
u32 stash_id;
- u32 snoop_id;
struct iommu_domain iommu_domain;
spinlock_t domain_lock;
};
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/18] iommu/fsl_pamu: enable the liodn when attaching a device
Instead of a separate call to enable all devices from the list, just
enablde the liodn one the device is attached to the iommu domain.
This also remove the DOMAIN_ATTR_FSL_PAMU_ENABLE iommu_attr.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 47 ++---
drivers/iommu/fsl_pamu_domain.h | 10 --
drivers/soc/fsl/qbman/qman_portal.c | 11 ---
include/linux/iommu.h | 1 -
4 files changed, 3 insertions(+), 66 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 962cdc1a4a1924..21c6d9e79eddf9 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -195,9 +195,6 @@ static void fsl_pamu_domain_free(struct iommu_domain
*domain)
/* remove all the devices from the device list */
detach_device(NULL, dma_domain);
-
- dma_domain->enabled = 0;
-
kmem_cache_free(fsl_pamu_domain_cache, dma_domain);
}
@@ -285,6 +282,9 @@ static int fsl_pamu_attach_device(struct iommu_domain
*domain,
ret = pamu_set_liodn(dma_domain, dev, liodn[i]);
if (ret)
break;
+ ret = pamu_enable_liodn(liodn[i]);
+ if (ret)
+ break;
}
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return ret;
@@ -341,46 +341,6 @@ int fsl_pamu_configure_l1_stash(struct iommu_domain
*domain, u32 cpu)
return ret;
}
-/* Configure domain dma state i.e. enable/disable DMA */
-static int configure_domain_dma_state(struct fsl_dma_domain *dma_domain, bool
enable)
-{
- struct device_domain_info *info;
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&dma_domain->domain_lock, flags);
- dma_domain->enabled = enable;
- list_for_each_entry(info, &dma_domain->devices, link) {
- ret = (enable) ? pamu_enable_liodn(info->liodn) :
- pamu_disable_liodn(info->liodn);
- if (ret)
- pr_debug("Unable to set dma state for liodn %d",
-info->liodn);
- }
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
-
- return 0;
-}
-
-static int fsl_pamu_set_domain_attr(struct iommu_domain *domain,
- enum iommu_attr attr_type, void *data)
-{
- struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
- int ret = 0;
-
- switch (attr_type) {
- case DOMAIN_ATTR_FSL_PAMU_ENABLE:
- ret = configure_domain_dma_state(dma_domain, *(int *)data);
- break;
- default:
- pr_debug("Unsupported attribute type\n");
- ret = -EINVAL;
- break;
- }
-
- return ret;
-}
-
static struct iommu_group *get_device_iommu_group(struct device *dev)
{
struct iommu_group *group;
@@ -505,7 +465,6 @@ static const struct iommu_ops fsl_pamu_ops = {
.attach_dev = fsl_pamu_attach_device,
.detach_dev = fsl_pamu_detach_device,
.iova_to_phys = fsl_pamu_iova_to_phys,
- .domain_set_attr = fsl_pamu_set_domain_attr,
.probe_device = fsl_pamu_probe_device,
.release_device = fsl_pamu_release_device,
.device_group = fsl_pamu_device_group,
diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h
index cd488004acd1b3..5f4ed253f61b31 100644
--- a/drivers/iommu/fsl_pamu_domain.h
+++ b/drivers/iommu/fsl_pamu_domain.h
@@ -12,16 +12,6 @@
struct fsl_dma_domain {
/* list of devices associated with the domain */
struct list_headdevices;
- /* dma_domain states:
-* enabled - DMA has been enabled for the given
-* domain. This translates to setting of the
-* valid bit for the primary PAACE in the PAMU
-* PAACT table. Domain geometry should be set and
-* it must have a valid mapping before DMA can be
-* enabled for it.
-*
-*/
- int enabled;
u32 stash_id;
u32 snoop_id;
struct iommu_domain iommu_domain;
diff --git a/drivers/soc/fsl/qbman/qman_portal.c
b/drivers/soc/fsl/qbman/qman_portal.c
index 798b3a1ffd0b9c..bf38eb0042ed52 100644
--- a/drivers/soc/fsl/qbman/qman_portal.c
+++ b/drivers/soc/fsl/qbman/qman_portal.c
@@ -46,7 +46,6 @@ static void portal_set_cpu(struct qm_portal_config *pcfg, int
cpu)
{
#ifdef CONFIG_FSL_PAMU
struct device *dev = pcfg->dev;
- int window_count = 1;
int ret;
pcfg->iommu_domain = iommu_domain_alloc(&platform_bus_type);
@@ -66,14 +65,6 @@ static void portal_set_cpu(struct qm_portal_config *pcfg,
int cpu)
ret);
goto out_domain_free;
}
- ret = iommu_domain_set_attr
[PATCH 09/18] iommu/fsl_pamu: merge handle_attach_device into fsl_pamu_attach_device
No good reason to split this functionality over two functions.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 59 +++--
1 file changed, 20 insertions(+), 39 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 4a4944332674f7..962cdc1a4a1924 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -240,45 +240,13 @@ static int update_domain_stash(struct fsl_dma_domain
*dma_domain, u32 val)
return ret;
}
-/*
- * Attach the LIODN to the DMA domain and configure the geometry
- * and window mappings.
- */
-static int handle_attach_device(struct fsl_dma_domain *dma_domain,
- struct device *dev, const u32 *liodn,
- int num)
-{
- unsigned long flags;
- int ret = 0;
- int i;
-
- spin_lock_irqsave(&dma_domain->domain_lock, flags);
- for (i = 0; i < num; i++) {
- /* Ensure that LIODN value is valid */
- if (liodn[i] >= PAACE_NUMBER_ENTRIES) {
- pr_debug("Invalid liodn %d, attach device failed for
%pOF\n",
-liodn[i], dev->of_node);
- ret = -EINVAL;
- break;
- }
-
- attach_device(dma_domain, liodn[i], dev);
- ret = pamu_set_liodn(dma_domain, dev, liodn[i]);
- if (ret)
- break;
- }
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
-
- return ret;
-}
-
static int fsl_pamu_attach_device(struct iommu_domain *domain,
struct device *dev)
{
struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
+ unsigned long flags;
+ int len, ret = 0, i;
const u32 *liodn;
- u32 liodn_cnt;
- int len, ret = 0;
struct pci_dev *pdev = NULL;
struct pci_controller *pci_ctl;
@@ -298,14 +266,27 @@ static int fsl_pamu_attach_device(struct iommu_domain
*domain,
}
liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
- if (liodn) {
- liodn_cnt = len / sizeof(u32);
- ret = handle_attach_device(dma_domain, dev, liodn, liodn_cnt);
- } else {
+ if (!liodn) {
pr_debug("missing fsl,liodn property at %pOF\n", dev->of_node);
- ret = -EINVAL;
+ return -EINVAL;
}
+ spin_lock_irqsave(&dma_domain->domain_lock, flags);
+ for (i = 0; i < len / sizeof(u32); i++) {
+ /* Ensure that LIODN value is valid */
+ if (liodn[i] >= PAACE_NUMBER_ENTRIES) {
+ pr_debug("Invalid liodn %d, attach device failed for
%pOF\n",
+liodn[i], dev->of_node);
+ ret = -EINVAL;
+ break;
+ }
+
+ attach_device(dma_domain, liodn[i], dev);
+ ret = pamu_set_liodn(dma_domain, dev, liodn[i]);
+ if (ret)
+ break;
+ }
+ spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return ret;
}
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 09/11] block: Add BLK_STS_P2PDMA
On 2021-03-16 2:00 a.m., Christoph Hellwig wrote: > On Thu, Mar 11, 2021 at 04:31:39PM -0700, Logan Gunthorpe wrote: >> Create a specific error code for when P2PDMA pages are passed to a block >> devices that cannot map them (due to no IOMMU support or ACS protections). >> >> This makes request errors in these cases more informative of as to what >> caused the error. > > I really don't think we should bother with a specific error code here, > we don't add a new status for every single possible logic error in the > caller. I originally had BLK_STS_IOERR but those errors suggested to people that the hardware had failed on the request when in fact it was a user error. I'll try BLK_STS_TARGET unless there's any objection or someone thinks another error code would make more sense. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 08/18] iommu/fsl_pamu: merge pamu_set_liodn and map_liodn
Merge the two fuctions that configure the ppaace into a single coherent
function. I somehow doubt we need the two pamu_config_ppaace calls,
but keep the existing behavior just to be on the safe side.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 65 +
1 file changed, 17 insertions(+), 48 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 40eff4b7bc5d42..4a4944332674f7 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -54,25 +54,6 @@ static int __init iommu_init_mempool(void)
return 0;
}
-/* Map the DMA window corresponding to the LIODN */
-static int map_liodn(int liodn, struct fsl_dma_domain *dma_domain)
-{
- int ret;
- struct iommu_domain_geometry *geom = &dma_domain->iommu_domain.geometry;
- unsigned long flags;
-
- spin_lock_irqsave(&iommu_lock, flags);
- ret = pamu_config_ppaace(liodn, geom->aperture_start,
-geom->aperture_end - 1, ~(u32)0,
-0, dma_domain->snoop_id, dma_domain->stash_id,
-PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE);
- spin_unlock_irqrestore(&iommu_lock, flags);
- if (ret)
- pr_debug("PAACE configuration failed for liodn %d\n", liodn);
-
- return ret;
-}
-
static int update_liodn_stash(int liodn, struct fsl_dma_domain *dma_domain,
u32 val)
{
@@ -94,11 +75,11 @@ static int update_liodn_stash(int liodn, struct
fsl_dma_domain *dma_domain,
}
/* Set the geometry parameters for a LIODN */
-static int pamu_set_liodn(int liodn, struct device *dev,
- struct fsl_dma_domain *dma_domain,
- struct iommu_domain_geometry *geom_attr)
+static int pamu_set_liodn(struct fsl_dma_domain *dma_domain, struct device
*dev,
+ int liodn)
{
- phys_addr_t window_addr, window_size;
+ struct iommu_domain *domain = &dma_domain->iommu_domain;
+ struct iommu_domain_geometry *geom = &domain->geometry;
u32 omi_index = ~(u32)0;
unsigned long flags;
int ret;
@@ -110,22 +91,25 @@ static int pamu_set_liodn(int liodn, struct device *dev,
*/
get_ome_index(&omi_index, dev);
- window_addr = geom_attr->aperture_start;
- window_size = geom_attr->aperture_end + 1;
-
spin_lock_irqsave(&iommu_lock, flags);
ret = pamu_disable_liodn(liodn);
- if (!ret)
- ret = pamu_config_ppaace(liodn, window_addr, window_size,
omi_index,
-0, dma_domain->snoop_id,
-dma_domain->stash_id, 0);
+ if (ret)
+ goto out_unlock;
+ ret = pamu_config_ppaace(liodn, geom->aperture_start,
+geom->aperture_end - 1, omi_index, 0,
+dma_domain->snoop_id, dma_domain->stash_id, 0);
+ if (ret)
+ goto out_unlock;
+ ret = pamu_config_ppaace(liodn, geom->aperture_start,
+geom->aperture_end - 1, ~(u32)0,
+0, dma_domain->snoop_id, dma_domain->stash_id,
+PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE);
+out_unlock:
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret) {
pr_debug("PAACE configuration failed for liodn %d\n",
liodn);
- return ret;
}
-
return ret;
}
@@ -265,7 +249,6 @@ static int handle_attach_device(struct fsl_dma_domain
*dma_domain,
int num)
{
unsigned long flags;
- struct iommu_domain *domain = &dma_domain->iommu_domain;
int ret = 0;
int i;
@@ -280,21 +263,7 @@ static int handle_attach_device(struct fsl_dma_domain
*dma_domain,
}
attach_device(dma_domain, liodn[i], dev);
- /*
-* Check if geometry has already been configured
-* for the domain. If yes, set the geometry for
-* the LIODN.
-*/
- ret = pamu_set_liodn(liodn[i], dev, dma_domain,
-&domain->geometry);
- if (ret)
- break;
-
- /*
-* Create window/subwindow mapping for
-* the LIODN.
-*/
- ret = map_liodn(liodn[i], dma_domain);
+ ret = pamu_set_liodn(dma_domain, dev, liodn[i]);
if (ret)
break;
}
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 07/18] iommu/fsl_pamu: replace DOMAIN_ATTR_FSL_PAMU_STASH with a direct call
Add a fsl_pamu_configure_l1_stash API that qman_portal can call directly
instead of indirecting through the iommu attr API.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
arch/powerpc/include/asm/fsl_pamu_stash.h | 12 +++-
drivers/iommu/fsl_pamu_domain.c | 16 +++-
drivers/iommu/fsl_pamu_domain.h | 2 --
drivers/soc/fsl/qbman/qman_portal.c | 18 +++---
include/linux/iommu.h | 1 -
5 files changed, 9 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/include/asm/fsl_pamu_stash.h
b/arch/powerpc/include/asm/fsl_pamu_stash.h
index 30a31ad2123d86..c0fbadb70b5dad 100644
--- a/arch/powerpc/include/asm/fsl_pamu_stash.h
+++ b/arch/powerpc/include/asm/fsl_pamu_stash.h
@@ -7,6 +7,8 @@
#ifndef __FSL_PAMU_STASH_H
#define __FSL_PAMU_STASH_H
+struct iommu_domain;
+
/* cache stash targets */
enum pamu_stash_target {
PAMU_ATTR_CACHE_L1 = 1,
@@ -14,14 +16,6 @@ enum pamu_stash_target {
PAMU_ATTR_CACHE_L3,
};
-/*
- * This attribute allows configuring stashig specific parameters
- * in the PAMU hardware.
- */
-
-struct pamu_stash_attribute {
- u32 cpu;/* cpu number */
- u32 cache; /* cache to stash to: L1,L2,L3 */
-};
+int fsl_pamu_configure_l1_stash(struct iommu_domain *domain, u32 cpu);
#endif /* __FSL_PAMU_STASH_H */
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index fd2bc88b690465..40eff4b7bc5d42 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -372,27 +372,20 @@ static void fsl_pamu_detach_device(struct iommu_domain
*domain,
}
/* Set the domain stash attribute */
-static int configure_domain_stash(struct fsl_dma_domain *dma_domain, void
*data)
+int fsl_pamu_configure_l1_stash(struct iommu_domain *domain, u32 cpu)
{
- struct pamu_stash_attribute *stash_attr = data;
+ struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
unsigned long flags;
int ret;
spin_lock_irqsave(&dma_domain->domain_lock, flags);
-
- memcpy(&dma_domain->dma_stash, stash_attr,
- sizeof(struct pamu_stash_attribute));
-
- dma_domain->stash_id = get_stash_id(stash_attr->cache,
- stash_attr->cpu);
+ dma_domain->stash_id = get_stash_id(PAMU_ATTR_CACHE_L1, cpu);
if (dma_domain->stash_id == ~(u32)0) {
pr_debug("Invalid stash attributes\n");
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return -EINVAL;
}
-
ret = update_domain_stash(dma_domain, dma_domain->stash_id);
-
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
return ret;
@@ -426,9 +419,6 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain
*domain,
int ret = 0;
switch (attr_type) {
- case DOMAIN_ATTR_FSL_PAMU_STASH:
- ret = configure_domain_stash(dma_domain, data);
- break;
case DOMAIN_ATTR_FSL_PAMU_ENABLE:
ret = configure_domain_dma_state(dma_domain, *(int *)data);
break;
diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h
index 13ee06e0ef0136..cd488004acd1b3 100644
--- a/drivers/iommu/fsl_pamu_domain.h
+++ b/drivers/iommu/fsl_pamu_domain.h
@@ -22,9 +22,7 @@ struct fsl_dma_domain {
*
*/
int enabled;
- /* stash_id obtained from the stash attribute details */
u32 stash_id;
- struct pamu_stash_attribute dma_stash;
u32 snoop_id;
struct iommu_domain iommu_domain;
spinlock_t domain_lock;
diff --git a/drivers/soc/fsl/qbman/qman_portal.c
b/drivers/soc/fsl/qbman/qman_portal.c
index 9ee1663f422cbf..798b3a1ffd0b9c 100644
--- a/drivers/soc/fsl/qbman/qman_portal.c
+++ b/drivers/soc/fsl/qbman/qman_portal.c
@@ -47,7 +47,6 @@ static void portal_set_cpu(struct qm_portal_config *pcfg, int
cpu)
#ifdef CONFIG_FSL_PAMU
struct device *dev = pcfg->dev;
int window_count = 1;
- struct pamu_stash_attribute stash_attr;
int ret;
pcfg->iommu_domain = iommu_domain_alloc(&platform_bus_type);
@@ -55,13 +54,9 @@ static void portal_set_cpu(struct qm_portal_config *pcfg,
int cpu)
dev_err(dev, "%s(): iommu_domain_alloc() failed", __func__);
goto no_iommu;
}
- stash_attr.cpu = cpu;
- stash_attr.cache = PAMU_ATTR_CACHE_L1;
- ret = iommu_domain_set_attr(pcfg->iommu_domain,
- DOMAIN_ATTR_FSL_PAMU_STASH,
- &stash_attr);
+ ret = fsl_pamu_configure_l1_stash(pcfg->iommu_domain, cpu);
if (ret < 0) {
- dev_err(dev, "%s(): iommu_domain_set_attr() = %d",
+ dev_err(dev, "%s(): fs
[PATCH 06/18] iommu/fsl_pamu: remove ->domain_window_enable
The only thing that fsl_pamu_window_enable does for the current caller
is to fill in the prot value in the only dma_window structure, and to
propagate a few values from the iommu_domain_geometry struture into the
dma_window. Remove the dma_window entirely, hardcode the prot value and
otherwise use the iommu_domain_geometry structure instead.
Remove the now unused ->domain_window_enable iommu method.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 182 +++-
drivers/iommu/fsl_pamu_domain.h | 17 ---
drivers/iommu/iommu.c | 11 --
drivers/soc/fsl/qbman/qman_portal.c | 7 --
include/linux/iommu.h | 17 ---
5 files changed, 14 insertions(+), 220 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index e6bdd38fc18409..fd2bc88b690465 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -54,34 +54,18 @@ static int __init iommu_init_mempool(void)
return 0;
}
-static phys_addr_t get_phys_addr(struct fsl_dma_domain *dma_domain, dma_addr_t
iova)
-{
- struct dma_window *win_ptr = &dma_domain->win_arr[0];
- struct iommu_domain_geometry *geom;
-
- geom = &dma_domain->iommu_domain.geometry;
-
- if (win_ptr->valid)
- return win_ptr->paddr + (iova & (win_ptr->size - 1));
-
- return 0;
-}
-
/* Map the DMA window corresponding to the LIODN */
static int map_liodn(int liodn, struct fsl_dma_domain *dma_domain)
{
int ret;
- struct dma_window *wnd = &dma_domain->win_arr[0];
- phys_addr_t wnd_addr = dma_domain->iommu_domain.geometry.aperture_start;
+ struct iommu_domain_geometry *geom = &dma_domain->iommu_domain.geometry;
unsigned long flags;
spin_lock_irqsave(&iommu_lock, flags);
- ret = pamu_config_ppaace(liodn, wnd_addr,
-wnd->size,
-~(u32)0,
-wnd->paddr >> PAMU_PAGE_SHIFT,
-dma_domain->snoop_id, dma_domain->stash_id,
-wnd->prot);
+ ret = pamu_config_ppaace(liodn, geom->aperture_start,
+geom->aperture_end - 1, ~(u32)0,
+0, dma_domain->snoop_id, dma_domain->stash_id,
+PAACE_AP_PERMS_QUERY | PAACE_AP_PERMS_UPDATE);
spin_unlock_irqrestore(&iommu_lock, flags);
if (ret)
pr_debug("PAACE configuration failed for liodn %d\n", liodn);
@@ -89,33 +73,6 @@ static int map_liodn(int liodn, struct fsl_dma_domain
*dma_domain)
return ret;
}
-/* Update window/subwindow mapping for the LIODN */
-static int update_liodn(int liodn, struct fsl_dma_domain *dma_domain, u32
wnd_nr)
-{
- int ret;
- struct dma_window *wnd = &dma_domain->win_arr[wnd_nr];
- phys_addr_t wnd_addr;
- unsigned long flags;
-
- spin_lock_irqsave(&iommu_lock, flags);
-
- wnd_addr = dma_domain->iommu_domain.geometry.aperture_start;
-
- ret = pamu_config_ppaace(liodn, wnd_addr,
-wnd->size,
-~(u32)0,
-wnd->paddr >> PAMU_PAGE_SHIFT,
-dma_domain->snoop_id, dma_domain->stash_id,
-wnd->prot);
- if (ret)
- pr_debug("Window reconfiguration failed for liodn %d\n",
-liodn);
-
- spin_unlock_irqrestore(&iommu_lock, flags);
-
- return ret;
-}
-
static int update_liodn_stash(int liodn, struct fsl_dma_domain *dma_domain,
u32 val)
{
@@ -172,26 +129,6 @@ static int pamu_set_liodn(int liodn, struct device *dev,
return ret;
}
-static int check_size(u64 size, dma_addr_t iova)
-{
- /*
-* Size must be a power of two and at least be equal
-* to PAMU page size.
-*/
- if ((size & (size - 1)) || size < PAMU_PAGE_SIZE) {
- pr_debug("Size too small or not a power of two\n");
- return -EINVAL;
- }
-
- /* iova must be page size aligned */
- if (iova & (size - 1)) {
- pr_debug("Address is not aligned with window size\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
static void remove_device_ref(struct device_domain_info *info)
{
unsigned long flags;
@@ -257,13 +194,10 @@ static void attach_device(struct fsl_dma_domain
*dma_domain, int liodn, struct d
static phys_addr_t fsl_pamu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova)
{
- struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
-
if (iova < domain->geometry.aperture_start ||
iova > domain->geometry.aperture_end)
return 0;
-
- return g
Re: [RFC PATCH v2 06/11] dma-direct: Support PCI P2PDMA pages in dma-direct map_sg
On 2021-03-16 1:58 a.m., Christoph Hellwig wrote: > On Fri, Mar 12, 2021 at 11:27:46AM -0700, Logan Gunthorpe wrote: >> So then we reject the patches that make that change. Seems like an odd >> argument to say that we can't do something that won't cause problems >> because someone might use it as an example and do something that will >> cause problems. Reject the change that causes the problem. > > No, the problem is a mess of calling conventions. A calling convention > returning 0 for error, positive values for success is fine. One returning > a negative errno for error and positive values for success is fine a well. > One returning 0 for the usual errors and negativ errnos for an unusual > corner case is just a complete mess. Fair enough. I can try implementing a dma_map_sg_p2p() roughly as Robin suggested that has a more reasonable calling convention. Most of your other feedback seems easy enough so I'll address it in a future series. Thanks, Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/18] iommu/fsl_pamu: remove support for multiple windows
The only domains allocated forces use of a single window. Remove all
the code related to multiple window support, as well as the need for
qman_portal to force a single window.
Remove the now unused DOMAIN_ATTR_WINDOWS iommu_attr.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu.c| 264 +-
drivers/iommu/fsl_pamu.h| 10 +-
drivers/iommu/fsl_pamu_domain.c | 275 +---
drivers/iommu/fsl_pamu_domain.h | 12 +-
drivers/soc/fsl/qbman/qman_portal.c | 7 -
include/linux/iommu.h | 1 -
6 files changed, 59 insertions(+), 510 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index b9a974d9783113..3e1647cd5ad47a 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -63,19 +63,6 @@ static const struct of_device_id l3_device_ids[] = {
/* maximum subwindows permitted per liodn */
static u32 max_subwindow_count;
-/* Pool for fspi allocation */
-static struct gen_pool *spaace_pool;
-
-/**
- * pamu_get_max_subwin_cnt() - Return the maximum supported
- * subwindow count per liodn.
- *
- */
-u32 pamu_get_max_subwin_cnt(void)
-{
- return max_subwindow_count;
-}
-
/**
* pamu_get_ppaace() - Return the primary PACCE
* @liodn: liodn PAACT index for desired PAACE
@@ -155,13 +142,6 @@ static unsigned int map_addrspace_size_to_wse(phys_addr_t
addrspace_size)
return fls64(addrspace_size) - 2;
}
-/* Derive the PAACE window count encoding for the subwindow count */
-static unsigned int map_subwindow_cnt_to_wce(u32 subwindow_cnt)
-{
- /* window count is 2^(WCE+1) bytes */
- return __ffs(subwindow_cnt) - 1;
-}
-
/*
* Set the PAACE type as primary and set the coherency required domain
* attribute
@@ -174,89 +154,11 @@ static void pamu_init_ppaace(struct paace *ppaace)
PAACE_M_COHERENCE_REQ);
}
-/*
- * Set the PAACE type as secondary and set the coherency required domain
- * attribute.
- */
-static void pamu_init_spaace(struct paace *spaace)
-{
- set_bf(spaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
- set_bf(spaace->domain_attr.to_host.coherency_required, PAACE_DA_HOST_CR,
- PAACE_M_COHERENCE_REQ);
-}
-
-/*
- * Return the spaace (corresponding to the secondary window index)
- * for a particular ppaace.
- */
-static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum)
-{
- u32 subwin_cnt;
- struct paace *spaace = NULL;
-
- subwin_cnt = 1UL << (get_bf(paace->impl_attr, PAACE_IA_WCE) + 1);
-
- if (wnum < subwin_cnt)
- spaace = &spaact[paace->fspi + wnum];
- else
- pr_debug("secondary paace out of bounds\n");
-
- return spaace;
-}
-
-/**
- * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows
- *required for primary PAACE in the secondary
- *PAACE table.
- * @subwin_cnt: Number of subwindows to be reserved.
- *
- * A PPAACE entry may have a number of associated subwindows. A subwindow
- * corresponds to a SPAACE entry in the SPAACT table. Each PAACE entry stores
- * the index (fspi) of the first SPAACE entry in the SPAACT table. This
- * function returns the index of the first SPAACE entry. The remaining
- * SPAACE entries are reserved contiguously from that index.
- *
- * Returns a valid fspi index in the range of 0 - SPAACE_NUMBER_ENTRIES on
success.
- * If no SPAACE entry is available or the allocator can not reserve the
required
- * number of contiguous entries function returns ULONG_MAX indicating a
failure.
- *
- */
-static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
-{
- unsigned long spaace_addr;
-
- spaace_addr = gen_pool_alloc(spaace_pool, subwin_cnt * sizeof(struct
paace));
- if (!spaace_addr)
- return ULONG_MAX;
-
- return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
-}
-
-/* Release the subwindows reserved for a particular LIODN */
-void pamu_free_subwins(int liodn)
-{
- struct paace *ppaace;
- u32 subwin_cnt, size;
-
- ppaace = pamu_get_ppaace(liodn);
- if (!ppaace) {
- pr_debug("Invalid liodn entry\n");
- return;
- }
-
- if (get_bf(ppaace->addr_bitfields, PPAACE_AF_MW)) {
- subwin_cnt = 1UL << (get_bf(ppaace->impl_attr, PAACE_IA_WCE) +
1);
- size = (subwin_cnt - 1) * sizeof(struct paace);
- gen_pool_free(spaace_pool, (unsigned
long)&spaact[ppaace->fspi], size);
- set_bf(ppaace->addr_bitfields, PPAACE_AF_MW, 0);
- }
-}
-
/*
* Function used for updating stash destination for the coressponding
* LIODN.
*/
-int pamu_update_paace_stash(int liodn, u32 subwin, u32 value)
+int pamu_update_paace_stash(int liodn, u32 value)
{
struct paace *paace;
@@ -265,11 +167,6 @@ int pamu_update_paace_sta
[PATCH 04/18] iommu/fsl_pamu: merge iommu_alloc_dma_domain into fsl_pamu_domain_alloc
Keep the functionality to allocate the domain together.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 34 ++---
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 7bd08ddad07779..a4da5597755d3d 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -292,25 +292,6 @@ static int check_size(u64 size, dma_addr_t iova)
return 0;
}
-static struct fsl_dma_domain *iommu_alloc_dma_domain(void)
-{
- struct fsl_dma_domain *domain;
-
- domain = kmem_cache_zalloc(fsl_pamu_domain_cache, GFP_KERNEL);
- if (!domain)
- return NULL;
-
- domain->stash_id = ~(u32)0;
- domain->snoop_id = ~(u32)0;
- domain->win_cnt = pamu_get_max_subwin_cnt();
-
- INIT_LIST_HEAD(&domain->devices);
-
- spin_lock_init(&domain->domain_lock);
-
- return domain;
-}
-
static void remove_device_ref(struct device_domain_info *info, u32 win_cnt)
{
unsigned long flags;
@@ -412,12 +393,17 @@ static struct iommu_domain
*fsl_pamu_domain_alloc(unsigned type)
if (type != IOMMU_DOMAIN_UNMANAGED)
return NULL;
- dma_domain = iommu_alloc_dma_domain();
- if (!dma_domain) {
- pr_debug("dma_domain allocation failed\n");
+ dma_domain = kmem_cache_zalloc(fsl_pamu_domain_cache, GFP_KERNEL);
+ if (!dma_domain)
return NULL;
- }
- /* defaul geometry 64 GB i.e. maximum system address */
+
+ dma_domain->stash_id = ~(u32)0;
+ dma_domain->snoop_id = ~(u32)0;
+ dma_domain->win_cnt = pamu_get_max_subwin_cnt();
+ INIT_LIST_HEAD(&dma_domain->devices);
+ spin_lock_init(&dma_domain->domain_lock);
+
+ /* default geometry 64 GB i.e. maximum system address */
dma_domain->iommu_domain. geometry.aperture_start = 0;
dma_domain->iommu_domain.geometry.aperture_end = (1ULL << 36) - 1;
dma_domain->iommu_domain.geometry.force_aperture = true;
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 03/18] iommu/fsl_pamu: remove support for setting DOMAIN_ATTR_GEOMETRY
The default geometry is the same as the one set by qman_port given
that FSL_PAMU depends on having 64-bit physical and thus DMA addresses.
Remove the support to update the geometry and remove the now pointless
geom_size field.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 55 +++--
drivers/iommu/fsl_pamu_domain.h | 6
drivers/soc/fsl/qbman/qman_portal.c | 12 ---
3 files changed, 5 insertions(+), 68 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index e587ec43f7e750..7bd08ddad07779 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -62,7 +62,7 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain
*dma_domain, dma_addr_t i
geom = &dma_domain->iommu_domain.geometry;
- if (!win_cnt || !dma_domain->geom_size) {
+ if (!win_cnt) {
pr_debug("Number of windows/geometry not configured for the
domain\n");
return 0;
}
@@ -72,7 +72,7 @@ static phys_addr_t get_phys_addr(struct fsl_dma_domain
*dma_domain, dma_addr_t i
dma_addr_t subwin_iova;
u32 wnd;
- subwin_size = dma_domain->geom_size >> ilog2(win_cnt);
+ subwin_size = (geom->aperture_end + 1) >> ilog2(win_cnt);
subwin_iova = iova & ~(subwin_size - 1);
wnd = (subwin_iova - geom->aperture_start) >>
ilog2(subwin_size);
win_ptr = &dma_domain->win_arr[wnd];
@@ -234,7 +234,7 @@ static int pamu_set_liodn(int liodn, struct device *dev,
get_ome_index(&omi_index, dev);
window_addr = geom_attr->aperture_start;
- window_size = dma_domain->geom_size;
+ window_size = geom_attr->aperture_end + 1;
spin_lock_irqsave(&iommu_lock, flags);
ret = pamu_disable_liodn(liodn);
@@ -303,7 +303,6 @@ static struct fsl_dma_domain *iommu_alloc_dma_domain(void)
domain->stash_id = ~(u32)0;
domain->snoop_id = ~(u32)0;
domain->win_cnt = pamu_get_max_subwin_cnt();
- domain->geom_size = 0;
INIT_LIST_HEAD(&domain->devices);
@@ -502,7 +501,8 @@ static int fsl_pamu_window_enable(struct iommu_domain
*domain, u32 wnd_nr,
return -EINVAL;
}
- win_size = dma_domain->geom_size >> ilog2(dma_domain->win_cnt);
+ win_size = (domain->geometry.aperture_end + 1) >>
+ ilog2(dma_domain->win_cnt);
if (size > win_size) {
pr_debug("Invalid window size\n");
spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
@@ -665,41 +665,6 @@ static void fsl_pamu_detach_device(struct iommu_domain
*domain,
pr_debug("missing fsl,liodn property at %pOF\n", dev->of_node);
}
-static int configure_domain_geometry(struct iommu_domain *domain, void *data)
-{
- struct iommu_domain_geometry *geom_attr = data;
- struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
- dma_addr_t geom_size;
- unsigned long flags;
-
- geom_size = geom_attr->aperture_end - geom_attr->aperture_start + 1;
- /*
-* Sanity check the geometry size. Also, we do not support
-* DMA outside of the geometry.
-*/
- if (check_size(geom_size, geom_attr->aperture_start) ||
- !geom_attr->force_aperture) {
- pr_debug("Invalid PAMU geometry attributes\n");
- return -EINVAL;
- }
-
- spin_lock_irqsave(&dma_domain->domain_lock, flags);
- if (dma_domain->enabled) {
- pr_debug("Can't set geometry attributes as domain is active\n");
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
- return -EBUSY;
- }
-
- /* Copy the domain geometry information */
- memcpy(&domain->geometry, geom_attr,
- sizeof(struct iommu_domain_geometry));
- dma_domain->geom_size = geom_size;
-
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
-
- return 0;
-}
-
/* Set the domain stash attribute */
static int configure_domain_stash(struct fsl_dma_domain *dma_domain, void
*data)
{
@@ -769,13 +734,6 @@ static int fsl_pamu_set_windows(struct iommu_domain
*domain, u32 w_count)
return -EBUSY;
}
- /* Ensure that the geometry has been set for the domain */
- if (!dma_domain->geom_size) {
- pr_debug("Please configure geometry before setting the number
of windows\n");
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
- return -EINVAL;
- }
-
/*
* Ensure we have valid window count i.e. it should be less than
* maximum permissible limit and should be a power of two.
@@ -811,9 +769,6 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain
*domain,
int ret = 0;
switch (attr_type) {
- cas
[PATCH 02/18] iommu/fsl_pamu: remove fsl_pamu_get_domain_attr
None of the values returned by this function are ever queried. Also
remove the DOMAIN_ATTR_FSL_PAMUV1 enum value that is not otherwise used.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 30 --
include/linux/iommu.h | 4
2 files changed, 34 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 53380cf1fa452f..e587ec43f7e750 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -832,35 +832,6 @@ static int fsl_pamu_set_domain_attr(struct iommu_domain
*domain,
return ret;
}
-static int fsl_pamu_get_domain_attr(struct iommu_domain *domain,
- enum iommu_attr attr_type, void *data)
-{
- struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
- int ret = 0;
-
- switch (attr_type) {
- case DOMAIN_ATTR_FSL_PAMU_STASH:
- memcpy(data, &dma_domain->dma_stash,
- sizeof(struct pamu_stash_attribute));
- break;
- case DOMAIN_ATTR_FSL_PAMU_ENABLE:
- *(int *)data = dma_domain->enabled;
- break;
- case DOMAIN_ATTR_FSL_PAMUV1:
- *(int *)data = DOMAIN_ATTR_FSL_PAMUV1;
- break;
- case DOMAIN_ATTR_WINDOWS:
- *(u32 *)data = dma_domain->win_cnt;
- break;
- default:
- pr_debug("Unsupported attribute type\n");
- ret = -EINVAL;
- break;
- }
-
- return ret;
-}
-
static struct iommu_group *get_device_iommu_group(struct device *dev)
{
struct iommu_group *group;
@@ -987,7 +958,6 @@ static const struct iommu_ops fsl_pamu_ops = {
.domain_window_enable = fsl_pamu_window_enable,
.iova_to_phys = fsl_pamu_iova_to_phys,
.domain_set_attr = fsl_pamu_set_domain_attr,
- .domain_get_attr = fsl_pamu_get_domain_attr,
.probe_device = fsl_pamu_probe_device,
.release_device = fsl_pamu_release_device,
.device_group = fsl_pamu_device_group,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 47c8b318d8f523..52874ae164dd60 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -104,9 +104,6 @@ enum iommu_cap {
* -the actual size of the mapped region of a window must be power
* of 2 starting with 4KB and physical address must be naturally
* aligned.
- * DOMAIN_ATTR_FSL_PAMUV1 corresponds to the above mentioned contraints.
- * The caller can invoke iommu_domain_get_attr to check if the underlying
- * iommu implementation supports these constraints.
*/
enum iommu_attr {
@@ -115,7 +112,6 @@ enum iommu_attr {
DOMAIN_ATTR_WINDOWS,
DOMAIN_ATTR_FSL_PAMU_STASH,
DOMAIN_ATTR_FSL_PAMU_ENABLE,
- DOMAIN_ATTR_FSL_PAMUV1,
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
DOMAIN_ATTR_IO_PGTABLE_CFG,
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/18] iommu: remove the unused domain_window_disable method
domain_window_disable is wired up by fsl_pamu, but never actually called.
Signed-off-by: Christoph Hellwig
Acked-by: Li Yang
---
drivers/iommu/fsl_pamu_domain.c | 48 -
include/linux/iommu.h | 2 --
2 files changed, 50 deletions(-)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index b2110767caf49c..53380cf1fa452f 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -473,53 +473,6 @@ static int update_domain_mapping(struct fsl_dma_domain
*dma_domain, u32 wnd_nr)
return ret;
}
-static int disable_domain_win(struct fsl_dma_domain *dma_domain, u32 wnd_nr)
-{
- struct device_domain_info *info;
- int ret = 0;
-
- list_for_each_entry(info, &dma_domain->devices, link) {
- if (dma_domain->win_cnt == 1 && dma_domain->enabled) {
- ret = pamu_disable_liodn(info->liodn);
- if (!ret)
- dma_domain->enabled = 0;
- } else {
- ret = pamu_disable_spaace(info->liodn, wnd_nr);
- }
- }
-
- return ret;
-}
-
-static void fsl_pamu_window_disable(struct iommu_domain *domain, u32 wnd_nr)
-{
- struct fsl_dma_domain *dma_domain = to_fsl_dma_domain(domain);
- unsigned long flags;
- int ret;
-
- spin_lock_irqsave(&dma_domain->domain_lock, flags);
- if (!dma_domain->win_arr) {
- pr_debug("Number of windows not configured\n");
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
- return;
- }
-
- if (wnd_nr >= dma_domain->win_cnt) {
- pr_debug("Invalid window index\n");
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
- return;
- }
-
- if (dma_domain->win_arr[wnd_nr].valid) {
- ret = disable_domain_win(dma_domain, wnd_nr);
- if (!ret) {
- dma_domain->win_arr[wnd_nr].valid = 0;
- dma_domain->mapped--;
- }
- }
-
- spin_unlock_irqrestore(&dma_domain->domain_lock, flags);
-}
static int fsl_pamu_window_enable(struct iommu_domain *domain, u32 wnd_nr,
phys_addr_t paddr, u64 size, int prot)
@@ -1032,7 +985,6 @@ static const struct iommu_ops fsl_pamu_ops = {
.attach_dev = fsl_pamu_attach_device,
.detach_dev = fsl_pamu_detach_device,
.domain_window_enable = fsl_pamu_window_enable,
- .domain_window_disable = fsl_pamu_window_disable,
.iova_to_phys = fsl_pamu_iova_to_phys,
.domain_set_attr = fsl_pamu_set_domain_attr,
.domain_get_attr = fsl_pamu_get_domain_attr,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e7fe519430af4..47c8b318d8f523 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -209,7 +209,6 @@ struct iommu_iotlb_gather {
* @put_resv_regions: Free list of reserved regions for a device
* @apply_resv_region: Temporary helper call-back for iova reserved ranges
* @domain_window_enable: Configure and enable a particular window for a domain
- * @domain_window_disable: Disable a particular window for a domain
* @of_xlate: add OF master IDs to iommu grouping
* @is_attach_deferred: Check if domain attach should be deferred from iommu
* driver init to device driver init (default no)
@@ -270,7 +269,6 @@ struct iommu_ops {
/* Window handling functions */
int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
phys_addr_t paddr, u64 size, int prot);
- void (*domain_window_disable)(struct iommu_domain *domain, u32 wnd_nr);
int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
bool (*is_attach_deferred)(struct iommu_domain *domain, struct device
*dev);
--
2.30.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
cleanup unused or almost unused IOMMU APIs and the FSL PAMU driver v2
Hi all, there are a bunch of IOMMU APIs that are entirely unused, or only used as a private communication channel between the FSL PAMU driver and it's only consumer, the qbman portal driver. So this series drops a huge chunk of entirely unused FSL PAMU functionality, then drops all kinds of unused IOMMU APIs, and then replaces what is left of the iommu_attrs with properly typed, smaller and easier to use specific APIs. Changes since v1: - use a different way to control strict flushing behavior (from Robin) - remove the iommu_cmd_line wrappers - simplify the pagetbl quirks a little more - slightly improved patch ordering - better changelogs Diffstat: arch/powerpc/include/asm/fsl_pamu_stash.h | 12 drivers/gpu/drm/msm/adreno/adreno_gpu.c |5 drivers/iommu/amd/iommu.c | 23 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 75 --- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |1 drivers/iommu/arm/arm-smmu/arm-smmu.c | 111 +--- drivers/iommu/arm/arm-smmu/arm-smmu.h |2 drivers/iommu/dma-iommu.c |9 drivers/iommu/fsl_pamu.c| 264 -- drivers/iommu/fsl_pamu.h| 10 drivers/iommu/fsl_pamu_domain.c | 694 ++-- drivers/iommu/fsl_pamu_domain.h | 46 - drivers/iommu/intel/iommu.c | 95 --- drivers/iommu/iommu.c | 115 +--- drivers/soc/fsl/qbman/qman_portal.c | 55 -- drivers/vfio/vfio_iommu_type1.c | 31 - drivers/vhost/vdpa.c| 10 include/linux/io-pgtable.h |4 include/linux/iommu.h | 76 --- 19 files changed, 203 insertions(+), 1435 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
On Tue, Mar 16, 2021 at 09:36:02PM +0800, Huang Rui wrote: > Thanks for the comments. Could you please elaborate this? > > Do you mean while amd_iommu=off, we won't prepare the IVRS, and even > needn't get all ACPI talbes. Because they are never be used and the next > state will always goes into IOMMU_CMDLINE_DISABLED, am I right? The first problem was that amd_iommu_irq_remap is never set back to false when irq-remapping initialization fails in amd_iommu_prepare(). But there are other problems, like that even when the IOMMU is set to disabled on the command line with amd_iommu=off, the code still sets up all IOMMUs and registers IRQ domains for them. Later the code checks wheter the IOMMU should stay disabled and tears everything down, except for the IRQ domains, which stay in the global list. The APIs do not really support tearing down IRQ domains well, so its not so easy to add this to the tear-down path. Now that the IRQ domains stay in the list, the ACPI code will come along later and calls the ->select() call-back for every IRQ domain, which gets execution to irq_remapping_select(), depite IOMMU being disabled and amd_iommu_rlookup_table already de-allocated. But since amd_iommu_irq_remap is still true the NULL pointer is dereferenced, causing the crash. When the IRQ domains would not be around, this would also not happen. So my patches also change the initializtion to not do all the setup work when amd_iommu=off was passed on the command line. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
On Tue, Mar 16, 2021 at 09:16:34PM +0800, Joerg Roedel wrote: > Hi Huang, > > On Thu, Mar 11, 2021 at 10:28:07PM +0800, Huang Rui wrote: > > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c > > index f0adbc48fd17..a08e885403b7 100644 > > --- a/drivers/iommu/amd/iommu.c > > +++ b/drivers/iommu/amd/iommu.c > > @@ -3862,7 +3862,7 @@ static int irq_remapping_select(struct irq_domain *d, > > struct irq_fwspec *fwspec, > > else if (x86_fwspec_is_hpet(fwspec)) > > devid = get_hpet_devid(fwspec->param[0]); > > > > - if (devid < 0) > > + if (devid < 0 || !amd_iommu_rlookup_table) > > return 0; > > The problem is deeper than this fix suggests. I prepared other fixes for > this particular problem. Please find them here: > Thanks for the comments. Could you please elaborate this? Do you mean while amd_iommu=off, we won't prepare the IVRS, and even needn't get all ACPI talbes. Because they are never be used and the next state will always goes into IOMMU_CMDLINE_DISABLED, am I right? Thanks, Ray > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fjoro%2Flinux.git%2Flog%2F%3Fh%3Diommu-fixes&data=04%7C01%7Cray.huang%40amd.com%7Cce731dda3b444ac9a14108d8e87dbb3e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637514974013915073%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NVahf1tIfgno%2BNWPu8hY4DygiGWdKXBJ8G6OsD%2BHC14%3D&reserved=0 > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Fix iommu remap panic while amd_iommu is set to disable
Hi Huang, On Thu, Mar 11, 2021 at 10:28:07PM +0800, Huang Rui wrote: > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c > index f0adbc48fd17..a08e885403b7 100644 > --- a/drivers/iommu/amd/iommu.c > +++ b/drivers/iommu/amd/iommu.c > @@ -3862,7 +3862,7 @@ static int irq_remapping_select(struct irq_domain *d, > struct irq_fwspec *fwspec, > else if (x86_fwspec_is_hpet(fwspec)) > devid = get_hpet_devid(fwspec->param[0]); > > - if (devid < 0) > + if (devid < 0 || !amd_iommu_rlookup_table) > return 0; The problem is deeper than this fix suggests. I prepared other fixes for this particular problem. Please find them here: https://git.kernel.org/pub/scm/linux/kernel/git/joro/linux.git/log/?h=iommu-fixes Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 14/17] iommu: remove DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE
On 2021-03-15 08:33, Christoph Hellwig wrote: On Fri, Mar 12, 2021 at 04:18:24PM +, Robin Murphy wrote: Let me know what you think of the version here: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/iommu-cleanup I'll happily switch the patch to you as the author if you're fine with that as well. I still have reservations about removing the attribute API entirely and pretending that io_pgtable_cfg is anything other than a SoC-specific private interface, I think a private inteface would make more sense. For now I've just condensed it down to a generic set of quirk bits and dropped the attrs structure, which seems like an ok middle ground for now. That being said I wonder why that quirk isn't simply set in the device tree? Because it's a software policy decision rather than any inherent property of the platform, and the DT certainly doesn't know *when* any particular device might prefer its IOMMU to use cacheable pagetables to minimise TLB miss latency vs. saving the cache capacity for larger data buffers. It really is most logical to decide this at the driver level. In truth the overall concept *is* relatively generic (a trend towards larger system caches and cleverer usage is about both raw performance and saving power on off-SoC DRAM traffic), it's just the particular implementation of using io-pgtable to set an outer-cacheable walk attribute in an SMMU TCR that's pretty much specific to Qualcomm SoCs. Hence why having a common abstraction at the iommu_domain level, but where the exact details are free to vary across different IOMMUs and their respective client drivers, is in many ways an ideal fit. but the reworked patch on its own looks reasonable to me, thanks! (I wasn't too convinced about the iommu_cmd_line wrappers either...) Just iommu_get_dma_strict() needs an export since the SMMU drivers can be modular - I consciously didn't add that myself since I was mistakenly thinking only iommu-dma would call it. Fixed. Can I get your signoff for the patch? Then I'll switch it to over to being attributed to you. Sure - I would have thought that the one I originally posted still stands, but for the avoidance of doubt, for the parts of commit 8b6d45c495bd in your tree that remain from what I wrote: Signed-off-by: Robin Murphy Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 04/11] iommu/arm-smmu-v3: Split block descriptor when start dirty log
Hi Yi, On 2021/3/16 17:17, Yi Sun wrote: > On 21-03-10 17:06:07, Keqian Zhu wrote: >> From: jiangkunkun >> >> Block descriptor is not a proper granule for dirty log tracking. >> Take an extreme example, if DMA writes one byte, under 1G mapping, >> the dirty amount reported to userspace is 1G, but under 4K mapping, >> the dirty amount is just 4K. >> >> This adds a new interface named start_dirty_log in iommu layer and >> arm smmuv3 implements it, which splits block descriptor to an span >> of page descriptors. Other types of IOMMU will perform architecture >> specific actions to start dirty log. >> >> To allow code reuse, the split_block operation is realized as an >> iommu_ops too. We flush all iotlbs after the whole procedure is >> completed to ease the pressure of iommu, as we will hanle a huge >> range of mapping in general. >> >> Spliting block does not simultaneously work with other pgtable ops, >> as the only designed user is vfio, which always hold a lock, so race >> condition is not considered in the pgtable ops. >> >> Co-developed-by: Keqian Zhu >> Signed-off-by: Kunkun Jiang >> --- >> >> changelog: >> >> v2: >> - Change the return type of split_block(). size_t -> int. >> - Change commit message to properly describe race condition. (Robin) >> - Change commit message to properly describe the need of split block. >> - Add a new interface named start_dirty_log(). (Sun Yi) >> - Change commit message to explain the realtionship of split_block() and >> start_dirty_log(). >> >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 52 + >> drivers/iommu/io-pgtable-arm.c | 122 >> drivers/iommu/iommu.c | 48 >> include/linux/io-pgtable.h | 2 + >> include/linux/iommu.h | 24 >> 5 files changed, 248 insertions(+) >> > Could you please split iommu common interface to a separate patch? > This may make review and comments easier. Yup, good suggestion. > > IMHO, I think the start/stop interfaces could be merged into one, e.g: > int iommu_domain_set_hwdbm(struct iommu_domain *domain, bool enable, >unsigned long iova, size_t size, >int prot); Looks good, this reduces some code. but I have a concern that this causes loss of flexibility, as we must pass same arguments when start|stop dirty log. What's your opinion about this? > > Same comments to patch 5. OK. Thanks. > > BRs, > Yi Sun > >> -- >> 2.19.1 > . Thanks, Keqian ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5] iommu/tegra-smmu: Add pagetable mappings to debugfs
On Mon, Mar 15, 2021 at 01:36:31PM -0700, Nicolin Chen wrote:
> This patch dumps all active mapping entries from pagetable
> to a debugfs directory named "mappings".
>
> Attaching an example:
>
> SWGROUP: hc
> ASID: 0
> reg: 0x250
> PTB_ASID: 0xe0080004
> as->pd_dma: 0x80004000
> {
> [1023] 0xf008000b (1)
> {
> PTE RANGE | ATTR | PHYS | IOVA
> | SIZE
> [#1023, #1023] | 0x5 | 0x000111a8d000 |
> 0xf000 | 0x1000
> }
> }
> Total PDE count: 1
> Total PTE count: 1
>
> Tested-by: Dmitry Osipenko
> Reviewed-by: Dmitry Osipenko
> Signed-off-by: Nicolin Chen
> ---
>
> Changelog
> v5:
> * Fixed a typo in commit message
> * Splitted a long line into two lines
> * Rearranged variable defines by length
> * Added Tested-by and Reviewed-by from Dmitry
> v4: https://lkml.org/lkml/2021/3/14/429
> * Changed %d to %u for unsigned variables
> * Fixed print format mismatch warnings on ARM32
> v3: https://lkml.org/lkml/2021/3/14/30
> * Fixed PHYS and IOVA print formats
> * Changed variables to unsigned int type
> * Changed the table outputs to be compact
> v2: https://lkml.org/lkml/2021/3/9/1382
> * Expanded mutex range to the entire function
> * Added as->lock to protect pagetable walkthrough
> * Replaced devm_kzalloc with devm_kcalloc for group_debug
> * Added "PTE RANGE" and "SIZE" columns to group contiguous mappings
> * Dropped as->count check; added WARN_ON when as->count mismatches
> pd[pd_index]
> v1: https://lkml.org/lkml/2020/9/26/70
>
> drivers/iommu/tegra-smmu.c | 181 -
> 1 file changed, 176 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> index 97eb62f667d2..b728cae63314 100644
> --- a/drivers/iommu/tegra-smmu.c
> +++ b/drivers/iommu/tegra-smmu.c
> @@ -19,6 +19,11 @@
> #include
> #include
>
> +struct tegra_smmu_group_debug {
> + const struct tegra_smmu_swgroup *group;
> + void *priv;
This always stores the address space, so why not make this:
struct tegra_smmu_as *as;
? While at it, perhaps throw in a const to make sure we don't modify
this structure in the debugfs code.
> +};
> +
> struct tegra_smmu_group {
> struct list_head list;
> struct tegra_smmu *smmu;
> @@ -47,6 +52,8 @@ struct tegra_smmu {
> struct dentry *debugfs;
>
> struct iommu_device iommu; /* IOMMU Core code handle */
> +
> + struct tegra_smmu_group_debug *group_debug;
> };
>
> struct tegra_smmu_as {
> @@ -152,6 +159,9 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu,
> unsigned long offset)
>
> #define SMMU_PDE_ATTR(SMMU_PDE_READABLE | SMMU_PDE_WRITABLE
> | \
>SMMU_PDE_NONSECURE)
> +#define SMMU_PTE_ATTR(SMMU_PTE_READABLE | SMMU_PTE_WRITABLE
> | \
> + SMMU_PTE_NONSECURE)
> +#define SMMU_PTE_ATTR_SHIFT (29)
No need for the parentheses here.
>
> static unsigned int iova_pd_index(unsigned long iova)
> {
> @@ -163,6 +173,12 @@ static unsigned int iova_pt_index(unsigned long iova)
> return (iova >> SMMU_PTE_SHIFT) & (SMMU_NUM_PTE - 1);
> }
>
> +static unsigned long pd_pt_index_iova(unsigned int pd_index, unsigned int
> pt_index)
> +{
> + return ((dma_addr_t)pd_index & (SMMU_NUM_PDE - 1)) << SMMU_PDE_SHIFT |
> +((dma_addr_t)pt_index & (SMMU_NUM_PTE - 1)) << SMMU_PTE_SHIFT;
> +}
> +
> static bool smmu_dma_addr_valid(struct tegra_smmu *smmu, dma_addr_t addr)
> {
> addr >>= 12;
> @@ -334,7 +350,7 @@ static void tegra_smmu_domain_free(struct iommu_domain
> *domain)
> }
>
> static const struct tegra_smmu_swgroup *
> -tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup)
> +tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned int swgroup, int
> *index)
> {
> const struct tegra_smmu_swgroup *group = NULL;
> unsigned int i;
> @@ -342,6 +358,8 @@ tegra_smmu_find_swgroup(struct tegra_smmu *smmu, unsigned
> int swgroup)
> for (i = 0; i < smmu->soc->num_swgroups; i++) {
> if (smmu->soc->swgroups[i].swgroup == swgroup) {
> group = &smmu->soc->swgroups[i];
> + if (index)
> + *index = i;
This doesn't look like the right place for this. And this also makes
things hard to follow because it passes out-of-band data in the index
parameter.
I'm thinking that this could benefit from a bit of refactoring where
we could for example embed struct tegra_smmu_group_debug into struct
tegra_smmu_group and then reference that when necessary instead of
carrying all that data in an orthogonal array. That should also make
it easier to track this.
Come to think of it, everything that's currently in your new struct
tegra_smmu_group_debug would be useful in struct tegra_smmu_group,
irrespective of debugfs support.
>
Re: [PATCH v2 04/11] iommu/arm-smmu-v3: Split block descriptor when start dirty log
On 21-03-10 17:06:07, Keqian Zhu wrote: > From: jiangkunkun > > Block descriptor is not a proper granule for dirty log tracking. > Take an extreme example, if DMA writes one byte, under 1G mapping, > the dirty amount reported to userspace is 1G, but under 4K mapping, > the dirty amount is just 4K. > > This adds a new interface named start_dirty_log in iommu layer and > arm smmuv3 implements it, which splits block descriptor to an span > of page descriptors. Other types of IOMMU will perform architecture > specific actions to start dirty log. > > To allow code reuse, the split_block operation is realized as an > iommu_ops too. We flush all iotlbs after the whole procedure is > completed to ease the pressure of iommu, as we will hanle a huge > range of mapping in general. > > Spliting block does not simultaneously work with other pgtable ops, > as the only designed user is vfio, which always hold a lock, so race > condition is not considered in the pgtable ops. > > Co-developed-by: Keqian Zhu > Signed-off-by: Kunkun Jiang > --- > > changelog: > > v2: > - Change the return type of split_block(). size_t -> int. > - Change commit message to properly describe race condition. (Robin) > - Change commit message to properly describe the need of split block. > - Add a new interface named start_dirty_log(). (Sun Yi) > - Change commit message to explain the realtionship of split_block() and > start_dirty_log(). > > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 52 + > drivers/iommu/io-pgtable-arm.c | 122 > drivers/iommu/iommu.c | 48 > include/linux/io-pgtable.h | 2 + > include/linux/iommu.h | 24 > 5 files changed, 248 insertions(+) > Could you please split iommu common interface to a separate patch? This may make review and comments easier. IMHO, I think the start/stop interfaces could be merged into one, e.g: int iommu_domain_set_hwdbm(struct iommu_domain *domain, bool enable, unsigned long iova, size_t size, int prot); Same comments to patch 5. BRs, Yi Sun > -- > 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v13 00/15] SMMUv3 Nested Stage Setup (IOMMU part)
Hi Krishna, On 3/15/21 7:04 PM, Krishna Reddy wrote: > Tested-by: Krishna Reddy > >> 1) pass the guest stage 1 configuration > > Validated Nested SMMUv3 translations for NVMe PCIe device from Guest VM along > with patch series "v11 SMMUv3 Nested Stage Setup (VFIO part)" and QEMU patch > series "vSMMUv3/pSMMUv3 2 stage VFIO integration" from v5.2.0-2stage-rfcv8. > NVMe PCIe device is functional with 2-stage translations and no issues > observed. Thank you very much for your testing efforts. For your info, there are more recent kernel series: [PATCH v14 00/13] SMMUv3 Nested Stage Setup (IOMMU part) (Feb 23) [PATCH v12 00/13] SMMUv3 Nested Stage Setup (VFIO part) (Feb 23) working along with QEMU RFC [RFC v8 00/28] vSMMUv3/pSMMUv3 2 stage VFIO integration (Feb 25) If you have cycles to test with those, this would be higly appreciated. Thanks Eric > > -KR > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 07/11] dma-mapping: Add flags to dma_map_ops to indicate PCI P2PDMA support
On Thu, Mar 11, 2021 at 04:31:37PM -0700, Logan Gunthorpe wrote:
> +int dma_pci_p2pdma_supported(struct device *dev)
> +{
> + const struct dma_map_ops *ops = get_dma_ops(dev);
> +
> + return !ops || ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED;
> +}
> +EXPORT_SYMBOL(dma_pci_p2pdma_supported);
EXPORT_SYMBOL_GPL like all new DMA APIs.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 04/11] PCI/P2PDMA: Introduce pci_p2pdma_should_map_bus() and pci_p2pdma_bus_offset()
On Thu, Mar 11, 2021 at 04:31:34PM -0700, Logan Gunthorpe wrote:
> Introduce pci_p2pdma_should_map_bus() which is meant to be called by
> DMA map functions to determine how to map a given p2pdma page.
>
> pci_p2pdma_bus_offset() is also added to allow callers to get the bus
> offset if they need to map the bus address.
>
> Signed-off-by: Logan Gunthorpe
> ---
> drivers/pci/p2pdma.c | 50 ++
> include/linux/pci-p2pdma.h | 11 +
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 7f6836732bce..66d16b7eb668 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -912,6 +912,56 @@ void pci_p2pdma_unmap_sg_attrs(struct device *dev,
> struct scatterlist *sg,
> }
> EXPORT_SYMBOL_GPL(pci_p2pdma_unmap_sg_attrs);
>
> +/**
> + * pci_p2pdma_bus_offset - returns the bus offset for a given page
> + * @page: page to get the offset for
> + *
> + * Must be passed a PCI p2pdma page.
> + */
> +u64 pci_p2pdma_bus_offset(struct page *page)
> +{
> + struct pci_p2pdma_pagemap *p2p_pgmap = to_p2p_pgmap(page->pgmap);
> +
> + WARN_ON(!is_pci_p2pdma_page(page));
> +
> + return p2p_pgmap->bus_offset;
> +}
> +EXPORT_SYMBOL_GPL(pci_p2pdma_bus_offset);
I don't see why this would be exported.
> +EXPORT_SYMBOL_GPL(pci_p2pdma_dma_map_type);
Same here. Also the two helpers don't really look very related.
I suspect you really want to move the p2p handling bits currenly added
to kernel/dma/direct.c into drivers/pci/p2pdma.c instead, as that will
allow direct access to the pci_p2pdma_pagemap and should thus simplify
things quite a bit.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 06/11] dma-direct: Support PCI P2PDMA pages in dma-direct map_sg
On Thu, Mar 11, 2021 at 04:31:36PM -0700, Logan Gunthorpe wrote:
> for_each_sg(sgl, sg, nents, i) {
> + if (is_pci_p2pdma_page(sg_page(sg))) {
> + if (sg_page(sg)->pgmap != pgmap) {
> + pgmap = sg_page(sg)->pgmap;
> + map = pci_p2pdma_dma_map_type(dev, pgmap);
> + bus_off = pci_p2pdma_bus_offset(sg_page(sg));
> + }
> +
> + if (map < 0) {
> + sg->dma_address = DMA_MAPPING_ERROR;
> + ret = -EREMOTEIO;
> + goto out_unmap;
> + }
> +
> + if (map) {
> + sg->dma_address = sg_phys(sg) + sg->offset -
> + bus_off;
> + sg_dma_len(sg) = sg->length;
> + sg_mark_pci_p2pdma(sg);
> + continue;
> + }
> + }
This code needs to go into a separate noinline helper to reduce the impact
on the fast path. Also as Robin noted the offset is already
accounted for in sg_phys. We also don't ever set the dma_address in
the scatterlist to DMA_MAPPING_ERROR, that is just a return value
for the single entry mapping routines.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 09/11] block: Add BLK_STS_P2PDMA
On Thu, Mar 11, 2021 at 04:31:39PM -0700, Logan Gunthorpe wrote: > Create a specific error code for when P2PDMA pages are passed to a block > devices that cannot map them (due to no IOMMU support or ACS protections). > > This makes request errors in these cases more informative of as to what > caused the error. I really don't think we should bother with a specific error code here, we don't add a new status for every single possible logic error in the caller. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 07/11] dma-mapping: Add flags to dma_map_ops to indicate PCI P2PDMA support
On Mon, Mar 15, 2021 at 10:33:13AM -0600, Logan Gunthorpe wrote: > >> + return !ops || ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED; > > > > Is this logic correct? I would have expected. > > > > return (ops && ops->flags & DMA_F_PCI_P2PDMA_SUPPORTED); > > > If ops is NULL then the operations in kernel/dma/direct.c are used and > support is added to those in patch 6. So it is correct as written. It is not quite that easy. There also is the bypass flag and for the specific case where that is ignored the code needs a really good comment. And to assist that formatted so that it makes sense. The above line is indeed highly confusing even if it ends up being correct. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 06/11] dma-direct: Support PCI P2PDMA pages in dma-direct map_sg
On Fri, Mar 12, 2021 at 11:27:46AM -0700, Logan Gunthorpe wrote: > So then we reject the patches that make that change. Seems like an odd > argument to say that we can't do something that won't cause problems > because someone might use it as an example and do something that will > cause problems. Reject the change that causes the problem. No, the problem is a mess of calling conventions. A calling convention returning 0 for error, positive values for success is fine. One returning a negative errno for error and positive values for success is fine a well. One returning 0 for the usual errors and negativ errnos for an unusual corner case is just a complete mess. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 06/11] dma-direct: Support PCI P2PDMA pages in dma-direct map_sg
On Fri, Mar 12, 2021 at 06:11:17PM +, Robin Murphy wrote: > Sure, that's how things stand immediately after this patch. But then > someone comes along with the perfectly reasonable argument for returning > more expressive error information for regular mapping failures as well > (because sometimes those can be terminal too, as above), we start to get > divergent behaviour across architectures and random bits of old code subtly > breaking down the line. *That* is what makes me wary of making a > fundamental change to a long-standing "nonzero means success" interface... Agreed. IMHO dma_map_sg actually needs to be switched to return unsigned to help root this out, going the other way is no helpful. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
