Re: [PATCH v5 01/14] iommu: Add dma ownership management interfaces
Hi Christoph,
On 1/4/22 6:08 PM, Christoph Hellwig wrote:
On Tue, Jan 04, 2022 at 09:56:31AM +0800, Lu Baolu wrote:
Multiple devices may be placed in the same IOMMU group because they
cannot be isolated from each other. These devices must either be
entirely under kernel control or userspace control, never a mixture.
This adds dma ownership management in iommu core and exposes several
interfaces for the device drivers and the device userspace assignment
framework (i.e. vfio), so that any conflict between user and kernel
controlled DMA could be detected at the beginning.
The device driver oriented interfaces are,
int iommu_device_use_dma_api(struct device *dev);
void iommu_device_unuse_dma_api(struct device *dev);
Devices under kernel drivers control must call iommu_device_use_dma_api()
before driver probes. The driver binding process must be aborted if it
returns failure.
The vfio oriented interfaces are,
int iommu_group_set_dma_owner(struct iommu_group *group,
void *owner);
void iommu_group_release_dma_owner(struct iommu_group *group);
bool iommu_group_dma_owner_claimed(struct iommu_group *group);
The device userspace assignment must be disallowed if the set dma owner
interface returns failure.
Signed-off-by: Jason Gunthorpe
Signed-off-by: Kevin Tian
Signed-off-by: Lu Baolu
---
include/linux/iommu.h | 31
drivers/iommu/iommu.c | 161 +-
2 files changed, 189 insertions(+), 3 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index de0c57a567c8..568f285468cf 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -682,6 +682,13 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+int iommu_device_use_dma_api(struct device *dev);
+void iommu_device_unuse_dma_api(struct device *dev);
+
+int iommu_group_set_dma_owner(struct iommu_group *group, void *owner);
+void iommu_group_release_dma_owner(struct iommu_group *group);
+bool iommu_group_dma_owner_claimed(struct iommu_group *group);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -1082,6 +1089,30 @@ static inline struct iommu_fwspec
*dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline int iommu_device_use_dma_api(struct device *dev)
+{
+ return 0;
+}
+
+static inline void iommu_device_unuse_dma_api(struct device *dev)
+{
+}
+
+static inline int
+iommu_group_set_dma_owner(struct iommu_group *group, void *owner)
+{
+ return -ENODEV;
+}
+
+static inline void iommu_group_release_dma_owner(struct iommu_group *group)
+{
+}
+
+static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group)
+{
+ return false;
+}
#endif /* CONFIG_IOMMU_API */
/**
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b86406b7162..ff0c8c1ad5af 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -48,6 +48,8 @@ struct iommu_group {
struct iommu_domain *default_domain;
struct iommu_domain *domain;
struct list_head entry;
+ unsigned int owner_cnt;
+ void *owner;
};
struct group_device {
@@ -289,7 +291,12 @@ int iommu_probe_device(struct device *dev)
mutex_lock(>mutex);
iommu_alloc_default_domain(group, dev);
- if (group->default_domain) {
+ /*
+* If device joined an existing group which has been claimed
+* for none kernel DMA purpose, avoid attaching the default
+* domain.
+*/
+ if (group->default_domain && !group->owner) {
ret = __iommu_attach_device(group->default_domain, dev);
if (ret) {
mutex_unlock(>mutex);
@@ -2320,7 +2327,7 @@ static int __iommu_attach_group(struct iommu_domain
*domain,
{
int ret;
- if (group->default_domain && group->domain != group->default_domain)
+ if (group->domain && group->domain != group->default_domain)
return -EBUSY;
ret = __iommu_group_for_each_dev(group, domain,
@@ -2357,7 +2364,11 @@ static void __iommu_detach_group(struct iommu_domain
*domain,
{
int ret;
- if (!group->default_domain) {
+ /*
+* If group has been claimed for none kernel DMA purpose, avoid
+* re-attaching the default domain.
+*/
none kernel reads odd. But maybe drop that and just say 'claimed
already' ala:
/*
* If the group has been claimed already, do not re-attach the default
* domain.
*/
Sure!
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 00/14] Fix BUG_ON in vfio_iommu_group_notifier()
Hi Jason, On 1/4/22 8:48 PM, Jason Gunthorpe wrote: On Tue, Jan 04, 2022 at 09:56:30AM +0800, Lu Baolu wrote: v5: - Move kernel dma ownership auto-claiming from driver core to bus callback. (Greg) - Refactor the iommu interfaces to make them more specific. (Jason/Robin) - Simplify the dma ownership implementation by removing the owner type. (Jason) - Commit message refactoring for PCI drivers. (Bjorn) - Move iommu_attach/detach_device() improvement patches into another series as there are a lot of code refactoring and cleanup staffs in various device drivers. Since you already have the code you should make this 'other series' right now. It should delete iommu_group_attach() and fix iommu_device_attach(). Yes. I am doing the functional and compile tests. I will post it once I complete the testing. You also didn't really do my suggestion, this messes up the normal __iommu_attach_group()/__iommu_detach_group() instead of adding the clear to purpose iommu_replace_group() for VFIO to use. This just makes it more difficult to normalize the APIs. I didn't forget that. :-) It's part of the new series. Otherwise it does seem to have turned out to be more understandable. Jason Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 09/14] PCI: portdrv: Suppress kernel DMA ownership auto-claiming
On Tue, Jan 04, 2022 at 01:51:30PM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 04, 2022 at 03:26:14PM -0400, Jason Gunthorpe wrote:
> > On Tue, Jan 04, 2022 at 11:06:31AM -0600, Bjorn Helgaas wrote:
> >
> > > > The existing vfio framework allows the portdrv driver to be bound
> > > > to the bridge while its downstream devices are assigned to user space.
> > >
> > > I.e., the existing VFIO framework allows a switch to be in the same
> > > IOMMU group as the devices below it, even though the switch has a
> > > kernel driver and the other devices may have userspace drivers?
> >
> > Yes, this patch exists to maintain current VFIO behavior which has this
> > same check.
> >
> > I belive the basis for VFIO doing this is that the these devices
> > cannot do DMA, so don't care about the DMA API or the group->domain,
> > and do not expose MMIO memory so do not care about the P2P attack.
>
> "These devices" means bridges, right? Not sure why we wouldn't care
> about the P2P attack.
Yes bridges. I said "I belive" because VFIO was changed to ignore
bridges a long time ago and the security rational was a bit unclear in
the commit.
See 5f096b14d421 ("vfio: Whitelist PCI bridges")
> PCIe switches use MSI or MSI-X for hotplug, PME, etc, so they do DMA
> for that. Is that not relevant here?
Alex's comment in the above commit notes about interrupts, but I think
the answer today is that it does not matter.
For platforms like x86 that keep interrupts and DMA seperate it
works fine.
For platforms that comingle the iommu_domain and interrupts (do some
exist?) we expect the platform to do whatever is necessary at
iommu_domain attach time to ensure interrupts continue to
work. (AFAICT at least)
In other words we don't have an API restriction to use
iommu_device_use_dma_api() to use request_irq().
So the main concern should be P2P attacks on bridge MMIO registers.
> Is there something that *prohibits* a bridge from having
> device-specific functionality including DMA?
I'm not sure, I think not, but the 'Bus Master Enable' language does
have a different definition for Type 1..
However, it doesn't matter - the question here is not about what the
device HW is capable of, but what does the kernel driver do. The
portdrv does not use the DMA API, so that alone is half the
requirement to skip the iommu_device_use_dma_api() call.
Some future device-specific bridge driver that is able to issue the
device-specific MMIO's and operate the DMA API should be coded to use
iommu_device_use_dma_api(), probably by using a different
device_driver for the bridge.
> I know some bridges have device-specific BARs for performance counters
> and the like.
Yep.
IMHO it is probably not so great as-is..
However, this patch is just porting the status-quo of commit 5f09 into
this new framework.
What this new framework does allow is for portdrv to police itself. eg
it could check if there is a MMIO BAR and call
iommu_device_use_dma_api() out of caution. It could also have an
allowance list of devices where we know hostile writes to the MMIO are
known harmless.
Without knowing more about what motivated 5f09 it is hard to say,
other than it has been 7 years like this and nobody has complained
yet :)
Jason
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Memory clearing in swiotlb_update_mem_attributes()
Hi Tom, For larger TDX VM, memset() after set_memory_decrypted() in swiotlb_update_mem_attributes() takes substantial portion of boot time. It makes me wounder why do we need it there? Malicious VMM can mess with decrypted/shared buffer at any point and for normal use it will be populated with real data anyway. Can we drop it? -- Kirill A. Shutemov ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] dt-bindings: iommu: Convert msm,iommu-v0 to yaml
On Sat, Dec 25, 2021 at 08:35:55PM +0100, David Heidelberg wrote:
> Convert Qualcomm IOMMU v0 implementation to yaml format.
>
> Signed-off-by: David Heidelberg
> ---
> v2:
> - fix wrong path in binding $id
> - comment qcom,mdp4 node example (we don't want to validate it yet)
>
> .../bindings/iommu/msm,iommu-v0.txt | 64 -
> .../bindings/iommu/qcom,iommu-v0.yaml | 96 +++
qcom,apq8064-iommu.yaml
At this point, I don't think we'll get a 2nd user.
> 2 files changed, 96 insertions(+), 64 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> create mode 100644 Documentation/devicetree/bindings/iommu/qcom,iommu-v0.yaml
>
> diff --git a/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> b/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> deleted file mode 100644
> index 20236385f26e..
> --- a/Documentation/devicetree/bindings/iommu/msm,iommu-v0.txt
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -* QCOM IOMMU
> -
> -The MSM IOMMU is an implementation compatible with the ARM VMSA short
> -descriptor page tables. It provides address translation for bus masters
> outside
> -of the CPU, each connected to the IOMMU through a port called micro-TLB.
> -
> -Required Properties:
> -
> - - compatible: Must contain "qcom,apq8064-iommu".
> - - reg: Base address and size of the IOMMU registers.
> - - interrupts: Specifiers for the MMU fault interrupts. For instances that
> -support secure mode two interrupts must be specified, for non-secure and
> -secure mode, in that order. For instances that don't support secure mode
> a
> -single interrupt must be specified.
> - - #iommu-cells: The number of cells needed to specify the stream id. This
> - is always 1.
> - - qcom,ncb: The total number of context banks in the IOMMU.
> - - clocks : List of clocks to be used during SMMU register access. See
> - Documentation/devicetree/bindings/clock/clock-bindings.txt
> - for information about the format. For each clock specified
> - here, there must be a corresponding entry in clock-names
> - (see below).
> -
> - - clock-names : List of clock names corresponding to the clocks
> specified in
> - the "clocks" property (above).
> - Should be "smmu_pclk" for specifying the interface clock
> - required for iommu's register accesses.
> - Should be "smmu_clk" for specifying the functional clock
> - required by iommu for bus accesses.
> -
> -Each bus master connected to an IOMMU must reference the IOMMU in its device
> -node with the following property:
> -
> - - iommus: A reference to the IOMMU in multiple cells. The first cell is a
> - phandle to the IOMMU and the second cell is the stream id.
> - A single master device can be connected to more than one iommu
> - and multiple contexts in each of the iommu. So multiple entries
> - are required to list all the iommus and the stream ids that the
> - master is connected to.
> -
> -Example: mdp iommu and its bus master
> -
> -mdp_port0: iommu@750 {
> - compatible = "qcom,apq8064-iommu";
> - #iommu-cells = <1>;
> - clock-names =
> - "smmu_pclk",
> - "smmu_clk";
> - clocks =
> - < SMMU_AHB_CLK>,
> - < MDP_AXI_CLK>;
> - reg = <0x0750 0x10>;
> - interrupts =
> - ,
> - ;
> - qcom,ncb = <2>;
> - };
> -
> - mdp: qcom,mdp@510 {
> - compatible = "qcom,mdp";
> - ...
> - iommus = <_port0 0
> - _port0 2>;
> - };
> diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu-v0.yaml
> b/Documentation/devicetree/bindings/iommu/qcom,iommu-v0.yaml
> new file mode 100644
> index ..6f166c30b9ec
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/qcom,iommu-v0.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: "http://devicetree.org/schemas/iommu/qcom,iommu-v0.yaml#;
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> +
> +title: Qualcomm IOMMU for APQ8064
> +
> +maintainers:
> + - Will Deacon
> +
> +description: >
> + The MSM IOMMU is an implementation compatible with the ARM VMSA short
> + descriptor page tables. It provides address translation for bus masters
> + outside of the CPU, each connected to the IOMMU through a port called
> micro-TLB.
> +
> +properties:
> + compatible:
> +const: qcom,apq8064-iommu
> +
> + clocks:
> +items:
>
Re: [PATCH v5 09/14] PCI: portdrv: Suppress kernel DMA ownership auto-claiming
On Tue, Jan 04, 2022 at 03:26:14PM -0400, Jason Gunthorpe wrote: > On Tue, Jan 04, 2022 at 11:06:31AM -0600, Bjorn Helgaas wrote: > > > > The existing vfio framework allows the portdrv driver to be bound > > > to the bridge while its downstream devices are assigned to user space. > > > > I.e., the existing VFIO framework allows a switch to be in the same > > IOMMU group as the devices below it, even though the switch has a > > kernel driver and the other devices may have userspace drivers? > > Yes, this patch exists to maintain current VFIO behavior which has this > same check. > > I belive the basis for VFIO doing this is that the these devices > cannot do DMA, so don't care about the DMA API or the group->domain, > and do not expose MMIO memory so do not care about the P2P attack. "These devices" means bridges, right? Not sure why we wouldn't care about the P2P attack. PCIe switches use MSI or MSI-X for hotplug, PME, etc, so they do DMA for that. Is that not relevant here? Is there something that *prohibits* a bridge from having device-specific functionality including DMA? I know some bridges have device-specific BARs for performance counters and the like. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 09/14] PCI: portdrv: Suppress kernel DMA ownership auto-claiming
On Tue, Jan 04, 2022 at 11:06:31AM -0600, Bjorn Helgaas wrote: > > The existing vfio framework allows the portdrv driver to be bound > > to the bridge while its downstream devices are assigned to user space. > > I.e., the existing VFIO framework allows a switch to be in the same > IOMMU group as the devices below it, even though the switch has a > kernel driver and the other devices may have userspace drivers? Yes, this patch exists to maintain current VFIO behavior which has this same check. I belive the basis for VFIO doing this is that the these devices cannot do DMA, so don't care about the DMA API or the group->domain, and do not expose MMIO memory so do not care about the P2P attack. A comment in the code to this effect would be good, IMHO. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 01/14] iommu: Add dma ownership management interfaces
On Tue, Jan 04, 2022 at 10:41:00AM -0600, Bjorn Helgaas wrote: > On Tue, Jan 04, 2022 at 02:08:00AM -0800, Christoph Hellwig wrote: > > On Tue, Jan 04, 2022 at 09:56:31AM +0800, Lu Baolu wrote: > > > Multiple devices may be placed in the same IOMMU group because they > > > cannot be isolated from each other. These devices must either be > > > entirely under kernel control or userspace control, never a mixture. > > I guess the reason is that if a group contained a mixture, userspace > could attack the kernel by programming a device to DMA to a device > owned by the kernel? There are several of reasons, including what you guess, but for the design of the series now we can just focus on the group's domain. If the kernel is using a device then the kernel driver uses the DMA API and the group's domain must always point to the default domain so long as a DMA API user exists. Hopefully it is clear to understand > > > The device driver oriented interfaces are, > > > > > > int iommu_device_use_dma_api(struct device *dev); > > > void iommu_device_unuse_dma_api(struct device *dev); > > Nit, do we care whether it uses the actual DMA API? Or is it just > that iommu_device_use_dma_api() tells us the driver may program the > device to do DMA? As the main purpose, yes this is all about the DMA API because it asserts the group domain is the DMA API's domain. There is a secondary purpose that has to do with the user/kernel attack you mentioned above. Maintaining the DMA API domain also prevents VFIO from allowing userspace to operate any device in the group which blocks P2P attacks to MMIO of other devices. This is why, even if the driver doesn't use DMA, it should still do a iommu_device_use_dma_api(), except in the special cases where we don't care about P2P attacks (eg pci-stub, bridges, etc). > > > The vfio oriented interfaces are, > > > > > > int iommu_group_set_dma_owner(struct iommu_group *group, > > > void *owner); > > > void iommu_group_release_dma_owner(struct iommu_group *group); > > > bool iommu_group_dma_owner_claimed(struct iommu_group *group); > > > > > > The device userspace assignment must be disallowed if the set dma owner > > > interface returns failure. > > Can you connect this back to the "never a mixture" from the beginning? > If all you cared about was prevent an IOMMU group from containing > devices with a mixture of kernel drivers and userspace drivers, I > assume you could do that without iommu_device_use_dma_api(). So is > this a way to *allow* a mixture under certain restricted conditions? It is not about user/kernel, it is about arbitrating the shared group->domain against multiple different requests to set it to something else. Lu, Given that the word 'user' was deleted from the API entirely it makes sense to reword these commit messages to focus less on user vs kernel and more on ownership of the domain pointer. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 09/14] PCI: portdrv: Suppress kernel DMA ownership auto-claiming
On Tue, Jan 04, 2022 at 09:56:39AM +0800, Lu Baolu wrote:
> If a switch lacks ACS P2P Request Redirect, a device below the switch can
> bypass the IOMMU and DMA directly to other devices below the switch, so
> all the downstream devices must be in the same IOMMU group as the switch
> itself.
Help me think through what's going on here. IIUC, we put devices in
the same IOMMU group when they can interfere with each other in any
way (DMA, config access, etc).
(We said "DMA" above, but I guess this would also apply to config
requests, right?)
*This* patch doesn't check for any ACS features. Can you connect the
dots for me? I guess the presence or absence of P2P Request Redirect
determines the size of the IOMMU group. And the following says
something about what is allowed in the group? And .no_kernel_api_dma
allows an exception to the general rule?
> The existing vfio framework allows the portdrv driver to be bound
> to the bridge while its downstream devices are assigned to user space.
I.e., the existing VFIO framework allows a switch to be in the same
IOMMU group as the devices below it, even though the switch has a
kernel driver and the other devices may have userspace drivers?
Is this a function of VFIO design or of the IOMMU driver?
> The pci_dma_configure() marks the iommu_group as containing only devices
> with kernel drivers that manage DMA. Avoid this default behavior for the
> portdrv driver in order for compatibility with the current vfio policy.
I assume "IOMMU group" means the same as "iommu_group"; maybe we can
use one of them consistently?
> Suggested-by: Jason Gunthorpe
> Suggested-by: Kevin Tian
> Signed-off-by: Lu Baolu
> ---
> drivers/pci/pcie/portdrv_pci.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index 35eca6277a96..2116f821c005 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -202,6 +202,8 @@ static struct pci_driver pcie_portdriver = {
>
> .err_handler= _portdrv_err_handler,
>
> + .no_kernel_api_dma = true,
> +
> .driver.pm = PCIE_PORTDRV_PM_OPS,
> };
>
> --
> 2.25.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 01/14] iommu: Add dma ownership management interfaces
On Tue, Jan 04, 2022 at 02:08:00AM -0800, Christoph Hellwig wrote:
> On Tue, Jan 04, 2022 at 09:56:31AM +0800, Lu Baolu wrote:
> > Multiple devices may be placed in the same IOMMU group because they
> > cannot be isolated from each other. These devices must either be
> > entirely under kernel control or userspace control, never a mixture.
I guess the reason is that if a group contained a mixture, userspace
could attack the kernel by programming a device to DMA to a device
owned by the kernel?
> > This adds dma ownership management in iommu core and exposes several
> > interfaces for the device drivers and the device userspace assignment
> > framework (i.e. vfio), so that any conflict between user and kernel
> > controlled DMA could be detected at the beginning.
Maybe I'm missing the point because I don't know what "conflict
between user and kernel controlled DMA" is. Are you talking about
both userspace and the kernel programming the same device to do DMA?
> > The device driver oriented interfaces are,
> >
> > int iommu_device_use_dma_api(struct device *dev);
> > void iommu_device_unuse_dma_api(struct device *dev);
Nit, do we care whether it uses the actual DMA API? Or is it just
that iommu_device_use_dma_api() tells us the driver may program the
device to do DMA?
> > Devices under kernel drivers control must call iommu_device_use_dma_api()
> > before driver probes. The driver binding process must be aborted if it
> > returns failure.
"Devices" don't call functions. Drivers do, or in this case, it looks
like the bus DMA code (platform, amba, fsl, pci, etc).
These functions are EXPORT_SYMBOL_GPL(), but it looks like all the
callers are built-in, so maybe the export is unnecessary?
You use "iommu"/"IOMMU" and "dma"/"DMA" interchangeably above. Would
be easier to read if you picked one.
> > The vfio oriented interfaces are,
> >
> > int iommu_group_set_dma_owner(struct iommu_group *group,
> > void *owner);
> > void iommu_group_release_dma_owner(struct iommu_group *group);
> > bool iommu_group_dma_owner_claimed(struct iommu_group *group);
> >
> > The device userspace assignment must be disallowed if the set dma owner
> > interface returns failure.
Can you connect this back to the "never a mixture" from the beginning?
If all you cared about was prevent an IOMMU group from containing
devices with a mixture of kernel drivers and userspace drivers, I
assume you could do that without iommu_device_use_dma_api(). So is
this a way to *allow* a mixture under certain restricted conditions?
Another nit below.
> > Signed-off-by: Jason Gunthorpe
> > Signed-off-by: Kevin Tian
> > Signed-off-by: Lu Baolu
> > ---
> > include/linux/iommu.h | 31
> > drivers/iommu/iommu.c | 161 +-
> > 2 files changed, 189 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> > index de0c57a567c8..568f285468cf 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -682,6 +682,13 @@ struct iommu_sva *iommu_sva_bind_device(struct device
> > *dev,
> > void iommu_sva_unbind_device(struct iommu_sva *handle);
> > u32 iommu_sva_get_pasid(struct iommu_sva *handle);
> >
> > +int iommu_device_use_dma_api(struct device *dev);
> > +void iommu_device_unuse_dma_api(struct device *dev);
> > +
> > +int iommu_group_set_dma_owner(struct iommu_group *group, void *owner);
> > +void iommu_group_release_dma_owner(struct iommu_group *group);
> > +bool iommu_group_dma_owner_claimed(struct iommu_group *group);
> > +
> > #else /* CONFIG_IOMMU_API */
> >
> > struct iommu_ops {};
> > @@ -1082,6 +1089,30 @@ static inline struct iommu_fwspec
> > *dev_iommu_fwspec_get(struct device *dev)
> > {
> > return NULL;
> > }
> > +
> > +static inline int iommu_device_use_dma_api(struct device *dev)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void iommu_device_unuse_dma_api(struct device *dev)
> > +{
> > +}
> > +
> > +static inline int
> > +iommu_group_set_dma_owner(struct iommu_group *group, void *owner)
> > +{
> > + return -ENODEV;
> > +}
> > +
> > +static inline void iommu_group_release_dma_owner(struct iommu_group *group)
> > +{
> > +}
> > +
> > +static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group)
> > +{
> > + return false;
> > +}
> > #endif /* CONFIG_IOMMU_API */
> >
> > /**
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 8b86406b7162..ff0c8c1ad5af 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -48,6 +48,8 @@ struct iommu_group {
> > struct iommu_domain *default_domain;
> > struct iommu_domain *domain;
> > struct list_head entry;
> > + unsigned int owner_cnt;
> > + void *owner;
> > };
> >
> > struct group_device {
> > @@ -289,7 +291,12 @@ int iommu_probe_device(struct device *dev)
> > mutex_lock(>mutex);
> > iommu_alloc_default_domain(group,
Re: [PATCH] Swiotlb: Add CONFIG_HAS_IOMEM check around memremap() in the swiotlb_mem_remap()
On Tue, Jan 04, 2022 at 04:03:07PM +0100, Christoph Hellwig wrote: > On Tue, Jan 04, 2022 at 02:51:55PM +, Wei Liu wrote: > > > Please stub out all of swiotlb_mem_remap instead of the ifdef inside the > > > function. > > > > Does this look okay to you? > > Yes, thanks! Thanks for confirming! I will turn this into an ack and apply the diff to hyperv-next. Wei. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 15/33] iommu/mediatek: Add SUB_COMMON_3BITS flag
Il 23/09/21 13:58, Yong Wu ha scritto: In prevous SoC, the sub common id occupy 2 bits. the mt8195's sub common id has 3bits. Add a new flag for this. and rename the prevous flag to _2BITS. For readable, I put these two flags together, then move the other flags. no functional change. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 05/33] iommu/mediatek: Adapt sharing and non-sharing pgtable case
Il 23/09/21 13:58, Yong Wu ha scritto: In previous mt2712, Both IOMMUs are MM IOMMU, and they will share pgtable. However in the latest SoC, another is infra IOMMU, there is no reason to share pgtable between MM with INFRA IOMMU. This patch manage to implement the two case(sharing and non-sharing pgtable). Currently we use for_each_m4u to loop the 2 HWs. Add the list_head into this macro. In the sharing pgtable case, the list_head is the global "m4ulist". In the non-sharing pgtable case, the list_head is hw_list_head which is a variable in the "data". then for_each_m4u will only loop itself. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 12/33] iommu/mediatek: Always tlb_flush_all when each PM resume
Il 23/09/21 13:58, Yong Wu ha scritto: Prepare for 2 HWs that sharing pgtable in different power-domains. When there are 2 M4U HWs, it may has problem in the flush_range in which we get the pm_status via the m4u dev, BUT that function don't reflect the real power-domain status of the HW since there may be other HW also use that power-domain. The function dma_alloc_attrs help allocate the iommu buffer which need the corresponding power domain since tlb flush is needed when preparing iova. BUT this function only is for allocating buffer, we have no good reason to request the user always call pm_runtime_get before calling dma_alloc_xxx. Therefore, we add a tlb_flush_all in the pm_runtime_resume to make sure the tlb always is clean. Another solution is always call pm_runtime_get in the tlb_flush_range. This will trigger pm runtime resume/backup so often when the iommu power is not active at some time(means user don't call pm_runtime_get before calling dma_alloc_xxx), This may cause the performance drop. thus we don't use this. In other case, the iommu's power should always be active via device link with smi. The previous SoC don't have PM except mt8192. the mt8192 IOMMU is display's power-domain which nearly always is enabled. thus no need fix tags here. Prepare for mt8195. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 14/33] iommu/mediatek: Always enable output PA over 32bits in isr
Il 23/09/21 13:58, Yong Wu ha scritto: Currently the output PA[32:33] is contained by the flag IOVA_34. This is not right. the iova_34 has no relation with pa[32:33], the 32bits iova still could map to pa[32:33]. Move it out from the flag. No need fix tag since currently only mt8192 use the calulation and it always has this IOVA_34 flag. Prepare for the IOMMU that still use IOVA 32bits but its dram size may be over 4GB. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 19/33] iommu/mediatek: Add list_del in mtk_iommu_remove
Il 23/09/21 13:58, Yong Wu ha scritto: Lack the list_del in the mtk_iommu_remove, and remove bus_set_iommu(*, NULL) since there may be several iommu HWs. we can not bus_set_iommu null when one iommu driver unbind. This issue is not so important, No need fix tags. Signed-off-by: Yong Wu I would still add a Fixes tag on this commit, as that would schedule it for backporting - and that's rather important, solidifying older releases. Please add the required tag, after which... Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 10/33] iommu/mediatek: Add tlb_lock in tlb_flush_all
Il 23/09/21 13:58, Yong Wu ha scritto: The tlb_flush_all also touches the registers about tlb operations. Add spinlock in it to protect the tlb registers. since the tlb_range already hold the spinlock, move it a bit outside the spinlock to print log. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 09/33] iommu/mediatek: Remove for_each_m4u in tlb_sync_all
Il 23/09/21 13:58, Yong Wu ha scritto:
The tlb_sync_all is called from these three functions:
a) flush_iotlb_all: it will be called for each a iommu HW.
b) tlb_flush_range_sync: it already has for_each_m4u.
c) in irq: When IOMMU HW translation fault, Only need flush itself.
Thus, No need for_each_m4u in this tlb_sync_all. Remove it.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 18 +++---
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6f4f6624e3ac..0b4c30baa864 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -206,19 +206,15 @@ static struct mtk_iommu_domain *to_mtk_domain(struct
iommu_domain *dom)
static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
- struct list_head *head = data->hw_list;
-
- for_each_m4u(data, head) {
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
- continue;
+ if (pm_runtime_get_if_in_use(data->dev) <= 0)
+ return;
- writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
- data->base + data->plat_data->inv_sel_reg);
- writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
- wmb(); /* Make sure the tlb flush all done */
+ writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
+ data->base + data->plat_data->inv_sel_reg);
+ writel_relaxed(F_ALL_INVLD, data->base + REG_MMU_INVALIDATE);
+ wmb(); /* Make sure the tlb flush all done */
There aren't a lot of writes here - not anymore, since you are no longer doing
this for_each_m4u()...
...so, please change writel_relaxed() to writel() calls, allowing you to also
remove the write barrier at the end (since in the non relaxed version, order
is already ensured).
- pm_runtime_put(data->dev);
- }
+ pm_runtime_put(data->dev);
}
static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 04/33] iommu/mediatek: Remove clk_disable in mtk_iommu_remove
Il 23/09/21 13:58, Yong Wu ha scritto:
After the commit b34ea31fe013 ("iommu/mediatek: Always enable the clk on
resume"), the iommu clock is controlled by the runtime callback.
thus remove the clk control in the mtk_iommu_remove.
Otherwise, it will warning like:
echo 14018000.iommu > /sys/bus/platform/drivers/mtk-iommu/unbind
[ 51.413044] [ cut here ]
[ 51.413648] vpp0_smi_iommu already disabled
[ 51.414233] WARNING: CPU: 2 PID: 157 at */v5.15-rc1/kernel/mediatek/
drivers/clk/clk.c:952 clk_core_disable+0xb0/0xb8
[ 51.417174] Hardware name: MT8195V/C(ENG) (DT)
[ 51.418635] pc : clk_core_disable+0xb0/0xb8
[ 51.419177] lr : clk_core_disable+0xb0/0xb8
...
[ 51.429375] Call trace:
[ 51.429694] clk_core_disable+0xb0/0xb8
[ 51.430193] clk_core_disable_lock+0x24/0x40
[ 51.430745] clk_disable+0x20/0x30
[ 51.431189] mtk_iommu_remove+0x58/0x118
[ 51.431705] platform_remove+0x28/0x60
[ 51.432197] device_release_driver_internal+0x110/0x1f0
[ 51.432873] device_driver_detach+0x18/0x28
[ 51.433418] unbind_store+0xd4/0x108
[ 51.433886] drv_attr_store+0x24/0x38
[ 51.434363] sysfs_kf_write+0x40/0x58
[ 51.434843] kernfs_fop_write_iter+0x164/0x1e0
Fixes: b34ea31fe013 ("iommu/mediatek: Always enable the clk on resume")
Reported-by: Hsin-Yi Wang
Signed-off-by: Yong Wu
Reviewed-by: AngeloGioacchino Del Regno
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 13/33] iommu/mediatek: Remove the power status checking in tlb flush all
Il 23/09/21 13:58, Yong Wu ha scritto:
To simplify the code, Remove the power status checking in the
tlb_flush_all, remove this:
if (pm_runtime_get_if_in_use(data->dev) <= 0)
continue;
After this patch, the mtk_iommu_tlb_flush_all will be called from
a) isr
b) pm runtime resume callback
c) tlb flush range fail case
d) iommu_create_device_direct_mappings
-> iommu_flush_iotlb_all
In first three cases, the power and clock always are enabled; d) is direct
mapping, the tlb flush is unnecessay since we already have tlb_flush_all
in the pm_runtime_resume callback. When the iommu's power status is
changed to active, the tlb always is clean.
In addition, there still are 2 reasons that don't add PM status checking
in the tlb flush all:
a) Write tlb flush all register also is ok even though the HW has no
power and clocks. Write ignore.
Do you mean that the register write seemingly succeeds but the hardware
discards it?
Please, reword the `a` sentence to be clearer.
b) pm_runtime_get_if_in_use(m4udev) is 0 when the tlb_flush_all
is called frm pm_runtime_resume cb. From this point, we can not add
this code above in this tlb_flush_all.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index e9e94944ed91..4a33b6c6b1db 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -204,10 +204,14 @@ static struct mtk_iommu_domain *to_mtk_domain(struct
iommu_domain *dom)
return container_of(dom, struct mtk_iommu_domain, domain);
}
-static void mtk_iommu_tlb_do_flush_all(struct mtk_iommu_data *data)
+static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
unsigned long flags;
+ /*
+* No need get power status since the HW PM status nearly is active
+* when entering here.
Please reword this comment to explain the entire situation.
+*/
spin_lock_irqsave(>tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
data->base + data->plat_data->inv_sel_reg);
@@ -216,16 +220,6 @@ static void mtk_iommu_tlb_do_flush_all(struct
mtk_iommu_data *data)
spin_unlock_irqrestore(>tlb_lock, flags);
}
-static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
-{
- if (pm_runtime_get_if_in_use(data->dev) <= 0)
- return;
-
- mtk_iommu_tlb_do_flush_all(data);
-
- pm_runtime_put(data->dev);
-}
-
static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
struct mtk_iommu_data *data)
{
@@ -263,7 +257,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
if (ret) {
dev_warn(data->dev,
"Partial TLB flush timed out, falling back to full
flush\n");
- mtk_iommu_tlb_do_flush_all(data);
+ mtk_iommu_tlb_flush_all(data);
}
if (has_pm)
@@ -993,7 +987,7 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct
device *dev)
*
* Thus, Make sure the tlb always is clean after each PM resume.
*/
- mtk_iommu_tlb_do_flush_all(data);
+ mtk_iommu_tlb_flush_all(data);
/*
* Uppon first resume, only enable the clk and return, since the values
of the
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 18/33] iommu/mediatek: Adjust device link when it is sub-common
Il 23/09/21 13:58, Yong Wu ha scritto: For MM IOMMU, We always add device link between smi-common and IOMMU HW. In mt8195, we add smi-sub-common. Thus, if the node is sub-common, we still need find again to get smi-common, then do device link. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 17/33] iommu/mediatek: Contain MM IOMMU flow with the MM TYPE
Il 23/09/21 13:58, Yong Wu ha scritto: Prepare for supporting INFRA_IOMMU, and APU_IOMMU later. For Infra IOMMU/APU IOMMU, it doesn't have the "larb""port". thus, Use the MM flag contain the MM_IOMMU special flow, Also, it moves a big chunk code about parsing the mediatek,larbs into a function, this is only needed for MM IOMMU. and all the current SoC are MM_IOMMU. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 22/33] iommu/mediatek: Add PCIe support
Il 23/09/21 13:58, Yong Wu ha scritto:
Currently the code for of_iommu_configure_dev_id is like this:
static int of_iommu_configure_dev_id(struct device_node *master_np,
struct device *dev,
const u32 *id)
{
struct of_phandle_args iommu_spec = { .args_count = 1 };
err = of_map_id(master_np, *id, "iommu-map",
"iommu-map-mask", _spec.np,
iommu_spec.args);
...
}
It supports only one id output. BUT our PCIe HW has two ID(one is for
writing, the other is for reading). I'm not sure if we should change
of_map_id to support output MAX_PHANDLE_ARGS.
Here add the solution in ourselve drivers. If it's pcie case, enable one
more bit.
Not all infra iommu support PCIe, thus add a PCIe support flag here.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 37d6dfb4feab..3f1fd8036345 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -132,6 +133,7 @@
#define MTK_IOMMU_TYPE_MM (0x0 << 13)
#define MTK_IOMMU_TYPE_INFRA (0x1 << 13)
#define MTK_IOMMU_TYPE_MASK (0x3 << 13)
+#define IFA_IOMMU_PCIe_SUPPORT BIT(15)
This definition looks like "breaking" the naming convention that's used in this
driver... what about MTK_INFRA_IOMMU_PCIE_SUPPORT?
#define MTK_IOMMU_HAS_FLAG(pdata, _x) (!!(((pdata)->flags) & (_x)))
@@ -401,8 +403,11 @@ static int mtk_iommu_config(struct mtk_iommu_data *data, struct device *dev,
larb_mmu->mmu &= ~MTK_SMI_MMU_EN(portid);
} else if (MTK_IOMMU_IS_TYPE(data->plat_data,
MTK_IOMMU_TYPE_INFRA)) {
peri_mmuen_msk = BIT(portid);
- peri_mmuen = enable ? peri_mmuen_msk : 0;
+ /* PCIdev has only one output id, enable the next
writing bit for PCIe */
+ if (dev_is_pci(dev))
+ peri_mmuen_msk |= BIT(portid + 1);
+ peri_mmuen = enable ? peri_mmuen_msk : 0;
ret = regmap_update_bits(data->pericfg, PERICFG_IOMMU_1,
peri_mmuen_msk, peri_mmuen);
if (ret)
@@ -977,6 +982,15 @@ static int mtk_iommu_probe(struct platform_device *pdev)
ret = component_master_add_with_match(dev, _iommu_com_ops,
match);
if (ret)
goto out_bus_set_null;
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
+ MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT))
{
+ #ifdef CONFIG_PCI
Please fix the indentation of this ifdef (do not indent).
+ if (!iommu_present(_bus_type)) {
+ ret = bus_set_iommu(_bus_type, _iommu_ops);
+ if (ret) /* PCIe fail don't affect platform_bus. */
+ goto out_list_del;
+ }
+ #endif
}
return ret;
@@ -1007,6 +1021,11 @@ static int mtk_iommu_remove(struct platform_device *pdev)
if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_MM)) {
device_link_remove(data->smicomm_dev, >dev);
component_master_del(>dev, _iommu_com_ops);
+ } else if (MTK_IOMMU_IS_TYPE(data->plat_data, MTK_IOMMU_TYPE_INFRA) &&
+ MTK_IOMMU_HAS_FLAG(data->plat_data, IFA_IOMMU_PCIe_SUPPORT))
{
+ #ifdef CONFIG_PCI
ditto.
+ bus_set_iommu(_bus_type, NULL);
+ #endif
}
pm_runtime_disable(>dev);
devm_free_irq(>dev, data->irq, data);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 20/33] iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO
Il 23/09/21 13:58, Yong Wu ha scritto: Allow the type IOMMU_DOMAIN_UNMANAGED since vfio_iommu_type1.c always call iommu_domain_alloc. The PCIe EP works ok when going through vfio. Signed-off-by: Yong Wu Acked-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 21/33] iommu/mediatek: Add infra iommu support
Il 23/09/21 13:58, Yong Wu ha scritto: The infra iommu enable bits in mt8195 is in the pericfg register segment, use regmap to update it. If infra iommu master translation fault, It don't have the larbid/portid, thus print out the whole register value. Since regmap_update_bits may fail, add return value for mtk_iommu_config. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 07/33] iommu/mediatek: Add a flag DCM_DISABLE
Il 23/09/21 13:58, Yong Wu ha scritto: In the infra iommu, we should disable DCM. add a new flag for this. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 08/33] iommu/mediatek: Add a flag NON_STD_AXI
Il 23/09/21 13:58, Yong Wu ha scritto: Add a new flag NON_STD_AXI, All the previous SoC support this flag. Prepare for adding infra and apu iommu which don't support this. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 11/33] iommu/mediatek: Remove the granule in the tlb flush
Il 23/09/21 13:58, Yong Wu ha scritto: The MediaTek IOMMU don't care about granule when tlb flushing. Remove this variable. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 23/33] iommu/mediatek: Add mt8195 support
Il 23/09/21 13:58, Yong Wu ha scritto: mt8195 has 3 IOMMU, containing 2 MM IOMMUs, one is for vdo, the other is for vpp. and 1 INFRA IOMMU. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 16/33] iommu/mediatek: Add IOMMU_TYPE flag
Il 23/09/21 13:58, Yong Wu ha scritto: Add IOMMU_TYPE definition. In the mt8195, we have another IOMMU_TYPE: infra iommu, also there will be another APU_IOMMU, thus, use 2bits for the IOMMU_TYPE. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 06/33] iommu/mediatek: Add 12G~16G support for multi domains
Il 23/09/21 13:58, Yong Wu ha scritto: In mt8192, we preassign 0-4G; 4G-8G; 8G-12G for different multimedia engines. This depends on the "dma-ranges=" in the iommu consumer's dtsi node. Adds 12G-16G region here. and reword the previous comment. we don't limit which master locate in which region. CCU still is 8G-12G. Don't change it here. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 24/33] iommu/mediatek: Only adjust code about register base
Il 23/09/21 13:58, Yong Wu ha scritto: No functional change. Use "base" instead of the data->base. This is avoid to touch too many lines in the next patches. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 27/33] iommu/mediatek: Initialise bank HW for each a bank
Il 23/09/21 13:58, Yong Wu ha scritto: The mt8195 IOMMU HW max support 5 banks, and regarding the banks' registers, it looks like: |bank0 | bank1 | bank2 | bank3 | bank4| |global | |control| null |regs | - |bank |bank |bank |bank |bank | |regs |regs |regs |regs |regs | | | | | | | - Each bank has some special bank registers and it share bank0's global control registers. this patch initialise the bank hw with the bankid. In the hw_init, we always initialise bank0's control register since we don't know if the bank0 is initialised. Additionally, About each bank's register base, always delta 0x1000. like bank[x + 1] = bank[x] + 0x1000. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 25/33] iommu/mediatek: Just move code position in hw_init
Il 23/09/21 13:58, Yong Wu ha scritto: No functional change too, prepare for mt8195 IOMMU support bank functions. Some global control settings are in bank0 while the other banks have their bank independent setting. Here only move the global control settings and the independent registers together. Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 29/33] iommu/mediatek: Change the domid to iova_region_id
Il 23/09/21 13:58, Yong Wu ha scritto: Prepare for adding bankid, also no functional change. In the previous SoC, each a iova_region is a domain; In the multi-banks case, each a bank is a domain, then the original function name "mtk_iommu_get_domain_id" is not proper. Use "iova_region_id" instead of "domain_id". Signed-off-by: Yong Wu Reviewed-by: AngeloGioacchino Del Regno ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 32/33] iommu/mediatek: Backup/restore regsiters for multi banks
Il 23/09/21 13:58, Yong Wu ha scritto:
Each bank has some independent registers. thus backup/restore them for
each a bank when suspend and resume.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 39 +++
drivers/iommu/mtk_iommu.h | 14 +++---
2 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 3925d1d4f2cf..3cb18ed28132 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1114,16 +1114,23 @@ static int __maybe_unused
mtk_iommu_runtime_suspend(struct device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = >reg;
- void __iomem *base = data->bank[0].base;
+ void __iomem *base;
+ int i = 0;
+ base = data->bank[i].base;
reg->wr_len_ctrl = readl_relaxed(base + REG_MMU_WR_LEN_CTRL);
reg->misc_ctrl = readl_relaxed(base + REG_MMU_MISC_CTRL);
reg->dcm_dis = readl_relaxed(base + REG_MMU_DCM_DIS);
reg->ctrl_reg = readl_relaxed(base + REG_MMU_CTRL_REG);
- reg->int_control0 = readl_relaxed(base + REG_MMU_INT_CONTROL0);
- reg->int_main_control = readl_relaxed(base + REG_MMU_INT_MAIN_CONTROL);
- reg->ivrp_paddr = readl_relaxed(base + REG_MMU_IVRP_PADDR);
reg->vld_pa_rng = readl_relaxed(base + REG_MMU_VLD_PA_RNG);
+ do {
+ if (!data->plat_data->bank_enable[i])
+ continue;
+ base = data->bank[i].base;
+ reg->int_control[i] = readl_relaxed(base +
REG_MMU_INT_CONTROL0);
+ reg->int_main_control[i] = readl_relaxed(base +
REG_MMU_INT_MAIN_CONTROL);
+ reg->ivrp_paddr[i] = readl_relaxed(base + REG_MMU_IVRP_PADDR);
+ } while (++i < data->plat_data->bank_nr);
clk_disable_unprepare(data->bclk);
return 0;
}
@@ -1132,9 +1139,9 @@ static int __maybe_unused mtk_iommu_runtime_resume(struct
device *dev)
{
struct mtk_iommu_data *data = dev_get_drvdata(dev);
struct mtk_iommu_suspend_reg *reg = >reg;
- struct mtk_iommu_domain *m4u_dom = data->bank[0].m4u_dom;
- void __iomem *base = data->bank[0].base;
- int ret;
+ struct mtk_iommu_domain *m4u_dom;
+ void __iomem *base;
+ int ret, i = 0;
ret = clk_prepare_enable(data->bclk);
if (ret) {
@@ -1157,18 +1164,26 @@ static int __maybe_unused
mtk_iommu_runtime_resume(struct device *dev)
* Uppon first resume, only enable the clk and return, since the values
of the
* registers are not yet set.
*/
- if (!m4u_dom)
+ if (!reg->wr_len_ctrl)
return 0;
+ base = data->bank[i].base;
writel_relaxed(reg->wr_len_ctrl, base + REG_MMU_WR_LEN_CTRL);
writel_relaxed(reg->misc_ctrl, base + REG_MMU_MISC_CTRL);
writel_relaxed(reg->dcm_dis, base + REG_MMU_DCM_DIS);
writel_relaxed(reg->ctrl_reg, base + REG_MMU_CTRL_REG);
- writel_relaxed(reg->int_control0, base + REG_MMU_INT_CONTROL0);
- writel_relaxed(reg->int_main_control, base + REG_MMU_INT_MAIN_CONTROL);
- writel_relaxed(reg->ivrp_paddr, base + REG_MMU_IVRP_PADDR);
writel_relaxed(reg->vld_pa_rng, base + REG_MMU_VLD_PA_RNG);
- writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK, base +
REG_MMU_PT_BASE_ADDR);
+ do {
+ m4u_dom = data->bank[i].m4u_dom;
+ if (!data->plat_data->bank_enable[i] || !m4u_dom)
+ continue;
+ base = data->bank[i].base;
+ writel_relaxed(reg->int_control[i], base +
REG_MMU_INT_CONTROL0);
+ writel_relaxed(reg->int_main_control[i], base +
REG_MMU_INT_MAIN_CONTROL);
+ writel_relaxed(reg->ivrp_paddr[i], base + REG_MMU_IVRP_PADDR);
+ writel(m4u_dom->cfg.arm_v7s_cfg.ttbr & MMU_PT_ADDR_MASK,
+ base + REG_MMU_PT_BASE_ADDR);
+ } while (++i < data->plat_data->bank_nr);
return 0;
}
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index cf4b3d10cf2c..e781ad583131 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -33,11 +33,19 @@ struct mtk_iommu_suspend_reg {
};
u32 dcm_dis;
u32 ctrl_reg;
- u32 int_control0;
- u32 int_main_control;
- u32 ivrp_paddr;
u32 vld_pa_rng;
u32 wr_len_ctrl;
+ union {
+ struct { /* only for gen1 */
+ u32 int_control0;
+ };
+
+ struct { /* only for gen2 that support multi-banks */
+ u32 int_control[MTK_IOMMU_BANK_MAX];
+ u32
Re: [PATCH v3 28/33] iommu/mediatek: Add bank_nr and bank_enable
Il 23/09/21 13:58, Yong Wu ha scritto:
Prepare for supporting multi banks, Adds two variables in the plat_data:
bank_nr: the bank number that this SoC support;
bank_enable: list if the banks is enabled.
Add them for all the current SoC, bank_nr always is 1 and only
bank_enable[0] is enabled.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 18 ++
drivers/iommu/mtk_iommu.h | 3 +++
2 files changed, 21 insertions(+)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index c139675d99e6..4ad85469f68f 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -1134,6 +1134,8 @@ static const struct mtk_iommu_plat_data mt2712_data = {
NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.hw_list = ,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}},
@@ -1144,6 +1146,8 @@ static const struct mtk_iommu_plat_data mt6779_data = {
.flags = HAS_SUB_COMM_2BITS | OUT_ORDER_WR_EN | WR_THROT_EN |
NOT_STD_AXI_MODE | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {5}, {7, 8}, {10}, {9}},
@@ -1154,6 +1158,8 @@ static const struct mtk_iommu_plat_data mt8167_data = {
.flags= RESET_AXI | HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}}, /* Linear mapping. */
@@ -1165,6 +1171,8 @@ static const struct mtk_iommu_plat_data mt8173_data = {
HAS_LEGACY_IVRP_PADDR | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {1}, {2}, {3}, {4}, {5}}, /* Linear mapping. */
@@ -1174,6 +1182,8 @@ static const struct mtk_iommu_plat_data mt8183_data = {
.m4u_plat = M4U_MT8183,
.flags= RESET_AXI | MTK_IOMMU_TYPE_MM,
.inv_sel_reg = REG_MMU_INV_SEL_GEN1,
+ .bank_nr = 1,
+ .bank_enable = {true},
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
.larbid_remap = {{0}, {4}, {5}, {6}, {7}, {2}, {3}, {1}},
@@ -1185,6 +1195,8 @@ static const struct mtk_iommu_plat_data mt8192_data = {
WR_THROT_EN | IOVA_34_EN | NOT_STD_AXI_MODE |
MTK_IOMMU_TYPE_MM,
.inv_sel_reg= REG_MMU_INV_SEL_GEN2,
+ .bank_nr= 1,
+ .bank_enable= {true},
.iova_region= mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{0}, {1}, {4, 5}, {7}, {2}, {9, 11, 19, 20},
@@ -1196,6 +1208,8 @@ static const struct mtk_iommu_plat_data mt8195_data_infra
= {
.flags= WR_THROT_EN | DCM_DISABLE |
MTK_IOMMU_TYPE_INFRA | IFA_IOMMU_PCIe_SUPPORT,
.pericfg_comp_str = "mediatek,mt8195-pericfg_ao",
+ .bank_nr = 1,
+ .bank_enable = {true},
.inv_sel_reg = REG_MMU_INV_SEL_GEN2,
.iova_region = single_domain,
.iova_region_nr = ARRAY_SIZE(single_domain),
@@ -1208,6 +1222,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vdo =
{
SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
.hw_list= ,
.inv_sel_reg= REG_MMU_INV_SEL_GEN2,
+ .bank_nr= 1,
+ .bank_enable= {true},
.iova_region= mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{2, 0}, {21}, {24}, {7}, {19}, {9, 10, 11},
@@ -1221,6 +1237,8 @@ static const struct mtk_iommu_plat_data mt8195_data_vpp =
{
SHARE_PGTABLE | MTK_IOMMU_TYPE_MM,
.hw_list= ,
.inv_sel_reg= REG_MMU_INV_SEL_GEN2,
+ .bank_nr= 1,
+ .bank_enable= {true},
.iova_region= mt8192_multi_dom,
.iova_region_nr = ARRAY_SIZE(mt8192_multi_dom),
.larbid_remap = {{1}, {3}, {22, 0, 0, 0, 23}, {8},
diff --git a/drivers/iommu/mtk_iommu.h b/drivers/iommu/mtk_iommu.h
index 881fade8d39a..dc0190edebf0 100644
--- a/drivers/iommu/mtk_iommu.h
+++ b/drivers/iommu/mtk_iommu.h
@@ -62,6 +62,9 @@ struct mtk_iommu_plat_data {
struct
Re: [PATCH v3 26/33] iommu/mediatek: Add mtk_iommu_bank_data structure
Il 23/09/21 13:58, Yong Wu ha scritto:
Prepare for supporting multi-banks for the IOMMU HW, No functional change.
Add a new structure(mtk_iommu_bank_data) for each a bank. Each a bank have
the independent HW base/IRQ/tlb-range ops, and each a bank has its special
iommu-domain(independent pgtable), thus, also move the domain information
into it.
In previous SoC, we have only one bank which could be treated as bank0(
bankid always is 0 for the previous SoC).
After adding this structure, the tlb operations and irq could use
bank_data as parameter.
Signed-off-by: Yong Wu
---
drivers/iommu/mtk_iommu.c | 137 ++
drivers/iommu/mtk_iommu.h | 24 ++-
2 files changed, 99 insertions(+), 62 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index ac31fe8b7aaf..6ef3eb3cad92 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -146,7 +146,7 @@ struct mtk_iommu_domain {
struct io_pgtable_cfg cfg;
struct io_pgtable_ops *iop;
- struct mtk_iommu_data *data;
+ struct mtk_iommu_bank_data *bank;
struct iommu_domain domain;
};
@@ -221,25 +221,29 @@ static struct mtk_iommu_domain *to_mtk_domain(struct iommu_domain *dom)
static void mtk_iommu_tlb_flush_all(struct mtk_iommu_data *data)
{
- void __iomem *base = data->base;
+ /* Tlb flush all always is in bank0. */
+ struct mtk_iommu_bank_data *bank = >bank[0];
+ void __iomem *base = bank->base;
unsigned long flags;
/*
* No need get power status since the HW PM status nearly is active
* when entering here.
*/
- spin_lock_irqsave(>tlb_lock, flags);
+ spin_lock_irqsave(>tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0, base +
data->plat_data->inv_sel_reg);
writel_relaxed(F_ALL_INVLD, base + REG_MMU_INVALIDATE);
wmb(); /* Make sure the tlb flush all done */
- spin_unlock_irqrestore(>tlb_lock, flags);
+ spin_unlock_irqrestore(>tlb_lock, flags);
}
static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
- struct mtk_iommu_data *data)
+ struct mtk_iommu_bank_data *bank)
{
- struct list_head *head = data->hw_list;
- bool has_pm = !!data->dev->pm_domain;
+ struct list_head *head = bank->pdata->hw_list;
+ bool has_pm = !!bank->pdev->pm_domain;
+ struct mtk_iommu_bank_data *curbank;
+ struct mtk_iommu_data *data;
unsigned long flags;
void __iomem *base;
int ret;
@@ -251,9 +255,10 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
continue;
}
- base = data->base;
+ curbank = >bank[bank->id];
+ base = curbank->base;
- spin_lock_irqsave(>tlb_lock, flags);
+ spin_lock_irqsave(>tlb_lock, flags);
writel_relaxed(F_INVLD_EN1 | F_INVLD_EN0,
base + data->plat_data->inv_sel_reg);
@@ -268,7 +273,7 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long iova, size_t size,
/* Clear the CPE status */
writel_relaxed(0, base + REG_MMU_CPE_DONE);
- spin_unlock_irqrestore(>tlb_lock, flags);
+ spin_unlock_irqrestore(>tlb_lock, flags);
if (ret) {
dev_warn(data->dev,
@@ -283,12 +288,13 @@ static void mtk_iommu_tlb_flush_range_sync(unsigned long
iova, size_t size,
static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
{
- struct mtk_iommu_data *data = dev_id;
- struct mtk_iommu_domain *dom = data->m4u_dom;
+ struct mtk_iommu_bank_data *bank = dev_id;
+ struct mtk_iommu_data *data = bank->pdata;
+ struct mtk_iommu_domain *dom = bank->m4u_dom;
unsigned int fault_larb = 0, fault_port = 0, sub_comm = 0;
u32 int_state, regval, va34_32, pa34_32;
const struct mtk_iommu_plat_data *plat_data = data->plat_data;
- void __iomem *base = data->base;
+ void __iomem *base = bank->base;
u64 fault_iova, fault_pa;
bool layer, write;
@@ -327,10 +333,10 @@ static irqreturn_t mtk_iommu_isr(int irq, void *dev_id)
fault_larb =
data->plat_data->larbid_remap[fault_larb][sub_comm];
}
- if (report_iommu_fault(>domain, data->dev, fault_iova,
+ if (report_iommu_fault(>domain, bank->pdev, fault_iova,
write ? IOMMU_FAULT_WRITE : IOMMU_FAULT_READ)) {
dev_err_ratelimited(
- data->dev,
+ bank->pdev,
"fault type=0x%x iova=0x%llx pa=0x%llx master=0x%x(larb=%d
port=%d) layer=%d %s\n",
int_state, fault_iova, fault_pa, regval, fault_larb,
fault_port,
Re: [PATCH] Swiotlb: Add CONFIG_HAS_IOMEM check around memremap() in the swiotlb_mem_remap()
On Tue, Jan 04, 2022 at 02:51:55PM +, Wei Liu wrote: > > Please stub out all of swiotlb_mem_remap instead of the ifdef inside the > > function. > > Does this look okay to you? Yes, thanks! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] Swiotlb: Add CONFIG_HAS_IOMEM check around memremap() in the swiotlb_mem_remap()
On Sun, Jan 02, 2022 at 11:54:46PM -0800, Christoph Hellwig wrote:
> On Fri, Dec 31, 2021 at 11:56:40AM -0500, Tianyu Lan wrote:
> > From: Tianyu Lan
> >
> > HAS_IOMEM option may not be selected on some platforms(e.g, s390) and this
> > will cause compile error due to miss memremap() implementation. Fix it via
> > adding HAS_IOMEM check around memremap() in the swiotlb.c.
> >
> > Reported-by: kernel test robot
> > Signed-off-by: Tianyu Lan
> > ---
> > kernel/dma/swiotlb.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index b36c1cdd0c4f..3de651ba38cc 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -167,6 +167,7 @@ static void *swiotlb_mem_remap(struct io_tlb_mem *mem,
> > unsigned long bytes)
> > {
> > void *vaddr = NULL;
> >
> > +#ifdef CONFIG_HAS_IOMEM
>
> Please stub out all of swiotlb_mem_remap instead of the ifdef inside the
> function.
Does this look okay to you?
---8<---
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index b36c1cdd0c4f..f1e7ea160b43 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -163,6 +163,7 @@ static inline unsigned long nr_slots(u64 val)
* when swiotlb_unencrypted_base is set. (e.g. for Hyper-V AMD SEV-SNP
* Isolation VMs).
*/
+#ifdef CONFIG_HAS_IOMEM
static void *swiotlb_mem_remap(struct io_tlb_mem *mem, unsigned long bytes)
{
void *vaddr = NULL;
@@ -178,6 +179,12 @@ static void *swiotlb_mem_remap(struct io_tlb_mem *mem,
unsigned long bytes)
return vaddr;
}
+#else
+static void *swiotlb_mem_remap(struct io_tlb_mem *mem, unsigned long bytes)
+{
+ return NULL;
+}
+#endif
/*
* Early SWIOTLB allocation may be too early to allow an architecture to
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 02/14] driver core: Add dma_cleanup callback in bus_type
On Tue, Jan 04, 2022 at 08:39:11AM -0400, Jason Gunthorpe wrote: > On Tue, Jan 04, 2022 at 02:08:36AM -0800, Christoph Hellwig wrote: > > All these bus callouts still looks horrible and just create tons of > > boilerplate code. > > Yes, Lu - Greg asked questions then didn't respond to their answers > meaning he accepts them, you should stick with the v4 version. Trying to catch up on emails from the break, that was way down my list of things to get back to as it's messy and non-obvious. I'll revisit it again after 5.17-rc1 is out, this is too late for that merge window anyway. thanks, greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 00/14] Fix BUG_ON in vfio_iommu_group_notifier()
On Tue, Jan 04, 2022 at 09:56:30AM +0800, Lu Baolu wrote: > v5: > - Move kernel dma ownership auto-claiming from driver core to bus > callback. (Greg) > - Refactor the iommu interfaces to make them more specific. > (Jason/Robin) > - Simplify the dma ownership implementation by removing the owner > type. (Jason) > - Commit message refactoring for PCI drivers. (Bjorn) > - Move iommu_attach/detach_device() improvement patches into another > series as there are a lot of code refactoring and cleanup staffs > in various device drivers. Since you already have the code you should make this 'other series' right now. It should delete iommu_group_attach() and fix iommu_device_attach(). You also didn't really do my suggestion, this messes up the normal __iommu_attach_group()/__iommu_detach_group() instead of adding the clear to purpose iommu_replace_group() for VFIO to use. This just makes it more difficult to normalize the APIs. Otherwise it does seem to have turned out to be more understandable. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 02/14] driver core: Add dma_cleanup callback in bus_type
On Tue, Jan 04, 2022 at 02:08:36AM -0800, Christoph Hellwig wrote: > All these bus callouts still looks horrible and just create tons of > boilerplate code. Yes, Lu - Greg asked questions then didn't respond to their answers meaning he accepts them, you should stick with the v4 version. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 02/14] driver core: Add dma_cleanup callback in bus_type
All these bus callouts still looks horrible and just create tons of boilerplate code. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 01/14] iommu: Add dma ownership management interfaces
On Tue, Jan 04, 2022 at 09:56:31AM +0800, Lu Baolu wrote:
> Multiple devices may be placed in the same IOMMU group because they
> cannot be isolated from each other. These devices must either be
> entirely under kernel control or userspace control, never a mixture.
>
> This adds dma ownership management in iommu core and exposes several
> interfaces for the device drivers and the device userspace assignment
> framework (i.e. vfio), so that any conflict between user and kernel
> controlled DMA could be detected at the beginning.
>
> The device driver oriented interfaces are,
>
> int iommu_device_use_dma_api(struct device *dev);
> void iommu_device_unuse_dma_api(struct device *dev);
>
> Devices under kernel drivers control must call iommu_device_use_dma_api()
> before driver probes. The driver binding process must be aborted if it
> returns failure.
>
> The vfio oriented interfaces are,
>
> int iommu_group_set_dma_owner(struct iommu_group *group,
> void *owner);
> void iommu_group_release_dma_owner(struct iommu_group *group);
> bool iommu_group_dma_owner_claimed(struct iommu_group *group);
>
> The device userspace assignment must be disallowed if the set dma owner
> interface returns failure.
>
> Signed-off-by: Jason Gunthorpe
> Signed-off-by: Kevin Tian
> Signed-off-by: Lu Baolu
> ---
> include/linux/iommu.h | 31
> drivers/iommu/iommu.c | 161 +-
> 2 files changed, 189 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index de0c57a567c8..568f285468cf 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -682,6 +682,13 @@ struct iommu_sva *iommu_sva_bind_device(struct device
> *dev,
> void iommu_sva_unbind_device(struct iommu_sva *handle);
> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>
> +int iommu_device_use_dma_api(struct device *dev);
> +void iommu_device_unuse_dma_api(struct device *dev);
> +
> +int iommu_group_set_dma_owner(struct iommu_group *group, void *owner);
> +void iommu_group_release_dma_owner(struct iommu_group *group);
> +bool iommu_group_dma_owner_claimed(struct iommu_group *group);
> +
> #else /* CONFIG_IOMMU_API */
>
> struct iommu_ops {};
> @@ -1082,6 +1089,30 @@ static inline struct iommu_fwspec
> *dev_iommu_fwspec_get(struct device *dev)
> {
> return NULL;
> }
> +
> +static inline int iommu_device_use_dma_api(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline void iommu_device_unuse_dma_api(struct device *dev)
> +{
> +}
> +
> +static inline int
> +iommu_group_set_dma_owner(struct iommu_group *group, void *owner)
> +{
> + return -ENODEV;
> +}
> +
> +static inline void iommu_group_release_dma_owner(struct iommu_group *group)
> +{
> +}
> +
> +static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group)
> +{
> + return false;
> +}
> #endif /* CONFIG_IOMMU_API */
>
> /**
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 8b86406b7162..ff0c8c1ad5af 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -48,6 +48,8 @@ struct iommu_group {
> struct iommu_domain *default_domain;
> struct iommu_domain *domain;
> struct list_head entry;
> + unsigned int owner_cnt;
> + void *owner;
> };
>
> struct group_device {
> @@ -289,7 +291,12 @@ int iommu_probe_device(struct device *dev)
> mutex_lock(>mutex);
> iommu_alloc_default_domain(group, dev);
>
> - if (group->default_domain) {
> + /*
> + * If device joined an existing group which has been claimed
> + * for none kernel DMA purpose, avoid attaching the default
> + * domain.
> + */
> + if (group->default_domain && !group->owner) {
> ret = __iommu_attach_device(group->default_domain, dev);
> if (ret) {
> mutex_unlock(>mutex);
> @@ -2320,7 +2327,7 @@ static int __iommu_attach_group(struct iommu_domain
> *domain,
> {
> int ret;
>
> - if (group->default_domain && group->domain != group->default_domain)
> + if (group->domain && group->domain != group->default_domain)
> return -EBUSY;
>
> ret = __iommu_group_for_each_dev(group, domain,
> @@ -2357,7 +2364,11 @@ static void __iommu_detach_group(struct iommu_domain
> *domain,
> {
> int ret;
>
> - if (!group->default_domain) {
> + /*
> + * If group has been claimed for none kernel DMA purpose, avoid
> + * re-attaching the default domain.
> + */
none kernel reads odd. But maybe drop that and just say 'claimed
already' ala:
/*
* If the group has been claimed already, do not re-attach the default
* domain.
*/
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
