Re: [GIT PULL] dma-mapping fix for 4.20
The pull request you sent on Wed, 19 Dec 2018 15:24:19 +0100: > git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.20-4 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2dd516ff7d852c2cda8c5b883d6625d1c812714e Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 0/7] Add virtio-iommu driver
On Thu, Dec 13, 2018 at 12:50:29PM +, Jean-Philippe Brucker wrote: > >> [3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.1 > >> git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9 > > > > Unfortunatly gitweb seems to be broken on linux-arm.org. What is missing > > in this patch-set to make this work on x86? > > You should be able to access it here: > http://www.linux-arm.org/git?p=linux-jpb.git;a=shortlog;h=refs/heads/virtio-iommu/devel > > That branch contains missing bits for x86 support: > > * ACPI support. We have the code but it's waiting for an IORT spec > update, to reserve the IORT node ID. I expect it to take a while, given > that I'm alone requesting a change for something that's not upstream or > in hardware. Frankly I think you should take a hard look at just getting the data needed from the PCI device itself. You don't need to depend on virtio, it can be a small driver that gets you that data from the device config space and then just goes away. If you want help with writing such a small driver let me know. If there's an advantage to virtio-iommu then that would be its portability, and it all goes out of the window because of dependencies on ACPI and DT and OF and the rest of the zoo. > * DMA ops for x86 (see "HACK" commit). I'd like to use dma-iommu but I'm > not sure how to implement the glue that sets dma_ops properly. > > Thanks, > Jean OK so IIUC you are looking into Christoph's suggestions to fix that up? There's still a bit of time left before the merge window, maybe you can make above changes. -- MST ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v3 2/6] iommu/arm-smmu: Add support to program multiple ARM SMMU's identically
Hi Robin, Thanks for the feedback :) >The whole point of the library idea was to factor out the code in such a way >that all the details >specific to a particular implementation can be kept together. But what this >patch does is insert >Tegra194-specific handling all through the 'common' code, which is the exact >opposite of that concept >and just makes more hard-to-maintain mess. In an attempt to reuse most of the ARM SMMU implementation, which heavily relies on data from arm_smmu_device, The library code has been added with some functionality only usable by Tegra194 SMMU driver. In V4 patches, I am working on to add a mechanism to override writel/readl functions in library so that Tegra smmu driver can override read/write functions and handle programming of multiple instances on its own. >The amount of copy-paste duplication in patch #4 has the opposite problem - >about 95% of that isn't >Tegra194-specific at all (I mean, how many fsl_mc instances does it have?), >and having multiple copies of > generic code with the potential to diverge is also not what anyone wants. I have split the code in a way that library only contains the code that deals with register programming. And avoided platform driver code and DT parsing code getting into library, which can allow drivers changing Independently if necessary in future. > Plus I don't think ending up building > multiple separate drivers will even work in general - thanks to the current > state of >bus_set_iommu() etc., you can't use the regular driver for your third SMMU at >the same time. Good point! >From code, platform_dma_configure/of_dma_configure/of_iommu_configure takes >care of setting right iommu_ops for devices based on the iommu DT node they have in iommus=<> entry. If iommu.c is updated to use dev->bus->dma_configure(), then it doesn't really need to use dev->bus->iommu_ops. dev->bus->dma_configure() can be used to set dev->dma_ops to the right one, if dev->dma_ops is not already set. If this approach looks good, I can make a patch to clean up bus->iommu_ops usage related code to allow devices to use specific SMMU instance as they need. >I think what really needs to be done is to conceptually split the driver into >"architecture" and "implementation" > layers - at some point after the holidays we're probably going to sit down > and go through all the various quirks and > specifics we know about to try and figure out what that should actually look > like. If you can provide some high level details on what to keep in library vs implementation after holidays, I would be happy to rework the patches. Will look forward for further discussions on this. -KR ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/6] iommu/arm-smmu: Add support to program multiple ARM SMMU's identically
Hi Krishna,
On 04/12/2018 01:36, Krishna Reddy wrote:
Add support to program multiple ARM SMMU's identically as one SMMU device.
Tegra194 uses Two ARM SMMU's as one SMMU device and both ARM SMMU's need
to be programmed identically.
The whole point of the library idea was to factor out the code in such a
way that all the details specific to a particular implementation can be
kept together. But what this patch does is insert Tegra194-specific
handling all through the 'common' code, which is the exact opposite of
that concept and just makes more hard-to-maintain mess.
The amount of copy-paste duplication in patch #4 has the opposite
problem - about 95% of that isn't Tegra194-specific at all (I mean, how
many fsl_mc instances does it have?), and having multiple copies of
generic code with the potential to diverge is also not what anyone
wants. Plus I don't think ending up building multiple separate drivers
will even work in general - thanks to the current state of
bus_set_iommu() etc., you can't use the regular driver for your third
SMMU at the same time.
I think what really needs to be done is to conceptually split the driver
into "architecture" and "implementation" layers - at some point after
the holidays we're probably going to sit down and go through all the
various quirks and specifics we know about to try and figure out what
that should actually look like.
Robin.
Signed-off-by: Krishna Reddy
---
drivers/iommu/lib-arm-smmu.c | 191 ---
1 file changed, 144 insertions(+), 47 deletions(-)
diff --git a/drivers/iommu/lib-arm-smmu.c b/drivers/iommu/lib-arm-smmu.c
index 6aba5db..7036763 100644
--- a/drivers/iommu/lib-arm-smmu.c
+++ b/drivers/iommu/lib-arm-smmu.c
@@ -69,9 +69,9 @@
* therefore this actually makes more sense than it might first appear.
*/
#ifdef CONFIG_64BIT
-#define smmu_write_atomic_lq writeq_relaxed
+#define smmu_write_atomic_lq writeq_all_relaxed
#else
-#define smmu_write_atomic_lq writel_relaxed
+#define smmu_write_atomic_lq writel_all_relaxed
#endif
/* Translation context bank */
@@ -135,6 +135,48 @@ struct arm_smmu_domain {
struct iommu_domain domain;
};
+#define to_smmu_intance(smmu, inst, addr) \
+ (addr - smmu->bases[0] + smmu->bases[inst])
+
+static void writel_all(struct arm_smmu_device *smmu,
+ u32 value, void __iomem *addr)
+{
+ int i;
+
+ writel(value, addr);
+ for (i = 1; i < smmu->num_smmus; i++) {
+ void __iomem *reg_addr = to_smmu_intance(smmu, i, addr);
+
+ writel(value, reg_addr);
+ }
+}
+
+static void writel_all_relaxed(struct arm_smmu_device *smmu,
+ u32 value, void __iomem *addr)
+{
+ int i;
+
+ writel_relaxed(value, addr);
+ for (i = 1; i < smmu->num_smmus; i++) {
+ void __iomem *reg_addr = to_smmu_intance(smmu, i, addr);
+
+ writel_relaxed(value, reg_addr);
+ }
+}
+
+static void writeq_all_relaxed(struct arm_smmu_device *smmu,
+ u64 value, void __iomem *addr)
+{
+ int i;
+
+ writeq_relaxed(value, addr);
+ for (i = 1; i < smmu->num_smmus; i++) {
+ void __iomem *reg_addr = to_smmu_intance(smmu, i, addr);
+
+ writeq_relaxed(value, reg_addr);
+ }
+}
+
static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
{
return container_of(dom, struct arm_smmu_domain, domain);
@@ -179,25 +221,37 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device
*smmu,
static void arm_smmu_tlb_sync_global(struct arm_smmu_device *smmu)
{
- void __iomem *base = ARM_SMMU_GR0(smmu);
+ int i;
unsigned long flags;
spin_lock_irqsave(>global_sync_lock, flags);
- __arm_smmu_tlb_sync(smmu, base + ARM_SMMU_GR0_sTLBGSYNC,
- base + ARM_SMMU_GR0_sTLBGSTATUS);
+ for (i = 0; i < smmu->num_smmus; i++) {
+ void __iomem *base = ARM_SMMU_GR0(smmu);
+
+ if (i > 0)
+ base = to_smmu_intance(smmu, i, base);
+ __arm_smmu_tlb_sync(smmu, base + ARM_SMMU_GR0_sTLBGSYNC,
+ base + ARM_SMMU_GR0_sTLBGSTATUS);
+ }
spin_unlock_irqrestore(>global_sync_lock, flags);
}
static void arm_smmu_tlb_sync_context(void *cookie)
{
+ int i;
struct arm_smmu_domain *smmu_domain = cookie;
struct arm_smmu_device *smmu = smmu_domain->smmu;
- void __iomem *base = ARM_SMMU_CB(smmu, smmu_domain->cfg.cbndx);
unsigned long flags;
spin_lock_irqsave(_domain->cb_lock, flags);
- __arm_smmu_tlb_sync(smmu, base + ARM_SMMU_CB_TLBSYNC,
- base + ARM_SMMU_CB_TLBSTATUS);
+ for (i = 0; i < smmu->num_smmus; i++) {
+ void __iomem *base = ARM_SMMU_CB(smmu,
Re: [PATCH dma-mapping tree] arm64: default to the direct mapping in get_arch_dma_ops
On Wed, Dec 19, 2018 at 04:59:03PM +, Robin Murphy wrote: > On 14/12/2018 15:02, Christoph Hellwig wrote: >> Otherwise the direct mapping won't work at all given that a NULL >> dev->dma_ops causes a fallback. Note that we already explicitly set >> dev->dma_ops to dma_dummy_ops for dma-incapable devices, so this >> fallback should not be needed anyway. > > Sorry, I'd somehow completely missed that you'd sent a proper patch for > this - indeed it looks like the right change to make. > > Reviewed-by: Robin Murphy Thanks. I've added this given that I hadn't pushed out the tree yet. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: ensure dma_alloc_coherent always returns zeroed memory
FYI, I've picked this up for dma-mapping for-next now. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH dma-mapping tree] arm64: default to the direct mapping in get_arch_dma_ops
On 14/12/2018 15:02, Christoph Hellwig wrote:
Otherwise the direct mapping won't work at all given that a NULL
dev->dma_ops causes a fallback. Note that we already explicitly set
dev->dma_ops to dma_dummy_ops for dma-incapable devices, so this
fallback should not be needed anyway.
Sorry, I'd somehow completely missed that you'd sent a proper patch for
this - indeed it looks like the right change to make.
Reviewed-by: Robin Murphy
Fixes: 356da6d0cd ("dma-mapping: bypass indirect calls for dma-direct")
Signed-off-by: Christoph Hellwig
Reported-by: Marek Szyprowski
Tested-by: Marek Szyprowski
---
arch/arm64/include/asm/dma-mapping.h | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/dma-mapping.h
b/arch/arm64/include/asm/dma-mapping.h
index 273e778f7de2..95dbf3ef735a 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -26,11 +26,7 @@
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
- /*
-* We expect no ISA devices, and all other DMA masters are expected to
-* have someone call arch_setup_dma_ops at device creation time.
-*/
- return _dummy_ops;
+ return NULL;
}
void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH dma-mapping tree] arm64: default to the direct mapping in get_arch_dma_ops
As all maintainers seem to be off to their holidays already I've applied this now given that I don't want to leave arm64 in broken state in linux-next any longer. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/of: Don't call iommu_ops->add_device directly
Hi Joerg,
On 2018-12-19 15:34, Joerg Roedel wrote:
> Hi Marek,
>
> thanks for the report!
>
> On Wed, Dec 19, 2018 at 10:54:18AM +0100, Marek Szyprowski wrote:
>> On 2018-12-11 16:05, Joerg Roedel wrote:
>>> From: Joerg Roedel
>>>
>>> Make sure to invoke this call-back through the proper
>>> function of the IOMMU-API.
>>>
>>> Signed-off-by: Joerg Roedel
>>> ---
>>> drivers/iommu/of_iommu.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>> index c5dd63072529..4d4847de727e 100644
>>> --- a/drivers/iommu/of_iommu.c
>>> +++ b/drivers/iommu/of_iommu.c
>>> @@ -218,10 +218,10 @@ const struct iommu_ops *of_iommu_configure(struct
>>> device *dev,
>>> ops = dev->iommu_fwspec->ops;
>>> /*
>>> * If we have reason to believe the IOMMU driver missed the initial
>>> -* add_device callback for dev, replay it to get things in order.
>>> +* probe for dev, replay it to get things in order.
>>> */
>>> - if (ops && ops->add_device && dev->bus && !dev->iommu_group)
>>> - err = ops->add_device(dev);
>>> + if (dev->bus && !dev->iommu_group)
>>> + err = iommu_probe_device(dev);
>> This change removes a check for NULL ops, what causes NULL pointer
>> exception on first device without IOMMU.
> Bummer, this check was supposed to be in iommu_probe_device(), but
> apparently it got lost. Does the attached patch fix it?
Yes, it fixes this issue.
>> I'm also not sure if this is a good idea to call iommu_probe_device(),
>> which comes from dev->bus->iommu_ops, which might be different from ops
>> from local variable.
> The local variable comes from dev->iommu_fwspec->ops, which should be
> exactly the same as dev->bus->iommu_ops. I'll leave that for now until
> it turns out to be a problem (which I don't expect).
>
>
> Regards,
>
> Joerg
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index a2131751dcff..3ed4db334341 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -114,10 +114,14 @@ void iommu_device_unregister(struct iommu_device *iommu)
> int iommu_probe_device(struct device *dev)
> {
> const struct iommu_ops *ops = dev->bus->iommu_ops;
> + int ret = -EINVAL;
>
> WARN_ON(dev->iommu_group);
>
> - return ops->add_device(dev);
> + if (ops)
> + ret = ops->add_device(dev);
> +
> + return ret;
> }
>
> void iommu_release_device(struct device *dev)
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/of: Don't call iommu_ops->add_device directly
Hi Marek,
thanks for the report!
On Wed, Dec 19, 2018 at 10:54:18AM +0100, Marek Szyprowski wrote:
> On 2018-12-11 16:05, Joerg Roedel wrote:
> > From: Joerg Roedel
> >
> > Make sure to invoke this call-back through the proper
> > function of the IOMMU-API.
> >
> > Signed-off-by: Joerg Roedel
> > ---
> > drivers/iommu/of_iommu.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index c5dd63072529..4d4847de727e 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -218,10 +218,10 @@ const struct iommu_ops *of_iommu_configure(struct
> > device *dev,
> > ops = dev->iommu_fwspec->ops;
> > /*
> > * If we have reason to believe the IOMMU driver missed the initial
> > -* add_device callback for dev, replay it to get things in order.
> > +* probe for dev, replay it to get things in order.
> > */
> > - if (ops && ops->add_device && dev->bus && !dev->iommu_group)
> > - err = ops->add_device(dev);
> > + if (dev->bus && !dev->iommu_group)
> > + err = iommu_probe_device(dev);
>
> This change removes a check for NULL ops, what causes NULL pointer
> exception on first device without IOMMU.
Bummer, this check was supposed to be in iommu_probe_device(), but
apparently it got lost. Does the attached patch fix it?
> I'm also not sure if this is a good idea to call iommu_probe_device(),
> which comes from dev->bus->iommu_ops, which might be different from ops
> from local variable.
The local variable comes from dev->iommu_fwspec->ops, which should be
exactly the same as dev->bus->iommu_ops. I'll leave that for now until
it turns out to be a problem (which I don't expect).
Regards,
Joerg
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a2131751dcff..3ed4db334341 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -114,10 +114,14 @@ void iommu_device_unregister(struct iommu_device *iommu)
int iommu_probe_device(struct device *dev)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
+ int ret = -EINVAL;
WARN_ON(dev->iommu_group);
- return ops->add_device(dev);
+ if (ops)
+ ret = ops->add_device(dev);
+
+ return ret;
}
void iommu_release_device(struct device *dev)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[GIT PULL] dma-mapping fix for 4.20
The following changes since commit 6531e115b7ab84f563fcd7f0d2d05ccf971aaaf9: Merge branch 'akpm' (patches from Andrew) (2018-12-14 15:35:30 -0800) are available in the Git repository at: git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.20-4 for you to fetch changes up to c92a54cfa0257e8ffd66b2a17d49e9c0bd4b769f: dma-direct: do not include SME mask in the DMA supported check (2018-12-17 18:02:11 +0100) dma-mapping fix for 4.21 Fix a regression in dma-direct that didn't take account the magic AMD memory encryption mask in the DMA address. Lendacky, Thomas (1): dma-direct: do not include SME mask in the DMA supported check kernel/dma/direct.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup
On Wed, Dec 19, 2018 at 12:21:35PM +, Robin Murphy wrote:
> On 18/12/2018 18:48, Andrew Jones wrote:
> > The sum of dmaaddr and size may overflow, particularly considering
> > there are cases where size will be U64_MAX.
>
> Only if the firmware is broken in the first place, though. It would be weird
> to describe an explicit _DMA range of base=0 and size=U64_MAX, because it's
> effectively the same as just not having one at all, but it's not strictly
> illegal. However, since the ACPI System Memory address space is at most
> 64-bit, anything that would actually overflow here is already describing an
> impossibility - really, we should probably scream even louder about a
> firmware bug and reject it entirely, rather than quietly hiding it.
Ah, looking again I see the paths. Either acpi_dma_get_range() returns
success, in which case base and size are fine, or it returns an EINVAL,
in which case base=size=0, or it returns ENODEV in which case base is
zero, so size may be set to U64_MAX by rc_dma_get_range() with no problem.
The !dev_is_pci(dev) path is also fine since base=0.
While I agree that we should complain when firmware provides bad ACPI
tables, my understanding of setting iort.memory_address_limit=64 was
that it simply meant "no limit", regardless of the base. However I now
see that it won't be used unless base=0. So I don't think we have a
problem, and we don't even seem to be missing firmware sanity checks.
It might be nice to have a comment explaining this stuff somewhere, but
otherwise sorry for the noise.
Thanks,
drew
>
> Robin.
>
> > Signed-off-by: Andrew Jones
> > ---
> > drivers/acpi/arm64/iort.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 70f4e80b9246..a0f4c157ba5e 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64
> > *dma_addr, u64 *dma_size)
> > }
> > if (!ret) {
> > - msb = fls64(dmaaddr + size - 1);
> > + u64 dmaaddr_max = dmaaddr + size - 1;
> > + if (dmaaddr_max >= dmaaddr)
> > + msb = fls64(dmaaddr_max);
> > + else
> > + msb = 64;
> > +
> > /*
> > * Round-up to the power-of-two mask or set
> > * the mask to the whole 64-bit address space
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/dma: Handle potential overflow in iommu_dma_init_domain
On 18/12/2018 18:48, Andrew Jones wrote:
The sum of base and size may overflow, particularly considering there
are cases where size will be U64_MAX. Also, end_pfn is unused, so we
remove it. Finally, as size doesn't actually need to be IOMMU page
aligned we remove it from the comment stating both it and base should
be. I wonder if we shouldn't at least warn when base is not aligned?
TBH if we're going to do anything here we may as well just get rid of
size altogether. It's pretty unrealistic that the check it's used in
would ever actually fail, and even if a sufficiently weird system did
exist for that to happen, I don't think it would make much practical
difference to just carry on at this point and let DMA mapping calls fail
later.
Robin.
Signed-off-by: Andrew Jones
---
drivers/iommu/dma-iommu.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index d1b04753b204..a0b01398b15c 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -281,9 +281,9 @@ static void iommu_dma_flush_iotlb_all(struct iova_domain
*iovad)
* @size: Size of IOVA space
* @dev: Device the domain is being initialised for
*
- * @base and @size should be exact multiples of IOMMU page granularity to
- * avoid rounding surprises. If necessary, we reserve the page at address 0
- * to ensure it is an invalid IOVA. It is safe to reinitialise a domain, but
+ * @base should be an exact multiple of IOMMU page granularity to avoid
+ * rounding surprises. If necessary, we reserve the page at address 0 to
+ * ensure it is an invalid IOVA. It is safe to reinitialise a domain, but
* any change which could make prior IOVAs invalid will fail.
*/
int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
@@ -291,21 +291,24 @@ int iommu_dma_init_domain(struct iommu_domain *domain,
dma_addr_t base,
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = >iovad;
- unsigned long order, base_pfn, end_pfn;
+ dma_addr_t max_addr = base + size - 1;
+ unsigned long order, base_pfn;
int attr;
if (!cookie || cookie->type != IOMMU_DMA_IOVA_COOKIE)
return -EINVAL;
+ if (max_addr < base)
+ max_addr = U64_MAX;
+
/* Use the smallest supported page size for IOVA granularity */
order = __ffs(domain->pgsize_bitmap);
base_pfn = max_t(unsigned long, 1, base >> order);
- end_pfn = (base + size - 1) >> order;
/* Check the domain allows at least some access to the device... */
if (domain->geometry.force_aperture) {
if (base > domain->geometry.aperture_end ||
- base + size <= domain->geometry.aperture_start) {
+ max_addr < domain->geometry.aperture_start) {
pr_warn("specified DMA range outside IOMMU
capability\n");
return -EFAULT;
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ACPI/IORT: Handle potential overflow in iort_dma_setup
On 18/12/2018 18:48, Andrew Jones wrote:
The sum of dmaaddr and size may overflow, particularly considering
there are cases where size will be U64_MAX.
Only if the firmware is broken in the first place, though. It would be
weird to describe an explicit _DMA range of base=0 and size=U64_MAX,
because it's effectively the same as just not having one at all, but
it's not strictly illegal. However, since the ACPI System Memory address
space is at most 64-bit, anything that would actually overflow here is
already describing an impossibility - really, we should probably scream
even louder about a firmware bug and reject it entirely, rather than
quietly hiding it.
Robin.
Signed-off-by: Andrew Jones
---
drivers/acpi/arm64/iort.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 70f4e80b9246..a0f4c157ba5e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1002,7 +1002,12 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr,
u64 *dma_size)
}
if (!ret) {
- msb = fls64(dmaaddr + size - 1);
+ u64 dmaaddr_max = dmaaddr + size - 1;
+ if (dmaaddr_max >= dmaaddr)
+ msb = fls64(dmaaddr_max);
+ else
+ msb = 64;
+
/*
* Round-up to the power-of-two mask or set
* the mask to the whole 64-bit address space
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/of: Don't call iommu_ops->add_device directly
Hi Joerg,
This patch landed in today's linux-next and causes a regression.
On 2018-12-11 16:05, Joerg Roedel wrote:
> From: Joerg Roedel
>
> Make sure to invoke this call-back through the proper
> function of the IOMMU-API.
>
> Signed-off-by: Joerg Roedel
> ---
> drivers/iommu/of_iommu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index c5dd63072529..4d4847de727e 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -218,10 +218,10 @@ const struct iommu_ops *of_iommu_configure(struct
> device *dev,
> ops = dev->iommu_fwspec->ops;
> /*
>* If we have reason to believe the IOMMU driver missed the initial
> - * add_device callback for dev, replay it to get things in order.
> + * probe for dev, replay it to get things in order.
>*/
> - if (ops && ops->add_device && dev->bus && !dev->iommu_group)
> - err = ops->add_device(dev);
> + if (dev->bus && !dev->iommu_group)
> + err = iommu_probe_device(dev);
This change removes a check for NULL ops, what causes NULL pointer
exception on first device without IOMMU.
I'm also not sure if this is a good idea to call iommu_probe_device(),
which comes from dev->bus->iommu_ops, which might be different from ops
from local variable.
>
> /* Ignore all other errors apart from EPROBE_DEFER */
> if (err == -EPROBE_DEFER) {
Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
