Re: [PATCH 1/4] dma-mapping: Add bounced DMA ops
On Tue, Jul 14, 2020 at 7:01 PM Christoph Hellwig wrote: > > On Mon, Jul 13, 2020 at 12:55:43PM +0100, Robin Murphy wrote: > > On 2020-07-13 10:12, Claire Chang wrote: > >> The bounced DMA ops provide an implementation of DMA ops that bounce > >> streaming DMA in and out of a specially allocated region. Only the > >> operations relevant to streaming DMA are supported. > > > > I think there are too many implicit assumptions here - apparently that > > coherent allocations will always be intercepted by > > dma_*_from_dev_coherent(), and that calling into dma-direct won't actually > > bounce things a second time beyond where you thought they were going, > > manage coherency for a different address, and make it all go subtly wrong. > > Consider "swiotlb=force", for instance... > > > > Again, plumbing this straight into dma-direct so that SWIOTLB can simply > > target a different buffer and always bounce regardless of masks would seem > > a far better option. > > I haven't really had time to read through the details, but I agree that > any bouncing scheme should reuse the swiotlb code and not invent a > parallel infrastructure. Thanks for the feedback. I'll try to reuse SWIOTLB. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/4] Bounced DMA support
On Mon, Jul 13, 2020 at 7:40 PM Robin Murphy wrote: > > On 2020-07-13 10:12, Claire Chang wrote: > > This series implements mitigations for lack of DMA access control on > > systems without an IOMMU, which could result in the DMA accessing the > > system memory at unexpected times and/or unexpected addresses, possibly > > leading to data leakage or corruption. > > > > For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus > > is not behind an IOMMU. As PCI-e, by design, gives the device full > > access to system memory, a vulnerability in the Wi-Fi firmware could > > easily escalate to a full system exploit (remote wifi exploits: [1a], > > [1b] that shows a full chain of exploits; [2], [3]). > > > > To mitigate the security concerns, we introduce bounced DMA. The bounced > > DMA ops provide an implementation of DMA ops that bounce streaming DMA > > in and out of a specially allocated region. The feature on its own > > provides a basic level of protection against the DMA overwriting buffer > > contents at unexpected times. However, to protect against general data > > leakage and system memory corruption, the system needs to provide a way > > to restrict the DMA to a predefined memory region (this is usually done > > at firmware level, e.g. in ATF on some ARM platforms). > > More to the point, this seems to need some fairly special interconnect > hardware too. On typical systems that just stick a TZASC directly in > front of the memory controller it would be hard to block DMA access > without also blocking CPU access. With something like Arm TZC-400 I > guess you could set up a "secure" region for most of DRAM that allows NS > accesses by NSAID from the CPUs, then similar regions for the pools with > NSAID access for both the respective device and the CPUs, but by that > point you've probably used up most of the available regions before even > considering what the firmware and TEE might want for actual Secure memory. > > In short, I don't foresee this being used by very many systems. We're going to use this on MTK SoC with MPU (memory protection unit) to restrict the DMA access for PCI-e Wi-Fi. > > That said,, although the motivation is different, it appears to end up > being almost exactly the same end result as the POWER secure > virtualisation thingy (essentially: constrain DMA to a specific portion > of RAM). The more code can be shared with that, the better. Could you share a bit more about the POWER secure virtualisation thingy? > > > Currently, 32-bit architectures are not supported because of the need to > > handle HIGHMEM, which increases code complexity and adds more > > performance penalty for such platforms. Also, bounced DMA can not be > > enabled on devices behind an IOMMU, as those require an IOMMU-aware > > implementation of DMA ops and do not require this kind of mitigation > > anyway. > > Note that we do actually have the notion of bounced DMA with IOMMUs > already (to avoid leakage of unrelated data in the same page). I think > it's only implemented for intel-iommu so far, but shouldn't take much > work to generalise to iommu-dma if anyone wanted to. That's already done > a bunch of work to generalise the SWIOTLB routines to be more reusable, > so building on top of that would be highly preferable. Yes, I'm aware of that and I'll try to put this on top of SWIOTLB. > > Thirdly, the concept of device-private bounce buffers does in fact > already exist to some degree too - there are various USB, crypto and > other devices that can only DMA to a local SRAM buffer (not to mention > subsystems doing their own bouncing for the sake of alignment/block > merging/etc.). Again, a slightly more generalised solution that makes > this a first-class notion for dma-direct itself and could help supplant > some of those hacks would be really really good. > > Robin. > > > [1a] > > https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html > > [1b] > > https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html > > [2] https://blade.tencent.com/en/advisories/qualpwn/ > > [3] > > https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/ > > > > > > Claire Chang (4): > >dma-mapping: Add bounced DMA ops > >dma-mapping: Add bounced DMA pool > >dt-bindings: of: Add plumbing for bounced DMA pool > >of: Add plumbing for bounced DMA pool > > > > .../reserved-memory/reserved-memory.txt | 36 +++ > > drivers/of/address.c | 37 +++ > > drivers/of/device.c | 3 + > > drivers/of/of_private.h | 6 + > > include/linux/device.h| 3 + > > include/linux/dma-mapping.h | 1 + > > kernel/dma/Kconfig| 17 + > > kernel/dma/Makefile | 1 + > > kernel/dma/bounced.c
RE: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
> From: Lu Baolu > Sent: Wednesday, July 15, 2020 9:00 AM > > Hi Christoph and Jacob, > > On 7/15/20 12:29 AM, Jacob Pan wrote: > > On Tue, 14 Jul 2020 09:25:14 +0100 > > Christoph Hellwig wrote: > > > >> On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > >>> Replace iommu_aux_at(de)tach_device() with > >>> iommu_aux_at(de)tach_group(). It also saves the > >>> IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data > >>> structure so that it could be reused in other places. > >> This removes the last user of iommu_aux_attach_device and > >> iommu_aux_detach_device, which can be removed now. > > it is still used in patch 2/4 inside iommu_aux_attach_group(), right? > > > > There is a need to use this interface. For example, an aux-domain is > attached to a subset of a physical device and used in the kernel. In > this usage scenario, there's no need to use vfio/mdev. The device driver > could just allocate an aux-domain and call iommu_aux_attach_device() to > setup the iommu. > and here is one example usage for adding per-instance pagetables for drm/msm: https://lore.kernel.org/lkml/20200626200414.14382-5-jcro...@codeaurora.org/ Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Hi Christoph and Jacob, On 7/15/20 12:29 AM, Jacob Pan wrote: On Tue, 14 Jul 2020 09:25:14 +0100 Christoph Hellwig wrote: On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group(). It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data structure so that it could be reused in other places. This removes the last user of iommu_aux_attach_device and iommu_aux_detach_device, which can be removed now. it is still used in patch 2/4 inside iommu_aux_attach_group(), right? There is a need to use this interface. For example, an aux-domain is attached to a subset of a physical device and used in the kernel. In this usage scenario, there's no need to use vfio/mdev. The device driver could just allocate an aux-domain and call iommu_aux_attach_device() to setup the iommu. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/4] iommu: Add iommu_aux_at(de)tach_group()
Hi Jacob,
On 7/15/20 12:39 AM, Jacob Pan wrote:
On Tue, 14 Jul 2020 13:57:01 +0800
Lu Baolu wrote:
This adds two new aux-domain APIs for a use case like vfio/mdev where
sub-devices derived from an aux-domain capable device are created and
put in an iommu_group.
/**
* iommu_aux_attach_group - attach an aux-domain to an iommu_group
which
* contains sub-devices (for example mdevs)
derived
* from @dev.
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from
@dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* Returns 0 on success, or an error value.
*/
int iommu_aux_attach_group(struct iommu_domain *domain,
struct iommu_group *group,
struct device *dev)
/**
* iommu_aux_detach_group - detach an aux-domain from an iommu_group
*
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from
@dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* @domain must have been attached to @group via
iommu_aux_attach_group(). */
void iommu_aux_detach_group(struct iommu_domain *domain,
struct iommu_group *group,
struct device *dev)
It also adds a flag in the iommu_group data structure to identify
an iommu_group with aux-domain attached from those normal ones.
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 58
+++ include/linux/iommu.h |
17 + 2 files changed, 75 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e1fdd3531d65..cad5a19ebf22 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -45,6 +45,7 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ unsigned int aux_domain_attached:1;
};
struct group_device {
@@ -2759,6 +2760,63 @@ int iommu_aux_get_pasid(struct iommu_domain
*domain, struct device *dev) }
EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
+/**
+ * iommu_aux_attach_group - attach an aux-domain to an iommu_group
which
+ * contains sub-devices (for example mdevs)
derived
+ * from @dev.
+ * @domain: an aux-domain;
+ * @group: an iommu_group which contains sub-devices derived from
@dev;
+ * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
+ *
+ * Returns 0 on success, or an error value.
+ */
+int iommu_aux_attach_group(struct iommu_domain *domain,
+ struct iommu_group *group, struct device
*dev) +{
+ int ret = -EBUSY;
+
+ mutex_lock(>mutex);
+ if (group->domain)
+ goto out_unlock;
+
Perhaps I missed something but are we assuming only one mdev per mdev
group? That seems to change the logic where vfio does:
iommu_group_for_each_dev()
iommu_aux_attach_device()
It has been changed in PATCH 4/4:
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
return iommu_aux_attach_group(domain->domain,
group->iommu_group,
group->iommu_device);
else
return iommu_attach_group(domain->domain,
group->iommu_group);
}
So, for both normal domain and aux-domain, we use the same concept:
attach a domain to a group.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/5] docs: IOMMU user API
On Tue, 14 Jul 2020 13:04:12 -0600 Alex Williamson wrote: > On Mon, 13 Jul 2020 22:00:23 -0700 > Jacob Pan wrote: > > > Hi Alex, > > > > On Mon, 13 Jul 2020 16:48:42 -0600 > > Alex Williamson wrote: > > > > > On Tue, 7 Jul 2020 16:43:45 -0700 > > > Jacob Pan wrote: > > > > > > > IOMMU UAPI is newly introduced to support communications between > > > > guest virtual IOMMU and host IOMMU. There has been lots of > > > > discussions on how it should work with VFIO UAPI and userspace > > > > in general. > > > > > > > > This document is indended to clarify the UAPI design and usage. > > > > The mechenics of how future extensions should be achieved are > > > > also covered > > > > > > mechanics > > > > > will fix. > > > > > > in this documentation. > > > > > > > > Signed-off-by: Liu Yi L > > > > Signed-off-by: Jacob Pan > > > > --- > > > > Documentation/userspace-api/iommu.rst | 312 > > > > ++ 1 file changed, 312 > > > > insertions(+) create mode 100644 > > > > Documentation/userspace-api/iommu.rst > > > > > > > > diff --git a/Documentation/userspace-api/iommu.rst > > > > b/Documentation/userspace-api/iommu.rst new file mode 100644 > > > > index ..581b462c2cec > > > > --- /dev/null > > > > +++ b/Documentation/userspace-api/iommu.rst > > > > @@ -0,0 +1,312 @@ > > > > +.. SPDX-License-Identifier: GPL-2.0 > > > > +.. iommu: > > > > + > > > > += > > > > +IOMMU Userspace API > > > > += > > > > + > > > > +IOMMU UAPI is used for virtualization cases where > > > > communications are +needed between physical and virtual IOMMU > > > > drivers. For native +usage, IOMMU is a system device which does > > > > not need to communicate +with user space directly. > > > > + > > > > +The primary use cases are guest Shared Virtual Address (SVA) > > > > and +guest IO virtual address (IOVA), wherein a virtual IOMMU > > > > (vIOMMU) is +required to communicate with the physical IOMMU in > > > > the host. + > > > > +.. contents:: :local: > > > > + > > > > +Functionalities > > > > +=== > > > > +Communications of user and kernel involve both directions. The > > > > +supported user-kernel APIs are as follows: > > > > + > > > > +1. Alloc/Free PASID > > > > +2. Bind/unbind guest PASID (e.g. Intel VT-d) > > > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU) > > > > +4. Invalidate IOMMU caches > > > > +5. Service page requests > > > > + > > > > +Requirements > > > > + > > > > +The IOMMU UAPIs are generic and extensible to meet the > > > > following +requirements: > > > > + > > > > +1. Emulated and para-virtualised vIOMMUs > > > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.) > > > > +3. Extensions to the UAPI shall not break existing user space > > > > + > > > > +Interfaces > > > > +== > > > > +Although the data structures defined in IOMMU UAPI are > > > > self-contained, +there is no user API functions introduced. > > > > Instead, IOMMU UAPI is +designed to work with existing user > > > > driver frameworks such as VFIO. + > > > > +Extension Rules & Precautions > > > > +- > > > > +When IOMMU UAPI gets extended, the data structures can *only* > > > > be +modified in two ways: > > > > + > > > > +1. Adding new fields by re-purposing the padding[] field. No > > > > size change. +2. Adding new union members at the end. May > > > > increase in size. + > > > > +No new fields can be added *after* the variable sized union in > > > > that it +will break backward compatibility when offset moves. In > > > > both cases, a +new flag must be accompanied with a new field > > > > such that the IOMMU +driver can process the data based on the > > > > new flag. Version field is +only reserved for the unlikely > > > > event of UAPI upgrade at its entirety. + > > > > +It's *always* the caller's responsibility to indicate the size > > > > of the +structure passed by setting argsz appropriately. > > > > +Though at the same time, argsz is user provided data which is > > > > not +trusted. The argsz field allows the user to indicate how > > > > much data +they're providing, it's still the kernel's > > > > responsibility to validate +whether it's correct and sufficient > > > > for the requested operation. + > > > > +Compatibility Checking > > > > +-- > > > > +When IOMMU UAPI extension results in size increase, user such > > > > as VFIO +has to handle the following cases: > > > > + > > > > +1. User and kernel has exact size match > > > > +2. An older user with older kernel header (smaller UAPI size) > > > > running on a > > > > + newer kernel (larger UAPI size) > > > > +3. A newer user with newer kernel header (larger UAPI size) > > > > running > > > > + on an older kernel. > > > > +4. A malicious/misbehaving user pass illegal/invalid size but > > > > within > > > > + range. The data may contain garbage. > > > > > > I'm
[PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS
The ACS "Translation Blocking" bit blocks the translated addresses from
the devices. We don't expect such traffic from devices unless ATS is
enabled on them. A device sending such traffic without ATS enabled,
indicates malicious intent, and thus should be blocked.
Enable PCI_ACS_TB by default for all devices, and it stays enabled until
atleast one of the devices downstream wants to enable ATS. It gets
disabled to enable ATS on a device downstream it, and then gets enabled
back on once all the downstream devices don't need ATS.
Signed-off-by: Rajat Jain
---
Note that I'm ignoring the devices that require quirks to enable or
disable ACS, instead of using the standard way for ACS configuration.
The reason is that it would require adding yet another quirk table or
quirk function pointer, that I don't know how to implement for those
devices, and will neither have the devices to test that code.
v5: Enable TB and disable ATS for all devices on boot. Disable TB later
only if needed to enable ATS on downstream devices.
v4: Add braces to avoid warning from kernel robot
print warning for only external-facing devices.
v3: print warning if ACS_TB not supported on external-facing/untrusted ports.
Minor code comments fixes.
v2: Commit log change
drivers/pci/ats.c | 5
drivers/pci/pci.c | 57 +
drivers/pci/pci.h | 2 ++
drivers/pci/probe.c | 2 +-
include/linux/pci.h | 2 ++
5 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index b761c1f72f67..e2ea9083f30f 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -28,6 +28,9 @@ void pci_ats_init(struct pci_dev *dev)
return;
dev->ats_cap = pos;
+
+ dev->ats_enabled = 1; /* To avoid WARN_ON from pci_disable_ats() */
+ pci_disable_ats(dev);
}
/**
@@ -82,6 +85,7 @@ int pci_enable_ats(struct pci_dev *dev, int ps)
}
pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
+ pci_disable_acs_trans_blocking(dev);
dev->ats_enabled = 1;
return 0;
}
@@ -102,6 +106,7 @@ void pci_disable_ats(struct pci_dev *dev)
ctrl &= ~PCI_ATS_CTRL_ENABLE;
pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
+ pci_enable_acs_trans_blocking(dev);
dev->ats_enabled = 0;
}
EXPORT_SYMBOL_GPL(pci_disable_ats);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 73a862782214..614e3c1e8c56 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -876,6 +876,9 @@ static void pci_std_enable_acs(struct pci_dev *dev)
/* Upstream Forwarding */
ctrl |= (cap & PCI_ACS_UF);
+ /* Translation Blocking */
+ ctrl |= (cap & PCI_ACS_TB);
+
pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
}
@@ -904,6 +907,60 @@ static void pci_enable_acs(struct pci_dev *dev)
pci_disable_acs_redir(dev);
}
+void pci_disable_acs_trans_blocking(struct pci_dev *pdev)
+{
+ u16 cap, ctrl, pos;
+ struct pci_dev *dev;
+
+ if (!pci_acs_enable)
+ return;
+
+ for (dev = pdev; dev; dev = pci_upstream_bridge(pdev)) {
+
+ pos = dev->acs_cap;
+ if (!pos)
+ continue;
+
+ /*
+* Disable translation blocking when first downstream
+* device that needs it (for ATS) wants to enable ATS
+*/
+ if (++dev->ats_dependencies == 1) {
+ pci_read_config_word(dev, pos + PCI_ACS_CAP, );
+ pci_read_config_word(dev, pos + PCI_ACS_CTRL, );
+ ctrl &= ~(cap & PCI_ACS_TB);
+ pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+ }
+ }
+}
+
+void pci_enable_acs_trans_blocking(struct pci_dev *pdev)
+{
+ u16 cap, ctrl, pos;
+ struct pci_dev *dev;
+
+ if (!pci_acs_enable)
+ return;
+
+ for (dev = pdev; dev; dev = pci_upstream_bridge(pdev)) {
+
+ pos = dev->acs_cap;
+ if (!pos)
+ continue;
+
+ /*
+* Enable translation blocking when last downstream device
+* that depends on it (for ATS), doesn't need ATS anymore
+*/
+ if (--dev->ats_dependencies == 0) {
+ pci_read_config_word(dev, pos + PCI_ACS_CAP, );
+ pci_read_config_word(dev, pos + PCI_ACS_CTRL, );
+ ctrl |= (cap & PCI_ACS_TB);
+ pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
+ }
+ }
+}
+
/**
* pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 12fb79fbe29d..f5d8ecb6ba96 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -552,6 +552,8 @@
Re: [PATCH v5] PCI/ACS: Enable PCI_ACS_TB and disable only when needed for ATS
On Tue, Jul 14, 2020 at 1:15 PM Rajat Jain wrote:
>
> The ACS "Translation Blocking" bit blocks the translated addresses from
> the devices. We don't expect such traffic from devices unless ATS is
> enabled on them. A device sending such traffic without ATS enabled,
> indicates malicious intent, and thus should be blocked.
>
> Enable PCI_ACS_TB by default for all devices, and it stays enabled until
> atleast one of the devices downstream wants to enable ATS. It gets
> disabled to enable ATS on a device downstream it, and then gets enabled
> back on once all the downstream devices don't need ATS.
>
> Signed-off-by: Rajat Jain
> ---
> Note that I'm ignoring the devices that require quirks to enable or
> disable ACS, instead of using the standard way for ACS configuration.
> The reason is that it would require adding yet another quirk table or
> quirk function pointer, that I don't know how to implement for those
> devices, and will neither have the devices to test that code.
>
> v5: Enable TB and disable ATS for all devices on boot. Disable TB later
> only if needed to enable ATS on downstream devices.
> v4: Add braces to avoid warning from kernel robot
> print warning for only external-facing devices.
> v3: print warning if ACS_TB not supported on external-facing/untrusted ports.
> Minor code comments fixes.
> v2: Commit log change
>
> drivers/pci/ats.c | 5
> drivers/pci/pci.c | 57 +
> drivers/pci/pci.h | 2 ++
> drivers/pci/probe.c | 2 +-
> include/linux/pci.h | 2 ++
> 5 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index b761c1f72f67..e2ea9083f30f 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -28,6 +28,9 @@ void pci_ats_init(struct pci_dev *dev)
> return;
>
> dev->ats_cap = pos;
> +
> + dev->ats_enabled = 1; /* To avoid WARN_ON from pci_disable_ats() */
> + pci_disable_ats(dev);
> }
>
> /**
> @@ -82,6 +85,7 @@ int pci_enable_ats(struct pci_dev *dev, int ps)
> }
> pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
>
> + pci_disable_acs_trans_blocking(dev);
> dev->ats_enabled = 1;
> return 0;
> }
> @@ -102,6 +106,7 @@ void pci_disable_ats(struct pci_dev *dev)
> ctrl &= ~PCI_ATS_CTRL_ENABLE;
> pci_write_config_word(dev, dev->ats_cap + PCI_ATS_CTRL, ctrl);
>
> + pci_enable_acs_trans_blocking(dev);
> dev->ats_enabled = 0;
> }
> EXPORT_SYMBOL_GPL(pci_disable_ats);
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 73a862782214..614e3c1e8c56 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -876,6 +876,9 @@ static void pci_std_enable_acs(struct pci_dev *dev)
> /* Upstream Forwarding */
> ctrl |= (cap & PCI_ACS_UF);
>
> + /* Translation Blocking */
> + ctrl |= (cap & PCI_ACS_TB);
> +
> pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> }
>
> @@ -904,6 +907,60 @@ static void pci_enable_acs(struct pci_dev *dev)
> pci_disable_acs_redir(dev);
> }
>
> +void pci_disable_acs_trans_blocking(struct pci_dev *pdev)
> +{
> + u16 cap, ctrl, pos;
> + struct pci_dev *dev;
> +
> + if (!pci_acs_enable)
> + return;
> +
> + for (dev = pdev; dev; dev = pci_upstream_bridge(pdev)) {
> +
> + pos = dev->acs_cap;
> + if (!pos)
> + continue;
> +
> + /*
> +* Disable translation blocking when first downstream
> +* device that needs it (for ATS) wants to enable ATS
> +*/
> + if (++dev->ats_dependencies == 1) {
I am a little worried about a potential race condition here. I know
that 2 PCI devices cannot be enumerating at the same time. Do we know
if multiple pci_enable_ats() and pci_disable_ats() function calls can
be simultaneously executing (even for different devices)? If so, we
may need an atomic_t variable for ats_dependencies.
Thanks,
Rajat
> + pci_read_config_word(dev, pos + PCI_ACS_CAP, );
> + pci_read_config_word(dev, pos + PCI_ACS_CTRL, );
> + ctrl &= ~(cap & PCI_ACS_TB);
> + pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> + }
> + }
> +}
> +
> +void pci_enable_acs_trans_blocking(struct pci_dev *pdev)
> +{
> + u16 cap, ctrl, pos;
> + struct pci_dev *dev;
> +
> + if (!pci_acs_enable)
> + return;
> +
> + for (dev = pdev; dev; dev = pci_upstream_bridge(pdev)) {
> +
> + pos = dev->acs_cap;
> + if (!pos)
> + continue;
> +
> + /*
> +* Enable translation blocking when last downstream device
> +* that depends on it (for ATS), doesn't need ATS anymore
> +*/
> +
Re: [PATCH v4 4/4] PCI/ACS: Enable PCI_ACS_TB for untrusted/external-facing devices
On Sat, Jul 11, 2020 at 12:53 PM Bjorn Helgaas wrote:
>
> On Fri, Jul 10, 2020 at 03:53:59PM -0700, Rajat Jain wrote:
> > On Fri, Jul 10, 2020 at 2:29 PM Raj, Ashok wrote:
> > > On Fri, Jul 10, 2020 at 03:29:22PM -0500, Bjorn Helgaas wrote:
> > > > On Tue, Jul 07, 2020 at 03:46:04PM -0700, Rajat Jain wrote:
> > > > > When enabling ACS, enable translation blocking for external facing
> > > > > ports
> > > > > and untrusted devices.
> > > > >
> > > > > Signed-off-by: Rajat Jain
> > > > > ---
> > > > > v4: Add braces to avoid warning from kernel robot
> > > > > print warning for only external-facing devices.
> > > > > v3: print warning if ACS_TB not supported on
> > > > > external-facing/untrusted ports.
> > > > > Minor code comments fixes.
> > > > > v2: Commit log change
> > > > >
> > > > > drivers/pci/pci.c| 8
> > > > > drivers/pci/quirks.c | 15 +++
> > > > > 2 files changed, 23 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > > index 73a8627822140..a5a6bea7af7ce 100644
> > > > > --- a/drivers/pci/pci.c
> > > > > +++ b/drivers/pci/pci.c
> > > > > @@ -876,6 +876,14 @@ static void pci_std_enable_acs(struct pci_dev
> > > > > *dev)
> > > > > /* Upstream Forwarding */
> > > > > ctrl |= (cap & PCI_ACS_UF);
> > > > >
> > > > > + /* Enable Translation Blocking for external devices */
> > > > > + if (dev->external_facing || dev->untrusted) {
> > > > > + if (cap & PCI_ACS_TB)
> > > > > + ctrl |= PCI_ACS_TB;
> > > > > + else if (dev->external_facing)
> > > > > + pci_warn(dev, "ACS: No Translation Blocking on
> > > > > external-facing dev\n");
> > > > > + }
> > > >
> > > > IIUC, this means that external devices can *never* use ATS and
> > > > can never cache translations.
> >
> > Yes, but it already exists today (and this patch doesn't change that):
> > 521376741b2c2 "PCI/ATS: Only enable ATS for trusted devices"
> >
> > IMHO any external device trying to send ATS traffic despite having ATS
> > disabled should count as a bad intent. And this patch is trying to
> > plug that loophole, by blocking the AT traffic from devices that we do
> > not expect to see AT from anyway.
>
> Thinking about this some more, I wonder if Linux should:
>
> - Explicitly disable ATS for every device at enumeration-time, e.g.,
> in pci_init_capabilities(),
>
> - Enable PCI_ACS_TB for every device (not just external-facing or
> untrusted ones),
>
> - Disable PCI_ACS_TB for the relevant devices along the path only
> when enabling ATS.
>
> One nice thing about doing that is that the "untrusted" test would be
> only in pci_enable_ats(), and we wouldn't need one in
> pci_std_enable_acs().
Sent out v5 with this approach here:
https://patchwork.kernel.org/patch/11663515/
Thanks,
Rajat
>
>
> It's possible BIOS gives us devices with ATS enabled, and this might
> break them, but that seems like something we'd want to find out about.
>
> Bjorn
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] OMAP: iommu: check for failure of a call to omap_iommu_dump_ctx
From: Colin Ian King
It is possible for the call to omap_iommu_dump_ctx to return
a negative error number, so check for the failure and return
the error number rather than pass the negative value to
simple_read_from_buffer.
Addresses-Coverity: ("Improper use of negative value")
Fixes: 14e0e6796a0d ("OMAP: iommu: add initial debugfs support")
Signed-off-by: Colin Ian King
---
drivers/iommu/omap-iommu-debug.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/omap-iommu-debug.c b/drivers/iommu/omap-iommu-debug.c
index 8e19bfa94121..a99afb5d9011 100644
--- a/drivers/iommu/omap-iommu-debug.c
+++ b/drivers/iommu/omap-iommu-debug.c
@@ -98,8 +98,11 @@ static ssize_t debug_read_regs(struct file *file, char
__user *userbuf,
mutex_lock(_debug_lock);
bytes = omap_iommu_dump_ctx(obj, p, count);
+ if (bytes < 0)
+ goto err;
bytes = simple_read_from_buffer(userbuf, count, ppos, buf, bytes);
+err:
mutex_unlock(_debug_lock);
kfree(buf);
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 1/5] docs: IOMMU user API
On Mon, 13 Jul 2020 22:00:23 -0700 Jacob Pan wrote: > Hi Alex, > > On Mon, 13 Jul 2020 16:48:42 -0600 > Alex Williamson wrote: > > > On Tue, 7 Jul 2020 16:43:45 -0700 > > Jacob Pan wrote: > > > > > IOMMU UAPI is newly introduced to support communications between > > > guest virtual IOMMU and host IOMMU. There has been lots of > > > discussions on how it should work with VFIO UAPI and userspace in > > > general. > > > > > > This document is indended to clarify the UAPI design and usage. The > > > mechenics of how future extensions should be achieved are also > > > covered > > > > mechanics > > > will fix. > > > > in this documentation. > > > > > > Signed-off-by: Liu Yi L > > > Signed-off-by: Jacob Pan > > > --- > > > Documentation/userspace-api/iommu.rst | 312 > > > ++ 1 file changed, 312 insertions(+) > > > create mode 100644 Documentation/userspace-api/iommu.rst > > > > > > diff --git a/Documentation/userspace-api/iommu.rst > > > b/Documentation/userspace-api/iommu.rst new file mode 100644 > > > index ..581b462c2cec > > > --- /dev/null > > > +++ b/Documentation/userspace-api/iommu.rst > > > @@ -0,0 +1,312 @@ > > > +.. SPDX-License-Identifier: GPL-2.0 > > > +.. iommu: > > > + > > > += > > > +IOMMU Userspace API > > > += > > > + > > > +IOMMU UAPI is used for virtualization cases where communications > > > are +needed between physical and virtual IOMMU drivers. For native > > > +usage, IOMMU is a system device which does not need to communicate > > > +with user space directly. > > > + > > > +The primary use cases are guest Shared Virtual Address (SVA) and > > > +guest IO virtual address (IOVA), wherein a virtual IOMMU (vIOMMU) > > > is +required to communicate with the physical IOMMU in the host. > > > + > > > +.. contents:: :local: > > > + > > > +Functionalities > > > +=== > > > +Communications of user and kernel involve both directions. The > > > +supported user-kernel APIs are as follows: > > > + > > > +1. Alloc/Free PASID > > > +2. Bind/unbind guest PASID (e.g. Intel VT-d) > > > +3. Bind/unbind guest PASID table (e.g. ARM sMMU) > > > +4. Invalidate IOMMU caches > > > +5. Service page requests > > > + > > > +Requirements > > > + > > > +The IOMMU UAPIs are generic and extensible to meet the following > > > +requirements: > > > + > > > +1. Emulated and para-virtualised vIOMMUs > > > +2. Multiple vendors (Intel VT-d, ARM sMMU, etc.) > > > +3. Extensions to the UAPI shall not break existing user space > > > + > > > +Interfaces > > > +== > > > +Although the data structures defined in IOMMU UAPI are > > > self-contained, +there is no user API functions introduced. > > > Instead, IOMMU UAPI is +designed to work with existing user driver > > > frameworks such as VFIO. + > > > +Extension Rules & Precautions > > > +- > > > +When IOMMU UAPI gets extended, the data structures can *only* be > > > +modified in two ways: > > > + > > > +1. Adding new fields by re-purposing the padding[] field. No size > > > change. +2. Adding new union members at the end. May increase in > > > size. + > > > +No new fields can be added *after* the variable sized union in > > > that it +will break backward compatibility when offset moves. In > > > both cases, a +new flag must be accompanied with a new field such > > > that the IOMMU +driver can process the data based on the new flag. > > > Version field is +only reserved for the unlikely event of UAPI > > > upgrade at its entirety. + > > > +It's *always* the caller's responsibility to indicate the size of > > > the +structure passed by setting argsz appropriately. > > > +Though at the same time, argsz is user provided data which is not > > > +trusted. The argsz field allows the user to indicate how much data > > > +they're providing, it's still the kernel's responsibility to > > > validate +whether it's correct and sufficient for the requested > > > operation. + > > > +Compatibility Checking > > > +-- > > > +When IOMMU UAPI extension results in size increase, user such as > > > VFIO +has to handle the following cases: > > > + > > > +1. User and kernel has exact size match > > > +2. An older user with older kernel header (smaller UAPI size) > > > running on a > > > + newer kernel (larger UAPI size) > > > +3. A newer user with newer kernel header (larger UAPI size) running > > > + on an older kernel. > > > +4. A malicious/misbehaving user pass illegal/invalid size but > > > within > > > + range. The data may contain garbage. > > > > I'm still not sure where VFIO has responsibility in managing any of > > these cases. I think we've determined that VFIO is just the wrapper > > and call-through mechanism, it's the UAPI core implementation and > > IOMMU drivers that are responsible for this. > > > That is right, I shall rewrite the responsibility to be held by
RE: [PATCH V4 1/3] iommu: Add support to change default domain of an iommu group
Hi Joerg,
Replying again because I noticed that I couldn't find this mail in the external
iommu mailing list while I was able to find your comments on my patch. Also,
could you please answer my two questions below?
> -Original Message-
> From: iommu On Behalf Of
> Prakhya, Sai Praneeth
> Sent: Tuesday, June 30, 2020 8:04 PM
> To: Joerg Roedel
> Cc: Raj, Ashok ; Will Deacon ;
> iommu@lists.linux-foundation.org; Robin Murphy ;
> Christoph Hellwig
> Subject: RE: [PATCH V4 1/3] iommu: Add support to change default domain of
> an iommu group
>
> Hi Joerg,
>
> > -Original Message-
> > From: Joerg Roedel
> > Sent: Tuesday, June 30, 2020 2:16 AM
> > To: Prakhya, Sai Praneeth
> > Cc: iommu@lists.linux-foundation.org; Christoph Hellwig ;
> > Raj, Ashok ; Will Deacon ;
> > Lu Baolu ; Mehta, Sohil
> > ; Robin Murphy ; Jacob
> > Pan
> > Subject: Re: [PATCH V4 1/3] iommu: Add support to change default
> > domain of an iommu group
> >
> > On Thu, Jun 04, 2020 at 06:32:06PM -0700, Sai Praneeth Prakhya wrote:
> > > +static int iommu_change_dev_def_domain(struct iommu_group *group,
> > > +int
> > > +type) {
> > > + struct iommu_domain *prev_dom;
> > > + struct group_device *grp_dev;
> > > + const struct iommu_ops *ops;
> > > + int ret, dev_def_dom;
> > > + struct device *dev;
> > > +
> > > + if (!group)
> > > + return -EINVAL;
> > > +
> > > + mutex_lock(>mutex);
> > > +
> > > + if (group->default_domain != group->domain) {
> > > + pr_err_ratelimited("Group assigned to user level for direct
> > > +access\n");
> >
> > Make this message: "Group not assigned to default domain\n".
>
> Sure! I will change it
>
> > > + ret = -EBUSY;
> > > + goto out;
> > > + }
> > > +
> > > + /*
> > > + * iommu group wasn't locked while acquiring device lock in
> > > + * iommu_group_store_type(). So, make sure that the device count
> > hasn't
> > > + * changed while acquiring device lock.
> > > + *
> > > + * Changing default domain of an iommu group with two or more
> > devices
> > > + * isn't supported because there could be a potential deadlock. Consider
> > > + * the following scenario. T1 is trying to acquire device locks of all
> > > + * the devices in the group and before it could acquire all of them,
> > > + * there could be another thread T2 (from different sub-system and use
> > > + * case) that has already acquired some of the device locks and might be
> > > + * waiting for T1 to release other device locks.
> > > + */
> > > + if (iommu_group_device_count(group) != 1) {
> > > + pr_err_ratelimited("Cannot change default domain of a group
> > with
> > > +two or more devices\n");
> >
> > "Can not change default domain: Group has more than one device\n"
>
> Ok.. make sense. I will change this.
>
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > > +
> > > + /* Since group has only one device */
> > > + list_for_each_entry(grp_dev, >devices, list)
> > > + dev = grp_dev->dev;
> > > +
> > > + prev_dom = group->default_domain;
> > > + if (!prev_dom || !prev_dom->ops || !prev_dom->ops-
> > >def_domain_type) {
> > > + pr_err_ratelimited("'def_domain_type' call back isn't
> > > +registered\n");
> >
> > This message isn't needed.
>
> Ok. I will remove it.
>
> > > + ret = __iommu_attach_device(group->default_domain, dev);
> > > + if (ret)
> > > + goto free_new_domain;
> > > +
> > > + group->domain = group->default_domain;
> > > +
> > > + ret = iommu_create_device_direct_mappings(group, dev);
> > > + if (ret)
> > > + goto free_new_domain;
> >
> > You need to create the direct mappings before you attach the device to
> > the new domain. Otherwise there might be a short time-window where
> > RMRR regions are not mapped.
>
> Ok.. makes sense. I will change this accordingly.
>
> > > +static ssize_t iommu_group_store_type(struct iommu_group *group,
> > > + const char *buf, size_t count) {
> > > + struct group_device *grp_dev;
> > > + struct device *dev;
> > > + int ret, req_type;
> > > +
> > > + if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> > > + return -EACCES;
> > > +
> > > + if (WARN_ON(!group))
> > > + return -EINVAL;
> > > +
> > > + if (sysfs_streq(buf, "identity"))
> > > + req_type = IOMMU_DOMAIN_IDENTITY;
> > > + else if (sysfs_streq(buf, "DMA"))
> > > + req_type = IOMMU_DOMAIN_DMA;
> > > + else if (sysfs_streq(buf, "auto"))
> > > + req_type = 0;
> > > + else
> > > + return -EINVAL;
> > > +
> > > + /*
> > > + * Lock/Unlock the group mutex here before device lock to
> > > + * 1. Make sure that the iommu group has only one device (this is a
> > > + *prerequisite for step 2)
> > > + * 2. Get struct *dev which is needed to lock device
> > > + */
> > > + mutex_lock(>mutex);
> > > + if (iommu_group_device_count(group) != 1) {
> > > + mutex_unlock(>mutex);
> > > + pr_err_ratelimited("Cannot change default
Re: [PATCH v3 2/4] iommu: Add iommu_aux_at(de)tach_group()
On Tue, 14 Jul 2020 13:57:01 +0800
Lu Baolu wrote:
> This adds two new aux-domain APIs for a use case like vfio/mdev where
> sub-devices derived from an aux-domain capable device are created and
> put in an iommu_group.
>
> /**
> * iommu_aux_attach_group - attach an aux-domain to an iommu_group
> which
> * contains sub-devices (for example mdevs)
> derived
> * from @dev.
> * @domain: an aux-domain;
> * @group: an iommu_group which contains sub-devices derived from
> @dev;
> * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
> *
> * Returns 0 on success, or an error value.
> */
> int iommu_aux_attach_group(struct iommu_domain *domain,
>struct iommu_group *group,
>struct device *dev)
>
> /**
> * iommu_aux_detach_group - detach an aux-domain from an iommu_group
> *
> * @domain: an aux-domain;
> * @group: an iommu_group which contains sub-devices derived from
> @dev;
> * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
> *
> * @domain must have been attached to @group via
> iommu_aux_attach_group(). */
> void iommu_aux_detach_group(struct iommu_domain *domain,
> struct iommu_group *group,
> struct device *dev)
>
> It also adds a flag in the iommu_group data structure to identify
> an iommu_group with aux-domain attached from those normal ones.
>
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/iommu.c | 58
> +++ include/linux/iommu.h |
> 17 + 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e1fdd3531d65..cad5a19ebf22 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -45,6 +45,7 @@ struct iommu_group {
> struct iommu_domain *default_domain;
> struct iommu_domain *domain;
> struct list_head entry;
> + unsigned int aux_domain_attached:1;
> };
>
> struct group_device {
> @@ -2759,6 +2760,63 @@ int iommu_aux_get_pasid(struct iommu_domain
> *domain, struct device *dev) }
> EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
>
> +/**
> + * iommu_aux_attach_group - attach an aux-domain to an iommu_group
> which
> + * contains sub-devices (for example mdevs)
> derived
> + * from @dev.
> + * @domain: an aux-domain;
> + * @group: an iommu_group which contains sub-devices derived from
> @dev;
> + * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
> + *
> + * Returns 0 on success, or an error value.
> + */
> +int iommu_aux_attach_group(struct iommu_domain *domain,
> +struct iommu_group *group, struct device
> *dev) +{
> + int ret = -EBUSY;
> +
> + mutex_lock(>mutex);
> + if (group->domain)
> + goto out_unlock;
> +
Perhaps I missed something but are we assuming only one mdev per mdev
group? That seems to change the logic where vfio does:
iommu_group_for_each_dev()
iommu_aux_attach_device()
> + ret = iommu_aux_attach_device(domain, dev);
> + if (!ret) {
> + group->domain = domain;
> + group->aux_domain_attached = true;
> + }
> +
> +out_unlock:
> + mutex_unlock(>mutex);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_aux_attach_group);
> +
> +/**
> + * iommu_aux_detach_group - detach an aux-domain from an iommu_group
> + *
> + * @domain: an aux-domain;
> + * @group: an iommu_group which contains sub-devices derived from
> @dev;
> + * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
> + *
> + * @domain must have been attached to @group via
> iommu_aux_attach_group().
> + */
> +void iommu_aux_detach_group(struct iommu_domain *domain,
> + struct iommu_group *group, struct device
> *dev) +{
> + mutex_lock(>mutex);
> +
> + if (WARN_ON(!group->aux_domain_attached || group->domain !=
> domain))
> + goto out_unlock;
> +
> + iommu_aux_detach_device(domain, dev);
> + group->aux_domain_attached = false;
> + group->domain = NULL;
> +
> +out_unlock:
> + mutex_unlock(>mutex);
> +}
> +EXPORT_SYMBOL_GPL(iommu_aux_detach_group);
> +
> /**
> * iommu_sva_bind_device() - Bind a process address space to a device
> * @dev: the device
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 5657d4fef9f2..9506551139ab 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -635,6 +635,10 @@ bool iommu_dev_feature_enabled(struct device
> *dev, enum iommu_dev_features f); int iommu_aux_attach_device(struct
> iommu_domain *domain, struct device *dev); void
> iommu_aux_detach_device(struct iommu_domain *domain, struct device
> *dev); int iommu_aux_get_pasid(struct iommu_domain *domain, struct
> device *dev); +int iommu_aux_attach_group(struct iommu_domain *domain,
> +struct iommu_group *group, struct
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Tue, 14 Jul 2020 09:25:14 +0100 Christoph Hellwig wrote: > On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > > Replace iommu_aux_at(de)tach_device() with > > iommu_aux_at(de)tach_group(). It also saves the > > IOMMU_DEV_FEAT_AUX-capable physcail device in the vfio_group data > > structure so that it could be reused in other places. > > This removes the last user of iommu_aux_attach_device and > iommu_aux_detach_device, which can be removed now. it is still used in patch 2/4 inside iommu_aux_attach_group(), right? > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu [Jacob Pan] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
Hi Geert,
Thank you for the review.
On Tue, Jul 14, 2020 at 9:09 AM Geert Uytterhoeven wrote:
>
> Hi Prabhakar,
>
> On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
> wrote:
> > From: Marian-Cristian Rotariu
> >
> > Add support for RZ/G2H (R8A774E1) SoC IPMMUs.
> >
> > Signed-off-by: Marian-Cristian Rotariu
> >
> > Signed-off-by: Lad Prabhakar
>
> Thanks for your patch!
>
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -751,6 +751,7 @@ static const struct soc_device_attribute
> > soc_rcar_gen3[] = {
> > static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> > { .soc_id = "r8a774b1", },
> > { .soc_id = "r8a774c0", },
> > + { .soc_id = "r8a774e1", },
>
> Adding an entry to soc_rcar_gen3_whitelist[] doesn't do anything, unless
> you also add the same entry to soc_rcar_gen3[].
>
I think the comment "For R-Car Gen3 use a white list to opt-in slave
devices." is misleading. Booting through the kernel I do see iommu
groups (attached is the logs). Also the recent patch to add
"r8a77961" just adds to soc_rcar_gen3_whitelist.
Cheers,
--Prabhakar
> > { .soc_id = "r8a7795", .revision = "ES3.*" },
> > { .soc_id = "r8a77961", },
> > { .soc_id = "r8a77965", },
> > @@ -963,6 +964,9 @@ static const struct of_device_id ipmmu_of_ids[] = {
> > }, {
> > .compatible = "renesas,ipmmu-r8a774c0",
> > .data = _features_rcar_gen3,
> > + }, {
> > + .compatible = "renesas,ipmmu-r8a774e1",
> > + .data = _features_rcar_gen3,
> > }, {
> > .compatible = "renesas,ipmmu-r8a7795",
> > .data = _features_rcar_gen3,
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
[0.000149] NOTICE: BL2: RZ G2H Initial Program Loader(CA57)
[0.004417] NOTICE: BL2: Initial Program Loader(Rev.2.0.6)
[0.009949] NOTICE: BL2: PRR is RZG G2H Ver.3.0
[0.014530] NOTICE: BL2: Board is HiHope RZ/G2H Rev.4.0
[0.019796] NOTICE: BL2: Boot device is QSPI Flash(40MHz)
[0.025236] NOTICE: BL2: LCM state is CM
[0.029203] NOTICE: BL2: CH0: 0x4 - 0x47fff, 2 GiB
[0.035071] NOTICE: BL2: CH1: 0x5 - 0x57fff, 2 GiB
[0.040957] NOTICE: BL2: DDR3200(rev.0.40)
[0.052413] NOTICE: BL2: [COLD_BOOT]
[0.058356] NOTICE: BL2: DRAM Split is 2ch(DDR 3)
[0.061655] NOTICE: BL2: QoS is default setting(rev.0.07)
[0.067098] NOTICE: BL2: DRAM refresh interval 1.95 usec
[0.072455] NOTICE: BL2: Periodic Write DQ Training
[0.077485] NOTICE: BL2: Lossy Decomp areas
[0.081614] NOTICE: Entry 0: DCMPAREACRAx:0x8540 DCMPAREACRBx:0x570
[0.088698] NOTICE: Entry 1: DCMPAREACRAx:0x4000 DCMPAREACRBx:0x0
[0.095611] NOTICE: Entry 2: DCMPAREACRAx:0x2000 DCMPAREACRBx:0x0
[0.102525] NOTICE: BL2: v1.5(release):af9f429-dirty
[0.107534] NOTICE: BL2: Built : 13:06:47, Jun 11 2020
[0.112722] NOTICE: BL2: Normal boot
[0.116360] NOTICE: BL2: dst=0xe6321100 src=0x818 len=512(0x200)
[0.122850] NOTICE: BL2: dst=0x43f0 src=0x8180400 len=6144(0x1800)
[0.130588] NOTICE: BL2: dst=0x4400 src=0x81c len=65536(0x1)
[0.149490] NOTICE: BL2: dst=0x5000 src=0x830 len=1048576(0x10)
[0.369751] NOTICE: BL2: Booting BL31
U-Boot 2018.09 (Jun 11 2020 - 13:06:59 +)
CPU: Renesas Electronics R8A774E1 rev 3.0
Model: Hoperun Technology HiHope RZ/G2H platform (hihope-rzg2h)
DRAM: 3.9 GiB
Bank #0: 0x04800 - 0x0bfff, 1.9 GiB
Bank #1: 0x5 - 0x57fff, 2 GiB
Watchdog: Not found by seq!
WDT: watchdog@e602
Watchdog: Started!
MMC: sd@ee10: 0, sd@ee16: 1
Loading Environment from MMC... OK
In:serial@e6e88000
Out: serial@e6e88000
Err: serial@e6e88000
Net: eth0: ethernet@e680
Hit any key to stop autoboot: 0
ethernet@e680 Waiting for PHY auto negotiation to complete... done
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
DHCP client bound to address 192.168.10.136 (1798 ms)
Using ethernet@e680 device
TFTP from server 192.168.10.1; our IP address is 192.168.10.136
Filename 'g2h/Image.gz'.
Load address: 0x4a08
Loading: T #
#
#
#
#
Re: [PATCH v10 4/5] dt-bindings: arm-smmu: add binding for Tegra194 SMMU
On Mon, Jul 13, 2020 at 8:10 AM Robin Murphy wrote:
>
> On 2020-07-10 21:29, Krishna Reddy wrote:
> > Thanks Rob. One question on setting "minItems: ". Please see below.
> >
> >>> +allOf:
> >>> + - if:
> >>> + properties:
> >>> +compatible:
> >>> + contains:
> >>> +enum:
> >>> + - nvidia,tegra194-smmu
> >>> +then:
> >>> + properties:
> >>> +reg:
> >>> + minItems: 2
> >>> + maxItems: 2
> >
> >> This doesn't work. The main part of the schema already said there's only
> >> 1 reg region. This part is ANDed with that, not an override. You need to
> >> add an else clause with 'maxItems: 1' and change the base schema to
> >> {minItems: 1, maxItems: 2}.
> >
> > As the earlier version of base schema doesn't have "minItems: " set, should
> > it be set to 0 for backward compatibility? Or can it just be omitted
> > setting in base schema as before?
>
> We've always needed at least 1 "reg" specifier in practice, so I don't
> think being backwards-compatible with broken DTs is a concern :)
'minItems: 0' would be a boolean (e.g. "reg;") and I'm not sure that's
even really valid json-schema. What you'd want here is 'reg' not
present (i.e. not in 'required').
Rob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v6 01/12] iommu: Change type of pasid to u32
> From: Yu, Fenghua > Sent: Tuesday, July 14, 2020 9:55 PM > On Mon, Jul 13, 2020 at 07:45:49PM -0700, Liu, Yi L wrote: > > > From: Fenghua Yu > > > Sent: Tuesday, July 14, 2020 7:48 AM > > > > > > PASID is defined as a few different types in iommu including "int", > > > "u32", and "unsigned int". To be consistent and to match with uapi > > > definitions, define PASID and its variations (e.g. max PASID) as "u32". > > > "u32" is also shorter and a little more explicit than "unsigned int". > > > > > > No PASID type change in uapi although it defines PASID as __u64 in > > > some places. > > > > just out of curious, why not using ioasid_t? In Linux kernel, PASID is > > managed by ioasid. > > ioasid_t is only used in limited underneath files (ioasid.c and ioasid.h). > Instead of changing hundreds of places to use ioasid_t, it's better to keep > ioasid_t > only used in the files. > > And it's explict and matches with uapi to define PASID as u32. Changing to > ioasid_t in > so many files (amd, gpu, crypto, etc) may confuse upper users on "why > ioasid_t". > > So we had better to explicitly define PASID as u32 and keep ioasid_t in the > limited > underneath files. fair enough, thanks, Regards, Yi Liu > Thanks. > > -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 01/12] iommu: Change type of pasid to u32
On Mon, Jul 13, 2020 at 07:45:49PM -0700, Liu, Yi L wrote: > > From: Fenghua Yu > > Sent: Tuesday, July 14, 2020 7:48 AM > > > > PASID is defined as a few different types in iommu including "int", > > "u32", and "unsigned int". To be consistent and to match with uapi > > definitions, define PASID and its variations (e.g. max PASID) as "u32". > > "u32" is also shorter and a little more explicit than "unsigned int". > > > > No PASID type change in uapi although it defines PASID as __u64 in > > some places. > > just out of curious, why not using ioasid_t? In Linux kernel, PASID is > managed by ioasid. ioasid_t is only used in limited underneath files (ioasid.c and ioasid.h). Instead of changing hundreds of places to use ioasid_t, it's better to keep ioasid_t only used in the files. And it's explict and matches with uapi to define PASID as u32. Changing to ioasid_t in so many files (amd, gpu, crypto, etc) may confuse upper users on "why ioasid_t". So we had better to explicitly define PASID as u32 and keep ioasid_t in the limited underneath files. Thanks. -Fenghua ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Tuesday, July 14, 2020 5:42 PM
>
> Hi Prabhakar,
>
> On Tue, Jul 14, 2020 at 10:30 AM Lad, Prabhakar
> wrote:
> > On Tue, Jul 14, 2020 at 9:09 AM Geert Uytterhoeven
> > wrote:
> > > On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
> > > wrote:
> > > > From: Marian-Cristian Rotariu
> > > >
> > > >
> > > > Add support for RZ/G2H (R8A774E1) SoC IPMMUs.
> > > >
> > > > Signed-off-by: Marian-Cristian Rotariu
> > > >
> > > > Signed-off-by: Lad Prabhakar
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > > @@ -751,6 +751,7 @@ static const struct soc_device_attribute
> > > > soc_rcar_gen3[] = {
> > > > static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> > > > { .soc_id = "r8a774b1", },
> > > > { .soc_id = "r8a774c0", },
> > > > + { .soc_id = "r8a774e1", },
> > >
> > > Adding an entry to soc_rcar_gen3_whitelist[] doesn't do anything, unless
> > > you also add the same entry to soc_rcar_gen3[].
> > >
> > I think the comment "For R-Car Gen3 use a white list to opt-in slave
> > devices." is misleading. Booting through the kernel I do see iommu
> > groups (attached is the logs).
>
> Indeed. Without an entry in soc_rcar_gen3[], the IPMMU is enabled
> unconditionally, and soc_rcar_gen3_whitelist[] is ignored.
> That's why you want an entry in both, unless you have an R-Car Gen3
> SoC where the IPMMU works correctly with all slave devices present.
> Perhaps soc_rcar_gen3[] should be renamed to soc_rcar_gen3_greylist[]
> (or soc_rcar_gen3_maybelist[]) to make this clear?
I think so (we should rename it).
> > Also the recent patch to add
> > "r8a77961" just adds to soc_rcar_gen3_whitelist.
>
> Oops, commit 17fe16181639801b ("iommu/renesas: Add support for r8a77961")
> did it wrong, too.
Thank you for the point it out. We should add r8a77961 to the soc_rcar_gen3[].
However, I don't know why I could not realize this issue...
So, I investigated this a little and then, IIUC, glob_match() which
soc_device_match() uses seems to return true, if *pat = "r8a7796" and *str =
"r8a77961".
Best regards,
Yoshihiro Shimoda
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 4/4] dma-pool: Make sure atomic pool suits device
When allocating DMA memory from a pool, the core can only guess which
atomic pool will fit a device's constraints. If it doesn't, get a safer
atomic pool and try again.
Fixes: c84dc6e68a1d ("dma-pool: add additional coherent pools to map to gfp
mask")
Reported-by: Jeremy Linton
Suggested-by: Robin Murphy
Signed-off-by: Nicolas Saenz Julienne
---
Changes since v1:
- Rebase to linus' master
kernel/dma/pool.c | 57 ++-
1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 5b9eaa2b498d..d48d9acb585f 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -240,39 +240,56 @@ static inline struct gen_pool *dma_guess_pool(struct
device *dev,
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
- struct gen_pool *pool = dma_guess_pool(dev, NULL);
- unsigned long val;
+ struct gen_pool *pool = NULL;
+ unsigned long val = 0;
void *ptr = NULL;
-
- if (!pool) {
- WARN(1, "%pGg atomic pool not initialised!\n", );
- return NULL;
+ phys_addr_t phys;
+
+ while (1) {
+ pool = dma_guess_pool(dev, pool);
+ if (!pool) {
+ WARN(1, "Failed to get suitable pool for %s\n",
+dev_name(dev));
+ break;
+ }
+
+ val = gen_pool_alloc(pool, size);
+ if (!val)
+ continue;
+
+ phys = gen_pool_virt_to_phys(pool, val);
+ if (dma_coherent_ok(dev, phys, size))
+ break;
+
+ gen_pool_free(pool, val, size);
+ val = 0;
}
- val = gen_pool_alloc(pool, size);
- if (likely(val)) {
- phys_addr_t phys = gen_pool_virt_to_phys(pool, val);
+ if (val) {
*ret_page = pfn_to_page(__phys_to_pfn(phys));
ptr = (void *)val;
memset(ptr, 0, size);
- } else {
- WARN_ONCE(1, "DMA coherent pool depleted, increase size "
-"(recommended min coherent_pool=%zuK)\n",
- gen_pool_size(pool) >> 9);
+
+ if (gen_pool_avail(pool) < atomic_pool_size)
+ schedule_work(_pool_work);
}
- if (gen_pool_avail(pool) < atomic_pool_size)
- schedule_work(_pool_work);
return ptr;
}
bool dma_free_from_pool(struct device *dev, void *start, size_t size)
{
- struct gen_pool *pool = dma_guess_pool(dev, NULL);
+ struct gen_pool *pool = NULL;
- if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
- return false;
- gen_pool_free(pool, (unsigned long)start, size);
- return true;
+ while (1) {
+ pool = dma_guess_pool(dev, pool);
+ if (!pool)
+ return false;
+
+ if (gen_pool_has_addr(pool, (unsigned long)start, size)) {
+ gen_pool_free(pool, (unsigned long)start, size);
+ return true;
+ }
+ }
}
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/4] dma-pool: Get rid of dma_in_atomic_pool()
The function is only used once and can be simplified to a one-liner.
Signed-off-by: Nicolas Saenz Julienne
---
kernel/dma/pool.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 39ca26fa41b5..318035e093fb 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -217,15 +217,6 @@ static inline struct gen_pool *dev_to_pool(struct device
*dev)
return atomic_pool_kernel;
}
-static bool dma_in_atomic_pool(struct device *dev, void *start, size_t size)
-{
- struct gen_pool *pool = dev_to_pool(dev);
-
- if (unlikely(!pool))
- return false;
- return gen_pool_has_addr(pool, (unsigned long)start, size);
-}
-
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
@@ -260,7 +251,7 @@ bool dma_free_from_pool(struct device *dev, void *start,
size_t size)
{
struct gen_pool *pool = dev_to_pool(dev);
- if (!dma_in_atomic_pool(dev, start, size))
+ if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
return false;
gen_pool_free(pool, (unsigned long)start, size);
return true;
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 3/4] dma-pool: Introduce dma_guess_pool()
dma-pool's dev_to_pool() creates the false impression that there is a
way to grantee a mapping between a device's DMA constraints and an
atomic pool. It tuns out it's just a guess, and the device might need to
use an atomic pool containing memory from a 'safer' (or lower) memory
zone.
To help mitigate this, introduce dma_guess_pool() which can be fed a
device's DMA constraints and atomic pools already known to be faulty, in
order for it to provide an better guess on which pool to use.
Signed-off-by: Nicolas Saenz Julienne
---
kernel/dma/pool.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
index 318035e093fb..5b9eaa2b498d 100644
--- a/kernel/dma/pool.c
+++ b/kernel/dma/pool.c
@@ -203,7 +203,7 @@ static int __init dma_atomic_pool_init(void)
}
postcore_initcall(dma_atomic_pool_init);
-static inline struct gen_pool *dev_to_pool(struct device *dev)
+static inline struct gen_pool *dma_guess_pool_from_device(struct device *dev)
{
u64 phys_mask;
gfp_t gfp;
@@ -217,10 +217,30 @@ static inline struct gen_pool *dev_to_pool(struct device
*dev)
return atomic_pool_kernel;
}
+static inline struct gen_pool *dma_get_safer_pool(struct gen_pool *bad_pool)
+{
+ if (bad_pool == atomic_pool_kernel)
+ return atomic_pool_dma32 ? : atomic_pool_dma;
+
+ if (bad_pool == atomic_pool_dma32)
+ return atomic_pool_dma;
+
+ return NULL;
+}
+
+static inline struct gen_pool *dma_guess_pool(struct device *dev,
+ struct gen_pool *bad_pool)
+{
+ if (bad_pool)
+ return dma_get_safer_pool(bad_pool);
+
+ return dma_guess_pool_from_device(dev);
+}
+
void *dma_alloc_from_pool(struct device *dev, size_t size,
struct page **ret_page, gfp_t flags)
{
- struct gen_pool *pool = dev_to_pool(dev);
+ struct gen_pool *pool = dma_guess_pool(dev, NULL);
unsigned long val;
void *ptr = NULL;
@@ -249,7 +269,7 @@ void *dma_alloc_from_pool(struct device *dev, size_t size,
bool dma_free_from_pool(struct device *dev, void *start, size_t size)
{
- struct gen_pool *pool = dev_to_pool(dev);
+ struct gen_pool *pool = dma_guess_pool(dev, NULL);
if (!pool || !gen_pool_has_addr(pool, (unsigned long)start, size))
return false;
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/4] dma-direct: Provide function to check physical memory area validity
dma_coherent_ok() checks if a physical memory area fits a device's DMA
constraints.
Signed-off-by: Nicolas Saenz Julienne
---
include/linux/dma-direct.h | 1 +
kernel/dma/direct.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
index 5184735a0fe8..ab2e20cba951 100644
--- a/include/linux/dma-direct.h
+++ b/include/linux/dma-direct.h
@@ -69,6 +69,7 @@ static inline bool dma_capable(struct device *dev, dma_addr_t
addr, size_t size,
u64 dma_direct_get_required_mask(struct device *dev);
gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
u64 *phys_mask);
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size);
void *dma_direct_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
gfp_t gfp, unsigned long attrs);
void dma_direct_free(struct device *dev, size_t size, void *cpu_addr,
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 95866b647581..67f060b86a73 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -70,7 +70,7 @@ gfp_t dma_direct_optimal_gfp_mask(struct device *dev, u64
dma_mask,
return 0;
}
-static bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
+bool dma_coherent_ok(struct device *dev, phys_addr_t phys, size_t size)
{
return phys_to_dma_direct(dev, phys) + size - 1 <=
min_not_zero(dev->coherent_dma_mask,
dev->bus_dma_limit);
--
2.27.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 0/4] dma-pool: Fix atomic pool selection
This is my attempt at fixing one of the regressions we've seen[1] after the introduction of per-zone atomic pools. This combined with "dma-pool: Do not allocate pool memory from CMA"[2] should fix the boot issues on Jeremy's RPi4 setup. [1] https://lkml.org/lkml/2020/7/2/974 [2] https://lkml.org/lkml/2020/7/8/1108 --- Changes since v1: - Rebase into linus' master Nicolas Saenz Julienne (4): dma-direct: Provide function to check physical memory area validity dma-pool: Get rid of dma_in_atomic_pool() dma-pool: Introduce dma_guess_pool() dma-pool: Make sure atomic pool suits device include/linux/dma-direct.h | 1 + kernel/dma/direct.c| 2 +- kernel/dma/pool.c | 80 +- 3 files changed, 56 insertions(+), 27 deletions(-) -- 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
Hi Shimoda-san,
On Tue, Jul 14, 2020 at 1:42 PM Yoshihiro Shimoda
wrote:
> > From: Geert Uytterhoeven, Sent: Tuesday, July 14, 2020 5:42 PM
> > On Tue, Jul 14, 2020 at 10:30 AM Lad, Prabhakar
> > wrote:
> > > On Tue, Jul 14, 2020 at 9:09 AM Geert Uytterhoeven
> > > wrote:
> > > > On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
> > > Also the recent patch to add
> > > "r8a77961" just adds to soc_rcar_gen3_whitelist.
> >
> > Oops, commit 17fe16181639801b ("iommu/renesas: Add support for r8a77961")
> > did it wrong, too.
>
> Thank you for the point it out. We should add r8a77961 to the soc_rcar_gen3[].
> However, I don't know why I could not realize this issue...
> So, I investigated this a little and then, IIUC, glob_match() which
> soc_device_match() uses seems to return true, if *pat = "r8a7796" and *str =
> "r8a77961".
Are you sure about this?
I enabled CONFIG_GLOB_SELFTEST, and globtest succeeded.
It does test glob_match("a", "aa"), which is a similar test.
To be 100% sure, I added:
--- a/lib/globtest.c
+++ b/lib/globtest.c
@@ -59,6 +59,7 @@ static char const glob_tests[] __initconst =
"1" "a\0" "a\0"
"0" "a\0" "b\0"
"0" "a\0" "aa\0"
+ "0" "r8a7796\0" "r8a77961\0"
"0" "a\0" "\0"
"1" "\0" "\0"
"0" "\0" "a\0"
and it still succeeded.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/exynos: Rename update_pte()
The name "update_pte" is a little too generic, and can end up clashing
with architecture pagetable code leaked out of common mm headers. Rename
it to something more appropriately namespaced.
Reported-by: kernel test robot
Acked-by: Marek Szyprowski
Signed-off-by: Robin Murphy
---
v2: fix typo - thanks Marek!
---
drivers/iommu/exynos-iommu.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 60c8a56e4a3f..75cdd37fae38 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -721,7 +721,7 @@ static struct platform_driver exynos_sysmmu_driver
__refdata = {
}
};
-static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
+static inline void exynos_iommu_set_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
{
dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent),
DMA_TO_DEVICE);
@@ -933,7 +933,7 @@ static sysmmu_pte_t *alloc_lv2entry(struct
exynos_iommu_domain *domain,
if (!pent)
return ERR_PTR(-ENOMEM);
- update_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
+ exynos_iommu_set_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
kmemleak_ignore(pent);
*pgcounter = NUM_LV2ENTRIES;
handle = dma_map_single(dma_dev, pent, LV2TABLE_SIZE,
@@ -994,7 +994,7 @@ static int lv1set_section(struct exynos_iommu_domain
*domain,
*pgcnt = 0;
}
- update_pte(sent, mk_lv1ent_sect(paddr, prot));
+ exynos_iommu_set_pte(sent, mk_lv1ent_sect(paddr, prot));
spin_lock(>lock);
if (lv1ent_page_zero(sent)) {
@@ -1018,7 +1018,7 @@ static int lv2set_page(sysmmu_pte_t *pent, phys_addr_t
paddr, size_t size,
if (WARN_ON(!lv2ent_fault(pent)))
return -EADDRINUSE;
- update_pte(pent, mk_lv2ent_spage(paddr, prot));
+ exynos_iommu_set_pte(pent, mk_lv2ent_spage(paddr, prot));
*pgcnt -= 1;
} else { /* size == LPAGE_SIZE */
int i;
@@ -1150,7 +1150,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
*iommu_domain,
}
/* workaround for h/w bug in System MMU v3.3 */
- update_pte(ent, ZERO_LV2LINK);
+ exynos_iommu_set_pte(ent, ZERO_LV2LINK);
size = SECT_SIZE;
goto done;
}
@@ -1171,7 +1171,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
*iommu_domain,
}
if (lv2ent_small(ent)) {
- update_pte(ent, 0);
+ exynos_iommu_set_pte(ent, 0);
size = SPAGE_SIZE;
domain->lv2entcnt[lv1ent_offset(iova)] += 1;
goto done;
--
2.26.2.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/dma: Avoid SAC address trick for PCIe devices
On 2020-07-13 14:14, Joerg Roedel wrote: On Wed, Jul 08, 2020 at 12:32:42PM +0100, Robin Murphy wrote: As for the intel-iommu implementation, relegate the opportunistic attempt to allocate a SAC address to the domain of conventional PCI devices only, to avoid it increasingly causing far more performance issues than possible benefits on modern PCI Express systems. Signed-off-by: Robin Murphy --- drivers/iommu/dma-iommu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c index 4959f5df21bd..0ff124f16ad4 100644 --- a/drivers/iommu/dma-iommu.c +++ b/drivers/iommu/dma-iommu.c @@ -426,7 +426,8 @@ static dma_addr_t iommu_dma_alloc_iova(struct iommu_domain *domain, dma_limit = min(dma_limit, (u64)domain->geometry.aperture_end); /* Try to get PCI devices a SAC address */ - if (dma_limit > DMA_BIT_MASK(32) && dev_is_pci(dev)) + if (dma_limit > DMA_BIT_MASK(32) && + dev_is_pci(dev) && !pci_is_pcie(to_pci_dev(dev))) iova = alloc_iova_fast(iovad, iova_len, DMA_BIT_MASK(32) >> shift, false); Unfortunatly this patch causes XHCI initialization failures on my AMD Ryzen system. I will remove both from the IOMMU tree for now. I guess the XHCI chip in my system does not support full 64bit dma addresses and needs a quirk or something like that. But until this is resolved its better to not change the IOVA allocation behavior. Oh bother - yes, this could have been masking all manner of bugs. That system will presumably also break if you managed to exhaust the 32-bit IOVA space such that the allocator moved up to the higher range anyway, or if you passed the XHCI through to a VM with a sufficiently wacky GPA layout, but I guess those are cases that simply nobody's run into yet. Does the firmware actually report any upper address constraint such that Sebastian's IVRS aperture patches might help? Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/exynos: Rename update_pte()
Hi Robin,
On 14.07.2020 13:33, Robin Murphy wrote:
> The name "update_pte" is a little too generic, and can end up clashing
> with architecture pagetable code leaked out of common mm headers. Rename
> it to something mroe appropriately namespaced.
s/mroe/more
> Reported-by: kernel test robot
> Signed-off-by: Robin Murphy
Acked-by: Marek Szyprowski
> ---
> drivers/iommu/exynos-iommu.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 60c8a56e4a3f..75cdd37fae38 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -721,7 +721,7 @@ static struct platform_driver exynos_sysmmu_driver
> __refdata = {
> }
> };
>
> -static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
> +static inline void exynos_iommu_set_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
> {
> dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent),
> DMA_TO_DEVICE);
> @@ -933,7 +933,7 @@ static sysmmu_pte_t *alloc_lv2entry(struct
> exynos_iommu_domain *domain,
> if (!pent)
> return ERR_PTR(-ENOMEM);
>
> - update_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
> + exynos_iommu_set_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
> kmemleak_ignore(pent);
> *pgcounter = NUM_LV2ENTRIES;
> handle = dma_map_single(dma_dev, pent, LV2TABLE_SIZE,
> @@ -994,7 +994,7 @@ static int lv1set_section(struct exynos_iommu_domain
> *domain,
> *pgcnt = 0;
> }
>
> - update_pte(sent, mk_lv1ent_sect(paddr, prot));
> + exynos_iommu_set_pte(sent, mk_lv1ent_sect(paddr, prot));
>
> spin_lock(>lock);
> if (lv1ent_page_zero(sent)) {
> @@ -1018,7 +1018,7 @@ static int lv2set_page(sysmmu_pte_t *pent, phys_addr_t
> paddr, size_t size,
> if (WARN_ON(!lv2ent_fault(pent)))
> return -EADDRINUSE;
>
> - update_pte(pent, mk_lv2ent_spage(paddr, prot));
> + exynos_iommu_set_pte(pent, mk_lv2ent_spage(paddr, prot));
> *pgcnt -= 1;
> } else { /* size == LPAGE_SIZE */
> int i;
> @@ -1150,7 +1150,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
> *iommu_domain,
> }
>
> /* workaround for h/w bug in System MMU v3.3 */
> - update_pte(ent, ZERO_LV2LINK);
> + exynos_iommu_set_pte(ent, ZERO_LV2LINK);
> size = SECT_SIZE;
> goto done;
> }
> @@ -1171,7 +1171,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
> *iommu_domain,
> }
>
> if (lv2ent_small(ent)) {
> - update_pte(ent, 0);
> + exynos_iommu_set_pte(ent, 0);
> size = SPAGE_SIZE;
> domain->lv2entcnt[lv1ent_offset(iova)] += 1;
> goto done;
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] dma-pool: Introduce dma_guess_pool()
On Tue, 2020-07-14 at 13:22 +0200, Christoph Hellwig wrote: > This one doesn't appear to actually apply on top of Linus' tree plus the > two previous patches. I'll take care of it. signature.asc Description: This is a digitally signed message part ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/exynos: Rename update_pte()
The name "update_pte" is a little too generic, and can end up clashing
with architecture pagetable code leaked out of common mm headers. Rename
it to something mroe appropriately namespaced.
Reported-by: kernel test robot
Signed-off-by: Robin Murphy
---
drivers/iommu/exynos-iommu.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 60c8a56e4a3f..75cdd37fae38 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -721,7 +721,7 @@ static struct platform_driver exynos_sysmmu_driver
__refdata = {
}
};
-static inline void update_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
+static inline void exynos_iommu_set_pte(sysmmu_pte_t *ent, sysmmu_pte_t val)
{
dma_sync_single_for_cpu(dma_dev, virt_to_phys(ent), sizeof(*ent),
DMA_TO_DEVICE);
@@ -933,7 +933,7 @@ static sysmmu_pte_t *alloc_lv2entry(struct
exynos_iommu_domain *domain,
if (!pent)
return ERR_PTR(-ENOMEM);
- update_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
+ exynos_iommu_set_pte(sent, mk_lv1ent_page(virt_to_phys(pent)));
kmemleak_ignore(pent);
*pgcounter = NUM_LV2ENTRIES;
handle = dma_map_single(dma_dev, pent, LV2TABLE_SIZE,
@@ -994,7 +994,7 @@ static int lv1set_section(struct exynos_iommu_domain
*domain,
*pgcnt = 0;
}
- update_pte(sent, mk_lv1ent_sect(paddr, prot));
+ exynos_iommu_set_pte(sent, mk_lv1ent_sect(paddr, prot));
spin_lock(>lock);
if (lv1ent_page_zero(sent)) {
@@ -1018,7 +1018,7 @@ static int lv2set_page(sysmmu_pte_t *pent, phys_addr_t
paddr, size_t size,
if (WARN_ON(!lv2ent_fault(pent)))
return -EADDRINUSE;
- update_pte(pent, mk_lv2ent_spage(paddr, prot));
+ exynos_iommu_set_pte(pent, mk_lv2ent_spage(paddr, prot));
*pgcnt -= 1;
} else { /* size == LPAGE_SIZE */
int i;
@@ -1150,7 +1150,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
*iommu_domain,
}
/* workaround for h/w bug in System MMU v3.3 */
- update_pte(ent, ZERO_LV2LINK);
+ exynos_iommu_set_pte(ent, ZERO_LV2LINK);
size = SECT_SIZE;
goto done;
}
@@ -1171,7 +1171,7 @@ static size_t exynos_iommu_unmap(struct iommu_domain
*iommu_domain,
}
if (lv2ent_small(ent)) {
- update_pte(ent, 0);
+ exynos_iommu_set_pte(ent, 0);
size = SPAGE_SIZE;
domain->lv2entcnt[lv1ent_offset(iova)] += 1;
goto done;
--
2.26.2.dirty
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] dma-pool: Introduce dma_guess_pool()
This one doesn't appear to actually apply on top of Linus' tree plus the two previous patches. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] dma-mapping: Add bounced DMA ops
On Mon, Jul 13, 2020 at 12:55:43PM +0100, Robin Murphy wrote: > On 2020-07-13 10:12, Claire Chang wrote: >> The bounced DMA ops provide an implementation of DMA ops that bounce >> streaming DMA in and out of a specially allocated region. Only the >> operations relevant to streaming DMA are supported. > > I think there are too many implicit assumptions here - apparently that > coherent allocations will always be intercepted by > dma_*_from_dev_coherent(), and that calling into dma-direct won't actually > bounce things a second time beyond where you thought they were going, > manage coherency for a different address, and make it all go subtly wrong. > Consider "swiotlb=force", for instance... > > Again, plumbing this straight into dma-direct so that SWIOTLB can simply > target a different buffer and always bounce regardless of masks would seem > a far better option. I haven't really had time to read through the details, but I agree that any bouncing scheme should reuse the swiotlb code and not invent a parallel infrastructure. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/4] Add system mmu support for Armada-806
Hi Will, On 14.07.2020 10:19, Will Deacon wrote: Hi Tomasz, On Thu, Jul 02, 2020 at 10:16:29PM +0200, Tomasz Nowicki wrote: There were already two versions of series to support SMMU for AP806, including workaround for accessing ARM SMMU 64bit registers. First [1] by Hanna Hawa and second [2] by Gregory CLEMENT. Since it got stuck this is yet another try. I incorporated the V2 comments, mainly by moving workaround code to arm-smmu-impl.c as requested. For the record, AP-806 can't access SMMU registers with 64bit width, this patches split the readq/writeq into two 32bit accesses instead and update DT bindings. The series was successfully tested on a vanilla v5.8-rc3 kernel and Intel e1000e PCIe NIC. The same for platform devices like SATA and USB. [1]: https://lkml.org/lkml/2018/10/15/373 [2]: https://lkml.org/lkml/2019/7/11/426 Do you have a v4 of this series? It looks like there were a few comments left to address, but with that I can pick it up for 5.9. Yes, I will send it out today. Thanks, Tomasz ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] iommu/ipmmu-vmsa: Add entry for R8A774E1 and r8a77961
Hi All, Patch 1/2 was posted as part of series [1] as pointed out by Geert we need to have an entry in both the lists soc_rcar_gen3 and soc_rcar_gen3_whitelist to enable iommu unconditionally, this is now fixed in patch 1/2, also note the DT binding documentation for R8A774E1 is part of [1]. Where as patch 2/2 is a similar FIX for r8a77961. Cheers, Prabhakar [1] https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=317627 Lad Prabhakar (1): iommu/ipmmu-vmsa: Add an entry for r8a77961 in soc_rcar_gen3[] Marian-Cristian Rotariu (1): iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code drivers/iommu/ipmmu-vmsa.c | 6 ++ 1 file changed, 6 insertions(+) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/2] iommu/ipmmu-vmsa: Add an entry for r8a77961 in soc_rcar_gen3[]
Add an entry for r8a77961 in soc_rcar_gen3[] list so that we dont
enable iommu unconditionally.
Fixes: 17fe161816398 ("iommu/renesas: Add support for r8a77961")
Signed-off-by: Lad Prabhakar
---
drivers/iommu/ipmmu-vmsa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 55c09cb8fc56..093b4ac5db69 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -741,6 +741,7 @@ static const struct soc_device_attribute soc_rcar_gen3[] = {
{ .soc_id = "r8a774c0", },
{ .soc_id = "r8a774e1", },
{ .soc_id = "r8a7795", },
+ { .soc_id = "r8a77961", },
{ .soc_id = "r8a7796", },
{ .soc_id = "r8a77965", },
{ .soc_id = "r8a77970", },
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
From: Marian-Cristian Rotariu
Add support for RZ/G2H (R8A774E1) SoC IPMMUs.
Signed-off-by: Marian-Cristian Rotariu
Signed-off-by: Lad Prabhakar
---
drivers/iommu/ipmmu-vmsa.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index b90cd9ff96f6..55c09cb8fc56 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -739,6 +739,7 @@ static const struct soc_device_attribute soc_rcar_gen3[] = {
{ .soc_id = "r8a774a1", },
{ .soc_id = "r8a774b1", },
{ .soc_id = "r8a774c0", },
+ { .soc_id = "r8a774e1", },
{ .soc_id = "r8a7795", },
{ .soc_id = "r8a7796", },
{ .soc_id = "r8a77965", },
@@ -751,6 +752,7 @@ static const struct soc_device_attribute soc_rcar_gen3[] = {
static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
{ .soc_id = "r8a774b1", },
{ .soc_id = "r8a774c0", },
+ { .soc_id = "r8a774e1", },
{ .soc_id = "r8a7795", .revision = "ES3.*" },
{ .soc_id = "r8a77961", },
{ .soc_id = "r8a77965", },
@@ -963,6 +965,9 @@ static const struct of_device_id ipmmu_of_ids[] = {
}, {
.compatible = "renesas,ipmmu-r8a774c0",
.data = _features_rcar_gen3,
+ }, {
+ .compatible = "renesas,ipmmu-r8a774e1",
+ .data = _features_rcar_gen3,
}, {
.compatible = "renesas,ipmmu-r8a7795",
.data = _features_rcar_gen3,
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v5 03/15] iommu/smmu: Report empty domain nesting info
Hi Will,
> From: Will Deacon
> Sent: Monday, July 13, 2020 9:15 PM
>
> On Sun, Jul 12, 2020 at 04:20:58AM -0700, Liu Yi L wrote:
> > This patch is added as instead of returning a boolean for
> > DOMAIN_ATTR_NESTING,
> > iommu_domain_get_attr() should return an iommu_nesting_info handle.
> >
> > Cc: Will Deacon
> > Cc: Robin Murphy
> > Cc: Eric Auger
> > Cc: Jean-Philippe Brucker
> > Suggested-by: Jean-Philippe Brucker
> > Signed-off-by: Liu Yi L
> > Signed-off-by: Jacob Pan
> > ---
> > v4 -> v5:
> > *) address comments from Eric Auger.
> > ---
> > drivers/iommu/arm-smmu-v3.c | 29 +++--
> > drivers/iommu/arm-smmu.c| 29 +++--
> > 2 files changed, 54 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> > index f578677..ec815d7 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -3019,6 +3019,32 @@ static struct iommu_group
> *arm_smmu_device_group(struct device *dev)
> > return group;
> > }
> >
> > +static int arm_smmu_domain_nesting_info(struct arm_smmu_domain
> *smmu_domain,
> > + void *data)
> > +{
> > + struct iommu_nesting_info *info = (struct iommu_nesting_info *)data;
> > + unsigned int size;
> > +
> > + if (!info || smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
> > + return -ENODEV;
> > +
> > + size = sizeof(struct iommu_nesting_info);
> > +
> > + /*
> > +* if provided buffer size is smaller than expected, should
> > +* return 0 and also the expected buffer size to caller.
> > +*/
> > + if (info->size < size) {
> > + info->size = size;
> > + return 0;
> > + }
> > +
> > + /* report an empty iommu_nesting_info for now */
> > + memset(info, 0x0, size);
> > + info->size = size;
> > + return 0;
> > +}
>
> Have you verified that this doesn't break the existing usage of
> DOMAIN_ATTR_NESTING in drivers/vfio/vfio_iommu_type1.c?
I didn't have ARM machine on my hand. But I contacted with Jean
Philippe, he confirmed no compiling issue. I didn't see any code
getting DOMAIN_ATTR_NESTING attr in current drivers/vfio/vfio_iommu_type1.c.
What I'm adding is to call iommu_domai_get_attr(, DOMAIN_ATTR_NESTIN)
and won't fail if the iommu_domai_get_attr() returns 0. This patch
returns an empty nesting info for DOMAIN_ATTR_NESTIN and return
value is 0 if no error. So I guess it won't fail nesting for ARM.
@Eric, how about your opinion? your dual-stage vSMMU support may
also share the vfio_iommu_type1.c code.
Regards,
Yi Liu
> Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 11/21] iommu/mediatek: Add power-domain operation
On Mon, 2020-07-13 at 15:03 +0800, Pi-Hsun Shih wrote:
> On Sat, Jul 11, 2020 at 2:51 PM Yong Wu wrote:
> >
> > In the previous SoC, the M4U HW is in the EMI power domain which is
> > always on. the latest M4U is in the display power domain which may be
> > turned on/off, thus we have to add pm_runtime interface for it.
> >
> > we should enable its power before M4U hw initial. and disable it after HW
> > initialize.
> >
> > When the engine work, the engine always enable the power and clocks for
> > smi-larb/smi-common, then the M4U's power will always be powered on
> > automatically via the device link with smi-common.
> >
> > Note: we don't enable the M4U power in iommu_map/unmap for tlb flush.
> > If its power already is on, of course it is ok. if the power is off,
> > the main tlb will be reset while M4U power on, thus the tlb flush while
> > m4u power off is unnecessary, just skip it.
> >
> > Signed-off-by: Yong Wu
> > ---
> > drivers/iommu/mtk_iommu.c | 54 ++-
> > 1 file changed, 47 insertions(+), 7 deletions(-)
> > ...
> > for_each_m4u(data) {
> > + /* skip tlb flush when pm is not active */
> > + if (pm_runtime_enabled(data->dev) &&
> > + !pm_runtime_active(data->dev))
> > + continue;
> > +
>
> pm_runtime_active(dev) == false implies dev->power.disable_depth == 0,
> which implies pm_runtime_enabled(dev) == true, so the
> pm_runtime_enabled(data->dev) can be omitted here.
Yes. Thanks.
Will fix in next version.
>
> > spin_lock_irqsave(>tlb_lock, flags);
> > writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
> >data->base + data->plat_data->inv_sel_reg);
> > ...
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 12/21] iommu/mediatek: Add iova reserved function
On Mon, 2020-07-13 at 15:33 +0800, Pi-Hsun Shih wrote:
> On Sat, Jul 11, 2020 at 2:51 PM Yong Wu wrote:
> >
> > For multiple iommu_domains, we need to reserve some iova regions, so we
> > will add mtk_iommu_iova_region structure. It includes the base address
> > and size of the range.
> > This is a preparing patch for supporting multi-domain.
> >
> > Signed-off-by: Anan sun
> > Signed-off-by: Hao Chao
> > Signed-off-by: Yong Wu
> > ---
> > drivers/iommu/mtk_iommu.c | 37 +
> > drivers/iommu/mtk_iommu.h | 5 +
> > 2 files changed, 42 insertions(+)
> >
> > diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> > index 03a6d66f4bef..fdfdb75706e0 100644
> > --- a/drivers/iommu/mtk_iommu.c
> > +++ b/drivers/iommu/mtk_iommu.c
> > @@ -151,6 +151,11 @@ static LIST_HEAD(m4ulist); /* List all the M4U HWs */
> > ...
> > +
> > +static void mtk_iommu_put_resv_regions(struct device *dev,
> > + struct list_head *head)
> > +{
> > + struct iommu_resv_region *entry, *next;
> > +
> > + list_for_each_entry_safe(entry, next, head, list)
> > + kfree(entry);
> > +}
> > +
>
> This is the same as generic_iommu_put_resv_regions, use that as the
> .put_resv_regions callback instead?
Thanks very much for the review.
Yes. I will fix it in next version.
>
> > ...
> > --
> > 2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
Hi Prabhakar,
On Tue, Jul 14, 2020 at 10:30 AM Lad, Prabhakar
wrote:
> On Tue, Jul 14, 2020 at 9:09 AM Geert Uytterhoeven
> wrote:
> > On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
> > wrote:
> > > From: Marian-Cristian Rotariu
> > >
> > > Add support for RZ/G2H (R8A774E1) SoC IPMMUs.
> > >
> > > Signed-off-by: Marian-Cristian Rotariu
> > >
> > > Signed-off-by: Lad Prabhakar
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > @@ -751,6 +751,7 @@ static const struct soc_device_attribute
> > > soc_rcar_gen3[] = {
> > > static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> > > { .soc_id = "r8a774b1", },
> > > { .soc_id = "r8a774c0", },
> > > + { .soc_id = "r8a774e1", },
> >
> > Adding an entry to soc_rcar_gen3_whitelist[] doesn't do anything, unless
> > you also add the same entry to soc_rcar_gen3[].
> >
> I think the comment "For R-Car Gen3 use a white list to opt-in slave
> devices." is misleading. Booting through the kernel I do see iommu
> groups (attached is the logs).
Indeed. Without an entry in soc_rcar_gen3[], the IPMMU is enabled
unconditionally, and soc_rcar_gen3_whitelist[] is ignored.
That's why you want an entry in both, unless you have an R-Car Gen3
SoC where the IPMMU works correctly with all slave devices present.
Perhaps soc_rcar_gen3[] should be renamed to soc_rcar_gen3_greylist[]
(or soc_rcar_gen3_maybelist[]) to make this clear?
> Also the recent patch to add
> "r8a77961" just adds to soc_rcar_gen3_whitelist.
Oops, commit 17fe16181639801b ("iommu/renesas: Add support for r8a77961")
did it wrong, too.
> > > { .soc_id = "r8a7795", .revision = "ES3.*" },
> > > { .soc_id = "r8a77961", },
> > > { .soc_id = "r8a77965", },
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] iommu: Add gfp parameter to io_pgtable_ops->map()
On Fri, Jun 12, 2020 at 11:39:55AM +0800, Baolin Wang wrote:
> Now the ARM page tables are always allocated by GFP_ATOMIC parameter,
> but the iommu_ops->map() function has been added a gfp_t parameter by
> commit 781ca2de89ba ("iommu: Add gfp parameter to iommu_ops::map"),
> thus io_pgtable_ops->map() should use the gfp parameter passed from
> iommu_ops->map() to allocate page pages, which can avoid wasting the
> memory allocators atomic pools for some non-atomic contexts.
>
> Signed-off-by: Baolin Wang
> ---
> drivers/gpu/drm/panfrost/panfrost_mmu.c | 2 +-
> drivers/iommu/arm-smmu-v3.c | 2 +-
> drivers/iommu/arm-smmu.c| 2 +-
> drivers/iommu/io-pgtable-arm-v7s.c | 18 +-
> drivers/iommu/io-pgtable-arm.c | 18 +-
> drivers/iommu/ipmmu-vmsa.c | 2 +-
> drivers/iommu/msm_iommu.c | 2 +-
> drivers/iommu/mtk_iommu.c | 2 +-
> drivers/iommu/qcom_iommu.c | 2 +-
> include/linux/io-pgtable.h | 2 +-
> 10 files changed, 26 insertions(+), 26 deletions(-)
I was a bit nervous about us passing GFP_KERNEL with a spinlock held, but
it looks like you've checked all the callsites and it looks fine to me, so:
Acked-by: Will Deacon
Joerg -- not sure what you want to do with this one, as it's likely to
conflict (trivially) with unrelated driver changes.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] iommu: Mark __iommu_map/__iommu_map_sg as static
On Fri, Jun 12, 2020 at 11:39:54AM +0800, Baolin Wang wrote: > Now the __iommu_map() and __iommu_map_sg() are used only in iommu.c > file, so mark them as static. > > Signed-off-by: Baolin Wang > --- > drivers/iommu/iommu.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Acked-by: Will Deacon (I'm assuming Joerg will pick this up for 5.9) Thanks, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
On Tue, Jul 14, 2020 at 01:57:03PM +0800, Lu Baolu wrote: > Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group(). > It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the > vfio_group data structure so that it could be reused in other places. This removes the last user of iommu_aux_attach_device and iommu_aux_detach_device, which can be removed now. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/4] Add system mmu support for Armada-806
Hi Tomasz, On Thu, Jul 02, 2020 at 10:16:29PM +0200, Tomasz Nowicki wrote: > There were already two versions of series to support SMMU for AP806, > including workaround for accessing ARM SMMU 64bit registers. > First [1] by Hanna Hawa and second [2] by Gregory CLEMENT. > Since it got stuck this is yet another try. I incorporated the V2 comments, > mainly by moving workaround code to arm-smmu-impl.c as requested. > > For the record, AP-806 can't access SMMU registers with 64bit width, > this patches split the readq/writeq into two 32bit accesses instead > and update DT bindings. > > The series was successfully tested on a vanilla v5.8-rc3 kernel and > Intel e1000e PCIe NIC. The same for platform devices like SATA and USB. > > [1]: https://lkml.org/lkml/2018/10/15/373 > [2]: https://lkml.org/lkml/2019/7/11/426 Do you have a v4 of this series? It looks like there were a few comments left to address, but with that I can pick it up for 5.9. Cheers, Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 8/9] dt-bindings: net: renesas,ravb: Add support for r8a774e1 SoC
Hello! On 14.07.2020 0:35, Lad Prabhakar wrote: From: Marian-Cristian Rotariu Document RZ/G2H (R8A774E1) SoC bindings. Signed-off-by: Marian-Cristian Rotariu Signed-off-by: Lad Prabhakar Reviewed-by: Sergei Shtylyov [...] MBR, Sergei ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/9] iommu/ipmmu-vmsa: Hook up R8A774E1 DT matching code
Hi Prabhakar,
On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
wrote:
> From: Marian-Cristian Rotariu
>
> Add support for RZ/G2H (R8A774E1) SoC IPMMUs.
>
> Signed-off-by: Marian-Cristian Rotariu
>
> Signed-off-by: Lad Prabhakar
Thanks for your patch!
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -751,6 +751,7 @@ static const struct soc_device_attribute soc_rcar_gen3[]
> = {
> static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> { .soc_id = "r8a774b1", },
> { .soc_id = "r8a774c0", },
> + { .soc_id = "r8a774e1", },
Adding an entry to soc_rcar_gen3_whitelist[] doesn't do anything, unless
you also add the same entry to soc_rcar_gen3[].
> { .soc_id = "r8a7795", .revision = "ES3.*" },
> { .soc_id = "r8a77961", },
> { .soc_id = "r8a77965", },
> @@ -963,6 +964,9 @@ static const struct of_device_id ipmmu_of_ids[] = {
> }, {
> .compatible = "renesas,ipmmu-r8a774c0",
> .data = _features_rcar_gen3,
> + }, {
> + .compatible = "renesas,ipmmu-r8a774e1",
> + .data = _features_rcar_gen3,
> }, {
> .compatible = "renesas,ipmmu-r8a7795",
> .data = _features_rcar_gen3,
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 8/9] dt-bindings: net: renesas,ravb: Add support for r8a774e1 SoC
On Mon, Jul 13, 2020 at 11:36 PM Lad Prabhakar
wrote:
> From: Marian-Cristian Rotariu
>
> Document RZ/G2H (R8A774E1) SoC bindings.
>
> Signed-off-by: Marian-Cristian Rotariu
>
> Signed-off-by: Lad Prabhakar
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 6/9] dt-bindings: gpio: renesas, rcar-gpio: Add r8a774e1 support
On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
wrote:
> Document Renesas RZ/G2H (R8A774E1) GPIO blocks compatibility within the
> relevant dt-bindings.
>
> Signed-off-by: Lad Prabhakar
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 5/5] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
On Mon, Jul 13, 2020 at 01:48:29PM -0700, John Stultz wrote: > On Mon, Jul 13, 2020 at 1:41 PM Will Deacon wrote: > > On Fri, Jul 10, 2020 at 03:21:53PM -0700, John Stultz wrote: > > > On Fri, Jul 10, 2020 at 12:54 AM Will Deacon wrote: > > > > On Thu, Jul 09, 2020 at 08:28:45PM -0700, John Stultz wrote: > > > > > On Thu, Jul 2, 2020 at 7:18 AM Will Deacon wrote: > > > > > > On Thu, Jun 25, 2020 at 12:10:39AM +, John Stultz wrote: > > > > > > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > > > > > > > index b510f67dfa49..714893535dd2 100644 > > > > > > > --- a/drivers/iommu/Kconfig > > > > > > > +++ b/drivers/iommu/Kconfig > > > > > > > @@ -381,6 +381,7 @@ config SPAPR_TCE_IOMMU > > > > > > > config ARM_SMMU > > > > > > > tristate "ARM Ltd. System MMU (SMMU) Support" > > > > > > > depends on (ARM64 || ARM || (COMPILE_TEST && > > > > > > > !GENERIC_ATOMIC64)) && MMU > > > > > > > + depends on QCOM_SCM || !QCOM_SCM #if QCOM_SCM=m this can't > > > > > > > be =y > > > > > > > select IOMMU_API > > > > > > > select IOMMU_IO_PGTABLE_LPAE > > > > > > > select ARM_DMA_USE_IOMMU if ARM > > > > > > > > > > > > This looks like a giant hack. Is there another way to handle this? > > > > > > > > > > Sorry for the slow response here. > > > > > > > > > > So, I agree the syntax looks strange (requiring a comment obviously > > > > > isn't a good sign), but it's a fairly common way to ensure drivers > > > > > don't get built in if they optionally depend on another driver that > > > > > can be built as a module. > > > > > See "RFKILL || !RFKILL", "EXTCON || !EXTCON", or "USB_GADGET || > > > > > !USB_GADGET" in various Kconfig files. > > > > > > > > > > I'm open to using a different method, and in a different thread you > > > > > suggested using something like symbol_get(). I need to look into it > > > > > more, but that approach looks even more messy and prone to runtime > > > > > failures. Blocking the unwanted case at build time seems a bit cleaner > > > > > to me, even if the syntax is odd. > > > > > > > > Maybe just split it out then, so that the ARM_SMMU entry doesn't have > > > > this, > > > > as that driver _really_ doesn't care about SoC details like this. In > > > > other > > > > words, add a new entry along the lines of: > > > > > > > > config ARM_SMMU_QCOM_IMPL > > > > default y > > > > #if QCOM_SCM=m this can't be =y > > > > depends on ARM_SMMU & (QCOM_SCM || !QCOM_SCM) > > > > > > > > and then have arm-smmu.h provide a static inline qcom_smmu_impl_init() > > > > which returns -ENODEV if CONFIG_ARM_SMMU_QCOM_IMPL=n and hack the > > > > Makefile > > > > so that we don't bother to compile arm-smmu-qcom.o in that case. > > > > > > > > Would that work? > > > > > > I think this proposal still has problems with the directionality of the > > > call. > > > > > > The arm-smmu-impl.o calls to arm-smmu-qcom.o which calls qcom_scm.o > > > So if qcom_scm.o is part of a module, the calling code in > > > arm-smmu-qcom.o also needs to be a module, which means CONFIG_ARM_SMMU > > > needs to be a module. > > > > > > I know you said the arm-smmu driver doesn't care about SoC details, > > > but the trouble is that currently the arm-smmu driver does directly > > > call the qcom-scm code. So it is a real dependency. However, if > > > QCOM_SCM is not configured, it calls stubs and that's ok. In that > > > way, the "depends on QCOM_SCM || !QCOM_SCM" line actually makes sense. > > > It looks terrible because we're used to boolean logic, but it's > > > ternary. > > > > Yes, it looks ugly, but the part I really have issues with is that building > > QCOM_SCM=m and ARM_SMMU=y is perfectly fine if you don't run on an SoC > > with the qcom implementation. I don't see why we need to enforce things > > here beyond making sure that all selectable permutations _build_ and > > fail gracefully at runtime on the qcom SoC if SCM isn't available. > > Ok. Thanks, that context/rationale makes sense to me now! I'll dig in > and see if I can figure out the runtime get_symbol handling you > suggested for the scm callout. Cheers, but in the interest of not being a massive pain in the bum, please yell if it ends up being tonnes of work and I'll close my eyes while applying your original patch. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/9] dt-bindings: dma: renesas,rcar-dmac: Document R8A774E1 bindings
On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
wrote:
> Renesas RZ/G2H (R8A774E1) SoC also has the R-Car gen3 compatible
> DMA controllers, therefore document RZ/G2H specific bindings.
>
> Signed-off-by: Lad Prabhakar
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] dma-mapping: add a dma_ops_bypass flag to struct device
On 14/07/2020 17:07, Christoph Hellwig wrote:
> On Mon, Jul 13, 2020 at 02:59:39PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 09/07/2020 01:24, Christoph Hellwig wrote:
>>> Several IOMMU drivers have a bypass mode where they can use a direct
>>> mapping if the devices DMA mask is large enough. Add generic support
>>> to the core dma-mapping code to do that to switch those drivers to
>>> a common solution.
>>>
>>> Signed-off-by: Christoph Hellwig
>>> ---
>>> include/linux/device.h | 8 +
>>> kernel/dma/Kconfig | 8 +
>>> kernel/dma/mapping.c | 74 +-
>>> 3 files changed, 68 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/include/linux/device.h b/include/linux/device.h
>>> index 4c4af98321ebd6..1f71acf37f78d7 100644
>>> --- a/include/linux/device.h
>>> +++ b/include/linux/device.h
>>> @@ -523,6 +523,11 @@ struct dev_links_info {
>>> * sync_state() callback.
>>> * @dma_coherent: this particular device is dma coherent, even if the
>>> * architecture supports non-coherent devices.
>>> + * @dma_ops_bypass: If set to %true then the dma_ops are bypassed for the
>>> + * streaming DMA operations (->map_* / ->unmap_* / ->sync_*),
>>> + * and optionall (if the coherent mask is large enough) also
>>
>>
>> s/optionall/optional/g
>>
>> Otherwise the series looks good and works well on powernv and pseries.
>> Thanks,
>
> Can you give a formal ACK?
It did never matter before but sure :)
Tested-by: Alexey Kardashevskiy
Reviewed-by: Alexey Kardashevskiy
or you want me to reply to individual patches? Thanks,
--
Alexey
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/9] dt-bindings: iommu: renesas, ipmmu-vmsa: Add r8a774e1 support
Hi Prabhakar,
On Mon, Jul 13, 2020 at 11:35 PM Lad Prabhakar
wrote:
> Document RZ/G2H (R8A774E1) SoC bindings.
>
> Signed-off-by: Lad Prabhakar
Thanks for your patch!
> --- a/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
> +++ b/Documentation/devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml
> @@ -32,6 +32,7 @@ properties:
>- enum:
>- renesas,ipmmu-r8a774a1 # RZ/G2M
>- renesas,ipmmu-r8a774b1 # RZ/G2N
> + - renesas,ipmmu-r8a774e1 # RZ/G2H
>- renesas,ipmmu-r8a774c0 # RZ/G2E
Please preserve alphabetical sort order.
>- renesas,ipmmu-r8a7795 # R-Car H3
>- renesas,ipmmu-r8a7796 # R-Car M3-W
With the above fixed:
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] dma-mapping: add a dma_ops_bypass flag to struct device
On Mon, Jul 13, 2020 at 02:59:39PM +1000, Alexey Kardashevskiy wrote:
>
>
> On 09/07/2020 01:24, Christoph Hellwig wrote:
> > Several IOMMU drivers have a bypass mode where they can use a direct
> > mapping if the devices DMA mask is large enough. Add generic support
> > to the core dma-mapping code to do that to switch those drivers to
> > a common solution.
> >
> > Signed-off-by: Christoph Hellwig
> > ---
> > include/linux/device.h | 8 +
> > kernel/dma/Kconfig | 8 +
> > kernel/dma/mapping.c | 74 +-
> > 3 files changed, 68 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index 4c4af98321ebd6..1f71acf37f78d7 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -523,6 +523,11 @@ struct dev_links_info {
> > * sync_state() callback.
> > * @dma_coherent: this particular device is dma coherent, even if the
> > * architecture supports non-coherent devices.
> > + * @dma_ops_bypass: If set to %true then the dma_ops are bypassed for the
> > + * streaming DMA operations (->map_* / ->unmap_* / ->sync_*),
> > + * and optionall (if the coherent mask is large enough) also
>
>
> s/optionall/optional/g
>
> Otherwise the series looks good and works well on powernv and pseries.
> Thanks,
Can you give a formal ACK?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 3/4] iommu: Add iommu_aux_get_domain_for_dev()
The device driver needs an API to get its aux-domain. A typical usage
scenario is:
unsigned long pasid;
struct iommu_domain *domain;
struct device *dev = mdev_dev(mdev);
struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
domain = iommu_aux_get_domain_for_dev(dev);
if (!domain)
return -ENODEV;
pasid = iommu_aux_get_pasid(domain, iommu_device);
if (pasid <= 0)
return -EINVAL;
/* Program the device context */
This adds an API for such use case.
Suggested-by: Alex Williamson
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 18 ++
include/linux/iommu.h | 7 +++
2 files changed, 25 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index cad5a19ebf22..434bf42b6b9b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2817,6 +2817,24 @@ void iommu_aux_detach_group(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_aux_detach_group);
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *dev)
+{
+ struct iommu_domain *domain = NULL;
+ struct iommu_group *group;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return NULL;
+
+ if (group->aux_domain_attached)
+ domain = group->domain;
+
+ iommu_group_put(group);
+
+ return domain;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_get_domain_for_dev);
+
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9506551139ab..cda6cef7579e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -639,6 +639,7 @@ int iommu_aux_attach_group(struct iommu_domain *domain,
struct iommu_group *group, struct device *dev);
void iommu_aux_detach_group(struct iommu_domain *domain,
struct iommu_group *group, struct device *dev);
+struct iommu_domain *iommu_aux_get_domain_for_dev(struct device *dev);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm,
@@ -1040,6 +1041,12 @@ iommu_aux_detach_group(struct iommu_domain *domain,
{
}
+static inline struct iommu_domain *
+iommu_aux_get_domain_for_dev(struct device *dev)
+{
+ return NULL;
+}
+
static inline struct iommu_sva *
iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, void *drvdata)
{
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 0/4] iommu aux-domain APIs extensions
This series aims to extend the IOMMU aux-domain API set so that it
could be more friendly to vfio/mdev usage. The interactions between
vfio/mdev and iommu during mdev creation and passthr are:
1. Create a group for mdev with iommu_group_alloc();
2. Add the device to the group with
group = iommu_group_alloc();
if (IS_ERR(group))
return PTR_ERR(group);
ret = iommu_group_add_device(group, >dev);
if (!ret)
dev_info(>dev, "MDEV: group_id = %d\n",
iommu_group_id(group));
3. Allocate an aux-domain with iommu_domain_alloc();
4. Attach the aux-domain to the iommu_group.
iommu_group_for_each_dev {
if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
return iommu_aux_attach_device(domain, iommu_device);
else
return iommu_attach_device(domain, iommu_device);
}
where, iommu_device is the aux-domain-capable device. The mdev's in
the group are all derived from it.
In the whole process, an iommu group was allocated for the mdev and an
iommu domain was attached to the group, but the group->domain leaves
NULL. As the result, iommu_get_domain_for_dev() (or other similar
interfaces) doesn't work anymore.
The iommu_get_domain_for_dev() is a necessary interface for device
drivers that want to support vfio/mdev based aux-domain. For example,
unsigned long pasid;
struct iommu_domain *domain;
struct device *dev = mdev_dev(mdev);
struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
domain = iommu_get_domain_for_dev(dev);
if (!domain)
return -ENODEV;
pasid = iommu_aux_get_pasid(domain, iommu_device);
if (pasid <= 0)
return -EINVAL;
/* Program the device context */
We tried to address this by extending iommu_aux_at(de)tach_device() so that
the users could pass in an optional device pointer (for example vfio/mdev).
(v2 of this series)
https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu...@linux.intel.com/
But that will cause a lock issue as group->mutex has been applied in
iommu_group_for_each_dev(), but has to be reapplied again in the
iommu_aux_attach_device().
This version tries to address this by introducing two new APIs into the
aux-domain API set:
/**
* iommu_aux_attach_group - attach an aux-domain to an iommu_group which
* contains sub-devices (for example mdevs)
* derived from @dev.
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from @dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* Returns 0 on success, or an error value.
*/
int iommu_aux_attach_group(struct iommu_domain *domain,
struct iommu_group *group, struct device *dev)
/**
* iommu_aux_detach_group - detach an aux-domain from an iommu_group
*
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from @dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* @domain must have been attached to @group via
* iommu_aux_attach_group().
*/
void iommu_aux_detach_group(struct iommu_domain *domain,
struct iommu_group *group, struct device *dev)
This version is evolved according to feedbacks from Robin(v1) and
Alex(v2). Your comments are very appreciated.
Best regards,
baolu
---
Change log:
- v1->v2:
-
https://lore.kernel.org/linux-iommu/20200627031532.28046-1-baolu...@linux.intel.com/
- Suggested by Robin.
- v2->v3:
-
https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu...@linux.intel.com/
- Suggested by Alex
Lu Baolu (4):
iommu: Check IOMMU_DEV_FEAT_AUX feature in aux api's
iommu: Add iommu_aux_at(de)tach_group()
iommu: Add iommu_aux_get_domain_for_dev()
vfio/type1: Use iommu_aux_at(de)tach_group() APIs
drivers/iommu/iommu.c | 92 ++---
drivers/vfio/vfio_iommu_type1.c | 44 +++-
include/linux/iommu.h | 24 +
3 files changed, 116 insertions(+), 44 deletions(-)
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 4/4] vfio/type1: Use iommu_aux_at(de)tach_group() APIs
Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group().
It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the
vfio_group data structure so that it could be reused in other places.
Signed-off-by: Lu Baolu
---
drivers/vfio/vfio_iommu_type1.c | 44 ++---
1 file changed, 7 insertions(+), 37 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac9102a..f8812e68de77 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -100,6 +100,7 @@ struct vfio_dma {
struct vfio_group {
struct iommu_group *iommu_group;
struct list_headnext;
+ struct device *iommu_device;
boolmdev_group; /* An mdev group */
boolpinned_page_dirty_scope;
};
@@ -1627,45 +1628,13 @@ static struct device *vfio_mdev_get_iommu_device(struct
device *dev)
return NULL;
}
-static int vfio_mdev_attach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- return iommu_aux_attach_device(domain, iommu_device);
- else
- return iommu_attach_device(domain, iommu_device);
- }
-
- return -EINVAL;
-}
-
-static int vfio_mdev_detach_domain(struct device *dev, void *data)
-{
- struct iommu_domain *domain = data;
- struct device *iommu_device;
-
- iommu_device = vfio_mdev_get_iommu_device(dev);
- if (iommu_device) {
- if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
- iommu_aux_detach_device(domain, iommu_device);
- else
- iommu_detach_device(domain, iommu_device);
- }
-
- return 0;
-}
-
static int vfio_iommu_attach_group(struct vfio_domain *domain,
struct vfio_group *group)
{
if (group->mdev_group)
- return iommu_group_for_each_dev(group->iommu_group,
- domain->domain,
- vfio_mdev_attach_domain);
+ return iommu_aux_attach_group(domain->domain,
+ group->iommu_group,
+ group->iommu_device);
else
return iommu_attach_group(domain->domain, group->iommu_group);
}
@@ -1674,8 +1643,8 @@ static void vfio_iommu_detach_group(struct vfio_domain
*domain,
struct vfio_group *group)
{
if (group->mdev_group)
- iommu_group_for_each_dev(group->iommu_group, domain->domain,
-vfio_mdev_detach_domain);
+ iommu_aux_detach_group(domain->domain, group->iommu_group,
+ group->iommu_device);
else
iommu_detach_group(domain->domain, group->iommu_group);
}
@@ -2007,6 +1976,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
return 0;
}
+ group->iommu_device = iommu_device;
bus = iommu_device->bus;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 1/4] iommu: Check IOMMU_DEV_FEAT_AUX feature in aux api's
The iommu aux-domain api's work only when IOMMU_DEV_FEAT_AUX is enabled
for the device. Add this check to avoid misuse.
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1ed1e14a1f0c..e1fdd3531d65 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2725,11 +2725,13 @@ EXPORT_SYMBOL_GPL(iommu_dev_feature_enabled);
*/
int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev)
{
- int ret = -ENODEV;
+ int ret;
- if (domain->ops->aux_attach_dev)
- ret = domain->ops->aux_attach_dev(domain, dev);
+ if (!iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX) ||
+ !domain->ops->aux_attach_dev)
+ return -ENODEV;
+ ret = domain->ops->aux_attach_dev(domain, dev);
if (!ret)
trace_attach_device_to_domain(dev);
@@ -2748,12 +2750,12 @@ EXPORT_SYMBOL_GPL(iommu_aux_detach_device);
int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
{
- int ret = -ENODEV;
+ if (!iommu_dev_feature_enabled(dev, IOMMU_DEV_FEAT_AUX) ||
+ !domain->ops->aux_get_pasid)
+ return -ENODEV;
- if (domain->ops->aux_get_pasid)
- ret = domain->ops->aux_get_pasid(domain, dev);
+ return domain->ops->aux_get_pasid(domain, dev);
- return ret;
}
EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3 2/4] iommu: Add iommu_aux_at(de)tach_group()
This adds two new aux-domain APIs for a use case like vfio/mdev where
sub-devices derived from an aux-domain capable device are created and
put in an iommu_group.
/**
* iommu_aux_attach_group - attach an aux-domain to an iommu_group which
* contains sub-devices (for example mdevs) derived
* from @dev.
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from @dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* Returns 0 on success, or an error value.
*/
int iommu_aux_attach_group(struct iommu_domain *domain,
struct iommu_group *group,
struct device *dev)
/**
* iommu_aux_detach_group - detach an aux-domain from an iommu_group
*
* @domain: an aux-domain;
* @group: an iommu_group which contains sub-devices derived from @dev;
* @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
*
* @domain must have been attached to @group via iommu_aux_attach_group().
*/
void iommu_aux_detach_group(struct iommu_domain *domain,
struct iommu_group *group,
struct device *dev)
It also adds a flag in the iommu_group data structure to identify
an iommu_group with aux-domain attached from those normal ones.
Signed-off-by: Lu Baolu
---
drivers/iommu/iommu.c | 58 +++
include/linux/iommu.h | 17 +
2 files changed, 75 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e1fdd3531d65..cad5a19ebf22 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -45,6 +45,7 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ unsigned int aux_domain_attached:1;
};
struct group_device {
@@ -2759,6 +2760,63 @@ int iommu_aux_get_pasid(struct iommu_domain *domain,
struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
+/**
+ * iommu_aux_attach_group - attach an aux-domain to an iommu_group which
+ * contains sub-devices (for example mdevs) derived
+ * from @dev.
+ * @domain: an aux-domain;
+ * @group: an iommu_group which contains sub-devices derived from @dev;
+ * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
+ *
+ * Returns 0 on success, or an error value.
+ */
+int iommu_aux_attach_group(struct iommu_domain *domain,
+ struct iommu_group *group, struct device *dev)
+{
+ int ret = -EBUSY;
+
+ mutex_lock(>mutex);
+ if (group->domain)
+ goto out_unlock;
+
+ ret = iommu_aux_attach_device(domain, dev);
+ if (!ret) {
+ group->domain = domain;
+ group->aux_domain_attached = true;
+ }
+
+out_unlock:
+ mutex_unlock(>mutex);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_aux_attach_group);
+
+/**
+ * iommu_aux_detach_group - detach an aux-domain from an iommu_group
+ *
+ * @domain: an aux-domain;
+ * @group: an iommu_group which contains sub-devices derived from @dev;
+ * @dev:the physical device which supports IOMMU_DEV_FEAT_AUX.
+ *
+ * @domain must have been attached to @group via iommu_aux_attach_group().
+ */
+void iommu_aux_detach_group(struct iommu_domain *domain,
+ struct iommu_group *group, struct device *dev)
+{
+ mutex_lock(>mutex);
+
+ if (WARN_ON(!group->aux_domain_attached || group->domain != domain))
+ goto out_unlock;
+
+ iommu_aux_detach_device(domain, dev);
+ group->aux_domain_attached = false;
+ group->domain = NULL;
+
+out_unlock:
+ mutex_unlock(>mutex);
+}
+EXPORT_SYMBOL_GPL(iommu_aux_detach_group);
+
/**
* iommu_sva_bind_device() - Bind a process address space to a device
* @dev: the device
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5657d4fef9f2..9506551139ab 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -635,6 +635,10 @@ bool iommu_dev_feature_enabled(struct device *dev, enum
iommu_dev_features f);
int iommu_aux_attach_device(struct iommu_domain *domain, struct device *dev);
void iommu_aux_detach_device(struct iommu_domain *domain, struct device *dev);
int iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev);
+int iommu_aux_attach_group(struct iommu_domain *domain,
+ struct iommu_group *group, struct device *dev);
+void iommu_aux_detach_group(struct iommu_domain *domain,
+ struct iommu_group *group, struct device *dev);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm,
@@ -1023,6 +1027,19 @@ iommu_aux_get_pasid(struct iommu_domain *domain, struct
device *dev)
return -ENODEV;
}
+static inline int
+iommu_aux_attach_group(struct
