Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Michael Ellerman]

Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
> On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote:
>> Hello Russell,
>> 
>> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I
>> don't find the property "dma-coherent" in the dtb source files.
>> 
>> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma":
>> 
>> dma0 = "/soc@ffe000000/dma@100300";
>>         dma1 = "/soc@ffe000000/dma@101300";
>>         dma@100300 {
>>             compatible = "fsl,eloplus-dma";
>>             dma-channel@0 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@80 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@100 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@180 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>         dma@101300 {
>>             compatible = "fsl,eloplus-dma";
>>             dma-channel@0 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@80 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@100 {
>>                 compatible = "fsl,eloplus-dma-channel";
>>             dma-channel@180 {
>>                 compatible = "fsl,eloplus-dma-channel";
>
> Hmm, so it looks like PowerPC doesn't mark devices that are dma
> coherent with a property that describes them as such.
>
> I think this opens a wider question - what should of_dma_is_coherent()
> return for PowerPC?  It seems right now that it returns false for
> devices that are DMA coherent, which seems to me to be a recipe for
> future mistakes.

Right, it seems of_dma_is_coherent() has baked in the assumption that
devices are non-coherent unless explicitly marked as coherent.

Which is wrong on all or at least most existing powerpc systems
according to Ben.

> Any ideas from the PPC maintainers?

Fixing it at the source seems like the best option to prevent future
breakage.

So I guess that would mean making of_dma_is_coherent() return true/false
based on CONFIG_NOT_COHERENT_CACHE on powerpc.

We could do it like below, which would still allow the dma-coherent
property to work if it ever makes sense on a future powerpc platform.

I don't really know any of this embedded stuff well, so happy to take
other suggestions on how to handle this mess.

cheers


diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 25aaa3903000..b96c9010acb6 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -760,6 +760,22 @@ static int __init check_cache_coherency(void)
 late_initcall(check_cache_coherency);
 #endif /* CONFIG_CHECK_CACHE_COHERENCY */
 
+#ifndef CONFIG_NOT_COHERENT_CACHE
+/*
+ * For historical reasons powerpc kernels are built with hard wired knowledge of
+ * whether or not DMA accesses are cache coherent. Additionally device trees on
+ * powerpc do not typically support the dma-coherent property.
+ *
+ * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to
+ * tell the drivers/of code that all devices are coherent regardless of whether
+ * they have a dma-coherent property.
+ */
+bool arch_of_dma_is_coherent(struct device_node *np)
+{
+	return true;
+}
+#endif
+
 #ifdef CONFIG_DEBUG_FS
 struct dentry *powerpc_debugfs_root;
 EXPORT_SYMBOL(powerpc_debugfs_root);
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 978427a9d5e6..3a4b2949a322 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
 }
 EXPORT_SYMBOL_GPL(of_dma_get_range);
 
+/*
+ * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA
+ */
+bool __weak arch_of_dma_is_coherent(struct device_node *np)
+{
+	return false;
+}
+
 /**
  * of_dma_is_coherent - Check if device is coherent
  * @np:	device node
@@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range);
  */
 bool of_dma_is_coherent(struct device_node *np)
 {
-	struct device_node *node = of_node_get(np);
+	struct device_node *node;
+
+	if (arch_of_dma_is_coherent(np))
+		return true;
 
+	np = of_node_get(np);
 	while (node) {
 		if (of_property_read_bool(node, "dma-coherent")) {
 			of_node_put(node);

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Benjamin Herrenschmidt]

On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote:
> 
> Right, it seems of_dma_is_coherent() has baked in the assumption that
> devices are non-coherent unless explicitly marked as coherent.
> 
> Which is wrong on all or at least most existing powerpc systems
> according to Ben.

This is probably broken on sparc(64) as well and whatever else uses
DT and is an intrinsicly coherent architecture (did we ever have
DT enabled x86s ? Wasn't OLPC such a beast ?).

I think this should have been done the other way around and default to
coherent since most traditional OF platforms are coherent, and you
can't just require those DTs to change.

> > Any ideas from the PPC maintainers?
> 
> Fixing it at the source seems like the best option to prevent future
> breakage.
> 
> So I guess that would mean making of_dma_is_coherent() return true/false
> based on CONFIG_NOT_COHERENT_CACHE on powerpc.
> 
> We could do it like below, which would still allow the dma-coherent
> property to work if it ever makes sense on a future powerpc platform.
> 
> I don't really know any of this embedded stuff well, so happy to take
> other suggestions on how to handle this mess.

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Rob Herring]

On Wed, Oct 23, 2019 at 1:41 AM Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote:
> >
> > Right, it seems of_dma_is_coherent() has baked in the assumption that
> > devices are non-coherent unless explicitly marked as coherent.
> >
> > Which is wrong on all or at least most existing powerpc systems
> > according to Ben.
>
> This is probably broken on sparc(64) as well and whatever else uses
> DT and is an intrinsicly coherent architecture (did we ever have
> DT enabled x86s ? Wasn't OLPC such a beast ?).

Only if those platforms use one of the 5 drivers that call this function:

drivers/ata/ahci_qoriq.c:       qoriq_priv->is_dmacoherent =
of_dma_is_coherent(np);
drivers/crypto/ccree/cc_driver.c:       new_drvdata->coherent =
of_dma_is_coherent(np);
drivers/iommu/arm-smmu-v3.c:    if (of_dma_is_coherent(dev->of_node))
drivers/iommu/arm-smmu.c:       if (of_dma_is_coherent(dev->of_node))
drivers/mmc/host/sdhci-of-esdhc.c:      if (of_dma_is_coherent(dev->of_node))

Curious that a PPC specific driver (ahci_qoriq) calls it...

Note that the value is also passed to arch_setup_dma_ops(), but only
arc, arm, arm64, and mips implement arch_setup_dma_ops.

> I think this should have been done the other way around and default to
> coherent since most traditional OF platforms are coherent, and you
> can't just require those DTs to change.

You can blame me. This was really only intended for cases where
coherency is configurable on a per peripheral basis and can't be
determined in other ways.

The simple solution here is simply to use the compatible string for
the device to determine coherent or not.

Rob

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Russell King - ARM Linux admin]

On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote:
> On Wed, Oct 23, 2019 at 1:41 AM Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> >
> > On Wed, 2019-10-23 at 16:42 +1100, Michael Ellerman wrote:
> > >
> > > Right, it seems of_dma_is_coherent() has baked in the assumption that
> > > devices are non-coherent unless explicitly marked as coherent.
> > >
> > > Which is wrong on all or at least most existing powerpc systems
> > > according to Ben.
> >
> > This is probably broken on sparc(64) as well and whatever else uses
> > DT and is an intrinsicly coherent architecture (did we ever have
> > DT enabled x86s ? Wasn't OLPC such a beast ?).
> 
> Only if those platforms use one of the 5 drivers that call this function:
> 
> drivers/ata/ahci_qoriq.c:       qoriq_priv->is_dmacoherent =
> of_dma_is_coherent(np);
> drivers/crypto/ccree/cc_driver.c:       new_drvdata->coherent =
> of_dma_is_coherent(np);
> drivers/iommu/arm-smmu-v3.c:    if (of_dma_is_coherent(dev->of_node))
> drivers/iommu/arm-smmu.c:       if (of_dma_is_coherent(dev->of_node))
> drivers/mmc/host/sdhci-of-esdhc.c:      if (of_dma_is_coherent(dev->of_node))
> 
> Curious that a PPC specific driver (ahci_qoriq) calls it...

Maybe because it is not PPC specific:

arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi:
compatible = "fsl,lx2160a-ahci";
arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi:
compatible = "fsl,lx2160a-ahci";
arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi:
compatible = "fsl,lx2160a-ahci";
arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi:
compatible = "fsl,lx2160a-ahci";

drivers/ata/ahci_qoriq.c:       { .compatible = "fsl,lx2160a-ahci", .data = (void *)AHCI_LX2160A},

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Russell King - ARM Linux admin]

On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote:
> > I think this should have been done the other way around and default to
> > coherent since most traditional OF platforms are coherent, and you
> > can't just require those DTs to change.
> 
> You can blame me. This was really only intended for cases where
> coherency is configurable on a per peripheral basis and can't be
> determined in other ways.
> 
> The simple solution here is simply to use the compatible string for
> the device to determine coherent or not.

It really isn't that simple.

There are two aspects to coherency, both of which must match:

1) The configuration of the device
2) The configuration of the kernel's DMA API

(1) is controlled by the driver, which can make the decision any way
it pleases.

(2) on ARM64 is controlled depending on whether or not "dma-coherent"
is specified in the device tree, since ARM64 can have a mixture of
DMA coherent and non-coherent devices.

A mismatch between (1) and (2) results in data corruption, potentially
eating your filesystem.  So, it's very important that the two match.

These didn't match for the LX2160A, but, due to the way CMA was working,
we sort of got away with it, but it was very dangerous as far as data
safety went.

Then, a change to CMA happened which moved where it was located, which
caused a regression.  Reverting the CMA changes didn't seem to be an
option, so another solution had to be found.

I started a discussion on how best to solve this:

https://archive.armlinux.org.uk/lurker/thread/20190919.041320.1e53541f.en.html

and the solution that the discussion came out with was the one that has
been merged - which we now know caused a regression on PPC.

Using compatible strings doesn't solve the issue: there is no way to
tell the DMA API from the driver that the device is coherent.  The
only way to do that is via the "dma-coherent" property in DT on ARM64.

To say that this is a mess is an under-statement, but we seem to have
ended up here because of a series of piece-meal changes that don't seem
to have been thought through enough.

So, what's the right way to solve this, and ensure that the DMA API and
device match as far as their coherency expectations go?  Revert all the
changes for sdhci-of-esdhc and CMA back to 5.0 or 5.1 state?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Rob Herring]

On Wed, Oct 23, 2019 at 9:32 AM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Oct 23, 2019 at 08:52:33AM -0500, Rob Herring wrote:
> > > I think this should have been done the other way around and default to
> > > coherent since most traditional OF platforms are coherent, and you
> > > can't just require those DTs to change.
> >
> > You can blame me. This was really only intended for cases where
> > coherency is configurable on a per peripheral basis and can't be
> > determined in other ways.
> >
> > The simple solution here is simply to use the compatible string for
> > the device to determine coherent or not.
>
> It really isn't that simple.

This doesn't work?:

        if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev->of_node))
                value |= ESDHC_DMA_SNOOP;
        else
                value &= ~ESDHC_DMA_SNOOP;

While I said use the compatibles, using the kconfig symbol is easier
than sorting out which compatibles are PPC SoCs. Though if that's
already done elsewhere in the driver, you could set a flag and use
that here. I'd be surprised if this was the only difference between
ARM and PPC SoCs for this block.

> There are two aspects to coherency, both of which must match:
>
> 1) The configuration of the device
> 2) The configuration of the kernel's DMA API
>
> (1) is controlled by the driver, which can make the decision any way
> it pleases.
>
> (2) on ARM64 is controlled depending on whether or not "dma-coherent"
> is specified in the device tree, since ARM64 can have a mixture of
> DMA coherent and non-coherent devices.
>
> A mismatch between (1) and (2) results in data corruption, potentially
> eating your filesystem.  So, it's very important that the two match.
>
> These didn't match for the LX2160A, but, due to the way CMA was working,
> we sort of got away with it, but it was very dangerous as far as data
> safety went.
>
> Then, a change to CMA happened which moved where it was located, which
> caused a regression.  Reverting the CMA changes didn't seem to be an
> option, so another solution had to be found.
>
> I started a discussion on how best to solve this:
>
> https://archive.armlinux.org.uk/lurker/thread/20190919.041320.1e53541f.en.html
>
> and the solution that the discussion came out with was the one that has
> been merged - which we now know caused a regression on PPC.
>
> Using compatible strings doesn't solve the issue: there is no way to
> tell the DMA API from the driver that the device is coherent.  The
> only way to do that is via the "dma-coherent" property in DT on ARM64.
>
> To say that this is a mess is an under-statement, but we seem to have
> ended up here because of a series of piece-meal changes that don't seem
> to have been thought through enough.
>
> So, what's the right way to solve this, and ensure that the DMA API and
> device match as far as their coherency expectations go?  Revert all the
> changes for sdhci-of-esdhc and CMA back to 5.0 or 5.1 state?

The other option is similar to earlier in the thread and just add to
of_dma_is_coherent():

/* Powerpc is normally cache coherent DMA */
if (IS_ENABLED(CONFIG_PPC) && !IS_ENABLED(CONFIG_NOT_COHERENT_CACHE))
    return true;

We could do the all the weak arch hooks, but that seems like overkill
to me at this point.

Rob

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Christoph Hellwig]

On Fri, Oct 25, 2019 at 05:28:45PM -0500, Rob Herring wrote:
> This doesn't work?:
> 
>         if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev->of_node))
>                 value |= ESDHC_DMA_SNOOP;
>         else
>                 value &= ~ESDHC_DMA_SNOOP;
> 
> While I said use the compatibles, using the kconfig symbol is easier
> than sorting out which compatibles are PPC SoCs. Though if that's
> already done elsewhere in the driver, you could set a flag and use
> that here. I'd be surprised if this was the only difference between
> ARM and PPC SoCs for this block.

I think the right thing is a Kconfig variable that the architectures
selects which says if OF is by default coherent or incoherent, and then
use that in of_dma_is_coherent.

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Benjamin Herrenschmidt]

On Fri, 2019-10-25 at 23:39 -0700, Christoph Hellwig wrote:
> On Fri, Oct 25, 2019 at 05:28:45PM -0500, Rob Herring wrote:
> > This doesn't work?:
> > 
> >         if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev-
> > >of_node))
> >                 value |= ESDHC_DMA_SNOOP;
> >         else
> >                 value &= ~ESDHC_DMA_SNOOP;
> > 
> > While I said use the compatibles, using the kconfig symbol is
> > easier
> > than sorting out which compatibles are PPC SoCs. Though if that's
> > already done elsewhere in the driver, you could set a flag and use
> > that here. I'd be surprised if this was the only difference between
> > ARM and PPC SoCs for this block.
> 
> I think the right thing is a Kconfig variable that the architectures
> selects which says if OF is by default coherent or incoherent, and
> then use that in of_dma_is_coherent.

That too. We could also define properties for both ways so we can
override the default either way.

Cheers,
Ben.

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Benjamin Herrenschmidt]

On Fri, 2019-10-25 at 17:28 -0500, Rob Herring wrote:
> This doesn't work?:
> 
>         if (IS_ENABLED(CONFIG_PPC) || of_dma_is_coherent(dev-
> >of_node))
>                 value |= ESDHC_DMA_SNOOP;
>         else
>                 value &= ~ESDHC_DMA_SNOOP;

CONFIG_PPC is restrictive. What about sparc64 ? There could be others
.. .we can't suddenly requests people to add new properties for what
was implied behaviours before hand, esp since it's not in the base 1275
spec, no ?

I would suggest of_dma_is_coherent is *true* by default unless
overriden to do something else.

Cheers,
Ben.

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Christian Zigotzky]

Hello,

The patch below works. I compiled the RC4 of kernel 5.4 with this patch today and the onboard SD card works without any problems.

Thanks!

Christian

> On 23. Oct 2019, at 07:42, Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
>>> On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote:
>>> Hello Russell,
>>> 
>>> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I
>>> don't find the property "dma-coherent" in the dtb source files.
>>> 
>>> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma":
>>> 
>>> dma0 = "/soc@ffe000000/dma@100300";
>>>         dma1 = "/soc@ffe000000/dma@101300";
>>>         dma@100300 {
>>>             compatible = "fsl,eloplus-dma";
>>>             dma-channel@0 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@80 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@100 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@180 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>         dma@101300 {
>>>             compatible = "fsl,eloplus-dma";
>>>             dma-channel@0 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@80 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@100 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>>>             dma-channel@180 {
>>>                 compatible = "fsl,eloplus-dma-channel";
>> 
>> Hmm, so it looks like PowerPC doesn't mark devices that are dma
>> coherent with a property that describes them as such.
>> 
>> I think this opens a wider question - what should of_dma_is_coherent()
>> return for PowerPC?  It seems right now that it returns false for
>> devices that are DMA coherent, which seems to me to be a recipe for
>> future mistakes.
> 
> Right, it seems of_dma_is_coherent() has baked in the assumption that
> devices are non-coherent unless explicitly marked as coherent.
> 
> Which is wrong on all or at least most existing powerpc systems
> according to Ben.
> 
>> Any ideas from the PPC maintainers?
> 
> Fixing it at the source seems like the best option to prevent future
> breakage.
> 
> So I guess that would mean making of_dma_is_coherent() return true/false
> based on CONFIG_NOT_COHERENT_CACHE on powerpc.
> 
> We could do it like below, which would still allow the dma-coherent
> property to work if it ever makes sense on a future powerpc platform.
> 
> I don't really know any of this embedded stuff well, so happy to take
> other suggestions on how to handle this mess.
> 
> cheers
> 
> 
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 25aaa3903000..b96c9010acb6 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -760,6 +760,22 @@ static int __init check_cache_coherency(void)
> late_initcall(check_cache_coherency);
> #endif /* CONFIG_CHECK_CACHE_COHERENCY */
> 
> +#ifndef CONFIG_NOT_COHERENT_CACHE
> +/*
> + * For historical reasons powerpc kernels are built with hard wired knowledge of
> + * whether or not DMA accesses are cache coherent. Additionally device trees on
> + * powerpc do not typically support the dma-coherent property.
> + *
> + * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to
> + * tell the drivers/of code that all devices are coherent regardless of whether
> + * they have a dma-coherent property.
> + */
> +bool arch_of_dma_is_coherent(struct device_node *np)
> +{
> +    return true;
> +}
> +#endif
> +
> #ifdef CONFIG_DEBUG_FS
> struct dentry *powerpc_debugfs_root;
> EXPORT_SYMBOL(powerpc_debugfs_root);
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 978427a9d5e6..3a4b2949a322 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
> }
> EXPORT_SYMBOL_GPL(of_dma_get_range);
> 
> +/*
> + * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA
> + */
> +bool __weak arch_of_dma_is_coherent(struct device_node *np)
> +{
> +    return false;
> +}
> +
> /**
>  * of_dma_is_coherent - Check if device is coherent
>  * @np:    device node
> @@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range);
>  */
> bool of_dma_is_coherent(struct device_node *np)
> {
> -    struct device_node *node = of_node_get(np);
> +    struct device_node *node;
> +
> +    if (arch_of_dma_is_coherent(np))
> +        return true;
> 
> +    np = of_node_get(np);
>    while (node) {
>        if (of_property_read_bool(node, "dma-coherent")) {
>            of_node_put(node);

Re: Onboard SD card doesn't work anymore after the 'mmc-v5.4-2' updates

[Christian Zigotzky]

FYI: The onboard SD card works also with the RC6 of kernel 5.4 with the 
patch below.

-- Christian

On 23 October 2019 at 4:20pm, Christian Zigotzky wrote:
> Hello,
>
> The patch below works. I compiled the RC4 of kernel 5.4 with this patch today and the onboard SD card works without any problems.
>
> Thanks!
>
> Christian
>
>> On 23. Oct 2019, at 07:42, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Russell King - ARM Linux admin <linux@armlinux.org.uk> writes:
>>>> On Tue, Oct 15, 2019 at 03:12:49PM +0200, Christian Zigotzky wrote:
>>>> Hello Russell,
>>>>
>>>> You asked me about "dma-coherent" in the Cyrus device tree. Unfortunately I
>>>> don't find the property "dma-coherent" in the dtb source files.
>>>>
>>>> Output of "fdtdump cyrus_p5020_eth_poweroff.dtb | grep dma":
>>>>
>>>> dma0 = "/soc@ffe000000/dma@100300";
>>>>          dma1 = "/soc@ffe000000/dma@101300";
>>>>          dma@100300 {
>>>>              compatible = "fsl,eloplus-dma";
>>>>              dma-channel@0 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@80 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@100 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@180 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>          dma@101300 {
>>>>              compatible = "fsl,eloplus-dma";
>>>>              dma-channel@0 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@80 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@100 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>>>              dma-channel@180 {
>>>>                  compatible = "fsl,eloplus-dma-channel";
>>> Hmm, so it looks like PowerPC doesn't mark devices that are dma
>>> coherent with a property that describes them as such.
>>>
>>> I think this opens a wider question - what should of_dma_is_coherent()
>>> return for PowerPC?  It seems right now that it returns false for
>>> devices that are DMA coherent, which seems to me to be a recipe for
>>> future mistakes.
>> Right, it seems of_dma_is_coherent() has baked in the assumption that
>> devices are non-coherent unless explicitly marked as coherent.
>>
>> Which is wrong on all or at least most existing powerpc systems
>> according to Ben.
>>
>>> Any ideas from the PPC maintainers?
>> Fixing it at the source seems like the best option to prevent future
>> breakage.
>>
>> So I guess that would mean making of_dma_is_coherent() return true/false
>> based on CONFIG_NOT_COHERENT_CACHE on powerpc.
>>
>> We could do it like below, which would still allow the dma-coherent
>> property to work if it ever makes sense on a future powerpc platform.
>>
>> I don't really know any of this embedded stuff well, so happy to take
>> other suggestions on how to handle this mess.
>>
>> cheers
>>
>>
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index 25aaa3903000..b96c9010acb6 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -760,6 +760,22 @@ static int __init check_cache_coherency(void)
>> late_initcall(check_cache_coherency);
>> #endif /* CONFIG_CHECK_CACHE_COHERENCY */
>>
>> +#ifndef CONFIG_NOT_COHERENT_CACHE
>> +/*
>> + * For historical reasons powerpc kernels are built with hard wired knowledge of
>> + * whether or not DMA accesses are cache coherent. Additionally device trees on
>> + * powerpc do not typically support the dma-coherent property.
>> + *
>> + * So when we know that DMA is coherent, override arch_of_dma_is_coherent() to
>> + * tell the drivers/of code that all devices are coherent regardless of whether
>> + * they have a dma-coherent property.
>> + */
>> +bool arch_of_dma_is_coherent(struct device_node *np)
>> +{
>> +    return true;
>> +}
>> +#endif
>> +
>> #ifdef CONFIG_DEBUG_FS
>> struct dentry *powerpc_debugfs_root;
>> EXPORT_SYMBOL(powerpc_debugfs_root);
>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>> index 978427a9d5e6..3a4b2949a322 100644
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -993,6 +993,14 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
>> }
>> EXPORT_SYMBOL_GPL(of_dma_get_range);
>>
>> +/*
>> + * arch_of_dma_is_coherent - Arch hook to determine if device is coherent for DMA
>> + */
>> +bool __weak arch_of_dma_is_coherent(struct device_node *np)
>> +{
>> +    return false;
>> +}
>> +
>> /**
>>   * of_dma_is_coherent - Check if device is coherent
>>   * @np:    device node
>> @@ -1002,8 +1010,12 @@ EXPORT_SYMBOL_GPL(of_dma_get_range);
>>   */
>> bool of_dma_is_coherent(struct device_node *np)
>> {
>> -    struct device_node *node = of_node_get(np);
>> +    struct device_node *node;
>> +
>> +    if (arch_of_dma_is_coherent(np))
>> +        return true;
>>
>> +    np = of_node_get(np);
>>     while (node) {
>>         if (of_property_read_bool(node, "dma-coherent")) {
>>             of_node_put(node);

Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

Hi All,

We still have DMA problems with some PCI devices. Since the PowerPC 
updates 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if 
we want to work with our PCI devices. The FSL P5020 and P5040 have these 
problems currently.

Error message:

[   25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 of 
DMA mask ffffffff bus mask df000000

All 5.x Linux kernels can't initialize a SCSI PCI card anymore so 
booting of a Linux userland isn't possible.

PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. The 
kernel 4.20 works with all PCI devices without limitation of RAM.

We created a bug report a month ago. [2]

Thanks,
Christian

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8d6973327ee84c2f40dd9efd8928d4a1186c96e2
[2] https://bugzilla.kernel.org/show_bug.cgi?id=205201

Re: use generic DMA mapping code in powerpc V4

[Christoph Hellwig]

On Thu, Dec 13, 2018 at 12:19:26PM +0100, Christian Zigotzky wrote:
> I tried it again but I get the following error message:
>
> MODPOST vmlinux.o
> arch/powerpc/kernel/dma-iommu.o: In function `.dma_iommu_get_required_mask':
> (.text+0x274): undefined reference to `.dma_direct_get_required_mask'
> make: *** [vmlinux] Error 1

Sorry, you need this one liner before all the patches posted last time:

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index d8819e3a1eb1..7e78c2798f2f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -154,6 +154,7 @@ config PPC
 	select CLONE_BACKWARDS
 	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
+	select DMA_DIRECT_OPS
 	select EDAC_ATOMIC_SCRUB
 	select EDAC_SUPPORT
 	select GENERIC_ATOMIC64			if PPC32

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

On 13 December 2018 at 12:25PM, Christoph Hellwig wrote:
> On Thu, Dec 13, 2018 at 12:19:26PM +0100, Christian Zigotzky wrote:
>> I tried it again but I get the following error message:
>>
>> MODPOST vmlinux.o
>> arch/powerpc/kernel/dma-iommu.o: In function `.dma_iommu_get_required_mask':
>> (.text+0x274): undefined reference to `.dma_direct_get_required_mask'
>> make: *** [vmlinux] Error 1
> Sorry, you need this one liner before all the patches posted last time:
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d8819e3a1eb1..7e78c2798f2f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -154,6 +154,7 @@ config PPC
>   	select CLONE_BACKWARDS
>   	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
>   	select DYNAMIC_FTRACE			if FUNCTION_TRACER
> +	select DMA_DIRECT_OPS
>   	select EDAC_ATOMIC_SCRUB
>   	select EDAC_SUPPORT
>   	select GENERIC_ATOMIC64			if PPC32
>
Thanks. Result: PASEMI onboard ethernet works and the X5000 (P5020 
board) boots with the patch '0001-get_required_mask.patch'.

-- Christian

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

On 13 December 2018 at 2:34PM, Christian Zigotzky wrote:
> On 13 December 2018 at 12:25PM, Christoph Hellwig wrote:
>> On Thu, Dec 13, 2018 at 12:19:26PM +0100, Christian Zigotzky wrote:
>>> I tried it again but I get the following error message:
>>>
>>> MODPOST vmlinux.o
>>> arch/powerpc/kernel/dma-iommu.o: In function 
>>> `.dma_iommu_get_required_mask':
>>> (.text+0x274): undefined reference to `.dma_direct_get_required_mask'
>>> make: *** [vmlinux] Error 1
>> Sorry, you need this one liner before all the patches posted last time:
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index d8819e3a1eb1..7e78c2798f2f 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -154,6 +154,7 @@ config PPC
>>       select CLONE_BACKWARDS
>>       select DCACHE_WORD_ACCESS        if PPC64 && CPU_LITTLE_ENDIAN
>>       select DYNAMIC_FTRACE            if FUNCTION_TRACER
>> +    select DMA_DIRECT_OPS
>>       select EDAC_ATOMIC_SCRUB
>>       select EDAC_SUPPORT
>>       select GENERIC_ATOMIC64            if PPC32
>>
> Thanks. Result: PASEMI onboard ethernet works and the X5000 (P5020 
> board) boots with the patch '0001-get_required_mask.patch'.
>
> -- Christian
>
>
Next patch: '0002-swiotlb-dma_supported.patch' for the last good commit 
(977706f9755d2d697aa6f45b4f9f0e07516efeda).

The PASEMI onboard ethernet works and the X5000 (P5020 board) boots.

-- Christian

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

On 13 December 2018 at 6:48PM, Christian Zigotzky wrote:
> On 13 December 2018 at 2:34PM, Christian Zigotzky wrote:
>> On 13 December 2018 at 12:25PM, Christoph Hellwig wrote:
>>> On Thu, Dec 13, 2018 at 12:19:26PM +0100, Christian Zigotzky wrote:
>>>> I tried it again but I get the following error message:
>>>>
>>>> MODPOST vmlinux.o
>>>> arch/powerpc/kernel/dma-iommu.o: In function 
>>>> `.dma_iommu_get_required_mask':
>>>> (.text+0x274): undefined reference to `.dma_direct_get_required_mask'
>>>> make: *** [vmlinux] Error 1
>>> Sorry, you need this one liner before all the patches posted last time:
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index d8819e3a1eb1..7e78c2798f2f 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -154,6 +154,7 @@ config PPC
>>>       select CLONE_BACKWARDS
>>>       select DCACHE_WORD_ACCESS        if PPC64 && CPU_LITTLE_ENDIAN
>>>       select DYNAMIC_FTRACE            if FUNCTION_TRACER
>>> +    select DMA_DIRECT_OPS
>>>       select EDAC_ATOMIC_SCRUB
>>>       select EDAC_SUPPORT
>>>       select GENERIC_ATOMIC64            if PPC32
>>>
>> Thanks. Result: PASEMI onboard ethernet works and the X5000 (P5020 
>> board) boots with the patch '0001-get_required_mask.patch'.
>>
>> -- Christian
>>
>>
> Next patch: '0002-swiotlb-dma_supported.patch' for the last good 
> commit (977706f9755d2d697aa6f45b4f9f0e07516efeda).
>
> The PASEMI onboard ethernet works and the X5000 (P5020 board) boots.
>
> -- Christian
>
>
Next patch: '0003-nommu-dma_supported.patch'

No problems with the PASEMI onboard ethernet and the P5020 board boots.

-- Christian

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

On 12 December 2018 at 3:15PM, Christoph Hellwig wrote:
 > Thanks for bisecting.  I've spent some time going over the conversion
 > but can't really pinpoint it.  I have three little patches that switch
 > parts of the code to the generic version.  This is on top of the
 > last good commmit (977706f9755d2d697aa6f45b4f9f0e07516efeda).
 >
 > Can you check with whіch one things stop working?

Hello Christoph,

Great news! All your patches work!

I tested all your patches (including the patch '0004-alloc-free.patch' 
today) and the PASEMI onboard ethernet works and the P5020 board boots 
without any problems. Thank you for your work!
I have a few days off. That means, I will work less and only for the 
A-EON first level Linux support. I can test again on Thursday next week.

Have a nice weekend!

Cheers,
Christian

Re: use generic DMA mapping code in powerpc V4

[Christoph Hellwig]

Hi Christian,

happy new year and I hope you had a few restful deays off.

I've pushed a new tree to:

   git://git.infradead.org/users/hch/misc.git powerpc-dma.6

Gitweb:

   http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6

Which has been rebased to the latests Linus tree, which has a lot of
changes, and has also changed the patch split a bit to aid bisection.

I think 

   http://git.infradead.org/users/hch/misc.git/commitdiff/c446404b041130fbd9d1772d184f24715cf2362f

might be a good commit to re-start testing, then bisecting up to the
last commit using git bisect.

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

Hi Christoph,

Happy new year for you too. Unfortunately we have some problems with the latest DRM patches. They modified a lot and some graphics cards don’t work anymore. During the holidays we tried to figure out where the problems are but without any success.

I will try to test your patches next week.

Cheers,
Christian

Sent from my iPhone

> On 3. Jan 2019, at 08:36, Christoph Hellwig <hch@lst.de> wrote:
> 
> Hi Christian,
> 
> happy new year and I hope you had a few restful deays off.
> 
> I've pushed a new tree to:
> 
>   git://git.infradead.org/users/hch/misc.git powerpc-dma.6
> 
> Gitweb:
> 
>   http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6
> 
> Which has been rebased to the latests Linus tree, which has a lot of
> changes, and has also changed the patch split a bit to aid bisection.
> 
> I think 
> 
>   http://git.infradead.org/users/hch/misc.git/commitdiff/c446404b041130fbd9d1772d184f24715cf2362f
> 
> might be a good commit to re-start testing, then bisecting up to the
> last commit using git bisect.

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

Next step: c446404b041130fbd9d1772d184f24715cf2362f (powerpc/dma: remove 
dma_nommu_mmap_coherent)

git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout c446404b041130fbd9d1772d184f24715cf2362f

Output:

Note: checking out 'c446404b041130fbd9d1772d184f24715cf2362f'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

   git checkout -b <new-branch-name>

HEAD is now at c446404... powerpc/dma: remove dma_nommu_mmap_coherent

-----

Link to the Git: 
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6

Result: PASEMI onboard ethernet works and the X5000 (P5020 board) boots.

-- Christian

Re: use generic DMA mapping code in powerpc V4

[Christian Zigotzky]

Next step: a64e18ba191ba9102fb174f27d707485ffd9389c (powerpc/dma: remove 
dma_nommu_get_required_mask)

git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a

git checkout a64e18ba191ba9102fb174f27d707485ffd9389c

Link to the Git: 
http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6

Results: PASEMI onboard ethernet works and the X5000 (P5020 board) 
boots. I also successfully tested sound, hardware 3D acceleration, 
Bluetooth, network, booting with a label etc. The uImages work also in a 
virtual e5500 quad-core QEMU machine.

-- Christian


On 05 January 2019 at 5:03PM, Christian Zigotzky wrote:
> Next step: c446404b041130fbd9d1772d184f24715cf2362f (powerpc/dma: 
> remove dma_nommu_mmap_coherent)
>
> git clone git://git.infradead.org/users/hch/misc.git -b powerpc-dma.6 a
>
> git checkout c446404b041130fbd9d1772d184f24715cf2362f
>
> Output:
>
> Note: checking out 'c446404b041130fbd9d1772d184f24715cf2362f'.
>
> You are in 'detached HEAD' state. You can look around, make experimental
> changes and commit them, and you can discard any commits you make in this
> state without impacting any branches by performing another checkout.
>
> If you want to create a new branch to retain commits you create, you may
> do so (now or later) by using -b with the checkout command again. 
> Example:
>
>   git checkout -b <new-branch-name>
>
> HEAD is now at c446404... powerpc/dma: remove dma_nommu_mmap_coherent
>
> -----
>
> Link to the Git: 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6
>
> Result: PASEMI onboard ethernet works and the X5000 (P5020 board) boots.
>
> -- Christian
>

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 05 November 2019 at 5:28 pm, Christoph Hellwig wrote:
> On Tue, Nov 05, 2019 at 08:56:27AM +0100, Christian Zigotzky wrote:
>> Hi All,
>>
>> We still have DMA problems with some PCI devices. Since the PowerPC updates
>> 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if we want to
>> work with our PCI devices. The FSL P5020 and P5040 have these problems
>> currently.
>>
>> Error message:
>>
>> [   25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 of DMA
>> mask ffffffff bus mask df000000
>>
>> All 5.x Linux kernels can't initialize a SCSI PCI card anymore so booting
>> of a Linux userland isn't possible.
>>
>> PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. The kernel
>> 4.20 works with all PCI devices without limitation of RAM.
> Can you send me the .config and a dmesg?  And in the meantime try the
> patch below?
>
> ---
> >From 4d659b7311bd4141fdd3eeeb80fa2d7602ea01d4 Mon Sep 17 00:00:00 2001
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Date: Fri, 18 Oct 2019 13:00:43 +0200
> Subject: dma-direct: check for overflows on 32 bit DMA addresses
>
> As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation it is
> possible for a device configured with 32 bit DMA addresses and a partial
> DMA mapping located at the end of the address space to overflow. It
> happens when a higher physical address, not DMAable, is translated to
> it's DMA counterpart.
>
> For example the Raspberry Pi 4, configurable up to 4 GB of memory, has
> an interconnect capable of addressing the lower 1 GB of physical memory
> with a DMA offset of 0xc0000000. It transpires that, any attempt to
> translate physical addresses higher than the first GB will result in an
> overflow which dma_capable() can't detect as it only checks for
> addresses bigger then the maximum allowed DMA address.
>
> Fix this by verifying in dma_capable() if the DMA address range provided
> is at any point lower than the minimum possible DMA address on the bus.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>   include/linux/dma-direct.h | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
> index adf993a3bd58..6ad9e9ea7564 100644
> --- a/include/linux/dma-direct.h
> +++ b/include/linux/dma-direct.h
> @@ -3,6 +3,7 @@
>   #define _LINUX_DMA_DIRECT_H 1
>   
>   #include <linux/dma-mapping.h>
> +#include <linux/memblock.h> /* for min_low_pfn */
>   #include <linux/mem_encrypt.h>
>   
>   #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
> @@ -27,6 +28,13 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size)
>   	if (!dev->dma_mask)
>   		return false;
>   
> +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +	/* Check if DMA address overflowed */
> +	if (min(addr, addr + size - 1) <
> +		__phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
> +		return false;
> +#endif
> +
>   	return addr + size - 1 <=
>   		min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>   }
Hello Christoph,

Thanks a lot for your patch! Unfortunately this patch doesn't solve the 
issue.

Error messages:

[    6.041163] bttv: driver version 0.9.19 loaded
[    6.041167] bttv: using 8 buffers with 2080k (520 pages) each for capture
[    6.041559] bttv: Bt8xx card found (0)
[    6.041609] bttv: 0: Bt878 (rev 17) at 1000:04:05.0, irq: 19, 
latency: 128, mmio: 0xc20001000
[    6.041622] bttv: 0: using: Typhoon TView RDS + FM Stereo / KNC1 TV 
Station RDS [card=53,insmod option]
[    6.042216] bttv: 0: tuner type=5
[    6.111994] bttv: 0: audio absent, no audio device found!
[    6.176425] bttv: 0: Setting PLL: 28636363 => 35468950 (needs up to 
100ms)
[    6.200005] bttv: PLL set ok
[    6.209351] bttv: 0: registered device video0
[    6.211576] bttv: 0: registered device vbi0
[    6.214897] bttv: 0: registered device radio0
[  114.218806] bttv 1000:04:05.0: overflow 0x00000000ff507000+4096 of 
DMA mask ffffffff bus mask df000000
[  114.218848] Modules linked in: rfcomm bnep tuner_simple tuner_types 
tea5767 tuner tda7432 tvaudio msp3400 bttv tea575x tveeprom 
videobuf_dma_sg videobuf_core rc_core videodev mc btusb btrtl btbcm 
btintel bluetooth uio_pdrv_genirq uio ecdh_generic ecc
[  114.219012] [c0000001ecddf720] [80000000008ff6e8] 
.buffer_prepare+0x150/0x268 [bttv]
[  114.219029] [c0000001ecddf860] [80000000008fff6c] 
.bttv_qbuf+0x50/0x64 [bttv]

-----

Trace:

[  462.783184] Call Trace:
[  462.783187] [c0000001c6c67420] [c0000000000b3358] 
.report_addr+0xb8/0xc0 (unreliable)
[  462.783192] [c0000001c6c67490] [c0000000000b351c] 
.dma_direct_map_page+0xf0/0x128
[  462.783195] [c0000001c6c67530] [c0000000000b35b0] 
.dma_direct_map_sg+0x5c/0xac
[  462.783205] [c0000001c6c675e0] [8000000000862e88] 
.__videobuf_iolock+0x660/0x6d8 [videobuf_dma_sg]
[  462.783220] [c0000001c6c676b0] [8000000000854274] 
.videobuf_iolock+0x98/0xb4 [videobuf_core]
[  462.783271] [c0000001c6c67720] [80000000008686e8] 
.buffer_prepare+0x150/0x268 [bttv]
[  462.783276] [c0000001c6c677c0] [8000000000854afc] 
.videobuf_qbuf+0x2b8/0x428 [videobuf_core]
[  462.783288] [c0000001c6c67860] [8000000000868f6c] 
.bttv_qbuf+0x50/0x64 [bttv]
[  462.783383] [c0000001c6c678e0] [80000000007bf208] .v4l_qbuf+0x54/0x60 
[videodev]
[  462.783402] [c0000001c6c67970] [80000000007c1eac] 
.__video_do_ioctl+0x30c/0x3f8 [videodev]
[  462.783421] [c0000001c6c67a80] [80000000007c3c08] 
.video_usercopy+0x18c/0x3dc [videodev]
[  462.783440] [c0000001c6c67c00] [80000000007bb14c] 
.v4l2_ioctl+0x60/0x78 [videodev]
[  462.783460] [c0000001c6c67c90] [80000000007d3c48] 
.v4l2_compat_ioctl32+0x9b4/0x1850 [videodev]
[  462.783468] [c0000001c6c67d70] [c0000000001ad9cc] 
.__se_compat_sys_ioctl+0x284/0x127c
[  462.783473] [c0000001c6c67e20] [c00000000000067c] system_call+0x60/0x6c
[  462.783475] Instruction dump:
[  462.783477] 40fe0044 60000000 892255d0 2f890000 40fe0020 3c82ffc5 
39200001 60000000
[  462.783483] 38842029 992255d0 485ad0d9 60000000 <0fe00000> 38210070 
e8010010 7c0803a6
[  462.783490] ---[ end trace b677d4a00458e277 ]---

-----

dmesg fsl p5040: https://bugzilla.kernel.org/attachment.cgi?id=285813

Kernel 5.4-rc6 config for the Cyrus+ board and for the QEMU ppce500 
board (CPU: P5040 and P5020): 
https://bugzilla.kernel.org/attachment.cgi?id=285815

Bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205201

Thanks for your help,

Christian

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 07 November 2019 at 10:53 am, Christian Zigotzky wrote:
> On 05 November 2019 at 5:28 pm, Christoph Hellwig wrote:
>> On Tue, Nov 05, 2019 at 08:56:27AM +0100, Christian Zigotzky wrote:
>>> Hi All,
>>>
>>> We still have DMA problems with some PCI devices. Since the PowerPC 
>>> updates
>>> 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if we 
>>> want to
>>> work with our PCI devices. The FSL P5020 and P5040 have these problems
>>> currently.
>>>
>>> Error message:
>>>
>>> [   25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 
>>> of DMA
>>> mask ffffffff bus mask df000000
>>>
>>> All 5.x Linux kernels can't initialize a SCSI PCI card anymore so 
>>> booting
>>> of a Linux userland isn't possible.
>>>
>>> PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. The 
>>> kernel
>>> 4.20 works with all PCI devices without limitation of RAM.
>> Can you send me the .config and a dmesg?  And in the meantime try the
>> patch below?
>>
>> ---
>> >From 4d659b7311bd4141fdd3eeeb80fa2d7602ea01d4 Mon Sep 17 00:00:00 2001
>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> Date: Fri, 18 Oct 2019 13:00:43 +0200
>> Subject: dma-direct: check for overflows on 32 bit DMA addresses
>>
>> As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation it is
>> possible for a device configured with 32 bit DMA addresses and a partial
>> DMA mapping located at the end of the address space to overflow. It
>> happens when a higher physical address, not DMAable, is translated to
>> it's DMA counterpart.
>>
>> For example the Raspberry Pi 4, configurable up to 4 GB of memory, has
>> an interconnect capable of addressing the lower 1 GB of physical memory
>> with a DMA offset of 0xc0000000. It transpires that, any attempt to
>> translate physical addresses higher than the first GB will result in an
>> overflow which dma_capable() can't detect as it only checks for
>> addresses bigger then the maximum allowed DMA address.
>>
>> Fix this by verifying in dma_capable() if the DMA address range provided
>> is at any point lower than the minimum possible DMA address on the bus.
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> ---
>>   include/linux/dma-direct.h | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>> index adf993a3bd58..6ad9e9ea7564 100644
>> --- a/include/linux/dma-direct.h
>> +++ b/include/linux/dma-direct.h
>> @@ -3,6 +3,7 @@
>>   #define _LINUX_DMA_DIRECT_H 1
>>     #include <linux/dma-mapping.h>
>> +#include <linux/memblock.h> /* for min_low_pfn */
>>   #include <linux/mem_encrypt.h>
>>     #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>> @@ -27,6 +28,13 @@ static inline bool dma_capable(struct device *dev, 
>> dma_addr_t addr, size_t size)
>>       if (!dev->dma_mask)
>>           return false;
>>   +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +    /* Check if DMA address overflowed */
>> +    if (min(addr, addr + size - 1) <
>> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
>> +        return false;
>> +#endif
>> +
>>       return addr + size - 1 <=
>>           min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>>   }
> Hello Christoph,
>
> Thanks a lot for your patch! Unfortunately this patch doesn't solve 
> the issue.
>
> Error messages:
>
> [    6.041163] bttv: driver version 0.9.19 loaded
> [    6.041167] bttv: using 8 buffers with 2080k (520 pages) each for 
> capture
> [    6.041559] bttv: Bt8xx card found (0)
> [    6.041609] bttv: 0: Bt878 (rev 17) at 1000:04:05.0, irq: 19, 
> latency: 128, mmio: 0xc20001000
> [    6.041622] bttv: 0: using: Typhoon TView RDS + FM Stereo / KNC1 TV 
> Station RDS [card=53,insmod option]
> [    6.042216] bttv: 0: tuner type=5
> [    6.111994] bttv: 0: audio absent, no audio device found!
> [    6.176425] bttv: 0: Setting PLL: 28636363 => 35468950 (needs up to 
> 100ms)
> [    6.200005] bttv: PLL set ok
> [    6.209351] bttv: 0: registered device video0
> [    6.211576] bttv: 0: registered device vbi0
> [    6.214897] bttv: 0: registered device radio0
> [  114.218806] bttv 1000:04:05.0: overflow 0x00000000ff507000+4096 of 
> DMA mask ffffffff bus mask df000000
> [  114.218848] Modules linked in: rfcomm bnep tuner_simple tuner_types 
> tea5767 tuner tda7432 tvaudio msp3400 bttv tea575x tveeprom 
> videobuf_dma_sg videobuf_core rc_core videodev mc btusb btrtl btbcm 
> btintel bluetooth uio_pdrv_genirq uio ecdh_generic ecc
> [  114.219012] [c0000001ecddf720] [80000000008ff6e8] 
> .buffer_prepare+0x150/0x268 [bttv]
> [  114.219029] [c0000001ecddf860] [80000000008fff6c] 
> .bttv_qbuf+0x50/0x64 [bttv]
>
> -----
>
> Trace:
>
> [  462.783184] Call Trace:
> [  462.783187] [c0000001c6c67420] [c0000000000b3358] 
> .report_addr+0xb8/0xc0 (unreliable)
> [  462.783192] [c0000001c6c67490] [c0000000000b351c] 
> .dma_direct_map_page+0xf0/0x128
> [  462.783195] [c0000001c6c67530] [c0000000000b35b0] 
> .dma_direct_map_sg+0x5c/0xac
> [  462.783205] [c0000001c6c675e0] [8000000000862e88] 
> .__videobuf_iolock+0x660/0x6d8 [videobuf_dma_sg]
> [  462.783220] [c0000001c6c676b0] [8000000000854274] 
> .videobuf_iolock+0x98/0xb4 [videobuf_core]
> [  462.783271] [c0000001c6c67720] [80000000008686e8] 
> .buffer_prepare+0x150/0x268 [bttv]
> [  462.783276] [c0000001c6c677c0] [8000000000854afc] 
> .videobuf_qbuf+0x2b8/0x428 [videobuf_core]
> [  462.783288] [c0000001c6c67860] [8000000000868f6c] 
> .bttv_qbuf+0x50/0x64 [bttv]
> [  462.783383] [c0000001c6c678e0] [80000000007bf208] 
> .v4l_qbuf+0x54/0x60 [videodev]
> [  462.783402] [c0000001c6c67970] [80000000007c1eac] 
> .__video_do_ioctl+0x30c/0x3f8 [videodev]
> [  462.783421] [c0000001c6c67a80] [80000000007c3c08] 
> .video_usercopy+0x18c/0x3dc [videodev]
> [  462.783440] [c0000001c6c67c00] [80000000007bb14c] 
> .v4l2_ioctl+0x60/0x78 [videodev]
> [  462.783460] [c0000001c6c67c90] [80000000007d3c48] 
> .v4l2_compat_ioctl32+0x9b4/0x1850 [videodev]
> [  462.783468] [c0000001c6c67d70] [c0000000001ad9cc] 
> .__se_compat_sys_ioctl+0x284/0x127c
> [  462.783473] [c0000001c6c67e20] [c00000000000067c] 
> system_call+0x60/0x6c
> [  462.783475] Instruction dump:
> [  462.783477] 40fe0044 60000000 892255d0 2f890000 40fe0020 3c82ffc5 
> 39200001 60000000
> [  462.783483] 38842029 992255d0 485ad0d9 60000000 <0fe00000> 38210070 
> e8010010 7c0803a6
> [  462.783490] ---[ end trace b677d4a00458e277 ]---
>
> -----
>
> dmesg fsl p5040: https://bugzilla.kernel.org/attachment.cgi?id=285813
>
> Kernel 5.4-rc6 config for the Cyrus+ board and for the QEMU ppce500 
> board (CPU: P5040 and P5020): 
> https://bugzilla.kernel.org/attachment.cgi?id=285815
>
> Bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205201
>
> Thanks for your help,
>
> Christian

Christoph,

Do you have another patch for testing or shall I bisect?

Thanks,
Christian

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 10 November 2019 at 08:27 am, Christian Zigotzky wrote:
> On 07 November 2019 at 10:53 am, Christian Zigotzky wrote:
>> On 05 November 2019 at 05:28 pm, Christoph Hellwig wrote:
>>> On Tue, Nov 05, 2019 at 08:56:27AM +0100, Christian Zigotzky wrote:
>>>> Hi All,
>>>>
>>>> We still have DMA problems with some PCI devices. Since the PowerPC 
>>>> updates
>>>> 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if we 
>>>> want to
>>>> work with our PCI devices. The FSL P5020 and P5040 have these problems
>>>> currently.
>>>>
>>>> Error message:
>>>>
>>>> [   25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 
>>>> of DMA
>>>> mask ffffffff bus mask df000000
>>>>
>>>> All 5.x Linux kernels can't initialize a SCSI PCI card anymore so 
>>>> booting
>>>> of a Linux userland isn't possible.
>>>>
>>>> PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. The 
>>>> kernel
>>>> 4.20 works with all PCI devices without limitation of RAM.
>>> Can you send me the .config and a dmesg?  And in the meantime try the
>>> patch below?
>>>
>>> ---
>>> >From 4d659b7311bd4141fdd3eeeb80fa2d7602ea01d4 Mon Sep 17 00:00:00 2001
>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>> Date: Fri, 18 Oct 2019 13:00:43 +0200
>>> Subject: dma-direct: check for overflows on 32 bit DMA addresses
>>>
>>> As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation 
>>> it is
>>> possible for a device configured with 32 bit DMA addresses and a 
>>> partial
>>> DMA mapping located at the end of the address space to overflow. It
>>> happens when a higher physical address, not DMAable, is translated to
>>> it's DMA counterpart.
>>>
>>> For example the Raspberry Pi 4, configurable up to 4 GB of memory, has
>>> an interconnect capable of addressing the lower 1 GB of physical memory
>>> with a DMA offset of 0xc0000000. It transpires that, any attempt to
>>> translate physical addresses higher than the first GB will result in an
>>> overflow which dma_capable() can't detect as it only checks for
>>> addresses bigger then the maximum allowed DMA address.
>>>
>>> Fix this by verifying in dma_capable() if the DMA address range 
>>> provided
>>> is at any point lower than the minimum possible DMA address on the bus.
>>>
>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>> ---
>>>   include/linux/dma-direct.h | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>> index adf993a3bd58..6ad9e9ea7564 100644
>>> --- a/include/linux/dma-direct.h
>>> +++ b/include/linux/dma-direct.h
>>> @@ -3,6 +3,7 @@
>>>   #define _LINUX_DMA_DIRECT_H 1
>>>     #include <linux/dma-mapping.h>
>>> +#include <linux/memblock.h> /* for min_low_pfn */
>>>   #include <linux/mem_encrypt.h>
>>>     #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>> @@ -27,6 +28,13 @@ static inline bool dma_capable(struct device 
>>> *dev, dma_addr_t addr, size_t size)
>>>       if (!dev->dma_mask)
>>>           return false;
>>>   +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>> +    /* Check if DMA address overflowed */
>>> +    if (min(addr, addr + size - 1) <
>>> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
>>> +        return false;
>>> +#endif
>>> +
>>>       return addr + size - 1 <=
>>>           min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>>>   }
>> Hello Christoph,
>>
>> Thanks a lot for your patch! Unfortunately this patch doesn't solve 
>> the issue.
>>
>> Error messages:
>>
>> [    6.041163] bttv: driver version 0.9.19 loaded
>> [    6.041167] bttv: using 8 buffers with 2080k (520 pages) each for 
>> capture
>> [    6.041559] bttv: Bt8xx card found (0)
>> [    6.041609] bttv: 0: Bt878 (rev 17) at 1000:04:05.0, irq: 19, 
>> latency: 128, mmio: 0xc20001000
>> [    6.041622] bttv: 0: using: Typhoon TView RDS + FM Stereo / KNC1 
>> TV Station RDS [card=53,insmod option]
>> [    6.042216] bttv: 0: tuner type=5
>> [    6.111994] bttv: 0: audio absent, no audio device found!
>> [    6.176425] bttv: 0: Setting PLL: 28636363 => 35468950 (needs up 
>> to 100ms)
>> [    6.200005] bttv: PLL set ok
>> [    6.209351] bttv: 0: registered device video0
>> [    6.211576] bttv: 0: registered device vbi0
>> [    6.214897] bttv: 0: registered device radio0
>> [  114.218806] bttv 1000:04:05.0: overflow 0x00000000ff507000+4096 of 
>> DMA mask ffffffff bus mask df000000
>> [  114.218848] Modules linked in: rfcomm bnep tuner_simple 
>> tuner_types tea5767 tuner tda7432 tvaudio msp3400 bttv tea575x 
>> tveeprom videobuf_dma_sg videobuf_core rc_core videodev mc btusb 
>> btrtl btbcm btintel bluetooth uio_pdrv_genirq uio ecdh_generic ecc
>> [  114.219012] [c0000001ecddf720] [80000000008ff6e8] 
>> .buffer_prepare+0x150/0x268 [bttv]
>> [  114.219029] [c0000001ecddf860] [80000000008fff6c] 
>> .bttv_qbuf+0x50/0x64 [bttv]
>>
>> -----
>>
>> Trace:
>>
>> [  462.783184] Call Trace:
>> [  462.783187] [c0000001c6c67420] [c0000000000b3358] 
>> .report_addr+0xb8/0xc0 (unreliable)
>> [  462.783192] [c0000001c6c67490] [c0000000000b351c] 
>> .dma_direct_map_page+0xf0/0x128
>> [  462.783195] [c0000001c6c67530] [c0000000000b35b0] 
>> .dma_direct_map_sg+0x5c/0xac
>> [  462.783205] [c0000001c6c675e0] [8000000000862e88] 
>> .__videobuf_iolock+0x660/0x6d8 [videobuf_dma_sg]
>> [  462.783220] [c0000001c6c676b0] [8000000000854274] 
>> .videobuf_iolock+0x98/0xb4 [videobuf_core]
>> [  462.783271] [c0000001c6c67720] [80000000008686e8] 
>> .buffer_prepare+0x150/0x268 [bttv]
>> [  462.783276] [c0000001c6c677c0] [8000000000854afc] 
>> .videobuf_qbuf+0x2b8/0x428 [videobuf_core]
>> [  462.783288] [c0000001c6c67860] [8000000000868f6c] 
>> .bttv_qbuf+0x50/0x64 [bttv]
>> [  462.783383] [c0000001c6c678e0] [80000000007bf208] 
>> .v4l_qbuf+0x54/0x60 [videodev]
>> [  462.783402] [c0000001c6c67970] [80000000007c1eac] 
>> .__video_do_ioctl+0x30c/0x3f8 [videodev]
>> [  462.783421] [c0000001c6c67a80] [80000000007c3c08] 
>> .video_usercopy+0x18c/0x3dc [videodev]
>> [  462.783440] [c0000001c6c67c00] [80000000007bb14c] 
>> .v4l2_ioctl+0x60/0x78 [videodev]
>> [  462.783460] [c0000001c6c67c90] [80000000007d3c48] 
>> .v4l2_compat_ioctl32+0x9b4/0x1850 [videodev]
>> [  462.783468] [c0000001c6c67d70] [c0000000001ad9cc] 
>> .__se_compat_sys_ioctl+0x284/0x127c
>> [  462.783473] [c0000001c6c67e20] [c00000000000067c] 
>> system_call+0x60/0x6c
>> [  462.783475] Instruction dump:
>> [  462.783477] 40fe0044 60000000 892255d0 2f890000 40fe0020 3c82ffc5 
>> 39200001 60000000
>> [  462.783483] 38842029 992255d0 485ad0d9 60000000 <0fe00000> 
>> 38210070 e8010010 7c0803a6
>> [  462.783490] ---[ end trace b677d4a00458e277 ]---
>>
>> -----
>>
>> dmesg fsl p5040: https://bugzilla.kernel.org/attachment.cgi?id=285813
>>
>> Kernel 5.4-rc6 config for the Cyrus+ board and for the QEMU ppce500 
>> board (CPU: P5040 and P5020): 
>> https://bugzilla.kernel.org/attachment.cgi?id=285815
>>
>> Bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205201
>>
>> Thanks for your help,
>>
>> Christian
>
> Christoph,
>
> Do you have another patch for testing or shall I bisect?
>
> Thanks,
> Christian

Hi Christoph,

I have seen that I have activated the kernel config option 
CONFIG_ARCH_DMA_ADDR_T_64BIT. That means your code in your patch won't 
work if this kernel option is enabled.

+#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
+    /* Check if DMA address overflowed */
+    if (min(addr, addr + size - 1) <
+        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
+        return false;
+#endif

I will delete the lines with ifndef and endif and will try it again.

Cheers,
Christian

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 11 November 2019 at 09:12 am, Christian Zigotzky wrote:
> On 10 November 2019 at 08:27 am, Christian Zigotzky wrote:
>> On 07 November 2019 at 10:53 am, Christian Zigotzky wrote:
>>> On 05 November 2019 at 05:28 pm, Christoph Hellwig wrote:
>>>> On Tue, Nov 05, 2019 at 08:56:27AM +0100, Christian Zigotzky wrote:
>>>>> Hi All,
>>>>>
>>>>> We still have DMA problems with some PCI devices. Since the 
>>>>> PowerPC updates
>>>>> 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if we 
>>>>> want to
>>>>> work with our PCI devices. The FSL P5020 and P5040 have these 
>>>>> problems
>>>>> currently.
>>>>>
>>>>> Error message:
>>>>>
>>>>> [   25.654852] bttv 1000:04:05.0: overflow 0x00000000fe077000+4096 
>>>>> of DMA
>>>>> mask ffffffff bus mask df000000
>>>>>
>>>>> All 5.x Linux kernels can't initialize a SCSI PCI card anymore so 
>>>>> booting
>>>>> of a Linux userland isn't possible.
>>>>>
>>>>> PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. 
>>>>> The kernel
>>>>> 4.20 works with all PCI devices without limitation of RAM.
>>>> Can you send me the .config and a dmesg?  And in the meantime try the
>>>> patch below?
>>>>
>>>> ---
>>>> >From 4d659b7311bd4141fdd3eeeb80fa2d7602ea01d4 Mon Sep 17 00:00:00 
>>>> 2001
>>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>> Date: Fri, 18 Oct 2019 13:00:43 +0200
>>>> Subject: dma-direct: check for overflows on 32 bit DMA addresses
>>>>
>>>> As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation 
>>>> it is
>>>> possible for a device configured with 32 bit DMA addresses and a 
>>>> partial
>>>> DMA mapping located at the end of the address space to overflow. It
>>>> happens when a higher physical address, not DMAable, is translated to
>>>> it's DMA counterpart.
>>>>
>>>> For example the Raspberry Pi 4, configurable up to 4 GB of memory, has
>>>> an interconnect capable of addressing the lower 1 GB of physical 
>>>> memory
>>>> with a DMA offset of 0xc0000000. It transpires that, any attempt to
>>>> translate physical addresses higher than the first GB will result 
>>>> in an
>>>> overflow which dma_capable() can't detect as it only checks for
>>>> addresses bigger then the maximum allowed DMA address.
>>>>
>>>> Fix this by verifying in dma_capable() if the DMA address range 
>>>> provided
>>>> is at any point lower than the minimum possible DMA address on the 
>>>> bus.
>>>>
>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>> ---
>>>>   include/linux/dma-direct.h | 8 ++++++++
>>>>   1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>>> index adf993a3bd58..6ad9e9ea7564 100644
>>>> --- a/include/linux/dma-direct.h
>>>> +++ b/include/linux/dma-direct.h
>>>> @@ -3,6 +3,7 @@
>>>>   #define _LINUX_DMA_DIRECT_H 1
>>>>     #include <linux/dma-mapping.h>
>>>> +#include <linux/memblock.h> /* for min_low_pfn */
>>>>   #include <linux/mem_encrypt.h>
>>>>     #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>>> @@ -27,6 +28,13 @@ static inline bool dma_capable(struct device 
>>>> *dev, dma_addr_t addr, size_t size)
>>>>       if (!dev->dma_mask)
>>>>           return false;
>>>>   +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>> +    /* Check if DMA address overflowed */
>>>> +    if (min(addr, addr + size - 1) <
>>>> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
>>>> +        return false;
>>>> +#endif
>>>> +
>>>>       return addr + size - 1 <=
>>>>           min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>>>>   }
>>> Hello Christoph,
>>>
>>> Thanks a lot for your patch! Unfortunately this patch doesn't solve 
>>> the issue.
>>>
>>> Error messages:
>>>
>>> [    6.041163] bttv: driver version 0.9.19 loaded
>>> [    6.041167] bttv: using 8 buffers with 2080k (520 pages) each for 
>>> capture
>>> [    6.041559] bttv: Bt8xx card found (0)
>>> [    6.041609] bttv: 0: Bt878 (rev 17) at 1000:04:05.0, irq: 19, 
>>> latency: 128, mmio: 0xc20001000
>>> [    6.041622] bttv: 0: using: Typhoon TView RDS + FM Stereo / KNC1 
>>> TV Station RDS [card=53,insmod option]
>>> [    6.042216] bttv: 0: tuner type=5
>>> [    6.111994] bttv: 0: audio absent, no audio device found!
>>> [    6.176425] bttv: 0: Setting PLL: 28636363 => 35468950 (needs up 
>>> to 100ms)
>>> [    6.200005] bttv: PLL set ok
>>> [    6.209351] bttv: 0: registered device video0
>>> [    6.211576] bttv: 0: registered device vbi0
>>> [    6.214897] bttv: 0: registered device radio0
>>> [  114.218806] bttv 1000:04:05.0: overflow 0x00000000ff507000+4096 
>>> of DMA mask ffffffff bus mask df000000
>>> [  114.218848] Modules linked in: rfcomm bnep tuner_simple 
>>> tuner_types tea5767 tuner tda7432 tvaudio msp3400 bttv tea575x 
>>> tveeprom videobuf_dma_sg videobuf_core rc_core videodev mc btusb 
>>> btrtl btbcm btintel bluetooth uio_pdrv_genirq uio ecdh_generic ecc
>>> [  114.219012] [c0000001ecddf720] [80000000008ff6e8] 
>>> .buffer_prepare+0x150/0x268 [bttv]
>>> [  114.219029] [c0000001ecddf860] [80000000008fff6c] 
>>> .bttv_qbuf+0x50/0x64 [bttv]
>>>
>>> -----
>>>
>>> Trace:
>>>
>>> [  462.783184] Call Trace:
>>> [  462.783187] [c0000001c6c67420] [c0000000000b3358] 
>>> .report_addr+0xb8/0xc0 (unreliable)
>>> [  462.783192] [c0000001c6c67490] [c0000000000b351c] 
>>> .dma_direct_map_page+0xf0/0x128
>>> [  462.783195] [c0000001c6c67530] [c0000000000b35b0] 
>>> .dma_direct_map_sg+0x5c/0xac
>>> [  462.783205] [c0000001c6c675e0] [8000000000862e88] 
>>> .__videobuf_iolock+0x660/0x6d8 [videobuf_dma_sg]
>>> [  462.783220] [c0000001c6c676b0] [8000000000854274] 
>>> .videobuf_iolock+0x98/0xb4 [videobuf_core]
>>> [  462.783271] [c0000001c6c67720] [80000000008686e8] 
>>> .buffer_prepare+0x150/0x268 [bttv]
>>> [  462.783276] [c0000001c6c677c0] [8000000000854afc] 
>>> .videobuf_qbuf+0x2b8/0x428 [videobuf_core]
>>> [  462.783288] [c0000001c6c67860] [8000000000868f6c] 
>>> .bttv_qbuf+0x50/0x64 [bttv]
>>> [  462.783383] [c0000001c6c678e0] [80000000007bf208] 
>>> .v4l_qbuf+0x54/0x60 [videodev]
>>> [  462.783402] [c0000001c6c67970] [80000000007c1eac] 
>>> .__video_do_ioctl+0x30c/0x3f8 [videodev]
>>> [  462.783421] [c0000001c6c67a80] [80000000007c3c08] 
>>> .video_usercopy+0x18c/0x3dc [videodev]
>>> [  462.783440] [c0000001c6c67c00] [80000000007bb14c] 
>>> .v4l2_ioctl+0x60/0x78 [videodev]
>>> [  462.783460] [c0000001c6c67c90] [80000000007d3c48] 
>>> .v4l2_compat_ioctl32+0x9b4/0x1850 [videodev]
>>> [  462.783468] [c0000001c6c67d70] [c0000000001ad9cc] 
>>> .__se_compat_sys_ioctl+0x284/0x127c
>>> [  462.783473] [c0000001c6c67e20] [c00000000000067c] 
>>> system_call+0x60/0x6c
>>> [  462.783475] Instruction dump:
>>> [  462.783477] 40fe0044 60000000 892255d0 2f890000 40fe0020 3c82ffc5 
>>> 39200001 60000000
>>> [  462.783483] 38842029 992255d0 485ad0d9 60000000 <0fe00000> 
>>> 38210070 e8010010 7c0803a6
>>> [  462.783490] ---[ end trace b677d4a00458e277 ]---
>>>
>>> -----
>>>
>>> dmesg fsl p5040: https://bugzilla.kernel.org/attachment.cgi?id=285813
>>>
>>> Kernel 5.4-rc6 config for the Cyrus+ board and for the QEMU ppce500 
>>> board (CPU: P5040 and P5020): 
>>> https://bugzilla.kernel.org/attachment.cgi?id=285815
>>>
>>> Bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205201
>>>
>>> Thanks for your help,
>>>
>>> Christian
>>
>> Christoph,
>>
>> Do you have another patch for testing or shall I bisect?
>>
>> Thanks,
>> Christian
>
> Hi Christoph,
>
> I have seen that I have activated the kernel config option 
> CONFIG_ARCH_DMA_ADDR_T_64BIT. That means your code in your patch won't 
> work if this kernel option is enabled.
>
> +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +    /* Check if DMA address overflowed */
> +    if (min(addr, addr + size - 1) <
> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
> +        return false;
> +#endif
>
> I will delete the lines with ifndef and endif and will try it again.
>
> Cheers,
> Christian

Christoph,

I have seen that the patch above isn't from you. It's from Nicolas Saenz 
Julienne.

Cheers,
Christian

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 11 November 2019 at 09:16 am, Christian Zigotzky wrote:
> On 11 November 2019 at 09:12 am, Christian Zigotzky wrote:
>> On 10 November 2019 at 08:27 am, Christian Zigotzky wrote:
>>> On 07 November 2019 at 10:53 am, Christian Zigotzky wrote:
>>>> On 05 November 2019 at 05:28 pm, Christoph Hellwig wrote:
>>>>> On Tue, Nov 05, 2019 at 08:56:27AM +0100, Christian Zigotzky wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> We still have DMA problems with some PCI devices. Since the 
>>>>>> PowerPC updates
>>>>>> 4.21-1 [1] we need to decrease the RAM to 3500MB (mem=3500M) if 
>>>>>> we want to
>>>>>> work with our PCI devices. The FSL P5020 and P5040 have these 
>>>>>> problems
>>>>>> currently.
>>>>>>
>>>>>> Error message:
>>>>>>
>>>>>> [   25.654852] bttv 1000:04:05.0: overflow 
>>>>>> 0x00000000fe077000+4096 of DMA
>>>>>> mask ffffffff bus mask df000000
>>>>>>
>>>>>> All 5.x Linux kernels can't initialize a SCSI PCI card anymore so 
>>>>>> booting
>>>>>> of a Linux userland isn't possible.
>>>>>>
>>>>>> PLEASE check the DMA changes in the PowerPC updates 4.21-1 [1]. 
>>>>>> The kernel
>>>>>> 4.20 works with all PCI devices without limitation of RAM.
>>>>> Can you send me the .config and a dmesg?  And in the meantime try the
>>>>> patch below?
>>>>>
>>>>> ---
>>>>> >From 4d659b7311bd4141fdd3eeeb80fa2d7602ea01d4 Mon Sep 17 00:00:00 
>>>>> 2001
>>>>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>>> Date: Fri, 18 Oct 2019 13:00:43 +0200
>>>>> Subject: dma-direct: check for overflows on 32 bit DMA addresses
>>>>>
>>>>> As seen on the new Raspberry Pi 4 and sta2x11's DMA implementation 
>>>>> it is
>>>>> possible for a device configured with 32 bit DMA addresses and a 
>>>>> partial
>>>>> DMA mapping located at the end of the address space to overflow. It
>>>>> happens when a higher physical address, not DMAable, is translated to
>>>>> it's DMA counterpart.
>>>>>
>>>>> For example the Raspberry Pi 4, configurable up to 4 GB of memory, 
>>>>> has
>>>>> an interconnect capable of addressing the lower 1 GB of physical 
>>>>> memory
>>>>> with a DMA offset of 0xc0000000. It transpires that, any attempt to
>>>>> translate physical addresses higher than the first GB will result 
>>>>> in an
>>>>> overflow which dma_capable() can't detect as it only checks for
>>>>> addresses bigger then the maximum allowed DMA address.
>>>>>
>>>>> Fix this by verifying in dma_capable() if the DMA address range 
>>>>> provided
>>>>> is at any point lower than the minimum possible DMA address on the 
>>>>> bus.
>>>>>
>>>>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>>>> ---
>>>>>   include/linux/dma-direct.h | 8 ++++++++
>>>>>   1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/include/linux/dma-direct.h b/include/linux/dma-direct.h
>>>>> index adf993a3bd58..6ad9e9ea7564 100644
>>>>> --- a/include/linux/dma-direct.h
>>>>> +++ b/include/linux/dma-direct.h
>>>>> @@ -3,6 +3,7 @@
>>>>>   #define _LINUX_DMA_DIRECT_H 1
>>>>>     #include <linux/dma-mapping.h>
>>>>> +#include <linux/memblock.h> /* for min_low_pfn */
>>>>>   #include <linux/mem_encrypt.h>
>>>>>     #ifdef CONFIG_ARCH_HAS_PHYS_TO_DMA
>>>>> @@ -27,6 +28,13 @@ static inline bool dma_capable(struct device 
>>>>> *dev, dma_addr_t addr, size_t size)
>>>>>       if (!dev->dma_mask)
>>>>>           return false;
>>>>>   +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>> +    /* Check if DMA address overflowed */
>>>>> +    if (min(addr, addr + size - 1) <
>>>>> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << 
>>>>> PAGE_SHIFT)))
>>>>> +        return false;
>>>>> +#endif
>>>>> +
>>>>>       return addr + size - 1 <=
>>>>>           min_not_zero(*dev->dma_mask, dev->bus_dma_mask);
>>>>>   }
>>>> Hello Christoph,
>>>>
>>>> Thanks a lot for your patch! Unfortunately this patch doesn't solve 
>>>> the issue.
>>>>
>>>> Error messages:
>>>>
>>>> [    6.041163] bttv: driver version 0.9.19 loaded
>>>> [    6.041167] bttv: using 8 buffers with 2080k (520 pages) each 
>>>> for capture
>>>> [    6.041559] bttv: Bt8xx card found (0)
>>>> [    6.041609] bttv: 0: Bt878 (rev 17) at 1000:04:05.0, irq: 19, 
>>>> latency: 128, mmio: 0xc20001000
>>>> [    6.041622] bttv: 0: using: Typhoon TView RDS + FM Stereo / KNC1 
>>>> TV Station RDS [card=53,insmod option]
>>>> [    6.042216] bttv: 0: tuner type=5
>>>> [    6.111994] bttv: 0: audio absent, no audio device found!
>>>> [    6.176425] bttv: 0: Setting PLL: 28636363 => 35468950 (needs up 
>>>> to 100ms)
>>>> [    6.200005] bttv: PLL set ok
>>>> [    6.209351] bttv: 0: registered device video0
>>>> [    6.211576] bttv: 0: registered device vbi0
>>>> [    6.214897] bttv: 0: registered device radio0
>>>> [  114.218806] bttv 1000:04:05.0: overflow 0x00000000ff507000+4096 
>>>> of DMA mask ffffffff bus mask df000000
>>>> [  114.218848] Modules linked in: rfcomm bnep tuner_simple 
>>>> tuner_types tea5767 tuner tda7432 tvaudio msp3400 bttv tea575x 
>>>> tveeprom videobuf_dma_sg videobuf_core rc_core videodev mc btusb 
>>>> btrtl btbcm btintel bluetooth uio_pdrv_genirq uio ecdh_generic ecc
>>>> [  114.219012] [c0000001ecddf720] [80000000008ff6e8] 
>>>> .buffer_prepare+0x150/0x268 [bttv]
>>>> [  114.219029] [c0000001ecddf860] [80000000008fff6c] 
>>>> .bttv_qbuf+0x50/0x64 [bttv]
>>>>
>>>> -----
>>>>
>>>> Trace:
>>>>
>>>> [  462.783184] Call Trace:
>>>> [  462.783187] [c0000001c6c67420] [c0000000000b3358] 
>>>> .report_addr+0xb8/0xc0 (unreliable)
>>>> [  462.783192] [c0000001c6c67490] [c0000000000b351c] 
>>>> .dma_direct_map_page+0xf0/0x128
>>>> [  462.783195] [c0000001c6c67530] [c0000000000b35b0] 
>>>> .dma_direct_map_sg+0x5c/0xac
>>>> [  462.783205] [c0000001c6c675e0] [8000000000862e88] 
>>>> .__videobuf_iolock+0x660/0x6d8 [videobuf_dma_sg]
>>>> [  462.783220] [c0000001c6c676b0] [8000000000854274] 
>>>> .videobuf_iolock+0x98/0xb4 [videobuf_core]
>>>> [  462.783271] [c0000001c6c67720] [80000000008686e8] 
>>>> .buffer_prepare+0x150/0x268 [bttv]
>>>> [  462.783276] [c0000001c6c677c0] [8000000000854afc] 
>>>> .videobuf_qbuf+0x2b8/0x428 [videobuf_core]
>>>> [  462.783288] [c0000001c6c67860] [8000000000868f6c] 
>>>> .bttv_qbuf+0x50/0x64 [bttv]
>>>> [  462.783383] [c0000001c6c678e0] [80000000007bf208] 
>>>> .v4l_qbuf+0x54/0x60 [videodev]
>>>> [  462.783402] [c0000001c6c67970] [80000000007c1eac] 
>>>> .__video_do_ioctl+0x30c/0x3f8 [videodev]
>>>> [  462.783421] [c0000001c6c67a80] [80000000007c3c08] 
>>>> .video_usercopy+0x18c/0x3dc [videodev]
>>>> [  462.783440] [c0000001c6c67c00] [80000000007bb14c] 
>>>> .v4l2_ioctl+0x60/0x78 [videodev]
>>>> [  462.783460] [c0000001c6c67c90] [80000000007d3c48] 
>>>> .v4l2_compat_ioctl32+0x9b4/0x1850 [videodev]
>>>> [  462.783468] [c0000001c6c67d70] [c0000000001ad9cc] 
>>>> .__se_compat_sys_ioctl+0x284/0x127c
>>>> [  462.783473] [c0000001c6c67e20] [c00000000000067c] 
>>>> system_call+0x60/0x6c
>>>> [  462.783475] Instruction dump:
>>>> [  462.783477] 40fe0044 60000000 892255d0 2f890000 40fe0020 
>>>> 3c82ffc5 39200001 60000000
>>>> [  462.783483] 38842029 992255d0 485ad0d9 60000000 <0fe00000> 
>>>> 38210070 e8010010 7c0803a6
>>>> [  462.783490] ---[ end trace b677d4a00458e277 ]---
>>>>
>>>> -----
>>>>
>>>> dmesg fsl p5040: https://bugzilla.kernel.org/attachment.cgi?id=285813
>>>>
>>>> Kernel 5.4-rc6 config for the Cyrus+ board and for the QEMU ppce500 
>>>> board (CPU: P5040 and P5020): 
>>>> https://bugzilla.kernel.org/attachment.cgi?id=285815
>>>>
>>>> Bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205201
>>>>
>>>> Thanks for your help,
>>>>
>>>> Christian
>>>
>>> Christoph,
>>>
>>> Do you have another patch for testing or shall I bisect?
>>>
>>> Thanks,
>>> Christian
>>
>> Hi Christoph,
>>
>> I have seen that I have activated the kernel config option 
>> CONFIG_ARCH_DMA_ADDR_T_64BIT. That means your code in your patch 
>> won't work if this kernel option is enabled.
>>
>> +#ifndef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +    /* Check if DMA address overflowed */
>> +    if (min(addr, addr + size - 1) <
>> +        __phys_to_dma(dev, (phys_addr_t)(min_low_pfn << PAGE_SHIFT)))
>> +        return false;
>> +#endif
>>
>> I will delete the lines with ifndef and endif and will try it again.
>>
>> Cheers,
>> Christian
>
> Christoph,
>
> I have seen that the patch above isn't from you. It's from Nicolas 
> Saenz Julienne.
>
> Cheers,
> Christian

Christoph,

Now, I can definitely say that this patch does not solve the issue.

Do you have another patch for testing or shall I bisect?

Thanks,
Christian

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christoph Hellwig]

On Mon, Nov 11, 2019 at 01:22:55PM +0100, Christian Zigotzky wrote:
> Now, I can definitely say that this patch does not solve the issue.
>
> Do you have another patch for testing or shall I bisect?

I'm still interested in the .config and dmesg.  Especially if the
board is using arch/powerpc/sysdev/fsl_pci.c, which is the only
place inside the powerpc arch code doing funny things with the
bus_dma_mask, which Robin pointed out looks fishy.

>
> Thanks,
> Christian
---end quoted text---

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

On 12 November 2019 at 3:41 pm, Christoph Hellwig wrote:
> On Mon, Nov 11, 2019 at 01:22:55PM +0100, Christian Zigotzky wrote:
>> Now, I can definitely say that this patch does not solve the issue.
>>
>> Do you have another patch for testing or shall I bisect?
> I'm still interested in the .config and dmesg.  Especially if the
> board is using arch/powerpc/sysdev/fsl_pci.c, which is the only
> place inside the powerpc arch code doing funny things with the
> bus_dma_mask, which Robin pointed out looks fishy.
>
Here you are:

.config: https://bugzilla.kernel.org/attachment.cgi?id=285815

dmesg: https://bugzilla.kernel.org/attachment.cgi?id=285813

Thanks

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christian Zigotzky]

Hello Christoph,

I have found the issue. :-)

GFP_DMA32 was renamed to GFP_DMA in the PowerPC updates 4.21-1 in 
December last year.

Some PCI devices still use GFP_DMA32 (grep -r GFP_DMA32 *). I renamed 
GFP_DMA32 to GFP_DMA in the file 
"drivers/media/v4l2-core/videobuf-dma-sg.c". After compiling the RC7 of 
kernel 5.4 my TV card works again.

Cheers,
Christian


On 12 November 2019 at 3:41 pm, Christoph Hellwig wrote:
> On Mon, Nov 11, 2019 at 01:22:55PM +0100, Christian Zigotzky wrote:
>> Now, I can definitely say that this patch does not solve the issue.
>>
>> Do you have another patch for testing or shall I bisect?
> I'm still interested in the .config and dmesg.  Especially if the
> board is using arch/powerpc/sysdev/fsl_pci.c, which is the only
> place inside the powerpc arch code doing funny things with the
> bus_dma_mask, which Robin pointed out looks fishy.
>
>> Thanks,
>> Christian
> ---end quoted text---
>

Re: Bug 205201 - overflow of DMA mask and bus mask

[Christoph Hellwig]

Interesting.  Give me some time to come up with a real fix, as drivers
really should not mess with GFP flags for these allocations, and even
if they did swiotlb is supposed to take care of any resulting problems.