[PATCH] iommu/sun50i: fix protection flag check
From: David Stevens Fix RW protection check when making a pte, so that it properly checks that both R and W flags are set, instead of either R or W. Signed-off-by: David Stevens --- drivers/iommu/sun50i-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index 181bb1c3437c..11cf5af30956 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -271,7 +271,7 @@ static u32 sun50i_mk_pte(phys_addr_t page, int prot) enum sun50i_iommu_aci aci; u32 flags = 0; - if (prot & (IOMMU_READ | IOMMU_WRITE)) + if ((prot & IOMMU_READ) && (prot & IOMMU_WRITE)) aci = SUN50I_IOMMU_ACI_RD_WR; else if (prot & IOMMU_READ) aci = SUN50I_IOMMU_ACI_RD; -- 2.32.0.402.g57bb445576-goog ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Jason Gunthorpe > Sent: Friday, July 16, 2021 2:13 AM > > On Thu, Jul 15, 2021 at 11:05:45AM -0700, Raj, Ashok wrote: > > On Thu, Jul 15, 2021 at 02:53:36PM -0300, Jason Gunthorpe wrote: > > > On Thu, Jul 15, 2021 at 10:48:36AM -0700, Raj, Ashok wrote: > > > > > > > > > Do we have any isolation requirements here? its the same process. > So if the > > > > > > page-request it sent to guest and even if you report it for mdev1, > after > > > > > > the PRQ is resolved by guest, the request from mdev2 from the > same guest > > > > > > should simply work? > > > > > > > > > > I think we already talked about this and said it should not be done. > > > > > > > > I get the should not be done, I'm wondering where should that be > > > > implemented? > > > > > > The iommu layer cannot have ambiguity. Every RID or RID,PASID slot > > > must have only one device attached to it. Attempting to connect two > > > devices to the same slot fails on the iommu layer. > > > > I guess we are talking about two different things. I was referring to SVM > > side of things. Maybe you are referring to the mdev. > > I'm talking about in the hypervisor. > > As I've said already, the vIOMMU interface is the problem here. The > guest VM should be able to know that it cannot use PASID 1 with two > devices, like the hypervisor knows. At the very least it should be > able to know that the PASID binding has failed and relay that failure > back to the process. > > Ideally the guest would know it should allocate another PASID for > these cases. > > But yes, if mdevs are going to be modeled with RIDs in the guest then > with the current vIOMMU we cannot cause a single hypervisor RID to > show up as two RIDs in the guest without breaking the vIOMMU model. > To summarize, for vIOMMU we can work with the spec owner to define a proper interface to feedback such restriction into the guest if necessary. For the kernel part, it's clear that IOMMU fd should disallow two devices attached to a single [RID] or [RID, PASID] slot in the first place. Then the next question is how to communicate such restriction to the userspace. It sounds like a group, but different in concept. An iommu group describes the minimal isolation boundary thus all devices in the group can be only assigned to a single user. But this case is opposite - the two mdevs (both support ENQCMD submission) with the same parent have problem when assigned to a single VM (in this case vPASID is vm-wide translated thus a same pPASID will be used cross both mdevs) while they instead work pretty well when assigned to different VMs (completely different vPASID spaces thus different pPASIDs). One thought is to have vfio device driver deal with it. In this proposal it is the vfio device driver to define the PASID virtualization policy and report it to userspace via VFIO_DEVICE_GET_INFO. The driver understands the restriction thus could just hide the vPASID capability when the user calls GET_INFO on the 2nd mdev in above scenario. In this way the user even doesn't need to know such restriction at all and both mdevs can be assigned to a single VM w/o any problem. Does it sound a right approach? Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [git pull] IOMMU Fixes for Linux v5.14-rc1
The pull request you sent on Thu, 15 Jul 2021 10:11:08 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git > tags/iommu-fixes-v5.14-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/f3523a226dbb0c925def650a658a0755185d60a8 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 11:05:45AM -0700, Raj, Ashok wrote: > On Thu, Jul 15, 2021 at 02:53:36PM -0300, Jason Gunthorpe wrote: > > On Thu, Jul 15, 2021 at 10:48:36AM -0700, Raj, Ashok wrote: > > > > > > > Do we have any isolation requirements here? its the same process. So > > > > > if the > > > > > page-request it sent to guest and even if you report it for mdev1, > > > > > after > > > > > the PRQ is resolved by guest, the request from mdev2 from the same > > > > > guest > > > > > should simply work? > > > > > > > > I think we already talked about this and said it should not be done. > > > > > > I get the should not be done, I'm wondering where should that be > > > implemented? > > > > The iommu layer cannot have ambiguity. Every RID or RID,PASID slot > > must have only one device attached to it. Attempting to connect two > > devices to the same slot fails on the iommu layer. > > I guess we are talking about two different things. I was referring to SVM > side of things. Maybe you are referring to the mdev. I'm talking about in the hypervisor. As I've said already, the vIOMMU interface is the problem here. The guest VM should be able to know that it cannot use PASID 1 with two devices, like the hypervisor knows. At the very least it should be able to know that the PASID binding has failed and relay that failure back to the process. Ideally the guest would know it should allocate another PASID for these cases. But yes, if mdevs are going to be modeled with RIDs in the guest then with the current vIOMMU we cannot cause a single hypervisor RID to show up as two RIDs in the guest without breaking the vIOMMU model. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 02:53:36PM -0300, Jason Gunthorpe wrote: > On Thu, Jul 15, 2021 at 10:48:36AM -0700, Raj, Ashok wrote: > > > > > Do we have any isolation requirements here? its the same process. So if > > > > the > > > > page-request it sent to guest and even if you report it for mdev1, after > > > > the PRQ is resolved by guest, the request from mdev2 from the same guest > > > > should simply work? > > > > > > I think we already talked about this and said it should not be done. > > > > I get the should not be done, I'm wondering where should that be > > implemented? > > The iommu layer cannot have ambiguity. Every RID or RID,PASID slot > must have only one device attached to it. Attempting to connect two > devices to the same slot fails on the iommu layer. I guess we are talking about two different things. I was referring to SVM side of things. Maybe you are referring to the mdev. A single guest process should be allowed to work with 2 different accelerators. The PASID for the process is just 1. Limiting that to just one accelerator per process seems wrong. Unless there is something else to prevent this, the best way seems never expose more than 1 mdev from same pdev to the same guest. I think this is a reasonable restriction compared to limiting a process to bind to no more than 1 accelerator. > > So the 2nd mdev will fail during IOASID binding when it tries to bind > to the same PASID that the first mdev is already bound to. > > Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 10:48:36AM -0700, Raj, Ashok wrote: > > > Do we have any isolation requirements here? its the same process. So if > > > the > > > page-request it sent to guest and even if you report it for mdev1, after > > > the PRQ is resolved by guest, the request from mdev2 from the same guest > > > should simply work? > > > > I think we already talked about this and said it should not be done. > > I get the should not be done, I'm wondering where should that be > implemented? The iommu layer cannot have ambiguity. Every RID or RID,PASID slot must have only one device attached to it. Attempting to connect two devices to the same slot fails on the iommu layer. So the 2nd mdev will fail during IOASID binding when it tries to bind to the same PASID that the first mdev is already bound to. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 02:18:26PM -0300, Jason Gunthorpe wrote: > On Thu, Jul 15, 2021 at 09:21:41AM -0700, Raj, Ashok wrote: > > On Thu, Jul 15, 2021 at 12:23:25PM -0300, Jason Gunthorpe wrote: > > > On Thu, Jul 15, 2021 at 06:57:57AM -0700, Raj, Ashok wrote: > > > > On Thu, Jul 15, 2021 at 09:48:13AM -0300, Jason Gunthorpe wrote: > > > > > On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > > > > > > > > > > > No. You are right on this case. I don't think there is a way to > > > > > > differentiate one mdev from the other if they come from the > > > > > > same parent and attached by the same guest process. In this > > > > > > case the fault could be reported on either mdev (e.g. the first > > > > > > matching one) to get it fixed in the guest. > > > > > > > > > > If the IOMMU can't distinguish the two mdevs they are not isolated > > > > > and would have to share a group. Since group sharing is not supported > > > > > today this seems like a non-issue > > > > > > > > Does this mean we have to prevent 2 mdev's from same pdev being > > > > assigned to > > > > the same guest? > > > > > > No, it means that the IOMMU layer has to be able to distinguish them. > > > > Ok, the guest has no control over it, as it see 2 separate pci devices and > > thinks they are all different. > > > > Only time when it can fail is during the bind operation. From guest > > perspective a bind in vIOMMU just turns into a write to local table and a > > invalidate will cause the host to update the real copy from the shadow. > > > > There is no way to fail the bind? and Allocation of the PASID is also a > > separate operation and has no clue how its going to be used in the guest. > > You can't attach the same RID to the same PASID twice. The IOMMU code > should prevent this. > > As we've talked about several times, it seems to me the vIOMMU > interface is misdesigned for the requirements you have. The hypervisor > should have a role in allocating the PASID since there are invisible > hypervisor restrictions. This is one of them. Allocating a PASID is a separate step from binding, isn't it? In vt-d we have a virtual command interface that can fail an allocation of PASID. But which device its bound to is a dynamic thing that only gets at bind_mm() right? > > > Do we have any isolation requirements here? its the same process. So if the > > page-request it sent to guest and even if you report it for mdev1, after > > the PRQ is resolved by guest, the request from mdev2 from the same guest > > should simply work? > > I think we already talked about this and said it should not be done. I get the should not be done, I'm wondering where should that be implemented? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 00/16] .map_sg() error cleanup
On 2021-07-15 10:53 a.m., Russell King (Oracle) wrote: > On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote: >> Hi, >> >> This series is spun out and expanded from my work to add P2PDMA support >> to DMA map operations[1]. >> >> The P2PDMA work requires distinguishing different error conditions in >> a map_sg operation. dma_map_sgtable() already allows for returning an >> error code (where as dma_map_sg() is only allowed to return zero) >> however, it currently only returns -EINVAL when a .map_sg() call returns >> zero. >> >> This series cleans up all .map_sg() implementations to return appropriate >> error codes. After the cleanup, dma_map_sg() will still return zero, >> however dma_map_sgtable() will pass the error code from the .map_sg() >> call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the >> obscure implementations. >> >> The patch set is based off of v5.14-rc1 and a git repo can be found >> here: > > Have all the callers for dma_map_sg() been updated to check for error > codes? If not, isn't that a pre-requisit to this patch set? No. Perhaps I wasn't clear enough: This series is changing only impelemntations of .map_sg(). It does *not* change the return code of dma_map_sg(). dma_map_sg() will continue to return zero on error for the foreseeable future. The dma_map_sgtable() call already allows returning error codes and it will pass the new error code through. This is what will be used in the P2PDMA work. Logan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 09:21:41AM -0700, Raj, Ashok wrote: > On Thu, Jul 15, 2021 at 12:23:25PM -0300, Jason Gunthorpe wrote: > > On Thu, Jul 15, 2021 at 06:57:57AM -0700, Raj, Ashok wrote: > > > On Thu, Jul 15, 2021 at 09:48:13AM -0300, Jason Gunthorpe wrote: > > > > On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > > > > > > > > > No. You are right on this case. I don't think there is a way to > > > > > differentiate one mdev from the other if they come from the > > > > > same parent and attached by the same guest process. In this > > > > > case the fault could be reported on either mdev (e.g. the first > > > > > matching one) to get it fixed in the guest. > > > > > > > > If the IOMMU can't distinguish the two mdevs they are not isolated > > > > and would have to share a group. Since group sharing is not supported > > > > today this seems like a non-issue > > > > > > Does this mean we have to prevent 2 mdev's from same pdev being assigned > > > to > > > the same guest? > > > > No, it means that the IOMMU layer has to be able to distinguish them. > > Ok, the guest has no control over it, as it see 2 separate pci devices and > thinks they are all different. > > Only time when it can fail is during the bind operation. From guest > perspective a bind in vIOMMU just turns into a write to local table and a > invalidate will cause the host to update the real copy from the shadow. > > There is no way to fail the bind? and Allocation of the PASID is also a > separate operation and has no clue how its going to be used in the guest. You can't attach the same RID to the same PASID twice. The IOMMU code should prevent this. As we've talked about several times, it seems to me the vIOMMU interface is misdesigned for the requirements you have. The hypervisor should have a role in allocating the PASID since there are invisible hypervisor restrictions. This is one of them. > Do we have any isolation requirements here? its the same process. So if the > page-request it sent to guest and even if you report it for mdev1, after > the PRQ is resolved by guest, the request from mdev2 from the same guest > should simply work? I think we already talked about this and said it should not be done. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 09/16] powerpc/iommu: return error code from .map_sg() ops
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
Propagate the error up if vio_dma_iommu_map_sg() fails.
ppc_iommu_map_sg() may fail either because of iommu_range_alloc() or
because of tbl->it_ops->set(). The former only supports returning an
error with DMA_MAPPING_ERROR and an examination of the latter indicates
that it may return arch-specific errors (for example,
tce_buildmulti_pSeriesLP()). Hence, coalesce all of those errors into
-EINVAL;
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Geoff Levand
---
arch/powerpc/kernel/iommu.c | 4 ++--
arch/powerpc/platforms/ps3/system-bus.c | 2 +-
arch/powerpc/platforms/pseries/vio.c| 5 +++--
3 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 2af89a5e379f..bd0ed618bfa5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -473,7 +473,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table
*tbl,
BUG_ON(direction == DMA_NONE);
if ((nelems == 0) || !tbl)
- return 0;
+ return -EINVAL;
outs = s = segstart = [0];
outcount = 1;
@@ -599,7 +599,7 @@ int ppc_iommu_map_sg(struct device *dev, struct iommu_table
*tbl,
if (s == outs)
break;
}
- return 0;
+ return -EINVAL;
}
diff --git a/arch/powerpc/platforms/ps3/system-bus.c
b/arch/powerpc/platforms/ps3/system-bus.c
index 1a5665875165..c54eb46f0cfb 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -663,7 +663,7 @@ static int ps3_ioc0_map_sg(struct device *_dev, struct
scatterlist *sg,
unsigned long attrs)
{
BUG();
- return 0;
+ return -EINVAL;
}
static void ps3_sb_unmap_sg(struct device *_dev, struct scatterlist *sg,
diff --git a/arch/powerpc/platforms/pseries/vio.c
b/arch/powerpc/platforms/pseries/vio.c
index e00f3725ec96..e31e59c54f30 100644
--- a/arch/powerpc/platforms/pseries/vio.c
+++ b/arch/powerpc/platforms/pseries/vio.c
@@ -560,7 +560,8 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct
scatterlist *sglist,
for_each_sg(sglist, sgl, nelems, count)
alloc_size += roundup(sgl->length, IOMMU_PAGE_SIZE(tbl));
- if (vio_cmo_alloc(viodev, alloc_size))
+ ret = vio_cmo_alloc(viodev, alloc_size);
+ if (ret)
goto out_fail;
ret = ppc_iommu_map_sg(dev, tbl, sglist, nelems, dma_get_mask(dev),
direction, attrs);
@@ -577,7 +578,7 @@ static int vio_dma_iommu_map_sg(struct device *dev, struct
scatterlist *sglist,
vio_cmo_dealloc(viodev, alloc_size);
out_fail:
atomic_inc(>cmo.allocs_failed);
- return 0;
+ return ret;
}
static void vio_dma_iommu_unmap_sg(struct device *dev,
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 06/16] ARM/dma-mapping: return error code from .map_sg() ops
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure,
so propagate any errors that may happen all the way up.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Russell King
Cc: Thomas Bogendoerfer
---
arch/arm/mm/dma-mapping.c | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c4b8df2ad328..8c286e690756 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -980,7 +980,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist
*sg, int nents,
{
const struct dma_map_ops *ops = get_dma_ops(dev);
struct scatterlist *s;
- int i, j;
+ int i, j, ret;
for_each_sg(sg, s, nents, i) {
#ifdef CONFIG_NEED_SG_DMA_LENGTH
@@ -988,7 +988,8 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist
*sg, int nents,
#endif
s->dma_address = ops->map_page(dev, sg_page(s), s->offset,
s->length, dir, attrs);
- if (dma_mapping_error(dev, s->dma_address))
+ ret = dma_mapping_error(dev, s->dma_address);
+ if (ret)
goto bad_mapping;
}
return nents;
@@ -996,7 +997,7 @@ int arm_dma_map_sg(struct device *dev, struct scatterlist
*sg, int nents,
bad_mapping:
for_each_sg(sg, s, i, j)
ops->unmap_page(dev, sg_dma_address(s), sg_dma_len(s), dir,
attrs);
- return 0;
+ return ret;
}
/**
@@ -1622,7 +1623,7 @@ static int __iommu_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
bool is_coherent)
{
struct scatterlist *s = sg, *dma = sg, *start = sg;
- int i, count = 0;
+ int i, count = 0, ret;
unsigned int offset = s->offset;
unsigned int size = s->offset + s->length;
unsigned int max = dma_get_max_seg_size(dev);
@@ -1634,8 +1635,10 @@ static int __iommu_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
s->dma_length = 0;
if (s->offset || (size & ~PAGE_MASK) || size + s->length > max)
{
- if (__map_sg_chunk(dev, start, size, >dma_address,
- dir, attrs, is_coherent) < 0)
+ ret = __map_sg_chunk(dev, start, size,
+>dma_address, dir, attrs,
+is_coherent);
+ if (ret < 0)
goto bad_mapping;
dma->dma_address += offset;
@@ -1648,8 +1651,9 @@ static int __iommu_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
}
size += s->length;
}
- if (__map_sg_chunk(dev, start, size, >dma_address, dir, attrs,
- is_coherent) < 0)
+ ret = __map_sg_chunk(dev, start, size, >dma_address, dir, attrs,
+is_coherent);
+ if (ret < 0)
goto bad_mapping;
dma->dma_address += offset;
@@ -1660,7 +1664,7 @@ static int __iommu_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
bad_mapping:
for_each_sg(sg, s, count, i)
__iommu_remove_mapping(dev, sg_dma_address(s), sg_dma_len(s));
- return 0;
+ return ret;
}
/**
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 03/16] iommu: Return full error code from iommu_map_sg[_atomic]()
Convert to ssize_t return code so the return code from __iommu_map()
can be returned all the way down through dma_iommu_map_sg().
Signed-off-by: Logan Gunthorpe
Cc: Joerg Roedel
Cc: Will Deacon
---
drivers/iommu/iommu.c | 15 +++
include/linux/iommu.h | 22 +++---
2 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..bf971b4e34aa 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2567,9 +2567,9 @@ size_t iommu_unmap_fast(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_unmap_fast);
-static size_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
-struct scatterlist *sg, unsigned int nents, int
prot,
-gfp_t gfp)
+static ssize_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+ struct scatterlist *sg, unsigned int nents, int prot,
+ gfp_t gfp)
{
const struct iommu_ops *ops = domain->ops;
size_t len = 0, mapped = 0;
@@ -2610,19 +2610,18 @@ static size_t __iommu_map_sg(struct iommu_domain
*domain, unsigned long iova,
/* undo mappings already done */
iommu_unmap(domain, iova, mapped);
- return 0;
-
+ return ret;
}
-size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
- struct scatterlist *sg, unsigned int nents, int prot)
+ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+struct scatterlist *sg, unsigned int nents, int prot)
{
might_sleep();
return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_KERNEL);
}
EXPORT_SYMBOL_GPL(iommu_map_sg);
-size_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
+ssize_t iommu_map_sg_atomic(struct iommu_domain *domain, unsigned long iova,
struct scatterlist *sg, unsigned int nents, int prot)
{
return __iommu_map_sg(domain, iova, sg, nents, prot, GFP_ATOMIC);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..9369458ba1bd 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -414,11 +414,11 @@ extern size_t iommu_unmap(struct iommu_domain *domain,
unsigned long iova,
extern size_t iommu_unmap_fast(struct iommu_domain *domain,
unsigned long iova, size_t size,
struct iommu_iotlb_gather *iotlb_gather);
-extern size_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
- struct scatterlist *sg,unsigned int nents, int prot);
-extern size_t iommu_map_sg_atomic(struct iommu_domain *domain,
- unsigned long iova, struct scatterlist *sg,
- unsigned int nents, int prot);
+extern ssize_t iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+ struct scatterlist *sg, unsigned int nents, int prot);
+extern ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
+ unsigned long iova, struct scatterlist *sg,
+ unsigned int nents, int prot);
extern phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t
iova);
extern void iommu_set_fault_handler(struct iommu_domain *domain,
iommu_fault_handler_t handler, void *token);
@@ -679,18 +679,18 @@ static inline size_t iommu_unmap_fast(struct iommu_domain
*domain,
return 0;
}
-static inline size_t iommu_map_sg(struct iommu_domain *domain,
- unsigned long iova, struct scatterlist *sg,
- unsigned int nents, int prot)
+static inline ssize_t iommu_map_sg(struct iommu_domain *domain,
+ unsigned long iova, struct scatterlist *sg,
+ unsigned int nents, int prot)
{
- return 0;
+ return -ENODEV;
}
-static inline size_t iommu_map_sg_atomic(struct iommu_domain *domain,
+static inline ssize_t iommu_map_sg_atomic(struct iommu_domain *domain,
unsigned long iova, struct scatterlist *sg,
unsigned int nents, int prot)
{
- return 0;
+ return -ENODEV;
}
static inline void iommu_flush_iotlb_all(struct iommu_domain *domain)
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 07/16] ia64/sba_iommu: return error code from sba_map_sg_attrs()
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
Propagate the return of dma_mapping_error() up, if it is an errno.
sba_coalesce_chunks() may only presently fail if sba_alloc_range()
fails, which in turn only fails if the iommu is out of mapping
resources, hence a -ENOMEM is used in that case.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Michael Ellerman
Cc: Niklas Schnelle
Cc: Thomas Bogendoerfer
---
arch/ia64/hp/common/sba_iommu.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
index 9148ddbf02e5..09dbe07a18c1 100644
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -1431,7 +1431,7 @@ static int sba_map_sg_attrs(struct device *dev, struct
scatterlist *sglist,
unsigned long attrs)
{
struct ioc *ioc;
- int coalesced, filled = 0;
+ int coalesced, filled = 0, ret;
#ifdef ASSERT_PDIR_SANITY
unsigned long flags;
#endif
@@ -1458,8 +1458,9 @@ static int sba_map_sg_attrs(struct device *dev, struct
scatterlist *sglist,
sglist->dma_length = sglist->length;
sglist->dma_address = sba_map_page(dev, sg_page(sglist),
sglist->offset, sglist->length, dir, attrs);
- if (dma_mapping_error(dev, sglist->dma_address))
- return 0;
+ ret = dma_mapping_error(dev, sglist->dma_address);
+ if (ret)
+ return ret;
return 1;
}
@@ -1486,7 +1487,7 @@ static int sba_map_sg_attrs(struct device *dev, struct
scatterlist *sglist,
coalesced = sba_coalesce_chunks(ioc, dev, sglist, nents);
if (coalesced < 0) {
sba_unmap_sg_attrs(dev, sglist, nents, dir, attrs);
- return 0;
+ return -ENOMEM;
}
/*
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 13/16] xen: swiotlb: return error code from xen_swiotlb_map_sg()
From: Martin Oliveira The .map_sg() op now expects an error code instead of zero on failure. xen_swiotlb_map_sg() may only fail if xen_swiotlb_map_page() fails, but xen_swiotlb_map_page() only supports returning errors as DMA_MAPPING_ERROR. So coalesce all errors into EINVAL. Signed-off-by: Martin Oliveira Signed-off-by: Logan Gunthorpe Cc: Konrad Rzeszutek Wilk Cc: Boris Ostrovsky Cc: Juergen Gross Cc: Stefano Stabellini --- drivers/xen/swiotlb-xen.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c index 24d11861ac7d..b5707127c9d7 100644 --- a/drivers/xen/swiotlb-xen.c +++ b/drivers/xen/swiotlb-xen.c @@ -509,7 +509,7 @@ xen_swiotlb_map_sg(struct device *dev, struct scatterlist *sgl, int nelems, out_unmap: xen_swiotlb_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC); sg_dma_len(sgl) = 0; - return 0; + return -EINVAL; } static void -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 00/16] .map_sg() error cleanup
Hi, This series is spun out and expanded from my work to add P2PDMA support to DMA map operations[1]. The P2PDMA work requires distinguishing different error conditions in a map_sg operation. dma_map_sgtable() already allows for returning an error code (where as dma_map_sg() is only allowed to return zero) however, it currently only returns -EINVAL when a .map_sg() call returns zero. This series cleans up all .map_sg() implementations to return appropriate error codes. After the cleanup, dma_map_sg() will still return zero, however dma_map_sgtable() will pass the error code from the .map_sg() call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the obscure implementations. The patch set is based off of v5.14-rc1 and a git repo can be found here: https://github.com/sbates130272/linux-p2pmem map_sg_err_cleanup_v1 Thanks, Logan [1] https://lore.kernel.org/linux-block/20210513223203.5542-1-log...@deltatee.com/ -- Logan Gunthorpe (5): dma-mapping: Allow map_sg() ops to return negative error codes dma-direct: Return appropriate error code from dma_direct_map_sg() iommu: Return full error code from iommu_map_sg[_atomic]() dma-iommu: Return error code from iommu_dma_map_sg() dma-mapping: Disallow .map_sg operations from returning zero on error Martin Oliveira (11): alpha: return error code from alpha_pci_map_sg() ARM/dma-mapping: return error code from .map_sg() ops ia64/sba_iommu: return error code from sba_map_sg_attrs() MIPS/jazzdma: return error code from jazz_dma_map_sg() powerpc/iommu: return error code from .map_sg() ops s390/pci: return error code from s390_dma_map_sg() sparc/iommu: return error codes from .map_sg() ops parisc: return error code from .map_sg() ops xen: swiotlb: return error code from xen_swiotlb_map_sg() x86/amd_gart: return error code from gart_map_sg() dma-mapping: return error code from dma_dummy_map_sg() arch/alpha/kernel/pci_iommu.c | 10 +++- arch/arm/mm/dma-mapping.c | 22 +--- arch/ia64/hp/common/sba_iommu.c | 9 +-- arch/mips/jazz/jazzdma.c| 2 +- arch/powerpc/kernel/iommu.c | 4 +- arch/powerpc/platforms/ps3/system-bus.c | 2 +- arch/powerpc/platforms/pseries/vio.c| 5 +- arch/s390/pci/pci_dma.c | 12 ++-- arch/sparc/kernel/iommu.c | 4 +- arch/sparc/kernel/pci_sun4v.c | 4 +- arch/sparc/mm/iommu.c | 2 +- arch/x86/kernel/amd_gart_64.c | 16 +++--- drivers/iommu/dma-iommu.c | 20 --- drivers/iommu/iommu.c | 15 +++-- drivers/parisc/ccio-dma.c | 2 +- drivers/parisc/sba_iommu.c | 2 +- drivers/xen/swiotlb-xen.c | 2 +- include/linux/dma-map-ops.h | 6 +- include/linux/dma-mapping.h | 35 +++- include/linux/iommu.h | 22 kernel/dma/direct.c | 2 +- kernel/dma/dummy.c | 2 +- kernel/dma/mapping.c| 73 ++--- 23 files changed, 165 insertions(+), 108 deletions(-) base-commit: e73f0f0ee7541171d89f2e2491130c7771ba58d3 -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 14/16] x86/amd_gart: return error code from gart_map_sg()
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
So make __dma_map_cont() return a valid errno (which is then propagated
to gart_map_sg() via dma_map_cont()) and return it in case of failure.
Also, return -EINVAL in case of invalid nents.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: Borislav Petkov
Cc: "H. Peter Anvin"
Cc: Niklas Schnelle
Cc: Thomas Bogendoerfer
Cc: Michael Ellerman
---
arch/x86/kernel/amd_gart_64.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 9ac696487b13..46aea9a4f26b 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -331,7 +331,7 @@ static int __dma_map_cont(struct device *dev, struct
scatterlist *start,
int i;
if (iommu_start == -1)
- return -1;
+ return -ENOMEM;
for_each_sg(start, s, nelems, i) {
unsigned long pages, addr;
@@ -380,13 +380,13 @@ static int gart_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs)
{
struct scatterlist *s, *ps, *start_sg, *sgmap;
- int need = 0, nextneed, i, out, start;
+ int need = 0, nextneed, i, out, start, ret;
unsigned long pages = 0;
unsigned int seg_size;
unsigned int max_seg_size;
if (nents == 0)
- return 0;
+ return -EINVAL;
out = 0;
start = 0;
@@ -414,8 +414,9 @@ static int gart_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
if (!iommu_merge || !nextneed || !need || s->offset ||
(s->length + seg_size > max_seg_size) ||
(ps->offset + ps->length) % PAGE_SIZE) {
- if (dma_map_cont(dev, start_sg, i - start,
-sgmap, pages, need) < 0)
+ ret = dma_map_cont(dev, start_sg, i - start,
+ sgmap, pages, need);
+ if (ret < 0)
goto error;
out++;
@@ -432,7 +433,8 @@ static int gart_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
pages += iommu_num_pages(s->offset, s->length, PAGE_SIZE);
ps = s;
}
- if (dma_map_cont(dev, start_sg, i - start, sgmap, pages, need) < 0)
+ ret = dma_map_cont(dev, start_sg, i - start, sgmap, pages, need);
+ if (ret < 0)
goto error;
out++;
flush_gart();
@@ -458,7 +460,7 @@ static int gart_map_sg(struct device *dev, struct
scatterlist *sg, int nents,
iommu_full(dev, pages << PAGE_SHIFT, dir);
for_each_sg(sg, s, nents, i)
s->dma_address = DMA_MAPPING_ERROR;
- return 0;
+ return ret;
}
/* allocate and map a coherent mapping */
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 12/16] parisc: return error code from .map_sg() ops
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: "James E.J. Bottomley"
Cc: Helge Deller
---
drivers/parisc/ccio-dma.c | 2 +-
drivers/parisc/sba_iommu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/parisc/ccio-dma.c b/drivers/parisc/ccio-dma.c
index b5f9ee81a46c..a3a5cfda3d93 100644
--- a/drivers/parisc/ccio-dma.c
+++ b/drivers/parisc/ccio-dma.c
@@ -918,7 +918,7 @@ ccio_map_sg(struct device *dev, struct scatterlist *sglist,
int nents,
BUG_ON(!dev);
ioc = GET_IOC(dev);
if (!ioc)
- return 0;
+ return -ENODEV;
DBG_RUN_SG("%s() START %d entries\n", __func__, nents);
diff --git a/drivers/parisc/sba_iommu.c b/drivers/parisc/sba_iommu.c
index dce4cdf786cd..9a6671a230ee 100644
--- a/drivers/parisc/sba_iommu.c
+++ b/drivers/parisc/sba_iommu.c
@@ -947,7 +947,7 @@ sba_map_sg(struct device *dev, struct scatterlist *sglist,
int nents,
ioc = GET_IOC(dev);
if (!ioc)
- return 0;
+ return -ENODEV;
/* Fast path single entry scatterlists. */
if (nents == 1) {
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 02/16] dma-direct: Return appropriate error code from dma_direct_map_sg()
Now that the map_sg() op expects error codes instead of return zero on error, convert dma_direct_map_sg() to return an error code. The only error to return presently is EINVAL if a page could not be mapped. Signed-off-by: Logan Gunthorpe --- kernel/dma/direct.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c index f737e3347059..803ee9321170 100644 --- a/kernel/dma/direct.c +++ b/kernel/dma/direct.c @@ -411,7 +411,7 @@ int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl, int nents, out_unmap: dma_direct_unmap_sg(dev, sgl, i, dir, attrs | DMA_ATTR_SKIP_CPU_SYNC); - return 0; + return -EINVAL; } dma_addr_t dma_direct_map_resource(struct device *dev, phys_addr_t paddr, -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 11/16] sparc/iommu: return error codes from .map_sg() ops
From: Martin Oliveira The .map_sg() op now expects an error code instead of zero on failure. Returning an errno from __sbus_iommu_map_sg() results in sbus_iommu_map_sg_gflush() and sbus_iommu_map_sg_pflush() returning an errno, as those functions are wrappers around __sbus_iommu_map_sg(). Signed-off-by: Martin Oliveira Signed-off-by: Logan Gunthorpe Cc: "David S. Miller" Cc: Niklas Schnelle Cc: Michael Ellerman --- arch/sparc/kernel/iommu.c | 4 ++-- arch/sparc/kernel/pci_sun4v.c | 4 ++-- arch/sparc/mm/iommu.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/sparc/kernel/iommu.c b/arch/sparc/kernel/iommu.c index a034f571d869..0589acd34201 100644 --- a/arch/sparc/kernel/iommu.c +++ b/arch/sparc/kernel/iommu.c @@ -448,7 +448,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist, iommu = dev->archdata.iommu; strbuf = dev->archdata.stc; if (nelems == 0 || !iommu) - return 0; + return -EINVAL; spin_lock_irqsave(>lock, flags); @@ -580,7 +580,7 @@ static int dma_4u_map_sg(struct device *dev, struct scatterlist *sglist, } spin_unlock_irqrestore(>lock, flags); - return 0; + return -EINVAL; } /* If contexts are being used, they are the same in all of the mappings diff --git a/arch/sparc/kernel/pci_sun4v.c b/arch/sparc/kernel/pci_sun4v.c index 9de57e88f7a1..d90e80fa5705 100644 --- a/arch/sparc/kernel/pci_sun4v.c +++ b/arch/sparc/kernel/pci_sun4v.c @@ -486,7 +486,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist, iommu = dev->archdata.iommu; if (nelems == 0 || !iommu) - return 0; + return -EINVAL; atu = iommu->atu; prot = HV_PCI_MAP_ATTR_READ; @@ -619,7 +619,7 @@ static int dma_4v_map_sg(struct device *dev, struct scatterlist *sglist, } local_irq_restore(flags); - return 0; + return -EINVAL; } static void dma_4v_unmap_sg(struct device *dev, struct scatterlist *sglist, diff --git a/arch/sparc/mm/iommu.c b/arch/sparc/mm/iommu.c index 0c0342e5b10d..01ffcedd159c 100644 --- a/arch/sparc/mm/iommu.c +++ b/arch/sparc/mm/iommu.c @@ -256,7 +256,7 @@ static int __sbus_iommu_map_sg(struct device *dev, struct scatterlist *sgl, sg->dma_address =__sbus_iommu_map_page(dev, sg_page(sg), sg->offset, sg->length, per_page_flush); if (sg->dma_address == DMA_MAPPING_ERROR) - return 0; + return -EINVAL; sg->dma_length = sg->length; } -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 01/16] dma-mapping: Allow map_sg() ops to return negative error codes
Allow dma_map_sgtable() to pass errors from the map_sg() ops. This
will be required for returning appropriate error codes when mapping
P2PDMA memory.
Introduce __dma_map_sg_attrs() which will return the raw error code
from the map_sg operation (whether it be negative or zero). Then add a
dma_map_sg_attrs() wrapper to convert any negative errors to zero to
satisfy the existing calling convention.
dma_map_sgtable() will convert a zero error return for old map_sg() ops
into a -EINVAL return and return any negative errors as reported.
This allows map_sg implementations to start returning multiple
negative error codes. Legacy map_sg implementations can continue
to return zero until they are all converted.
Signed-off-by: Logan Gunthorpe
---
include/linux/dma-map-ops.h | 8 +++-
include/linux/dma-mapping.h | 35 --
kernel/dma/mapping.c| 73 +
3 files changed, 78 insertions(+), 38 deletions(-)
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index 0d53a96a3d64..eaa969be8284 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -41,8 +41,12 @@ struct dma_map_ops {
size_t size, enum dma_data_direction dir,
unsigned long attrs);
/*
-* map_sg returns 0 on error and a value > 0 on success.
-* It should never return a value < 0.
+* map_sg should return a negative error code on error.
+* dma_map_sgtable() will return the error code returned and convert
+* a zero return (for legacy implementations) into -EINVAL.
+*
+* dma_map_sg() will always return zero on any negative or zero
+* return to satisfy its own calling convention.
*/
int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs);
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 183e7103a66d..daa1e360f0ee 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -110,6 +110,8 @@ int dma_map_sg_attrs(struct device *dev, struct scatterlist
*sg, int nents,
void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg,
int nents, enum dma_data_direction dir,
unsigned long attrs);
+int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+ enum dma_data_direction dir, unsigned long attrs);
dma_addr_t dma_map_resource(struct device *dev, phys_addr_t phys_addr,
size_t size, enum dma_data_direction dir, unsigned long attrs);
void dma_unmap_resource(struct device *dev, dma_addr_t addr, size_t size,
@@ -174,6 +176,11 @@ static inline void dma_unmap_sg_attrs(struct device *dev,
unsigned long attrs)
{
}
+static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
+ enum dma_data_direction dir, unsigned long attrs)
+{
+ return -EOPNOTSUPP;
+}
static inline dma_addr_t dma_map_resource(struct device *dev,
phys_addr_t phys_addr, size_t size, enum dma_data_direction dir,
unsigned long attrs)
@@ -343,34 +350,6 @@ static inline void dma_sync_single_range_for_device(struct
device *dev,
return dma_sync_single_for_device(dev, addr + offset, size, dir);
}
-/**
- * dma_map_sgtable - Map the given buffer for DMA
- * @dev: The device for which to perform the DMA operation
- * @sgt: The sg_table object describing the buffer
- * @dir: DMA direction
- * @attrs: Optional DMA attributes for the map operation
- *
- * Maps a buffer described by a scatterlist stored in the given sg_table
- * object for the @dir DMA operation by the @dev device. After success the
- * ownership for the buffer is transferred to the DMA domain. One has to
- * call dma_sync_sgtable_for_cpu() or dma_unmap_sgtable() to move the
- * ownership of the buffer back to the CPU domain before touching the
- * buffer by the CPU.
- *
- * Returns 0 on success or -EINVAL on error during mapping the buffer.
- */
-static inline int dma_map_sgtable(struct device *dev, struct sg_table *sgt,
- enum dma_data_direction dir, unsigned long attrs)
-{
- int nents;
-
- nents = dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
- if (nents <= 0)
- return -EINVAL;
- sgt->nents = nents;
- return 0;
-}
-
/**
* dma_unmap_sgtable - Unmap the given buffer for DMA
* @dev: The device for which to perform the DMA operation
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 2b06a809d0b9..30f89d244566 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -177,12 +177,8 @@ void dma_unmap_page_attrs(struct device *dev, dma_addr_t
addr, size_t size,
}
EXPORT_SYMBOL(dma_unmap_page_attrs);
-/*
- * dma_maps_sg_attrs returns 0 on error and > 0
[PATCH v1 05/16] alpha: return error code from alpha_pci_map_sg()
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
pci_map_single_1() can fail for different reasons, but since the only
supported type of error return is DMA_MAPPING_ERROR, we coalesce those
errors into EINVAL.
ENOMEM is returned when no page tables can be allocated.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Richard Henderson
Cc: Ivan Kokshaysky
Cc: Matt Turner
---
arch/alpha/kernel/pci_iommu.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c
index 35d7b3096d6e..72fc2465d13c 100644
--- a/arch/alpha/kernel/pci_iommu.c
+++ b/arch/alpha/kernel/pci_iommu.c
@@ -649,7 +649,9 @@ static int alpha_pci_map_sg(struct device *dev, struct
scatterlist *sg,
sg->dma_address
= pci_map_single_1(pdev, SG_ENT_VIRT_ADDRESS(sg),
sg->length, dac_allowed);
- return sg->dma_address != DMA_MAPPING_ERROR;
+ if (sg->dma_address == DMA_MAPPING_ERROR)
+ return -EINVAL;
+ return 1;
}
start = sg;
@@ -685,8 +687,10 @@ static int alpha_pci_map_sg(struct device *dev, struct
scatterlist *sg,
if (out < end)
out->dma_length = 0;
- if (out - start == 0)
+ if (out - start == 0) {
printk(KERN_WARNING "pci_map_sg failed: no entries?\n");
+ return -ENOMEM;
+ }
DBGA("pci_map_sg: %ld entries\n", out - start);
return out - start;
@@ -699,7 +703,7 @@ static int alpha_pci_map_sg(struct device *dev, struct
scatterlist *sg,
entries. Unmap them now. */
if (out > start)
pci_unmap_sg(pdev, start, out - start, dir);
- return 0;
+ return -ENOMEM;
}
/* Unmap a set of streaming mode DMA translations. Again, cpu read
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 10/16] s390/pci: return error code from s390_dma_map_sg()
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
So propagate the error from __s390_dma_map_sg() up.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
Cc: Niklas Schnelle
Cc: Gerald Schaefer
Cc: Heiko Carstens
Cc: Vasily Gorbik
Cc: Christian Borntraeger
---
arch/s390/pci/pci_dma.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index ebc9a49523aa..c78b02012764 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -487,7 +487,7 @@ static int s390_dma_map_sg(struct device *dev, struct
scatterlist *sg,
unsigned int max = dma_get_max_seg_size(dev);
unsigned int size = s->offset + s->length;
unsigned int offset = s->offset;
- int count = 0, i;
+ int count = 0, i, ret;
for (i = 1; i < nr_elements; i++) {
s = sg_next(s);
@@ -497,8 +497,9 @@ static int s390_dma_map_sg(struct device *dev, struct
scatterlist *sg,
if (s->offset || (size & ~PAGE_MASK) ||
size + s->length > max) {
- if (__s390_dma_map_sg(dev, start, size,
- >dma_address, dir))
+ ret = __s390_dma_map_sg(dev, start, size,
+ >dma_address, dir);
+ if (ret)
goto unmap;
dma->dma_address += offset;
@@ -511,7 +512,8 @@ static int s390_dma_map_sg(struct device *dev, struct
scatterlist *sg,
}
size += s->length;
}
- if (__s390_dma_map_sg(dev, start, size, >dma_address, dir))
+ ret = __s390_dma_map_sg(dev, start, size, >dma_address, dir);
+ if (ret)
goto unmap;
dma->dma_address += offset;
@@ -523,7 +525,7 @@ static int s390_dma_map_sg(struct device *dev, struct
scatterlist *sg,
s390_dma_unmap_pages(dev, sg_dma_address(s), sg_dma_len(s),
dir, attrs);
- return 0;
+ return ret;
}
static void s390_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 08/16] MIPS/jazzdma: return error code from jazz_dma_map_sg()
From: Martin Oliveira The .map_sg() op now expects an error code instead of zero on failure. vdma_alloc() may fail for different reasons, but since it only supports indicating an error via a return of DMA_MAPPING_ERROR, we coalesce the different reasons into -EINVAL. Signed-off-by: Martin Oliveira Signed-off-by: Logan Gunthorpe Cc: Thomas Bogendoerfer --- arch/mips/jazz/jazzdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/jazz/jazzdma.c b/arch/mips/jazz/jazzdma.c index 461457b28982..3b99743435db 100644 --- a/arch/mips/jazz/jazzdma.c +++ b/arch/mips/jazz/jazzdma.c @@ -552,7 +552,7 @@ static int jazz_dma_map_sg(struct device *dev, struct scatterlist *sglist, dir); sg->dma_address = vdma_alloc(sg_phys(sg), sg->length); if (sg->dma_address == DMA_MAPPING_ERROR) - return 0; + return -EINVAL; sg_dma_len(sg) = sg->length; } -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 04/16] dma-iommu: Return error code from iommu_dma_map_sg()
Pass through appropriate error codes from iommu_dma_map_sg() now that
the error code will be passed through dma_map_sgtable().
Signed-off-by: Logan Gunthorpe
Cc: Joerg Roedel
Cc: Will Deacon
---
drivers/iommu/dma-iommu.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 98ba927aee1a..9d35e9994306 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -972,7 +972,7 @@ static int iommu_dma_map_sg_swiotlb(struct device *dev,
struct scatterlist *sg,
out_unmap:
iommu_dma_unmap_sg_swiotlb(dev, sg, i, dir, attrs |
DMA_ATTR_SKIP_CPU_SYNC);
- return 0;
+ return -EINVAL;
}
/*
@@ -993,11 +993,14 @@ static int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg,
dma_addr_t iova;
size_t iova_len = 0;
unsigned long mask = dma_get_seg_boundary(dev);
+ ssize_t ret;
int i;
- if (static_branch_unlikely(_deferred_attach_enabled) &&
- iommu_deferred_attach(dev, domain))
- return 0;
+ if (static_branch_unlikely(_deferred_attach_enabled)) {
+ ret = iommu_deferred_attach(dev, domain);
+ if (ret)
+ return ret;
+ }
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC))
iommu_dma_sync_sg_for_device(dev, sg, nents, dir);
@@ -1045,14 +1048,17 @@ static int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg,
}
iova = iommu_dma_alloc_iova(domain, iova_len, dma_get_mask(dev), dev);
- if (!iova)
+ if (!iova) {
+ ret = -ENOMEM;
goto out_restore_sg;
+ }
/*
* We'll leave any physical concatenation to the IOMMU driver's
* implementation - it knows better than we do.
*/
- if (iommu_map_sg_atomic(domain, iova, sg, nents, prot) < iova_len)
+ ret = iommu_map_sg_atomic(domain, iova, sg, nents, prot);
+ if (ret < iova_len)
goto out_free_iova;
return __finalise_sg(dev, sg, nents, iova);
@@ -1061,7 +1067,7 @@ static int iommu_dma_map_sg(struct device *dev, struct
scatterlist *sg,
iommu_dma_free_iova(cookie, iova, iova_len, NULL);
out_restore_sg:
__invalidate_sg(sg, nents);
- return 0;
+ return ret;
}
static void iommu_dma_unmap_sg(struct device *dev, struct scatterlist *sg,
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 16/16] dma-mapping: Disallow .map_sg operations from returning zero on error
Now that all the .map_sg operations have been converted to returning
proper error codes, drop the code to handle a zero return value,
add a warning if a zero is returned and update the comment for the
map_sg operation.
Signed-off-by: Logan Gunthorpe
---
include/linux/dma-map-ops.h | 8 +++-
kernel/dma/mapping.c| 6 +++---
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/include/linux/dma-map-ops.h b/include/linux/dma-map-ops.h
index eaa969be8284..f299bc1e317b 100644
--- a/include/linux/dma-map-ops.h
+++ b/include/linux/dma-map-ops.h
@@ -42,11 +42,9 @@ struct dma_map_ops {
unsigned long attrs);
/*
* map_sg should return a negative error code on error.
-* dma_map_sgtable() will return the error code returned and convert
-* a zero return (for legacy implementations) into -EINVAL.
-*
-* dma_map_sg() will always return zero on any negative or zero
-* return to satisfy its own calling convention.
+* dma_map_sgtable() will return the error code returned by the
+* operation and dma_map_sg() will always convert any error to zero
+* to satisfy its own calling convention.
*/
int (*map_sg)(struct device *dev, struct scatterlist *sg, int nents,
enum dma_data_direction dir, unsigned long attrs);
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 30f89d244566..978a6a16aaf7 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -194,6 +194,8 @@ static int __dma_map_sg_attrs(struct device *dev, struct
scatterlist *sg,
else
ents = ops->map_sg(dev, sg, nents, dir, attrs);
+ WARN_ON_ONCE(ents == 0);
+
if (ents > 0)
debug_dma_map_sg(dev, sg, nents, ents, dir);
@@ -251,9 +253,7 @@ int dma_map_sgtable(struct device *dev, struct sg_table
*sgt,
int nents;
nents = __dma_map_sg_attrs(dev, sgt->sgl, sgt->orig_nents, dir, attrs);
- if (nents == 0)
- return -EINVAL;
- else if (nents < 0)
+ if (nents < 0)
return nents;
sgt->nents = nents;
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v1 15/16] dma-mapping: return error code from dma_dummy_map_sg()
From: Martin Oliveira
The .map_sg() op now expects an error code instead of zero on failure.
The only errno to return is -ENODEV in the case when DMA is not
supported.
Signed-off-by: Martin Oliveira
Signed-off-by: Logan Gunthorpe
---
kernel/dma/dummy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/dma/dummy.c b/kernel/dma/dummy.c
index eacd4c5b10bf..ae9abebed0c4 100644
--- a/kernel/dma/dummy.c
+++ b/kernel/dma/dummy.c
@@ -22,7 +22,7 @@ static int dma_dummy_map_sg(struct device *dev, struct
scatterlist *sgl,
int nelems, enum dma_data_direction dir,
unsigned long attrs)
{
- return 0;
+ return -ENODEV;
}
static int dma_dummy_supported(struct device *hwdev, u64 mask)
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses
On Thu, Jul 15, 2021 at 12:58:53PM -0400, Boris Ostrovsky wrote: > > On 7/15/21 3:39 AM, Roman Skakun wrote: > >> This looks like it wasn't picked up? Should it go in rc1? > > Hi, Konrad! > > > > This looks like an unambiguous bug, and should be in rc1. > > > Looks like you didn't copy Christoph which could be part of the problem. > Adding him. Can someone just send a clean patch that I can review and hopefully apply? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses
On 7/15/21 3:39 AM, Roman Skakun wrote:
>> This looks like it wasn't picked up? Should it go in rc1?
> Hi, Konrad!
>
> This looks like an unambiguous bug, and should be in rc1.
Looks like you didn't copy Christoph which could be part of the problem. Adding
him.
-boris
>
> Cheers!
>
> ср, 14 июл. 2021 г. в 03:15, Konrad Rzeszutek Wilk :
>> On Tue, Jun 22, 2021 at 04:34:14PM +0300, Roman Skakun wrote:
>>> This commit is dedicated to fix incorrect conversion from
>>> cpu_addr to page address in cases when we get virtual
>>> address which allocated in the vmalloc range.
>>> As the result, virt_to_page() cannot convert this address
>>> properly and return incorrect page address.
>>>
>>> Need to detect such cases and obtains the page address using
>>> vmalloc_to_page() instead.
>>>
>>> Signed-off-by: Roman Skakun
>>> Reviewed-by: Andrii Anisov
>>> ---
>>> Hey!
>>> Thanks for suggestions, Christoph!
>>> I updated the patch according to your advice.
>>> But, I'm so surprised because nobody catches this problem
>>> in the common code before. It looks a bit strange as for me.
>> This looks like it wasn't picked up? Should it go in rc1?
>>>
>>> kernel/dma/ops_helpers.c | 12 ++--
>>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
>>> index 910ae69cae77..782728d8a393 100644
>>> --- a/kernel/dma/ops_helpers.c
>>> +++ b/kernel/dma/ops_helpers.c
>>> @@ -5,6 +5,14 @@
>>> */
>>> #include
>>>
>>> +static struct page *cpu_addr_to_page(void *cpu_addr)
>>> +{
>>> + if (is_vmalloc_addr(cpu_addr))
>>> + return vmalloc_to_page(cpu_addr);
>>> + else
>>> + return virt_to_page(cpu_addr);
>>> +}
>>> +
>>> /*
>>> * Create scatter-list for the already allocated DMA buffer.
>>> */
>>> @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct
>>> sg_table *sgt,
>>>void *cpu_addr, dma_addr_t dma_addr, size_t size,
>>>unsigned long attrs)
>>> {
>>> - struct page *page = virt_to_page(cpu_addr);
>>> + struct page *page = cpu_addr_to_page(cpu_addr);
>>> int ret;
>>>
>>> ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
>>> @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct
>>> vm_area_struct *vma,
>>> return -ENXIO;
>>>
>>> return remap_pfn_range(vma, vma->vm_start,
>>> - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
>>> + page_to_pfn(cpu_addr_to_page(cpu_addr)) +
>>> vma->vm_pgoff,
>>> user_count << PAGE_SHIFT, vma->vm_page_prot);
>>> #else
>>> return -ENXIO;
>>> --
>>> 2.25.1
>>>
>
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 00/16] .map_sg() error cleanup
On Thu, Jul 15, 2021 at 10:45:28AM -0600, Logan Gunthorpe wrote: > Hi, > > This series is spun out and expanded from my work to add P2PDMA support > to DMA map operations[1]. > > The P2PDMA work requires distinguishing different error conditions in > a map_sg operation. dma_map_sgtable() already allows for returning an > error code (where as dma_map_sg() is only allowed to return zero) > however, it currently only returns -EINVAL when a .map_sg() call returns > zero. > > This series cleans up all .map_sg() implementations to return appropriate > error codes. After the cleanup, dma_map_sg() will still return zero, > however dma_map_sgtable() will pass the error code from the .map_sg() > call. Thanks go to Martn Oliveira for doing a lot of the cleanup of the > obscure implementations. > > The patch set is based off of v5.14-rc1 and a git repo can be found > here: Have all the callers for dma_map_sg() been updated to check for error codes? If not, isn't that a pre-requisit to this patch set? >From what I see in Linus' current tree, we still have cases today where the return value of dma_map_sg() is compared with zero to detect failure, so I think that needs fixing before we start changing the dma_map_sg() implementation to return negative numbers. I also notice that there are various places that don't check the return value - and returning a negative number instead of zero may well cause random other bits to be set in fields. So, I think there's a fair amount of work to do in all the drivers before this change can be considered. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/3] iommu: dart: Add DART iommu driver
Hi,
Awesome, thanks a lot for the detailed review!
On Wed, Jul 14, 2021, at 01:23, Robin Murphy wrote:
> ^^ Nit: the subsystem style for the subject format should be
> "iommu/dart: Add..." - similarly on patch #1, which I just realised I
> missed (sorry!)
Sure!
>
> On 2021-06-27 15:34, Sven Peter wrote:
> > Apple's new SoCs use iommus for almost all peripherals. These Device
> > Address Resolution Tables must be setup before these peripherals can
> > act as DMA masters.
> >
> > Signed-off-by: Sven Peter
> > ---
> > MAINTAINERS |1 +
> > drivers/iommu/Kconfig| 15 +
> > drivers/iommu/Makefile |1 +
> > drivers/iommu/apple-dart-iommu.c | 1058 ++
>
> I'd be inclined to drop "-iommu" from the filename, unless there's some
> other "apple-dart" functionality that might lead to a module name clash
> in future?
Sure, DART should only be an iommu.
>
> > 4 files changed, 1075 insertions(+)
> > create mode 100644 drivers/iommu/apple-dart-iommu.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 29e5541c8f21..c1ffaa56b5f9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1245,6 +1245,7 @@ M:Sven Peter
> > L:iommu@lists.linux-foundation.org
> > S:Maintained
> > F:Documentation/devicetree/bindings/iommu/apple,dart.yaml
> > +F: drivers/iommu/apple-dart-iommu.c
> >
> > APPLE SMC DRIVER
> > M:Henrik Rydberg
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index 1f111b399bca..87882c628b46 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -249,6 +249,21 @@ config SPAPR_TCE_IOMMU
> > Enables bits of IOMMU API required by VFIO. The iommu_ops
> > is not implemented as it is not necessary for VFIO.
> >
> > +config IOMMU_APPLE_DART
> > + tristate "Apple DART IOMMU Support"
> > + depends on ARM64 || (COMPILE_TEST && !GENERIC_ATOMIC64)
> > + select IOMMU_API
> > + select IOMMU_IO_PGTABLE
>
> This is redundant - the individual formats already select it.
Removed for the next version.
>
[...]
> > +#include
>
> Redundant duplicate
Whoops, removed for the next version as well.
>
> > +#define DART_MAX_STREAMS 16
[...]
> > +
> > +/*
> > + * This structure is used to identify a single stream attached to a domain.
> > + * It's used as a list inside that domain to be able to attach multiple
> > + * streams to a single domain. Since multiple devices can use a single
> > stream
> > + * it additionally keeps track of how many devices are represented by this
> > + * stream. Once that number reaches zero it is detached from the IOMMU
> > domain
> > + * and all translations from this stream are disabled.
>
> That sounds a lot like something you should be doing properly with groups.
The hint to look at arm-smmu for a similar flow was very helpful, thanks!
Now that I understand how these groups works I completely agree that this
needs to be reworked and done properly.
>
> > + * @dart: DART instance to which this stream belongs
> > + * @sid: stream id within the DART instance
> > + * @num_devices: count of devices attached to this stream
> > + * @stream_head: list head for the next stream
> > + */
> > +struct apple_dart_stream {
> > + struct apple_dart *dart;
> > + u32 sid;
>
> What are the actual SID values like? If they're large and sparse then
> maybe a list makes sense, but if they're small and dense then an array
> hanging off the apple_dart structure itself might be more efficient.
> Given DART_MAX_STREAMS, I'm thinking the latter, and considerably so.
>
> The impression I'm getting so far is that this seems conceptually a bit
> like arm-smmu with stream indexing.
There are two (very similar) types of DARTs.
The one supported with this series has up to 16 stream ids which will be
integers <16. There's another variant used for Thunderbolt for which I will
add support in a follow-up that supports up to 64 stream ids then.
So at worst this is an array with 64 entries if this structure won't
disappear completely.
And yes, this is conceptually a bit like arm-smmu's stream indexing I think.
>
> > + u32 num_devices;
> > +
> > + struct list_head stream_head;
> > +};
> > +
> > +/*
> > + * This structure is attached to each iommu domain handled by a DART.
> > + * A single domain is used to represent a single virtual address space.
> > + * It is always allocated together with a page table.
> > + *
> > + * Streams are the smallest units the DART hardware can differentiate.
> > + * These are pointed to the page table of a domain whenever a device is
> > + * attached to it. A single stream can only be assigned to a single domain.
> > + *
> > + * Devices are assigned to at least a single and sometimes multiple
> > individual
> > + * streams (using the iommus property in the device tree). Multiple devices
> > + * can theoretically be represented by the same stream, though this is
> >
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 12:23:25PM -0300, Jason Gunthorpe wrote: > On Thu, Jul 15, 2021 at 06:57:57AM -0700, Raj, Ashok wrote: > > On Thu, Jul 15, 2021 at 09:48:13AM -0300, Jason Gunthorpe wrote: > > > On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > > > > > > > No. You are right on this case. I don't think there is a way to > > > > differentiate one mdev from the other if they come from the > > > > same parent and attached by the same guest process. In this > > > > case the fault could be reported on either mdev (e.g. the first > > > > matching one) to get it fixed in the guest. > > > > > > If the IOMMU can't distinguish the two mdevs they are not isolated > > > and would have to share a group. Since group sharing is not supported > > > today this seems like a non-issue > > > > Does this mean we have to prevent 2 mdev's from same pdev being assigned to > > the same guest? > > No, it means that the IOMMU layer has to be able to distinguish them. Ok, the guest has no control over it, as it see 2 separate pci devices and thinks they are all different. Only time when it can fail is during the bind operation. From guest perspective a bind in vIOMMU just turns into a write to local table and a invalidate will cause the host to update the real copy from the shadow. There is no way to fail the bind? and Allocation of the PASID is also a separate operation and has no clue how its going to be used in the guest. > > This either means they are "SW mdevs" which does not involve the IOMMU > layer and puts both the responsibility for isolation and idenfication > on the mdev driver. When you mean SW mdev, is it the GPU like case where mdev is purely a SW construct? or SIOV type where RID+PASID case? > > Or they are some "PASID mdev" which does allow the IOMMU to isolate > them. > > What can't happen is to comingle /dev/iommu control over the pdev > between two mdevs. > > ie we can't talk about faults for IOMMU on SW mdevs - faults do not > come from the IOMMU layer, they have to come from inside the mdev it > self, somehow. Recoverable faults for guest needs to be sent to guest? A page-request from mdev1 and from mdev2 will both look alike when the process is sharing it. Do we have any isolation requirements here? its the same process. So if the page-request it sent to guest and even if you report it for mdev1, after the PRQ is resolved by guest, the request from mdev2 from the same guest should simply work? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 06:57:57AM -0700, Raj, Ashok wrote: > On Thu, Jul 15, 2021 at 09:48:13AM -0300, Jason Gunthorpe wrote: > > On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > > > > > No. You are right on this case. I don't think there is a way to > > > differentiate one mdev from the other if they come from the > > > same parent and attached by the same guest process. In this > > > case the fault could be reported on either mdev (e.g. the first > > > matching one) to get it fixed in the guest. > > > > If the IOMMU can't distinguish the two mdevs they are not isolated > > and would have to share a group. Since group sharing is not supported > > today this seems like a non-issue > > Does this mean we have to prevent 2 mdev's from same pdev being assigned to > the same guest? No, it means that the IOMMU layer has to be able to distinguish them. This either means they are "SW mdevs" which does not involve the IOMMU layer and puts both the responsibility for isolation and idenfication on the mdev driver. Or they are some "PASID mdev" which does allow the IOMMU to isolate them. What can't happen is to comingle /dev/iommu control over the pdev between two mdevs. ie we can't talk about faults for IOMMU on SW mdevs - faults do not come from the IOMMU layer, they have to come from inside the mdev it self, somehow. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
On 2021-07-15 15:07, Christoph Hellwig wrote: On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote: If people are going to insist on calling iommu_iova_to_phys() pointlessly and expecting it to work, Maybe we need to fix that? Feel free to try, but we didn't have much luck pushing back on it previously, so playing whack-a-mole against netdev now is a game I'm personally happy to stay away from ;) Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu: Streamline iommu_iova_to_phys()
On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote: > If people are going to insist on calling iommu_iova_to_phys() > pointlessly and expecting it to work, Maybe we need to fix that? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 09:48:13AM -0300, Jason Gunthorpe wrote: > On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > > > No. You are right on this case. I don't think there is a way to > > differentiate one mdev from the other if they come from the > > same parent and attached by the same guest process. In this > > case the fault could be reported on either mdev (e.g. the first > > matching one) to get it fixed in the guest. > > If the IOMMU can't distinguish the two mdevs they are not isolated > and would have to share a group. Since group sharing is not supported > today this seems like a non-issue Does this mean we have to prevent 2 mdev's from same pdev being assigned to the same guest? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu: Streamline iommu_iova_to_phys()
If people are going to insist on calling iommu_iova_to_phys()
pointlessly and expecting it to work, we can at least do ourselves a
favour by handling those cases in the core code, rather than repeatedly
across an inconsistent handful of drivers.
Since all the existing drivers implement the internal callback, and any
future ones are likely to want to work with iommu-dma which relies on
iova_to_phys a fair bit, we may as well remove that currently-redundant
check as well and consider it mandatory.
Reviewed-by: Lu Baolu
Signed-off-by: Robin Murphy
---
v2: Lowered the priority of the ops check per Baolu's suggestion, only
even further to the point of non-existence :)
---
drivers/iommu/amd/io_pgtable.c | 3 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ---
drivers/iommu/iommu.c | 5 -
4 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c b/drivers/iommu/amd/io_pgtable.c
index bb0ee5c9fde7..182c93a43efd 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -493,9 +493,6 @@ static phys_addr_t iommu_v1_iova_to_phys(struct
io_pgtable_ops *ops, unsigned lo
unsigned long offset_mask, pte_pgsize;
u64 *pte, __pte;
- if (pgtable->mode == PAGE_MODE_NONE)
- return iova;
-
pte = fetch_pte(pgtable, iova, _pgsize);
if (!pte || !IOMMU_PTE_PRESENT(*pte))
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 3e87a9cf6da3..c9fdd0d097be 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2481,9 +2481,6 @@ arm_smmu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0f181f76c31b..0d04eafa3fdb 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1317,9 +1317,6 @@ static phys_addr_t arm_smmu_iova_to_phys(struct
iommu_domain *domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 43a2041d9629..d4ba324fb0bc 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2371,7 +2371,10 @@ EXPORT_SYMBOL_GPL(iommu_detach_group);
phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain, dma_addr_t iova)
{
- if (unlikely(domain->ops->iova_to_phys == NULL))
+ if (domain->type == IOMMU_DOMAIN_IDENTITY)
+ return iova;
+
+ if (domain->type == IOMMU_DOMAIN_BLOCKED)
return 0;
return domain->ops->iova_to_phys(domain, iova);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On Thu, Jul 15, 2021 at 06:49:54AM +, Tian, Kevin wrote: > No. You are right on this case. I don't think there is a way to > differentiate one mdev from the other if they come from the > same parent and attached by the same guest process. In this > case the fault could be reported on either mdev (e.g. the first > matching one) to get it fixed in the guest. If the IOMMU can't distinguish the two mdevs they are not isolated and would have to share a group. Since group sharing is not supported today this seems like a non-issue Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 11/11] memory: mtk-smi: mt8195: Add initial setting for smi-larb
To improve the performance, We add some initial setting for smi larbs.
there are two part:
1), Each port has the special ostd(outstanding) value in each larb.
2), Two general setting for each larb.
In some SoC, this setting maybe changed dynamically for some special case
like 4K, and this initial setting is enough in mt8195.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 74 +++-
1 file changed, 73 insertions(+), 1 deletion(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index c52bf02458ff..1d9e67520433 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -32,6 +32,14 @@
#define SMI_DUMMY 0x444
/* SMI LARB */
+#define SMI_LARB_CMD_THRT_CON 0x24
+#define SMI_LARB_THRT_EN 0x370256
+
+#define SMI_LARB_SW_FLAG 0x40
+#define SMI_LARB_SW_FLAG_1 0x1
+
+#define SMI_LARB_OSTDL_PORT0x200
+#define SMI_LARB_OSTDL_PORTx(id) (SMI_LARB_OSTDL_PORT + (((id) & 0x1f)
<< 2))
/* Below are about mmu enable registers, they are different in SoCs */
/* mt2701 */
@@ -67,6 +75,11 @@
})
#define SMI_COMMON_INIT_REGS_NR6
+#define SMI_LARB_PORT_NR_MAX 32
+
+#define MTK_SMI_FLAG_LARB_THRT_EN BIT(0)
+#define MTK_SMI_FLAG_LARB_SW_FLAG BIT(1)
+#define MTK_SMI_CAPS(flags, _x)(!!((flags) & (_x)))
struct mtk_smi_reg_pair {
unsigned intoffset;
@@ -97,6 +110,8 @@ struct mtk_smi_larb_gen {
int port_in_larb[MTK_LARB_NR_MAX + 1];
void (*config_port)(struct device *dev);
unsigned intlarb_direct_to_common_mask;
+ unsigned intflags_general;
+ const u8(*ostd)[SMI_LARB_PORT_NR_MAX];
};
struct mtk_smi {
@@ -213,12 +228,22 @@ static void mtk_smi_larb_config_port_mt8173(struct device
*dev)
static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
{
struct mtk_smi_larb *larb = dev_get_drvdata(dev);
- u32 reg;
+ u32 reg, flags_general = larb->larb_gen->flags_general;
+ const u8 *larbostd = larb->larb_gen->ostd[larb->larbid];
int i;
if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
return;
+ if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_LARB_THRT_EN))
+ writel_relaxed(SMI_LARB_THRT_EN, larb->base +
SMI_LARB_CMD_THRT_CON);
+
+ if (MTK_SMI_CAPS(flags_general, MTK_SMI_FLAG_LARB_SW_FLAG))
+ writel_relaxed(SMI_LARB_SW_FLAG_1, larb->base +
SMI_LARB_SW_FLAG);
+
+ for (i = 0; i < SMI_LARB_PORT_NR_MAX && larbostd && !!larbostd[i]; i++)
+ writel_relaxed(larbostd[i], larb->base +
SMI_LARB_OSTDL_PORTx(i));
+
for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
reg = readl_relaxed(larb->base + SMI_LARB_NONSEC_CON(i));
reg |= F_MMU_EN;
@@ -227,6 +252,51 @@ static void mtk_smi_larb_config_port_gen2_general(struct
device *dev)
}
}
+static const u8 mtk_smi_larb_mt8195_ostd[][SMI_LARB_PORT_NR_MAX] = {
+ [0] = {0x0a, 0xc, 0x22, 0x22, 0x01, 0x0a,}, /* larb0 */
+ [1] = {0x0a, 0xc, 0x22, 0x22, 0x01, 0x0a,}, /* larb1 */
+ [2] = {0x12, 0x12, 0x12, 0x12, 0x0a,}, /* ... */
+ [3] = {0x12, 0x12, 0x12, 0x12, 0x28, 0x28, 0x0a,},
+ [4] = {0x06, 0x01, 0x17, 0x06, 0x0a,},
+ [5] = {0x06, 0x01, 0x17, 0x06, 0x06, 0x01, 0x06, 0x0a,},
+ [6] = {0x06, 0x01, 0x06, 0x0a,},
+ [7] = {0x0c, 0x0c, 0x12,},
+ [8] = {0x0c, 0x0c, 0x12,},
+ [9] = {0x0a, 0x08, 0x04, 0x06, 0x01, 0x01, 0x10, 0x18, 0x11, 0x0a,
+ 0x08, 0x04, 0x11, 0x06, 0x02, 0x06, 0x01, 0x11, 0x11, 0x06,},
+ [10] = {0x18, 0x08, 0x01, 0x01, 0x20, 0x12, 0x18, 0x06, 0x05, 0x10,
+ 0x08, 0x08, 0x10, 0x08, 0x08, 0x18, 0x0c, 0x09, 0x0b, 0x0d,
+ 0x0d, 0x06, 0x10, 0x10,},
+ [11] = {0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x0e, 0x01, 0x01, 0x01, 0x01,},
+ [12] = {0x09, 0x09, 0x05, 0x05, 0x0c, 0x18, 0x02, 0x02, 0x04, 0x02,},
+ [13] = {0x02, 0x02, 0x12, 0x12, 0x02, 0x02, 0x02, 0x02, 0x08, 0x01,},
+ [14] = {0x12, 0x12, 0x02, 0x02, 0x02, 0x02, 0x16, 0x01, 0x16, 0x01,
+ 0x01, 0x02, 0x02, 0x08, 0x02,},
+ [15] = {},
+ [16] = {0x28, 0x02, 0x02, 0x12, 0x02, 0x12, 0x10, 0x02, 0x02, 0x0a,
+ 0x12, 0x02, 0x0a, 0x16, 0x02, 0x04,},
+ [17] = {0x1a, 0x0e, 0x0a, 0x0a, 0x0c, 0x0e, 0x10,},
+ [18] = {0x12, 0x06, 0x12, 0x06,},
+ [19] = {0x01, 0x04, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x04, 0x01,
+ 0x01, 0x01, 0x04, 0x0a, 0x06, 0x01, 0x01, 0x01, 0x0a, 0x06,
+ 0x01, 0x01, 0x05, 0x03, 0x03, 0x04, 0x01,},
+ [20] = {0x01, 0x04, 0x01, 0x01, 0x01, 0x01, 0x01, 0x04, 0x04, 0x01,
+ 0x01, 0x01, 0x04, 0x0a, 0x06, 0x01, 0x01, 0x01, 0x0a, 0x06,
+ 0x01, 0x01, 0x05, 0x03, 0x03, 0x04,
[PATCH v2 10/11] memory: mtk-smi: mt8195: Add initial setting for smi-common
To improve the performance, add initial setting for smi-common.
some register use some fix setting(suggested from DE).
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 42
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 3c288716a378..c52bf02458ff 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -18,11 +18,19 @@
#include
/* SMI COMMON */
+#define SMI_L1LEN 0x100
+
#define SMI_BUS_SEL0x220
#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
/* All are MMU0 defaultly. Only specialize mmu1 here. */
#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid))
+#define SMI_M4U_TH 0x234
+#define SMI_FIFO_TH1 0x238
+#define SMI_FIFO_TH2 0x23c
+#define SMI_DCM0x300
+#define SMI_DUMMY 0x444
+
/* SMI LARB */
/* Below are about mmu enable registers, they are different in SoCs */
@@ -58,6 +66,13 @@
(_id << 8 | _id << 10 | _id << 12 | _id << 14); \
})
+#define SMI_COMMON_INIT_REGS_NR6
+
+struct mtk_smi_reg_pair {
+ unsigned intoffset;
+ u32 value;
+};
+
enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2, /* gen2 smi common */
@@ -74,6 +89,8 @@ static const char * const mtk_smi_larb_clks_optional[] =
{"gals"};
struct mtk_smi_common_plat {
enum mtk_smi_type type;
u32 bus_sel; /* Balance some larbs to enter mmu0 or
mmu1 */
+
+ const struct mtk_smi_reg_pair *init;
};
struct mtk_smi_larb_gen {
@@ -409,6 +426,15 @@ static struct platform_driver mtk_smi_larb_driver = {
}
};
+static const struct mtk_smi_reg_pair
mtk_smi_common_mt8195_init[SMI_COMMON_INIT_REGS_NR] = {
+ {SMI_L1LEN, 0xb},
+ {SMI_M4U_TH, 0xe100e10},
+ {SMI_FIFO_TH1, 0x506090a},
+ {SMI_FIFO_TH2, 0x506090a},
+ {SMI_DCM, 0x4f1},
+ {SMI_DUMMY, 0x1},
+};
+
static const struct mtk_smi_common_plat mtk_smi_common_gen1 = {
.type = MTK_SMI_GEN1,
};
@@ -439,11 +465,13 @@ static const struct mtk_smi_common_plat
mtk_smi_common_mt8195_vdo = {
.type = MTK_SMI_GEN2,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(3) | F_MMU1_LARB(5) |
F_MMU1_LARB(7),
+ .init = mtk_smi_common_mt8195_init,
};
static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vpp = {
.type = MTK_SMI_GEN2,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(7),
+ .init = mtk_smi_common_mt8195_init,
};
static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8195 = {
@@ -530,15 +558,21 @@ static int mtk_smi_common_remove(struct platform_device
*pdev)
static int __maybe_unused mtk_smi_common_resume(struct device *dev)
{
struct mtk_smi *common = dev_get_drvdata(dev);
- u32 bus_sel = common->plat->bus_sel;
- int ret;
+ const struct mtk_smi_reg_pair *init = common->plat->init;
+ u32 bus_sel = common->plat->bus_sel; /* default is 0 */
+ int ret, i;
ret = clk_bulk_prepare_enable(common->clk_num, common->clks);
if (ret)
return ret;
- if (common->plat->type == MTK_SMI_GEN2 && bus_sel)
- writel(bus_sel, common->base + SMI_BUS_SEL);
+ if (common->plat->type != MTK_SMI_GEN2)
+ return 0;
+
+ for (i = 0; i < SMI_COMMON_INIT_REGS_NR && init && init[i].offset; i++)
+ writel_relaxed(init[i].value, common->base + init[i].offset);
+
+ writel(bus_sel, common->base + SMI_BUS_SEL);
return 0;
}
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 09/11] memory: mtk-smi: mt8195: Add smi support
MT8195 has two smi-common, their IP are the same. Only the larbs that
connect with the smi-common are different. thus the bus_sel are different
for the two smi-common.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index e5a34b3952a0..3c288716a378 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -250,6 +250,10 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt8192 =
{
.config_port= mtk_smi_larb_config_port_gen2_general,
};
+static const struct mtk_smi_larb_gen mtk_smi_larb_mt8195 = {
+ .config_port= mtk_smi_larb_config_port_gen2_general,
+};
+
static const struct of_device_id mtk_smi_larb_of_ids[] = {
{.compatible = "mediatek,mt2701-smi-larb", .data =
_smi_larb_mt2701},
{.compatible = "mediatek,mt2712-smi-larb", .data =
_smi_larb_mt2712},
@@ -258,6 +262,7 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{.compatible = "mediatek,mt8173-smi-larb", .data =
_smi_larb_mt8173},
{.compatible = "mediatek,mt8183-smi-larb", .data =
_smi_larb_mt8183},
{.compatible = "mediatek,mt8192-smi-larb", .data =
_smi_larb_mt8192},
+ {.compatible = "mediatek,mt8195-smi-larb", .data =
_smi_larb_mt8195},
{}
};
@@ -430,6 +435,21 @@ static const struct mtk_smi_common_plat
mtk_smi_common_mt8192 = {
F_MMU1_LARB(6),
};
+static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vdo = {
+ .type = MTK_SMI_GEN2,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(3) | F_MMU1_LARB(5) |
+ F_MMU1_LARB(7),
+};
+
+static const struct mtk_smi_common_plat mtk_smi_common_mt8195_vpp = {
+ .type = MTK_SMI_GEN2,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(7),
+};
+
+static const struct mtk_smi_common_plat mtk_smi_sub_common_mt8195 = {
+ .type = MTK_SMI_GEN2_SUB_COMM,
+};
+
static const struct of_device_id mtk_smi_common_of_ids[] = {
{.compatible = "mediatek,mt2701-smi-common", .data =
_smi_common_gen1},
{.compatible = "mediatek,mt2712-smi-common", .data =
_smi_common_gen2},
@@ -438,6 +458,9 @@ static const struct of_device_id mtk_smi_common_of_ids[] = {
{.compatible = "mediatek,mt8173-smi-common", .data =
_smi_common_gen2},
{.compatible = "mediatek,mt8183-smi-common", .data =
_smi_common_mt8183},
{.compatible = "mediatek,mt8192-smi-common", .data =
_smi_common_mt8192},
+ {.compatible = "mediatek,mt8195-smi-common-vdo", .data =
_smi_common_mt8195_vdo},
+ {.compatible = "mediatek,mt8195-smi-common-vpp", .data =
_smi_common_mt8195_vpp},
+ {.compatible = "mediatek,mt8195-smi-sub-common", .data =
_smi_sub_common_mt8195},
{}
};
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 08/11] memory: mtk-smi: Use devm_platform_ioremap_resource
No functional change. Simplify probing code.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index ee49bb50f5f5..e5a34b3952a0 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -317,7 +317,6 @@ static int mtk_smi_dts_clk_init(struct device *dev, struct
mtk_smi *smi,
static int mtk_smi_larb_probe(struct platform_device *pdev)
{
struct mtk_smi_larb *larb;
- struct resource *res;
struct device *dev = >dev;
int ret;
@@ -326,8 +325,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
return -ENOMEM;
larb->larb_gen = of_device_get_match_data(dev);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- larb->base = devm_ioremap_resource(dev, res);
+ larb->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(larb->base))
return PTR_ERR(larb->base);
@@ -447,7 +445,6 @@ static int mtk_smi_common_probe(struct platform_device
*pdev)
{
struct device *dev = >dev;
struct mtk_smi *common;
- struct resource *res;
int ret;
common = devm_kzalloc(dev, sizeof(*common), GFP_KERNEL);
@@ -468,8 +465,7 @@ static int mtk_smi_common_probe(struct platform_device
*pdev)
* base.
*/
if (common->plat->type == MTK_SMI_GEN1) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- common->smi_ao_base = devm_ioremap_resource(dev, res);
+ common->smi_ao_base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(common->smi_ao_base))
return PTR_ERR(common->smi_ao_base);
@@ -481,8 +477,7 @@ static int mtk_smi_common_probe(struct platform_device
*pdev)
if (ret)
return ret;
} else {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- common->base = devm_ioremap_resource(dev, res);
+ common->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(common->base))
return PTR_ERR(common->base);
}
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 07/11] memory: mtk-smi: Add smi sub common support
In mt8195, there are some larbs connect with the smi-sub-common, then
connect with smi-common.
Before we create device link between smi-larb with smi-common. If we have
sub-common, we should use device link the smi-larb and smi-sub-common,
then use device link between the smi-sub-common with smi-common. This is
for enabling clock/power automatically.
Move the device link code to a new interface for reusing.
There is no SW extra setting for smi-sub-common.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 75 +++-
1 file changed, 51 insertions(+), 24 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index e68cbb51dd12..ee49bb50f5f5 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -60,7 +60,8 @@
enum mtk_smi_type {
MTK_SMI_GEN1,
- MTK_SMI_GEN2
+ MTK_SMI_GEN2, /* gen2 smi common */
+ MTK_SMI_GEN2_SUB_COMM, /* gen2 smi sub common */
};
#define MTK_SMI_CLK_NR_MAX 4
@@ -90,13 +91,14 @@ struct mtk_smi {
void __iomem*smi_ao_base; /* only for gen1 */
void __iomem*base;/* only for gen2 */
};
+ struct device *smi_common_dev; /* for sub common */
const struct mtk_smi_common_plat *plat;
};
struct mtk_smi_larb { /* larb: local arbiter */
struct mtk_smi smi;
void __iomem*base;
- struct device *smi_common_dev;
+ struct device *smi_common_dev; /* common or
sub-common dev */
const struct mtk_smi_larb_gen *larb_gen;
int larbid;
u32 *mmu;
@@ -259,6 +261,38 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{}
};
+static int mtk_smi_device_link_common(struct device *dev, struct device
**com_dev)
+{
+ struct platform_device *smi_com_pdev;
+ struct device_node *smi_com_node;
+ struct device *smi_com_dev;
+ struct device_link *link;
+
+ smi_com_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
+ if (!smi_com_node)
+ return -EINVAL;
+
+ smi_com_pdev = of_find_device_by_node(smi_com_node);
+ of_node_put(smi_com_node);
+ if (smi_com_pdev) {
+ /* smi common is the supplier, Make sure it is ready before */
+ if (!platform_get_drvdata(smi_com_pdev))
+ return -EPROBE_DEFER;
+ smi_com_dev = _com_pdev->dev;
+ link = device_link_add(dev, smi_com_dev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
+ if (!link) {
+ dev_err(dev, "Unable to link smi-common dev\n");
+ return -ENODEV;
+ }
+ *com_dev = smi_com_dev;
+ } else {
+ dev_err(dev, "Failed to get the smi_common device\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
unsigned int clk_nr_optional,
const char * const clk_optional[])
@@ -285,9 +319,6 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
struct mtk_smi_larb *larb;
struct resource *res;
struct device *dev = >dev;
- struct device_node *smi_node;
- struct platform_device *smi_pdev;
- struct device_link *link;
int ret;
larb = devm_kzalloc(dev, sizeof(*larb), GFP_KERNEL);
@@ -307,26 +338,10 @@ static int mtk_smi_larb_probe(struct platform_device
*pdev)
return ret;
larb->smi.dev = dev;
- smi_node = of_parse_phandle(dev->of_node, "mediatek,smi", 0);
- if (!smi_node)
- return -EINVAL;
- smi_pdev = of_find_device_by_node(smi_node);
- of_node_put(smi_node);
- if (smi_pdev) {
- if (!platform_get_drvdata(smi_pdev))
- return -EPROBE_DEFER;
- larb->smi_common_dev = _pdev->dev;
- link = device_link_add(dev, larb->smi_common_dev,
- DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
- if (!link) {
- dev_err(dev, "Unable to link smi-common dev\n");
- return -ENODEV;
- }
- } else {
- dev_err(dev, "Failed to get the smi_common device\n");
- return -EINVAL;
- }
+ ret = mtk_smi_device_link_common(dev, >smi_common_dev);
+ if (ret < 0)
+ return ret;
pm_runtime_enable(dev);
platform_set_drvdata(pdev, larb);
@@ -471,6 +486,14 @@ static int mtk_smi_common_probe(struct platform_device
*pdev)
if (IS_ERR(common->base))
[PATCH v2 06/11] memory: mtk-smi: Add error handle for smi_probe
Add error handle while component_add fail. Signed-off-by: Yong Wu --- It don't have the error handle when v1. it is not a fatal error. thus don't add fix tags. --- drivers/memory/mtk-smi.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c index 6f8e582bace5..e68cbb51dd12 100644 --- a/drivers/memory/mtk-smi.c +++ b/drivers/memory/mtk-smi.c @@ -330,7 +330,15 @@ static int mtk_smi_larb_probe(struct platform_device *pdev) pm_runtime_enable(dev); platform_set_drvdata(pdev, larb); - return component_add(dev, _smi_larb_component_ops); + ret = component_add(dev, _smi_larb_component_ops); + if (ret) + goto err_pm_disable; + return 0; + +err_pm_disable: + pm_runtime_disable(dev); + device_link_remove(dev, larb->smi_common_dev); + return ret; } static int mtk_smi_larb_remove(struct platform_device *pdev) -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 05/11] memory: mtk-smi: Adjust some code position
No functional change. Only move the code position to make the code more
readable.
1. Put the register smi-common above smi-larb. Prepare to add some others
register setting.
2. Put mtk_smi_larb_unbind around larb_bind.
3. Sort the SoC data alphabetically. and put them in one line as the
current kernel allow it.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 185 +++
1 file changed, 73 insertions(+), 112 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index ff07b14bcd66..6f8e582bace5 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -17,12 +17,15 @@
#include
#include
-/* mt8173 */
-#define SMI_LARB_MMU_EN0xf00
+/* SMI COMMON */
+#define SMI_BUS_SEL0x220
+#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
+/* All are MMU0 defaultly. Only specialize mmu1 here. */
+#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid))
-/* mt8167 */
-#define MT8167_SMI_LARB_MMU_EN 0xfc0
+/* SMI LARB */
+/* Below are about mmu enable registers, they are different in SoCs */
/* mt2701 */
#define REG_SMI_SECUR_CON_BASE 0x5c0
@@ -41,20 +44,20 @@
/* mt2701 domain should be set to 3 */
#define SMI_SECUR_CON_VAL_DOMAIN(id) (0x3 << id) & 0x7) << 2) + 1))
-/* mt2712 */
-#define SMI_LARB_NONSEC_CON(id)(0x380 + ((id) * 4))
-#define F_MMU_EN BIT(0)
-#define BANK_SEL(id) ({ \
+/* mt8167 */
+#define MT8167_SMI_LARB_MMU_EN 0xfc0
+
+/* mt8173 */
+#define MT8173_SMI_LARB_MMU_EN 0xf00
+
+/* larb gen2 */
+#define SMI_LARB_NONSEC_CON(id)(0x380 + ((id) * 4))
+#define F_MMU_EN BIT(0)
+#define BANK_SEL(id) ({ \
u32 _id = (id) & 0x3; \
(_id << 8 | _id << 10 | _id << 12 | _id << 14); \
})
-/* SMI COMMON */
-#define SMI_BUS_SEL0x220
-#define SMI_BUS_LARB_SHIFT(larbid) ((larbid) << 1)
-/* All are MMU0 defaultly. Only specialize mmu1 here. */
-#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid))
-
enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2
@@ -132,36 +135,16 @@ mtk_smi_larb_bind(struct device *dev, struct device
*master, void *data)
return -ENODEV;
}
-static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
-{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
- u32 reg;
- int i;
-
- if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
- return;
-
- for_each_set_bit(i, (unsigned long *)larb->mmu, 32) {
- reg = readl_relaxed(larb->base + SMI_LARB_NONSEC_CON(i));
- reg |= F_MMU_EN;
- reg |= BANK_SEL(larb->bank[i]);
- writel(reg, larb->base + SMI_LARB_NONSEC_CON(i));
- }
-}
-
-static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+static void
+mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-
- writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
+ /* Do nothing as the iommu is always enabled. */
}
-static void mtk_smi_larb_config_port_mt8167(struct device *dev)
-{
- struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-
- writel(*larb->mmu, larb->base + MT8167_SMI_LARB_MMU_EN);
-}
+static const struct component_ops mtk_smi_larb_component_ops = {
+ .bind = mtk_smi_larb_bind,
+ .unbind = mtk_smi_larb_unbind,
+};
static void mtk_smi_larb_config_port_gen1(struct device *dev)
{
@@ -194,26 +177,36 @@ static void mtk_smi_larb_config_port_gen1(struct device
*dev)
}
}
-static void
-mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
+static void mtk_smi_larb_config_port_mt8167(struct device *dev)
{
- /* Do nothing as the iommu is always enabled. */
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+
+ writel(*larb->mmu, larb->base + MT8167_SMI_LARB_MMU_EN);
}
-static const struct component_ops mtk_smi_larb_component_ops = {
- .bind = mtk_smi_larb_bind,
- .unbind = mtk_smi_larb_unbind,
-};
+static void mtk_smi_larb_config_port_mt8173(struct device *dev)
+{
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);
-static const struct mtk_smi_larb_gen mtk_smi_larb_mt8173 = {
- /* mt8173 do not need the port in larb */
- .config_port = mtk_smi_larb_config_port_mt8173,
-};
+ writel(*larb->mmu, larb->base + MT8173_SMI_LARB_MMU_EN);
+}
-static const struct mtk_smi_larb_gen mtk_smi_larb_mt8167 = {
- /* mt8167 do not need the port in larb */
- .config_port = mtk_smi_larb_config_port_mt8167,
-};
+static void mtk_smi_larb_config_port_gen2_general(struct device *dev)
+{
+ struct mtk_smi_larb *larb = dev_get_drvdata(dev);
+ u32 reg;
+ int
[PATCH v2 04/11] memory: mtk-smi: Rename smi_gen to smi_type
Prepare for adding smi sub common. Only rename from smi_gen to smi_type.
No functional change.
About the current "smi_gen", we have gen1/gen2 that stand for the
generation number for HW. I plan to add a new type(sub_common), then the
name "gen" is not prober.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index a2213452059d..ff07b14bcd66 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -55,7 +55,7 @@
/* All are MMU0 defaultly. Only specialize mmu1 here. */
#define F_MMU1_LARB(larbid)(0x1 << SMI_BUS_LARB_SHIFT(larbid))
-enum mtk_smi_gen {
+enum mtk_smi_type {
MTK_SMI_GEN1,
MTK_SMI_GEN2
};
@@ -68,8 +68,8 @@ static const char * const mtk_smi_common_clks_optional[] =
{"gals0", "gals1"};
static const char * const mtk_smi_larb_clks_optional[] = {"gals"};
struct mtk_smi_common_plat {
- enum mtk_smi_gen gen;
- u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
+ enum mtk_smi_type type;
+ u32 bus_sel; /* Balance some larbs to enter mmu0 or
mmu1 */
};
struct mtk_smi_larb_gen {
@@ -402,27 +402,27 @@ static struct platform_driver mtk_smi_larb_driver = {
};
static const struct mtk_smi_common_plat mtk_smi_common_gen1 = {
- .gen = MTK_SMI_GEN1,
+ .type = MTK_SMI_GEN1,
};
static const struct mtk_smi_common_plat mtk_smi_common_gen2 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
};
static const struct mtk_smi_common_plat mtk_smi_common_mt6779 = {
- .gen= MTK_SMI_GEN2,
- .bus_sel= F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(4) |
- F_MMU1_LARB(5) | F_MMU1_LARB(6) | F_MMU1_LARB(7),
+ .type = MTK_SMI_GEN2,
+ .bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(4) |
+ F_MMU1_LARB(5) | F_MMU1_LARB(6) | F_MMU1_LARB(7),
};
static const struct mtk_smi_common_plat mtk_smi_common_mt8183 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) |
F_MMU1_LARB(7),
};
static const struct mtk_smi_common_plat mtk_smi_common_mt8192 = {
- .gen = MTK_SMI_GEN2,
+ .type = MTK_SMI_GEN2,
.bus_sel = F_MMU1_LARB(1) | F_MMU1_LARB(2) | F_MMU1_LARB(5) |
F_MMU1_LARB(6),
};
@@ -483,7 +483,7 @@ static int mtk_smi_common_probe(struct platform_device
*pdev)
* clock into emi clock domain, but for mtk smi gen2, there's no smi ao
* base.
*/
- if (common->plat->gen == MTK_SMI_GEN1) {
+ if (common->plat->type == MTK_SMI_GEN1) {
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
common->smi_ao_base = devm_ioremap_resource(dev, res);
if (IS_ERR(common->smi_ao_base))
@@ -523,7 +523,7 @@ static int __maybe_unused mtk_smi_common_resume(struct
device *dev)
if (ret)
return ret;
- if (common->plat->gen == MTK_SMI_GEN2 && bus_sel)
+ if (common->plat->type == MTK_SMI_GEN2 && bus_sel)
writel(bus_sel, common->base + SMI_BUS_SEL);
return 0;
}
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 03/11] memory: mtk-smi: Use clk_bulk clock ops
Use clk_bulk interface instead of the orginal one to simplify the code.
SMI have several clocks: apb/smi/gals, the apb/smi clocks are required
for both smi-common and smi-larb while the gals clock are optional.
thus, use devm_clk_bulk_get for apb/smi and use
devm_clk_bulk_get_optional for gals.
For gals clocks, we already use get_optional for it, then the flag
"has_gals" is not helpful now, remove it.
Also remove clk fail logs since bulk interface already output fail log.
Signed-off-by: Yong Wu
---
drivers/memory/mtk-smi.c | 138 +--
1 file changed, 47 insertions(+), 91 deletions(-)
diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index c5fb51f73b34..a2213452059d 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -60,9 +60,15 @@ enum mtk_smi_gen {
MTK_SMI_GEN2
};
+#define MTK_SMI_CLK_NR_MAX 4
+
+/* Always require apb/smi clocks while gals clocks are optional. */
+static const char * const mtk_smi_clks_required[] = {"apb", "smi"};
+static const char * const mtk_smi_common_clks_optional[] = {"gals0", "gals1"};
+static const char * const mtk_smi_larb_clks_optional[] = {"gals"};
+
struct mtk_smi_common_plat {
enum mtk_smi_gen gen;
- bool has_gals;
u32 bus_sel; /* Balance some larbs to enter mmu0 or mmu1 */
};
@@ -70,13 +76,12 @@ struct mtk_smi_larb_gen {
int port_in_larb[MTK_LARB_NR_MAX + 1];
void (*config_port)(struct device *dev);
unsigned intlarb_direct_to_common_mask;
- boolhas_gals;
};
struct mtk_smi {
struct device *dev;
- struct clk *clk_apb, *clk_smi;
- struct clk *clk_gals0, *clk_gals1;
+ unsigned intclk_num;
+ struct clk_bulk_dataclks[MTK_SMI_CLK_NR_MAX];
struct clk *clk_async; /*only needed by mt2701*/
union {
void __iomem*smi_ao_base; /* only for gen1 */
@@ -95,45 +100,6 @@ struct mtk_smi_larb { /* larb: local arbiter */
unsigned char *bank;
};
-static int mtk_smi_clk_enable(const struct mtk_smi *smi)
-{
- int ret;
-
- ret = clk_prepare_enable(smi->clk_apb);
- if (ret)
- return ret;
-
- ret = clk_prepare_enable(smi->clk_smi);
- if (ret)
- goto err_disable_apb;
-
- ret = clk_prepare_enable(smi->clk_gals0);
- if (ret)
- goto err_disable_smi;
-
- ret = clk_prepare_enable(smi->clk_gals1);
- if (ret)
- goto err_disable_gals0;
-
- return 0;
-
-err_disable_gals0:
- clk_disable_unprepare(smi->clk_gals0);
-err_disable_smi:
- clk_disable_unprepare(smi->clk_smi);
-err_disable_apb:
- clk_disable_unprepare(smi->clk_apb);
- return ret;
-}
-
-static void mtk_smi_clk_disable(const struct mtk_smi *smi)
-{
- clk_disable_unprepare(smi->clk_gals1);
- clk_disable_unprepare(smi->clk_gals0);
- clk_disable_unprepare(smi->clk_smi);
- clk_disable_unprepare(smi->clk_apb);
-}
-
int mtk_smi_larb_get(struct device *larbdev)
{
int ret = pm_runtime_resume_and_get(larbdev);
@@ -270,7 +236,6 @@ static const struct mtk_smi_larb_gen mtk_smi_larb_mt6779 = {
};
static const struct mtk_smi_larb_gen mtk_smi_larb_mt8183 = {
- .has_gals = true,
.config_port= mtk_smi_larb_config_port_gen2_general,
.larb_direct_to_common_mask = BIT(2) | BIT(3) | BIT(7),
/* IPU0 | IPU1 | CCU */
@@ -312,6 +277,27 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = {
{}
};
+static int mtk_smi_dts_clk_init(struct device *dev, struct mtk_smi *smi,
+ unsigned int clk_nr_optional,
+ const char * const clk_optional[])
+{
+ int i, ret, clk_nr_required;
+
+ clk_nr_required = ARRAY_SIZE(mtk_smi_clks_required);
+ for (i = 0; i < clk_nr_required; i++)
+ smi->clks[i].id = mtk_smi_clks_required[i];
+ ret = devm_clk_bulk_get(dev, clk_nr_required, smi->clks);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < clk_nr_optional; i++)
+ smi->clks[i + clk_nr_required].id = clk_optional[i];
+ ret = devm_clk_bulk_get_optional(dev, clk_nr_optional,
+smi->clks + clk_nr_required);
+ smi->clk_num = clk_nr_required + clk_nr_optional;
+ return ret;
+}
+
static int mtk_smi_larb_probe(struct platform_device *pdev)
{
struct mtk_smi_larb *larb;
@@ -320,6 +306,7 @@ static int mtk_smi_larb_probe(struct platform_device *pdev)
struct device_node *smi_node;
struct platform_device *smi_pdev;
struct device_link *link;
+ int ret;
[PATCH v2 02/11] dt-bindings: memory: mediatek: Add mt8195 smi sub common
Add the binding for smi-sub-common. The SMI block diagram like this: IOMMU | | smi-common -- | | larb0 larb7 <-max is 8 The smi-common connects with smi-larb and IOMMU. The maximum larbs number that connects with a smi-common is 8. If the engines number is over 8, sometimes we use a smi-sub-common which is nearly same with smi-common. It supports up to 8 input and 1 output(smi-common has 2 output) Something like: IOMMU | | smi-common - | | ... larb0 sub-common ... <-max is 8 --- ||... <-max is 8 too. larb2 larb5 We don't need extra SW setting for smi-sub-common, only the sub-common has special clocks need to enable when the engines access dram. If it is sub-common, it should have a "mediatek,smi" phandle to point to its smi-common. meanwhile, the sub-common only has one gals clock. Signed-off-by: Yong Wu --- .../mediatek,smi-common.yaml | 25 +++ 1 file changed, 25 insertions(+) diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml index 602592b6c3f5..f79d99ebc440 100644 --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml @@ -38,6 +38,7 @@ properties: - mediatek,mt8192-smi-common - mediatek,mt8195-smi-common-vdo - mediatek,mt8195-smi-common-vpp + - mediatek,mt8195-smi-sub-common - description: for mt7623 items: @@ -67,6 +68,10 @@ properties: minItems: 2 maxItems: 4 + mediatek,smi: +$ref: /schemas/types.yaml#/definitions/phandle-array +description: a phandle to the smi-common node above. Only for sub-common. + required: - compatible - reg @@ -93,6 +98,26 @@ allOf: - const: smi - const: async + - if: # only for sub common + properties: +compatible: + contains: +enum: + - mediatek,mt8195-smi-sub-common +then: + required: +- mediatek,smi + properties: +clock: + items: +minItems: 3 +maxItems: 3 +clock-names: + items: +- const: apb +- const: smi +- const: gals0 + - if: # for gen2 HW that have gals properties: compatible: -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 01/11] dt-bindings: memory: mediatek: Add mt8195 smi binding
Add mt8195 smi supporting in the bindings. In mt8195, there are two smi-common HW, one is for vdo(video output), the other is for vpp(video processing pipe). They connect with different smi-larbs, then some setting(bus_sel) is different. Differentiate them with the compatible string. Something like this: IOMMU(VDO) IOMMU(VPP) | | SMI_COMMON_VDO SMI_COMMON_VPP | | ... | | ... larb0 larb2 ...larb1 larb3... Signed-off-by: Yong Wu --- .../bindings/memory-controllers/mediatek,smi-common.yaml| 6 +- .../bindings/memory-controllers/mediatek,smi-larb.yaml | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml index e87e4382807c..602592b6c3f5 100644 --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-common.yaml @@ -16,7 +16,7 @@ description: | MediaTek SMI have two generations of HW architecture, here is the list which generation the SoCs use: generation 1: mt2701 and mt7623. - generation 2: mt2712, mt6779, mt8167, mt8173, mt8183 and mt8192. + generation 2: mt2712, mt6779, mt8167, mt8173, mt8183, mt8192 and mt8195. There's slight differences between the two SMI, for generation 2, the register which control the iommu port is at each larb's register base. But @@ -36,6 +36,8 @@ properties: - mediatek,mt8173-smi-common - mediatek,mt8183-smi-common - mediatek,mt8192-smi-common + - mediatek,mt8195-smi-common-vdo + - mediatek,mt8195-smi-common-vpp - description: for mt7623 items: @@ -98,6 +100,8 @@ allOf: - mediatek,mt6779-smi-common - mediatek,mt8183-smi-common - mediatek,mt8192-smi-common +- mediatek,mt8195-smi-common-vdo +- mediatek,mt8195-smi-common-vpp then: properties: diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml index 2353f6cf3c80..eaeff1ada7f8 100644 --- a/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,smi-larb.yaml @@ -24,6 +24,7 @@ properties: - mediatek,mt8173-smi-larb - mediatek,mt8183-smi-larb - mediatek,mt8192-smi-larb + - mediatek,mt8195-smi-larb - description: for mt7623 items: @@ -74,6 +75,7 @@ allOf: compatible: enum: - mediatek,mt8183-smi-larb +- mediatek,mt8195-smi-larb then: properties: @@ -108,6 +110,7 @@ allOf: - mediatek,mt6779-smi-larb - mediatek,mt8167-smi-larb - mediatek,mt8192-smi-larb + - mediatek,mt8195-smi-larb then: required: -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 00/11] MT8195 SMI support
This patchset mainly adds SMI support for mt8195. Comparing with the previous version, add two new functions: a) add smi sub common b) add initial setting for smi-common and smi-larb. change note: v2: rebase on v5.14-rc1 1) Adjust clk_bulk flow: use devm_clk_bulk_get for necessary clocks. 2) Add two new little patches: a) use devm_platform_ioremap_resource b) Add error handle for smi_probe v1: https://lore.kernel.org/linux-mediatek/20210616114346.18812-1-yong...@mediatek.com/ Yong Wu (11): dt-bindings: memory: mediatek: Add mt8195 smi binding dt-bindings: memory: mediatek: Add mt8195 smi sub common memory: mtk-smi: Use clk_bulk instead of the clk ops memory: mtk-smi: Rename smi_gen to smi_type memory: mtk-smi: Adjust some code position memory: mtk-smi: Add error handle for smi_probe memory: mtk-smi: Add smi sub common support memory: mtk-smi: Use devm_platform_ioremap_resource memory: mtk-smi: mt8195: Add smi support memory: mtk-smi: mt8195: Add initial setting for smi-common memory: mtk-smi: mt8195: Add initial setting for smi-larb .../mediatek,smi-common.yaml | 31 +- .../memory-controllers/mediatek,smi-larb.yaml | 3 + drivers/memory/mtk-smi.c | 572 ++ 3 files changed, 356 insertions(+), 250 deletions(-) -- 2.18.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v7 10/15] iommu/io-pgtable-arm: Implement arm_lpae_unmap_pages()
On 2021/6/16 21:38, Georgi Djakov wrote:
From: "Isaac J. Manjarres"
Implement the unmap_pages() callback for the ARM LPAE io-pgtable
format.
Signed-off-by: Isaac J. Manjarres
Suggested-by: Will Deacon
Signed-off-by: Georgi Djakov
---
drivers/iommu/io-pgtable-arm.c | 120 +
1 file changed, 74 insertions(+), 46 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index ea66b10c04c4..fe8fa0ee9c98 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -46,6 +46,9 @@
#define ARM_LPAE_PGD_SIZE(d) \
(sizeof(arm_lpae_iopte) << (d)->pgd_bits)
+#define ARM_LPAE_PTES_PER_TABLE(d) \
+ (ARM_LPAE_GRANULE(d) >> ilog2(sizeof(arm_lpae_iopte)))
+
/*
* Calculate the index at level l used to map virtual address a using the
* pagetable in d.
@@ -239,22 +242,19 @@ static void __arm_lpae_sync_pte(arm_lpae_iopte *ptep, int
num_entries,
sizeof(*ptep) * num_entries, DMA_TO_DEVICE);
}
-static void __arm_lpae_set_pte(arm_lpae_iopte *ptep, arm_lpae_iopte pte,
- int num_entries, struct io_pgtable_cfg *cfg)
+static void __arm_lpae_clear_pte(arm_lpae_iopte *ptep, struct io_pgtable_cfg
*cfg)
{
- int i;
- for (i = 0; i < num_entries; i++)
- ptep[i] = pte;
+ *ptep = 0;
if (!cfg->coherent_walk)
- __arm_lpae_sync_pte(ptep, num_entries, cfg);
+ __arm_lpae_sync_pte(ptep, 1, cfg);
}
Thank you for providing this patchset, I am updating my patches based on it.
But can we keep __arm_lpae_set_pte()? I think it's better to remove
'num_entries'.
I am really need it. If you remove it, I have to add it back.
Thanks,
Kunkun Jiang
static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
struct iommu_iotlb_gather *gather,
- unsigned long iova, size_t size, int lvl,
- arm_lpae_iopte *ptep);
+ unsigned long iova, size_t size, size_t pgcount,
+ int lvl, arm_lpae_iopte *ptep);
static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
phys_addr_t paddr, arm_lpae_iopte prot,
@@ -298,7 +298,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable
*data,
size_t sz = ARM_LPAE_BLOCK_SIZE(lvl, data);
tblp = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data);
- if (__arm_lpae_unmap(data, NULL, iova + i * sz, sz,
+ if (__arm_lpae_unmap(data, NULL, iova + i * sz, sz, 1,
lvl, tblp) != sz) {
WARN_ON(1);
return -EINVAL;
@@ -526,14 +526,15 @@ static size_t arm_lpae_split_blk_unmap(struct
arm_lpae_io_pgtable *data,
struct iommu_iotlb_gather *gather,
unsigned long iova, size_t size,
arm_lpae_iopte blk_pte, int lvl,
- arm_lpae_iopte *ptep)
+ arm_lpae_iopte *ptep, size_t pgcount)
{
struct io_pgtable_cfg *cfg = >iop.cfg;
arm_lpae_iopte pte, *tablep;
phys_addr_t blk_paddr;
size_t tablesz = ARM_LPAE_GRANULE(data);
size_t split_sz = ARM_LPAE_BLOCK_SIZE(lvl, data);
- int i, unmap_idx = -1;
+ int ptes_per_table = ARM_LPAE_PTES_PER_TABLE(data);
+ int i, unmap_idx_start = -1, num_entries = 0, max_entries;
if (WARN_ON(lvl == ARM_LPAE_MAX_LEVELS))
return 0;
@@ -542,15 +543,18 @@ static size_t arm_lpae_split_blk_unmap(struct
arm_lpae_io_pgtable *data,
if (!tablep)
return 0; /* Bytes unmapped */
- if (size == split_sz)
- unmap_idx = ARM_LPAE_LVL_IDX(iova, lvl, data);
+ if (size == split_sz) {
+ unmap_idx_start = ARM_LPAE_LVL_IDX(iova, lvl, data);
+ max_entries = ptes_per_table - unmap_idx_start;
+ num_entries = min_t(int, pgcount, max_entries);
+ }
blk_paddr = iopte_to_paddr(blk_pte, data);
pte = iopte_prot(blk_pte);
- for (i = 0; i < tablesz / sizeof(pte); i++, blk_paddr += split_sz) {
+ for (i = 0; i < ptes_per_table; i++, blk_paddr += split_sz) {
/* Unmap! */
- if (i == unmap_idx)
+ if (i >= unmap_idx_start && i < (unmap_idx_start + num_entries))
continue;
__arm_lpae_init_pte(data, blk_paddr, pte, lvl, 1, [i]);
@@ -568,76 +572,92 @@ static size_t arm_lpae_split_blk_unmap(struct
arm_lpae_io_pgtable *data,
return 0;
tablep = iopte_deref(pte, data);
- } else if (unmap_idx >=
Re: [PATCH] iommu: Unify iova_to_phys for identity domains
On 2021-07-15 02:38, Lu Baolu wrote:
On 7/15/21 1:28 AM, Robin Murphy wrote:
If people are going to insist on calling iommu_iova_to_phys()
pointlessly and expecting it to work, we can at least do ourselves a
favour by handling those cases in the core code, rather than repeatedly
across an inconsistent handful of drivers.
Signed-off-by: Robin Murphy
---
drivers/iommu/amd/io_pgtable.c | 3 ---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 3 ---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 3 ---
drivers/iommu/iommu.c | 6 +-
4 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/amd/io_pgtable.c
b/drivers/iommu/amd/io_pgtable.c
index bb0ee5c9fde7..182c93a43efd 100644
--- a/drivers/iommu/amd/io_pgtable.c
+++ b/drivers/iommu/amd/io_pgtable.c
@@ -493,9 +493,6 @@ static phys_addr_t iommu_v1_iova_to_phys(struct
io_pgtable_ops *ops, unsigned lo
unsigned long offset_mask, pte_pgsize;
u64 *pte, __pte;
- if (pgtable->mode == PAGE_MODE_NONE)
- return iova;
-
pte = fetch_pte(pgtable, iova, _pgsize);
if (!pte || !IOMMU_PTE_PRESENT(*pte))
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 3e87a9cf6da3..c9fdd0d097be 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2481,9 +2481,6 @@ arm_smmu_iova_to_phys(struct iommu_domain
*domain, dma_addr_t iova)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0f181f76c31b..0d04eafa3fdb 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1317,9 +1317,6 @@ static phys_addr_t arm_smmu_iova_to_phys(struct
iommu_domain *domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
- if (domain->type == IOMMU_DOMAIN_IDENTITY)
- return iova;
-
if (!ops)
return 0;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 43a2041d9629..7c16f977b5a6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2371,7 +2371,11 @@ EXPORT_SYMBOL_GPL(iommu_detach_group);
phys_addr_t iommu_iova_to_phys(struct iommu_domain *domain,
dma_addr_t iova)
{
- if (unlikely(domain->ops->iova_to_phys == NULL))
+ if (domain->type == IOMMU_DOMAIN_IDENTITY)
+ return iova;
+
+ if (unlikely(domain->ops->iova_to_phys == NULL) ||
+ domain->type == IOMMU_DOMAIN_BLOCKED)
return 0;
Nit:
Logically we only needs ops->iova_to_phys for DMA and UNMANAGED domains,
so it looks better if we have
if (domain->type == IOMMU_DOMAIN_BLOCKED ||
unlikely(domain->ops->iova_to_phys == NULL))
return 0;
Yeah, I put IOMMU_DOMAIN_BLOCKED last as the least likely condition
since it's really just for completeness - I don't think it's possible to
see it legitimately used at the moment - but on second look I think
ops->iova_to_phys == NULL is equally theoretical now, so maybe I could
be removing that and we just make it mandatory for any new drivers?
Anyway,
Reviewed-by: Lu Baolu
Thanks!
Robin.
Best regards,
baolu
return domain->ops->iova_to_phys(domain, iova);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On 2021/7/15 14:49, Tian, Kevin wrote:
>> From: Shenming Lu
>> Sent: Thursday, July 15, 2021 2:29 PM
>>
>> On 2021/7/15 11:55, Tian, Kevin wrote:
From: Shenming Lu
Sent: Thursday, July 15, 2021 11:21 AM
On 2021/7/9 15:48, Tian, Kevin wrote:
> 4.6. I/O page fault
> +++
>
> uAPI is TBD. Here is just about the high-level flow from host IOMMU
>> driver
> to guest IOMMU driver and backwards. This flow assumes that I/O page
faults
> are reported via IOMMU interrupts. Some devices report faults via
>> device
> specific way instead of going through the IOMMU. That usage is not
covered
> here:
>
> - Host IOMMU driver receives a I/O page fault with raw fault_data {rid,
> pasid, addr};
>
> - Host IOMMU driver identifies the faulting I/O page table according to
> {rid, pasid} and calls the corresponding fault handler with an opaque
> object (registered by the handler) and raw fault_data (rid, pasid,
> addr);
>
> - IOASID fault handler identifies the corresponding ioasid and device
> cookie according to the opaque object, generates an user fault_data
> (ioasid, cookie, addr) in the fault region, and triggers eventfd to
> userspace;
>
Hi, I have some doubts here:
For mdev, it seems that the rid in the raw fault_data is the parent
device's,
then in the vSVA scenario, how can we get to know the mdev(cookie) from
the
rid and pasid?
And from this point of view，would it be better to register the mdev
(iommu_register_device()) with the parent device info?
>>>
>>> This is what is proposed in this RFC. A successful binding generates a new
>>> iommu_dev object for each vfio device. For mdev this object includes
>>> its parent device, the defPASID marking this mdev, and the cookie
>>> representing it in userspace. Later it is iommu_dev being recorded in
>>> the attaching_data when the mdev is attached to an IOASID:
>>>
>>> struct iommu_attach_data *__iommu_device_attach(
>>> struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags);
>>>
>>> Then when a fault is reported, the fault handler just needs to figure out
>>> iommu_dev according to {rid, pasid} in the raw fault data.
>>>
>>
>> Yeah, we have the defPASID that marks the mdev and refers to the default
>> I/O address space, but how about the non-default I/O address spaces?
>> Is there a case that two different mdevs (on the same parent device)
>> are used by the same process in the guest, thus have a same pasid route
>> in the physical IOMMU? It seems that we can't figure out the mdev from
>> the rid and pasid in this case...
>>
>> Did I misunderstand something?... :-)
>>
>
> No. You are right on this case. I don't think there is a way to
> differentiate one mdev from the other if they come from the
> same parent and attached by the same guest process. In this
> case the fault could be reported on either mdev (e.g. the first
> matching one) to get it fixed in the guest.
>
OK. Thanks,
Shenming
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[git pull] IOMMU Fixes for Linux v5.14-rc1
Hi Linus, The following changes since commit e73f0f0ee7541171d89f2e2491130c7771ba58d3: Linux 5.14-rc1 (2021-07-11 15:07:40 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.14-rc1 for you to fetch changes up to 4a5c155a5ab372516a1a5ddd29473f8f696feb79: MAINTAINERS: Add Suravee Suthikulpanit as Reviewer for AMD IOMMU (AMD-Vi) (2021-07-15 09:00:16 +0200) IOMMU Fixes for Linux v5.14-rc1 Including: - Revert a patch which caused boot failures with QCOM IOMMU - Two fixes for Intel VT-d context table handling - Physical address decoding fix for Rockchip IOMMU - Add a reviewer for AMD IOMMU Benjamin Gaignard (1): iommu/rockchip: Fix physical address decoding Lu Baolu (1): iommu/vt-d: Fix clearing real DMA device's scalable-mode context entries Marek Szyprowski (1): iommu/qcom: Revert "iommu/arm: Cleanup resources in case of probe error path" Sanjay Kumar (1): iommu/vt-d: Global devTLB flush when present context entry changed Suravee Suthikulpanit (1): MAINTAINERS: Add Suravee Suthikulpanit as Reviewer for AMD IOMMU (AMD-Vi) MAINTAINERS | 1 + drivers/iommu/arm/arm-smmu/qcom_iommu.c | 13 ++--- drivers/iommu/intel/iommu.c | 34 ++--- drivers/iommu/rockchip-iommu.c | 6 -- 4 files changed, 30 insertions(+), 24 deletions(-) Please pull. Thanks, Joerg signature.asc Description: Digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses
> This looks like it wasn't picked up? Should it go in rc1?
Hi, Konrad!
This looks like an unambiguous bug, and should be in rc1.
Cheers!
ср, 14 июл. 2021 г. в 03:15, Konrad Rzeszutek Wilk :
>
> On Tue, Jun 22, 2021 at 04:34:14PM +0300, Roman Skakun wrote:
> > This commit is dedicated to fix incorrect conversion from
> > cpu_addr to page address in cases when we get virtual
> > address which allocated in the vmalloc range.
> > As the result, virt_to_page() cannot convert this address
> > properly and return incorrect page address.
> >
> > Need to detect such cases and obtains the page address using
> > vmalloc_to_page() instead.
> >
> > Signed-off-by: Roman Skakun
> > Reviewed-by: Andrii Anisov
> > ---
> > Hey!
> > Thanks for suggestions, Christoph!
> > I updated the patch according to your advice.
> > But, I'm so surprised because nobody catches this problem
> > in the common code before. It looks a bit strange as for me.
>
> This looks like it wasn't picked up? Should it go in rc1?
> >
> >
> > kernel/dma/ops_helpers.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
> > index 910ae69cae77..782728d8a393 100644
> > --- a/kernel/dma/ops_helpers.c
> > +++ b/kernel/dma/ops_helpers.c
> > @@ -5,6 +5,14 @@
> > */
> > #include
> >
> > +static struct page *cpu_addr_to_page(void *cpu_addr)
> > +{
> > + if (is_vmalloc_addr(cpu_addr))
> > + return vmalloc_to_page(cpu_addr);
> > + else
> > + return virt_to_page(cpu_addr);
> > +}
> > +
> > /*
> > * Create scatter-list for the already allocated DMA buffer.
> > */
> > @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct
> > sg_table *sgt,
> >void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >unsigned long attrs)
> > {
> > - struct page *page = virt_to_page(cpu_addr);
> > + struct page *page = cpu_addr_to_page(cpu_addr);
> > int ret;
> >
> > ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > return -ENXIO;
> >
> > return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
> > + page_to_pfn(cpu_addr_to_page(cpu_addr)) +
> > vma->vm_pgoff,
> > user_count << PAGE_SHIFT, vma->vm_page_prot);
> > #else
> > return -ENXIO;
> > --
> > 2.25.1
> >
--
Best Regards, Roman.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: check if group is NULL before remove device
From: Frank Wunderlich
if probe is failing, iommu_group may be not initialized,
so freeing it will result in NULL pointer access
Fixes: d72e31c93746 ("iommu: IOMMU Groups")
Signed-off-by: Frank Wunderlich
---
drivers/iommu/iommu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..63f0af10c403 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -924,6 +924,9 @@ void iommu_group_remove_device(struct device *dev)
struct iommu_group *group = dev->iommu_group;
struct group_device *tmp_device, *device = NULL;
+ if (!group)
+ return;
+
dev_info(dev, "Removing from iommu group %d\n", group->id);
/* Pre-notify listeners that a device is being removed. */
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: check if group is NULL before remove device
From: Frank Wunderlich
if probe is failing, iommu_group may be not initialized,
so freeing it will result in NULL pointer access
Fixes: d72e31c93746 ("iommu: IOMMU Groups")
Signed-off-by: Frank Wunderlich
---
drivers/iommu/iommu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..63f0af10c403 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -924,6 +924,9 @@ void iommu_group_remove_device(struct device *dev)
struct iommu_group *group = dev->iommu_group;
struct group_device *tmp_device, *device = NULL;
+ if (!group)
+ return;
+
dev_info(dev, "Removing from iommu group %d\n", group->id);
/* Pre-notify listeners that a device is being removed. */
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] dma-mapping: use vmalloc_to_page for vmalloc addresses
Hi, Stefano!
> We have the same thing in xen_swiotlb_free_coherent. Can we find a way
> to call cpu_addr_to_page() from xen_swiotlb_free_coherent?
> Maybe we can make cpu_addr_to_page non-static?
Yes, right, If we want to reuse this helper instead of the same code
block in xen_swiotlb_free_coherent() need to make cpu_addr_to_page() as
global and add a new declaration for this helper in include/linux/dma-map-ops.h.
What do you think?
Cheers!
ср, 14 июл. 2021 г. в 04:23, Stefano Stabellini :
>
> On Tue, 22 Jun 2021, Roman Skakun wrote:
> > This commit is dedicated to fix incorrect conversion from
> > cpu_addr to page address in cases when we get virtual
> > address which allocated in the vmalloc range.
> > As the result, virt_to_page() cannot convert this address
> > properly and return incorrect page address.
> >
> > Need to detect such cases and obtains the page address using
> > vmalloc_to_page() instead.
> >
> > Signed-off-by: Roman Skakun
> > Reviewed-by: Andrii Anisov
> > ---
> > Hey!
> > Thanks for suggestions, Christoph!
> > I updated the patch according to your advice.
> > But, I'm so surprised because nobody catches this problem
> > in the common code before. It looks a bit strange as for me.
> >
> >
> > kernel/dma/ops_helpers.c | 12 ++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/dma/ops_helpers.c b/kernel/dma/ops_helpers.c
> > index 910ae69cae77..782728d8a393 100644
> > --- a/kernel/dma/ops_helpers.c
> > +++ b/kernel/dma/ops_helpers.c
> > @@ -5,6 +5,14 @@
> > */
> > #include
> >
> > +static struct page *cpu_addr_to_page(void *cpu_addr)
> > +{
> > + if (is_vmalloc_addr(cpu_addr))
> > + return vmalloc_to_page(cpu_addr);
> > + else
> > + return virt_to_page(cpu_addr);
> > +}
>
> We have the same thing in xen_swiotlb_free_coherent. Can we find a way
> to call cpu_addr_to_page() from xen_swiotlb_free_coherent? Maybe we can
> make cpu_addr_to_page non-static? No problem if it is too much trouble.
>
>
> > /*
> > * Create scatter-list for the already allocated DMA buffer.
> > */
> > @@ -12,7 +20,7 @@ int dma_common_get_sgtable(struct device *dev, struct
> > sg_table *sgt,
> >void *cpu_addr, dma_addr_t dma_addr, size_t size,
> >unsigned long attrs)
> > {
> > - struct page *page = virt_to_page(cpu_addr);
> > + struct page *page = cpu_addr_to_page(cpu_addr);
> > int ret;
> >
> > ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> > @@ -43,7 +51,7 @@ int dma_common_mmap(struct device *dev, struct
> > vm_area_struct *vma,
> > return -ENXIO;
> >
> > return remap_pfn_range(vma, vma->vm_start,
> > - page_to_pfn(virt_to_page(cpu_addr)) + vma->vm_pgoff,
> > + page_to_pfn(cpu_addr_to_page(cpu_addr)) +
> > vma->vm_pgoff,
> > user_count << PAGE_SHIFT, vma->vm_page_prot);
> > #else
> > return -ENXIO;
> > --
> > 2.25.1
> >
--
Best Regards, Roman.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 17/17] Documentation: Add documentation for VDUSE
On Thu, Jul 15, 2021 at 1:18 PM Jason Wang wrote:
>
>
> 在 2021/7/13 下午4:46, Xie Yongji 写道:
> > VDUSE (vDPA Device in Userspace) is a framework to support
> > implementing software-emulated vDPA devices in userspace. This
> > document is intended to clarify the VDUSE design and usage.
> >
> > Signed-off-by: Xie Yongji
> > ---
> > Documentation/userspace-api/index.rst | 1 +
> > Documentation/userspace-api/vduse.rst | 248
> > ++
> > 2 files changed, 249 insertions(+)
> > create mode 100644 Documentation/userspace-api/vduse.rst
> >
> > diff --git a/Documentation/userspace-api/index.rst
> > b/Documentation/userspace-api/index.rst
> > index 0b5eefed027e..c432be070f67 100644
> > --- a/Documentation/userspace-api/index.rst
> > +++ b/Documentation/userspace-api/index.rst
> > @@ -27,6 +27,7 @@ place where this information is gathered.
> > iommu
> > media/index
> > sysfs-platform_profile
> > + vduse
> >
> > .. only:: subproject and html
> >
> > diff --git a/Documentation/userspace-api/vduse.rst
> > b/Documentation/userspace-api/vduse.rst
> > new file mode 100644
> > index ..2c0d56d4b2da
> > --- /dev/null
> > +++ b/Documentation/userspace-api/vduse.rst
> > @@ -0,0 +1,248 @@
> > +==
> > +VDUSE - "vDPA Device in Userspace"
> > +==
> > +
> > +vDPA (virtio data path acceleration) device is a device that uses a
> > +datapath which complies with the virtio specifications with vendor
> > +specific control path. vDPA devices can be both physically located on
> > +the hardware or emulated by software. VDUSE is a framework that makes it
> > +possible to implement software-emulated vDPA devices in userspace. And
> > +to make the device emulation more secure, the emulated vDPA device's
> > +control path is handled in the kernel and only the data path is
> > +implemented in the userspace.
> > +
> > +Note that only virtio block device is supported by VDUSE framework now,
> > +which can reduce security risks when the userspace process that implements
> > +the data path is run by an unprivileged user. The support for other device
> > +types can be added after the security issue of corresponding device driver
> > +is clarified or fixed in the future.
> > +
> > +Start/Stop VDUSE devices
> > +
> > +
> > +VDUSE devices are started as follows:
>
>
> Not native speaker but "created" is probably better.
>
How about using "added"?
>
> > +
> > +1. Create a new VDUSE instance with ioctl(VDUSE_CREATE_DEV) on
> > + /dev/vduse/control.
> > +
> > +2. Setup each virtqueue with ioctl(VDUSE_VQ_SETUP) on /dev/vduse/$NAME.
> > +
> > +3. Begin processing VDUSE messages from /dev/vduse/$NAME. The first
> > + messages will arrive while attaching the VDUSE instance to vDPA bus.
> > +
> > +4. Send the VDPA_CMD_DEV_NEW netlink message to attach the VDUSE
> > + instance to vDPA bus.
>
>
> I think 4 should be done before 3?
>
VDPA_CMD_DEV_NEW netlink message should be done after userspace
listens to /dev/vduse/$NAME. Otherwise, the messages would be hung.
>
> > +
> > +VDUSE devices are stopped as follows:
>
>
> "removed" or "destroyed" is better than "stopped" here.
>
"removed" looks better?
>
> > +
> > +1. Send the VDPA_CMD_DEV_DEL netlink message to detach the VDUSE
> > + instance from vDPA bus.
> > +
> > +2. Close the file descriptor referring to /dev/vduse/$NAME.
> > +
> > +3. Destroy the VDUSE instance with ioctl(VDUSE_DESTROY_DEV) on
> > + /dev/vduse/control.
> > +
> > +The netlink messages can be sent via vdpa tool in iproute2 or use the
> > +below sample codes:
> > +
> > +.. code-block:: c
> > +
> > + static int netlink_add_vduse(const char *name, enum vdpa_command cmd)
> > + {
> > + struct nl_sock *nlsock;
> > + struct nl_msg *msg;
> > + int famid;
> > +
> > + nlsock = nl_socket_alloc();
> > + if (!nlsock)
> > + return -ENOMEM;
> > +
> > + if (genl_connect(nlsock))
> > + goto free_sock;
> > +
> > + famid = genl_ctrl_resolve(nlsock, VDPA_GENL_NAME);
> > + if (famid < 0)
> > + goto close_sock;
> > +
> > + msg = nlmsg_alloc();
> > + if (!msg)
> > + goto close_sock;
> > +
> > + if (!genlmsg_put(msg, NL_AUTO_PORT, NL_AUTO_SEQ, famid, 0, 0,
> > cmd, 0))
> > + goto nla_put_failure;
> > +
> > + NLA_PUT_STRING(msg, VDPA_ATTR_DEV_NAME, name);
> > + if (cmd == VDPA_CMD_DEV_NEW)
> > + NLA_PUT_STRING(msg, VDPA_ATTR_MGMTDEV_DEV_NAME,
> > "vduse");
> > +
> > + if (nl_send_sync(nlsock, msg))
> > + goto close_sock;
> > +
> > + nl_close(nlsock);
> > + nl_socket_free(nlsock);
> > +
> > + return 0;
> > + nla_put_failure:
> > +
Re: [PATCH] iommu: check if group is NULL before remove device
On Thu, Jul 15, 2021 at 09:11:50AM +0200, Frank Wunderlich wrote: > From: Frank Wunderlich > > if probe is failing, iommu_group may be not initialized, Sentences start with capital letters. IOMMU patch subjects too, after the 'iommu:' prefix. > so freeing it will result in NULL pointer access Please describe in more detail how this NULL-ptr dereference is triggered. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] MAINTAINERS: Add Suravee Suthikulpanit as Reviewer for AMD IOMMU (AMD-Vi)
On Thu, Jul 15, 2021 at 04:02:22AM +0700, Suravee Suthikulpanit wrote: > To help review changes related to AMD IOMMU. > > Signed-off-by: Suravee Suthikulpanit > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) Applied, thanks Suravee. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 0/3] Apple M1 DART IOMMU driver
On Wed, Jul 14, 2021 at 10:51:34PM +0200, Arnd Bergmann wrote: > The question is how we can best allow one but not the other. By only allowing to allocate domains of type IDENTITY and DMA, but fail to allocate UNMANAGED domains. Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v2] /dev/iommu uAPI proposal
> From: Shenming Lu
> Sent: Thursday, July 15, 2021 2:29 PM
>
> On 2021/7/15 11:55, Tian, Kevin wrote:
> >> From: Shenming Lu
> >> Sent: Thursday, July 15, 2021 11:21 AM
> >>
> >> On 2021/7/9 15:48, Tian, Kevin wrote:
> >>> 4.6. I/O page fault
> >>> +++
> >>>
> >>> uAPI is TBD. Here is just about the high-level flow from host IOMMU
> driver
> >>> to guest IOMMU driver and backwards. This flow assumes that I/O page
> >> faults
> >>> are reported via IOMMU interrupts. Some devices report faults via
> device
> >>> specific way instead of going through the IOMMU. That usage is not
> >> covered
> >>> here:
> >>>
> >>> - Host IOMMU driver receives a I/O page fault with raw fault_data {rid,
> >>> pasid, addr};
> >>>
> >>> - Host IOMMU driver identifies the faulting I/O page table according to
> >>> {rid, pasid} and calls the corresponding fault handler with an opaque
> >>> object (registered by the handler) and raw fault_data (rid, pasid,
> >>> addr);
> >>>
> >>> - IOASID fault handler identifies the corresponding ioasid and device
> >>> cookie according to the opaque object, generates an user fault_data
> >>> (ioasid, cookie, addr) in the fault region, and triggers eventfd to
> >>> userspace;
> >>>
> >>
> >> Hi, I have some doubts here:
> >>
> >> For mdev, it seems that the rid in the raw fault_data is the parent
> >> device's,
> >> then in the vSVA scenario, how can we get to know the mdev(cookie) from
> >> the
> >> rid and pasid?
> >>
> >> And from this point of view，would it be better to register the mdev
> >> (iommu_register_device()) with the parent device info?
> >>
> >
> > This is what is proposed in this RFC. A successful binding generates a new
> > iommu_dev object for each vfio device. For mdev this object includes
> > its parent device, the defPASID marking this mdev, and the cookie
> > representing it in userspace. Later it is iommu_dev being recorded in
> > the attaching_data when the mdev is attached to an IOASID:
> >
> > struct iommu_attach_data *__iommu_device_attach(
> > struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags);
> >
> > Then when a fault is reported, the fault handler just needs to figure out
> > iommu_dev according to {rid, pasid} in the raw fault data.
> >
>
> Yeah, we have the defPASID that marks the mdev and refers to the default
> I/O address space, but how about the non-default I/O address spaces?
> Is there a case that two different mdevs (on the same parent device)
> are used by the same process in the guest, thus have a same pasid route
> in the physical IOMMU? It seems that we can't figure out the mdev from
> the rid and pasid in this case...
>
> Did I misunderstand something?... :-)
>
No. You are right on this case. I don't think there is a way to
differentiate one mdev from the other if they come from the
same parent and attached by the same guest process. In this
case the fault could be reported on either mdev (e.g. the first
matching one) to get it fixed in the guest.
Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v2] /dev/iommu uAPI proposal
On 2021/7/15 11:55, Tian, Kevin wrote:
>> From: Shenming Lu
>> Sent: Thursday, July 15, 2021 11:21 AM
>>
>> On 2021/7/9 15:48, Tian, Kevin wrote:
>>> 4.6. I/O page fault
>>> +++
>>>
>>> uAPI is TBD. Here is just about the high-level flow from host IOMMU driver
>>> to guest IOMMU driver and backwards. This flow assumes that I/O page
>> faults
>>> are reported via IOMMU interrupts. Some devices report faults via device
>>> specific way instead of going through the IOMMU. That usage is not
>> covered
>>> here:
>>>
>>> - Host IOMMU driver receives a I/O page fault with raw fault_data {rid,
>>> pasid, addr};
>>>
>>> - Host IOMMU driver identifies the faulting I/O page table according to
>>> {rid, pasid} and calls the corresponding fault handler with an opaque
>>> object (registered by the handler) and raw fault_data (rid, pasid,
>>> addr);
>>>
>>> - IOASID fault handler identifies the corresponding ioasid and device
>>> cookie according to the opaque object, generates an user fault_data
>>> (ioasid, cookie, addr) in the fault region, and triggers eventfd to
>>> userspace;
>>>
>>
>> Hi, I have some doubts here:
>>
>> For mdev, it seems that the rid in the raw fault_data is the parent device's,
>> then in the vSVA scenario, how can we get to know the mdev(cookie) from
>> the
>> rid and pasid?
>>
>> And from this point of view，would it be better to register the mdev
>> (iommu_register_device()) with the parent device info?
>>
>
> This is what is proposed in this RFC. A successful binding generates a new
> iommu_dev object for each vfio device. For mdev this object includes
> its parent device, the defPASID marking this mdev, and the cookie
> representing it in userspace. Later it is iommu_dev being recorded in
> the attaching_data when the mdev is attached to an IOASID:
>
> struct iommu_attach_data *__iommu_device_attach(
> struct iommu_dev *dev, u32 ioasid, u32 pasid, int flags);
>
> Then when a fault is reported, the fault handler just needs to figure out
> iommu_dev according to {rid, pasid} in the raw fault data.
>
Yeah, we have the defPASID that marks the mdev and refers to the default
I/O address space, but how about the non-default I/O address spaces?
Is there a case that two different mdevs (on the same parent device)
are used by the same process in the guest, thus have a same pasid route
in the physical IOMMU? It seems that we can't figure out the mdev from
the rid and pasid in this case...
Did I misunderstand something?... :-)
Thanks,
Shenming
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
