Re: Consult on ARM SMMU debugfs
On Mon, Jan 11, 2021 at 08:01:48PM +, Robin Murphy wrote: > On 2021-01-07 02:45, chenxiang (M) wrote: > > Hi Will, Robin or other guys, > > > > When debugging SMMU/SVA issue on huawei ARM64 board, we find that it > > lacks of enough debugfs for ARM SMMU driver (such as > > > > the value of STE/CD which we need to check sometimes). Currently it > > creates top-level iommu directory in debugfs, but there is no debugfs > > > > for ARM SMMU driver specially. Do you know whether ARM have the plan to > > do that recently? > > FWIW I don't think I've ever felt the need to need to inspect the Stream > Table on a live system. So far the nature of the STE code has been simple > enough that it's very hard for any given STE to be *wrong* - either it's set > up as expected and thus works fine, or it's not initialised at all and you > get C_BAD_STE, where 99% of the time you then just cross-reference the > Stream ID against the firmware and find that the DT/IORT is wrong. > > Similarly I don't think I've even even *seen* an issue that could be > attributed to a context descriptor, although I appreciate that as we start > landing more PASID and SVA support the scope for that starts to widen > considerably. > > Feel free to propose a patch if you believe it would be genuinely useful and > won't just bit-rot into a maintenance burden, but it's not something that's > on our roadmap here. I do think that the IOMMU stuff needs better debugging. I've hit the WARN_ON() in __arm_lpae_map(), and it's been pretty much undebuggable, so I've resorted to putting the IOMMU into bypass mode permanently to work around the issue. The reason that it's undebuggable is if one puts printk() or trace statements in the code, boots the platform, you get flooded with those debugging messages, because every access to the rootfs generates and tears down a mapping. It would be nice to be able to inspect the IOMMU page tables and state of the IOMMU, rather than having to resort to effectively disabling the IOMMU. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ARM/omap1: switch to use dma_direct_set_offset for lbus DMA offsets
On Mon, Sep 21, 2020 at 08:47:23AM +0200, Christoph Hellwig wrote: > On Mon, Sep 21, 2020 at 09:44:18AM +0300, Tony Lindgren wrote: > > * Janusz Krzysztofik [200919 22:29]: > > > Hi Tony, > > > > > > On Friday, September 18, 2020 7:49:33 A.M. CEST Tony Lindgren wrote: > > > > * Christoph Hellwig [200917 17:37]: > > > > > Switch the omap1510 platform ohci device to use dma_direct_set_offset > > > > > to set the DMA offset instead of using direct hooks into the DMA > > > > > mapping code and remove the now unused hooks. > > > > > > > > Looks nice to me :) I still can't test this probably for few more weeks > > > > though but hopefully Aaro or Janusz (Added to Cc) can test it. > > > > > > Works for me on Amstrad Delta (tested with a USB ethernet adapter). > > > > > > Tested-by: Janusz Krzysztofik > > > > Great, good to hear! And thanks for testing it. > > > > Christoph, feel free to queue this along with the other patches: > > > > Acked-by: Tony Lindgren > > > > Or let me know if you want me to pick it up. > > I'd prefer to pick it up through the dma-mapping tree, but preferably > I'd pick the whole series up once Russell has tested footwinder. I don't think that's going to happen very soon... seems way too much effort to pull down the appropriate tree to build and test. Sorry. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/4] ARM/footbridge: switch to use dma_direct_set_offset for lbus DMA offsets
On Thu, Sep 17, 2020 at 07:32:27PM +0200, Christoph Hellwig wrote:
> static int __init cats_pci_init(void)
> {
> - if (machine_is_cats())
> - pci_common_init(_pci);
> + if (!machine_is_cats())
> + return 0;
> + bus_register_notifier(_bus_type, _pci_dma_nb);
> + pci_common_init(_pci);
I'd prefer these things to retain a positive-logic construct, so:
if (machine_is_cats()) {
bus_register_notifier(_bus_type, _pci_dma_nb);
pci_common_init(_pci);
}
It's the same number of lines.
Otherwise, I think it's fine. I'll try to find some spare time to give
it a go on a Netwinder.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] ARM/dma-mapping: don't handle NULL devices in dma-direct.h
On Thu, Sep 17, 2020 at 07:32:28PM +0200, Christoph Hellwig wrote: > The DMA API removed support for not passing in a device a long time > ago, so remove the NULL checks. What happens with ISA devices? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/3] ARM/keystone: move the DMA offset handling under ifdef CONFIG_ARM_LPAE
On Thu, Sep 10, 2020 at 07:40:37AM +0200, Christoph Hellwig wrote: > The DMA offset notifier can only be used if PHYS_OFFSET is at least > KEYSTONE_HIGH_PHYS_START, which can't be represented by a 32-bit > phys_addr_t. Currently the code compiles fine despite that, a pending > change to the DMA offset handling would create a compiler warning for > this case. Add an ifdef to not compile the code except for LPAE > configs. However, to have use of the high physical offset, LPAE needs to be enabled, which ensures that phys_addr_t is 64-bit. I believe that DMA is non-coherent on this platform unless the high physical address is used. Or something like that. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: decruft the vmalloc API
On Wed, Apr 08, 2020 at 01:58:58PM +0200, Christoph Hellwig wrote: > Hi all, > > Peter noticed that with some dumb luck you can toast the kernel address > space with exported vmalloc symbols. > > I used this as an opportunity to decruft the vmalloc.c API and make it > much more systematic. This also removes any chance to create vmalloc > mappings outside the designated areas or using executable permissions > from modules. Besides that it removes more than 300 lines of code. I haven't read all your patches yet. Have you tested it on 32-bit ARM, where the module area is located _below_ PAGE_OFFSET and outside of the vmalloc area? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Wed, Mar 04, 2020 at 12:33:14PM +0200, Laurentiu Tudor wrote: > > > On 04.03.2020 12:07, Russell King - ARM Linux admin wrote: > > On Wed, Mar 04, 2020 at 11:56:53AM +0200, Laurentiu Tudor wrote: > > > From 44ae46501b5379bd0890df87fd3827248626ed03 Mon Sep 17 00:00:00 2001 > > > From: Laurentiu Tudor > > > Date: Tue, 1 Oct 2019 16:22:48 +0300 > > > Subject: [PATCH 1/6] bus: fsl-mc: make mc work with smmu disable bypass on > > > Content-Type: text/plain; charset="us-ascii" > > > > > > Since this commit [1] booting kernel on MC based chips started to > > > fail because this firmware starts before the kernel and as soon as > > > SMMU is probed it starts to trigger contiguous faults. > > > > I think you mean "continuous" here. > > Yes, thanks. > > > > This is a workaround that allows MC firmware to run with SMMU > > > in disable bypass mode. It consists of the following steps: > > > 1. pause the firmware at early boot to get a chance to setup SMMU > > > 2. request direct mapping for MC device > > > 3. resume the firmware > > > The workaround relies on the fact that no state is lost when > > > pausing / resuming firmware, as per the docs. > > > With this patch, platforms with MC firmware can now boot without > > > having to change the default config to set: > > >CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n > > > > This alone is a definite improvement, and has been needed for a while. > > Please submit this patch with an appropriate Fixes: tag so stable trees > > can pick it up. > > The thing is that probably this workaround will never make it in the kernel > because it questionable to say the least, e.g. see [1]. My plan is to give > this approach [2] a try sometime soon. > > [1] https://patchwork.kernel.org/comment/23149049/ > [2] https://patchwork.kernel.org/cover/11279577/ So, if we want to reduce the iommu noise, we need to completely break the ability to use a mainline kernel on the LX2160A. This doesn't seem practical nor sensible. Someone has to give. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Wed, Mar 04, 2020 at 11:56:53AM +0200, Laurentiu Tudor wrote:
> From 44ae46501b5379bd0890df87fd3827248626ed03 Mon Sep 17 00:00:00 2001
> From: Laurentiu Tudor
> Date: Tue, 1 Oct 2019 16:22:48 +0300
> Subject: [PATCH 1/6] bus: fsl-mc: make mc work with smmu disable bypass on
> Content-Type: text/plain; charset="us-ascii"
>
> Since this commit [1] booting kernel on MC based chips started to
> fail because this firmware starts before the kernel and as soon as
> SMMU is probed it starts to trigger contiguous faults.
I think you mean "continuous" here.
> This is a workaround that allows MC firmware to run with SMMU
> in disable bypass mode. It consists of the following steps:
> 1. pause the firmware at early boot to get a chance to setup SMMU
> 2. request direct mapping for MC device
> 3. resume the firmware
> The workaround relies on the fact that no state is lost when
> pausing / resuming firmware, as per the docs.
> With this patch, platforms with MC firmware can now boot without
> having to change the default config to set:
> CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n
This alone is a definite improvement, and has been needed for a while.
Please submit this patch with an appropriate Fixes: tag so stable trees
can pick it up.
> [1] 954a03be033 ("iommu/arm-smmu: Break insecure users by disabling bypass by
> default")
Please put this where you're referencing it above - it's fine to wrap
the description of the commit when using it in the body of the commit
message. However, that should _never_ when providing a Fixes: tag
(linux-next has a script which will detect and complain about broken
Fixes: tags.)
Thanks.
>
> Signed-off-by: Laurentiu Tudor
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 53 +
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index a0f8854acb3a..683a6401ffe8 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -18,6 +18,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "fsl-mc-private.h"
>
> @@ -889,6 +891,12 @@ static int get_mc_addr_translation_ranges(struct device
> *dev,
> return 0;
> }
>
> +#define FSL_MC_GCR1 0x0
> +#define GCR1_P1_STOP BIT(31)
> +
> +static u32 boot_gcr1;
> +static void __iomem *fsl_mc_regs;
> +
> /**
> * fsl_mc_bus_probe - callback invoked when the root MC bus is being
> * added
> @@ -906,6 +914,21 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
> struct mc_version mc_version;
> struct resource res;
>
> + /*
> + * The MC firmware requires full access to the whole address space plus
> + * it has no way of dealing with any kind of address translation, so
> + * request direct mapping for it.
> + */
> + error = iommu_request_dm_for_dev(>dev);
> + if (error)
> + dev_warn(>dev, "iommu_request_dm_for_dev() failed: %d\n",
> + error);
> +
> + if (fsl_mc_regs) {
> + /* Resume the firmware */
> + writel(boot_gcr1 & ~GCR1_P1_STOP, fsl_mc_regs + FSL_MC_GCR1);
> + }
> +
> mc = devm_kzalloc(>dev, sizeof(*mc), GFP_KERNEL);
> if (!mc)
> return -ENOMEM;
> @@ -990,6 +1013,13 @@ static int fsl_mc_bus_remove(struct platform_device
> *pdev)
> if (!fsl_mc_is_root_dprc(>root_mc_bus_dev->dev))
> return -EINVAL;
>
> + /*
> + * Pause back the firmware so that it doesn't trigger faults by the
> + * time SMMU gets brought down.
> + */
> + writel(boot_gcr1 | GCR1_P1_STOP, fsl_mc_regs + FSL_MC_GCR1);
> + iounmap(fsl_mc_regs);
> +
> fsl_mc_device_remove(mc->root_mc_bus_dev);
>
> fsl_destroy_mc_io(mc->root_mc_bus_dev->mc_io);
> @@ -1018,6 +1048,8 @@ static struct platform_driver fsl_mc_bus_driver = {
> static int __init fsl_mc_bus_driver_init(void)
> {
> int error;
> + struct device_node *np;
> + struct resource res;
>
> error = bus_register(_mc_bus_type);
> if (error < 0) {
> @@ -1039,9 +1071,30 @@ static int __init fsl_mc_bus_driver_init(void)
> if (error < 0)
> goto error_cleanup_dprc_driver;
>
> + np = of_find_matching_node(NULL, fsl_mc_bus_match_table);
> + if (np && of_device_is_available(np)) {
> + error = of_address_to_resource(np, 1, );
> + if (error)
> + goto error_cleanup_dprc_driver;
> + fsl_mc_regs = ioremap(res.start, resource_size());
> + if (!fsl_mc_regs) {
> + error = -ENXIO;
> + goto error_cleanup_dprc_driver;
> + }
> +
> + boot_gcr1 = readl(fsl_mc_regs + FSL_MC_GCR1);
> + /*
> + * If found running, pause MC firmware in order to get a chance
> + * to setup SMMU for it.
> + */
> + if (!(boot_gcr1 & GCR1_P1_STOP))
Re: [PATCH] iommu: silence iommu group prints
On Wed, Mar 04, 2020 at 11:42:16AM +0200, Laurentiu Tudor wrote:
> On 04.03.2020 11:33, Russell King - ARM Linux admin wrote:
> > On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:
> > >
> > > On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:
> > > > On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> > > > > From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00
> > > > > 2001
> > > > > From: Laurentiu Tudor
> > > > > Date: Thu, 13 Feb 2020 11:59:12 +0200
> > > > > Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure
> > > > > implementation
> > > > > Content-Type: text/plain; charset="us-ascii"
> > > > >
> > > > > The devices on this bus are not discovered by way of device tree
> > > > > but by queries to the firmware. It makes little sense to trick the
> > > > > generic of layer into thinking that these devices are of related so
> > > > > that we can get our dma configuration. Instead of doing that, add
> > > > > our custom dma configuration implementation.
> > > >
> > > > Firstly, applying this to v5.5 results in a build failure, due to a
> > > > missing linux/iommu.h include.
> > > >
> > > > Secondly, this on its own appears to make the DPAA2 network interfaces
> > > > completely disappear. Looking in /sys/bus/fsl-mc/drivers/*, none of
> > > > the DPAA2 drivers are bound to anything, and looking in
> > > > /sys/bus/fsl-mc/devices/, there is:
> > > >
> > > > lrwxrwxrwx 1 root root 0 Mar 3 22:06 dprc.1 ->
> > > > ../../../devices/platform/soc/80c00.fsl-mc/dprc.1
> > > >
> > > > This is booting with u-boot, so using DT rather than ACPI.
> > > >
> > > > > Signed-off-by: Laurentiu Tudor
> > > > > ---
> > > > >drivers/bus/fsl-mc/fsl-mc-bus.c | 42
> > > > > -
> > > > >1 file changed, 41 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > index 36eb25f82c8e..3df015eedae4 100644
> > > > > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device
> > > > > *dev, struct kobj_uevent_env *env)
> > > > >static int fsl_mc_dma_configure(struct device *dev)
> > > > >{
> > > > > struct device *dma_dev = dev;
> > > > > + struct iommu_fwspec *fwspec;
> > > > > + const struct iommu_ops *iommu_ops;
> > > > > + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> > > > > + int ret;
> > > > > + u32 icid;
> > > > > +
> > > > > + /* Skip DMA setup for devices that are not DMA masters */
> > > > > + if (dev->type == _mc_bus_dpmcp_type ||
> > > > > + dev->type == _mc_bus_dpbp_type ||
> > > > > + dev->type == _mc_bus_dpcon_type ||
> > > > > + dev->type == _mc_bus_dpio_type)
> > > > > + return 0;
> > > > > while (dev_is_fsl_mc(dma_dev))
> > > > > dma_dev = dma_dev->parent;
> > > > > - return of_dma_configure(dev, dma_dev->of_node, 0);
> > > > > + fwspec = dev_iommu_fwspec_get(dma_dev);
> > > > > + if (!fwspec)
> > > > > + return -ENODEV;
> > > >
> > > > The problem appears to be here - fwspec is NULL for dprc.1.
> > >
> > > Ok, that's because the iommu config is missing from the DT node that's
> > > exposing the MC firmware. I've attached a fresh set of patches that
> > > include
> > > on top the missing config and a workaround that makes MC work over SMMU.
> > > Also added the missing #include, thanks for pointing out.
> > > Let me know how it goes.
> >
> > Shouldn't patch 6 should be first in the series, so that patch 1 does
> > not cause a regression and bisectability damage?
> >
>
> Correct, width one comment: both 5 and 6 should go first. Once iommu-map is
> added in the device tree the workaround for MC with the
> arm-smmu.disable_bypass boot arg will no longer work.
So, wouldn't it be reasonable to arrange the patch series like that?
--
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:
>
> On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:
> > On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> > > From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
> > > From: Laurentiu Tudor
> > > Date: Thu, 13 Feb 2020 11:59:12 +0200
> > > Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
> > > Content-Type: text/plain; charset="us-ascii"
> > >
> > > The devices on this bus are not discovered by way of device tree
> > > but by queries to the firmware. It makes little sense to trick the
> > > generic of layer into thinking that these devices are of related so
> > > that we can get our dma configuration. Instead of doing that, add
> > > our custom dma configuration implementation.
> >
> > Firstly, applying this to v5.5 results in a build failure, due to a
> > missing linux/iommu.h include.
> >
> > Secondly, this on its own appears to make the DPAA2 network interfaces
> > completely disappear. Looking in /sys/bus/fsl-mc/drivers/*, none of
> > the DPAA2 drivers are bound to anything, and looking in
> > /sys/bus/fsl-mc/devices/, there is:
> >
> > lrwxrwxrwx 1 root root 0 Mar 3 22:06 dprc.1 ->
> > ../../../devices/platform/soc/80c00.fsl-mc/dprc.1
> >
> > This is booting with u-boot, so using DT rather than ACPI.
> >
> > > Signed-off-by: Laurentiu Tudor
> > > ---
> > > drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
> > > 1 file changed, 41 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > index 36eb25f82c8e..3df015eedae4 100644
> > > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev,
> > > struct kobj_uevent_env *env)
> > > static int fsl_mc_dma_configure(struct device *dev)
> > > {
> > > struct device *dma_dev = dev;
> > > + struct iommu_fwspec *fwspec;
> > > + const struct iommu_ops *iommu_ops;
> > > + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> > > + int ret;
> > > + u32 icid;
> > > +
> > > + /* Skip DMA setup for devices that are not DMA masters */
> > > + if (dev->type == _mc_bus_dpmcp_type ||
> > > + dev->type == _mc_bus_dpbp_type ||
> > > + dev->type == _mc_bus_dpcon_type ||
> > > + dev->type == _mc_bus_dpio_type)
> > > + return 0;
> > > while (dev_is_fsl_mc(dma_dev))
> > > dma_dev = dma_dev->parent;
> > > - return of_dma_configure(dev, dma_dev->of_node, 0);
> > > + fwspec = dev_iommu_fwspec_get(dma_dev);
> > > + if (!fwspec)
> > > + return -ENODEV;
> >
> > The problem appears to be here - fwspec is NULL for dprc.1.
>
> Ok, that's because the iommu config is missing from the DT node that's
> exposing the MC firmware. I've attached a fresh set of patches that include
> on top the missing config and a workaround that makes MC work over SMMU.
> Also added the missing #include, thanks for pointing out.
> Let me know how it goes.
Shouldn't patch 6 should be first in the series, so that patch 1 does
not cause a regression and bisectability damage?
--
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
> From: Laurentiu Tudor
> Date: Thu, 13 Feb 2020 11:59:12 +0200
> Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
> Content-Type: text/plain; charset="us-ascii"
>
> The devices on this bus are not discovered by way of device tree
> but by queries to the firmware. It makes little sense to trick the
> generic of layer into thinking that these devices are of related so
> that we can get our dma configuration. Instead of doing that, add
> our custom dma configuration implementation.
Firstly, applying this to v5.5 results in a build failure, due to a
missing linux/iommu.h include.
Secondly, this on its own appears to make the DPAA2 network interfaces
completely disappear. Looking in /sys/bus/fsl-mc/drivers/*, none of
the DPAA2 drivers are bound to anything, and looking in
/sys/bus/fsl-mc/devices/, there is:
lrwxrwxrwx 1 root root 0 Mar 3 22:06 dprc.1 ->
../../../devices/platform/soc/80c00.fsl-mc/dprc.1
This is booting with u-boot, so using DT rather than ACPI.
> Signed-off-by: Laurentiu Tudor
> ---
> drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 36eb25f82c8e..3df015eedae4 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct
> kobj_uevent_env *env)
> static int fsl_mc_dma_configure(struct device *dev)
> {
> struct device *dma_dev = dev;
> + struct iommu_fwspec *fwspec;
> + const struct iommu_ops *iommu_ops;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + int ret;
> + u32 icid;
> +
> + /* Skip DMA setup for devices that are not DMA masters */
> + if (dev->type == _mc_bus_dpmcp_type ||
> + dev->type == _mc_bus_dpbp_type ||
> + dev->type == _mc_bus_dpcon_type ||
> + dev->type == _mc_bus_dpio_type)
> + return 0;
>
> while (dev_is_fsl_mc(dma_dev))
> dma_dev = dma_dev->parent;
>
> - return of_dma_configure(dev, dma_dev->of_node, 0);
> + fwspec = dev_iommu_fwspec_get(dma_dev);
> + if (!fwspec)
> + return -ENODEV;
The problem appears to be here - fwspec is NULL for dprc.1.
> + iommu_ops = iommu_ops_from_fwnode(fwspec->iommu_fwnode);
> + if (!iommu_ops)
> + return -ENODEV;
> +
> + ret = iommu_fwspec_init(dev, fwspec->iommu_fwnode, iommu_ops);
> + if (ret)
> + return ret;
> +
> + icid = mc_dev->icid;
> + ret = iommu_fwspec_add_ids(dev, , 1);
> + if (ret) {
> + iommu_fwspec_free(dev);
> + return ret;
> + }
> +
> + if (!device_iommu_mapped(dev)) {
> + ret = iommu_probe_device(dev);
> + if (ret) {
> + iommu_fwspec_free(dev);
> + return ret;
> + }
> + }
> +
> + arch_setup_dma_ops(dev, 0, *dma_dev->dma_mask + 1, iommu_ops, true);
> +
> + return 0;
> }
>
> static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
> --
> 2.17.1
>
--
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Tue, Mar 03, 2020 at 04:18:57PM +0200, Laurentiu Tudor wrote: > > > On 28.02.2020 20:32, Robin Murphy wrote: > > [ +Laurentiu ] > > > > Hi Russell, > > > > Thanks for sharing a log, now I properly understand what's up... further > > comments at the end (for context). > > > > On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote: > > > On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote: > > > > On 28/02/2020 02:16, Lu Baolu wrote: > > > > > Hi, > > > > > > > > > > On 2020/2/27 19:57, Russell King wrote: > > > > > > On the LX2160A, there are lots (about 160) of IOMMU messages > > > > > > produced > > > > > > during boot; this is excessive. Reduce the severity of these > > > > > > messages > > > > > > to debug level. > > > > > > > > > > > > Signed-off-by: Russell King > > > > > > --- > > > > > > drivers/iommu/iommu.c | 4 ++-- > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > > > > index 3ead597e1c57..304281ec623b 100644 > > > > > > --- a/drivers/iommu/iommu.c > > > > > > +++ b/drivers/iommu/iommu.c > > > > > > @@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group > > > > > > *group, struct device *dev) > > > > > > trace_add_device_to_group(group->id, dev); > > > > > > - dev_info(dev, "Adding to iommu group %d\n", group->id); > > > > > > + dev_dbg(dev, "Adding to iommu group %d\n", group->id); > > > > > > > > > > I'm not strongly against this. But to me this message seems > > > > > to be a good > > > > > indicator that a device was probed successfully by the iommu > > > > > subsystem. > > > > > Keeping it in the default kernel message always helps to the kernel > > > > > debugging. > > > > > > > > > > > > > I would tend to agree. > > > > > > Here's the boot messages. Notice how many of these "Adding to iommu > > > group" messages there are: > > > > > > [ 0.00] Booting Linux on physical CPU 0x00 [0x410fd083] > > > [ 0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2 > > > (GCC)) #655 SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020 > > > [ 0.00] Machine model: SolidRun LX2160A Clearfog CX > > > [ 0.00] earlycon: pl11 at MMIO32 0x021c (options '') > > > [ 0.00] printk: bootconsole [pl11] enabled > > > [ 0.00] efi: Getting EFI parameters from FDT: > > > [ 0.00] efi: UEFI not found. > > > [ 0.00] cma: Reserved 32 MiB at 0xf9c0 > > > [ 0.00] On node 0 totalpages: 1555968 > > > [ 0.00] DMA zone: 4096 pages used for memmap > > > [ 0.00] DMA zone: 0 pages reserved > > > [ 0.00] DMA zone: 262144 pages, LIFO batch:63 > > > [ 0.00] DMA32 zone: 3832 pages used for memmap > > > [ 0.00] DMA32 zone: 245248 pages, LIFO batch:63 > > > [ 0.00] Normal zone: 16384 pages used for memmap > > > [ 0.00] Normal zone: 1048576 pages, LIFO batch:63 > > > [ 0.00] psci: probing for conduit method from DT. > > > [ 0.00] psci: PSCIv1.1 detected in firmware. > > > [ 0.00] psci: Using standard PSCI v0.2 function IDs > > > [ 0.00] psci: MIGRATE_INFO_TYPE not supported. > > > [ 0.00] psci: SMC Calling Convention v1.1 > > > [ 0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976 > > > [ 0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096 > > > [ 0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05 > > > [0] 06 [0] 07 > > > [ 0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13 > > > [0] 14 [0] 15 > > > [ 0.00] Detected PIPT I-cache on CPU0 > > > [ 0.00] CPU features: detected: GIC system register CPU interface > > > [ 0.00] CPU features: detected: EL2 vector hardening > > > [ 0.00] Speculative Store Bypass Disable mitigation not required > > > [ 0.00] CPU features: detected: ARM erratum 1319367 > > > [
Re: [PATCH] iommu: silence iommu group prints
On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote: > On 28/02/2020 02:16, Lu Baolu wrote: > > Hi, > > > > On 2020/2/27 19:57, Russell King wrote: > > > On the LX2160A, there are lots (about 160) of IOMMU messages produced > > > during boot; this is excessive. Reduce the severity of these messages > > > to debug level. > > > > > > Signed-off-by: Russell King > > > --- > > > drivers/iommu/iommu.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > > index 3ead597e1c57..304281ec623b 100644 > > > --- a/drivers/iommu/iommu.c > > > +++ b/drivers/iommu/iommu.c > > > @@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group > > > *group, struct device *dev) > > > trace_add_device_to_group(group->id, dev); > > > - dev_info(dev, "Adding to iommu group %d\n", group->id); > > > + dev_dbg(dev, "Adding to iommu group %d\n", group->id); > > > > I'm not strongly against this. But to me this message seems to be a good > > indicator that a device was probed successfully by the iommu subsystem. > > Keeping it in the default kernel message always helps to the kernel > > debugging. > > > > I would tend to agree. Here's the boot messages. Notice how many of these "Adding to iommu group" messages there are: [0.00] Booting Linux on physical CPU 0x00 [0x410fd083] [0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2 (GCC)) #655 SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020 [0.00] Machine model: SolidRun LX2160A Clearfog CX [0.00] earlycon: pl11 at MMIO32 0x021c (options '') [0.00] printk: bootconsole [pl11] enabled [0.00] efi: Getting EFI parameters from FDT: [0.00] efi: UEFI not found. [0.00] cma: Reserved 32 MiB at 0xf9c0 [0.00] On node 0 totalpages: 1555968 [0.00] DMA zone: 4096 pages used for memmap [0.00] DMA zone: 0 pages reserved [0.00] DMA zone: 262144 pages, LIFO batch:63 [0.00] DMA32 zone: 3832 pages used for memmap [0.00] DMA32 zone: 245248 pages, LIFO batch:63 [0.00] Normal zone: 16384 pages used for memmap [0.00] Normal zone: 1048576 pages, LIFO batch:63 [0.00] psci: probing for conduit method from DT. [0.00] psci: PSCIv1.1 detected in firmware. [0.00] psci: Using standard PSCI v0.2 function IDs [0.00] psci: MIGRATE_INFO_TYPE not supported. [0.00] psci: SMC Calling Convention v1.1 [0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976 [0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096 [0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05 [0] 06 [0] 07 [0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13 [0] 14 [0] 15 [0.00] Detected PIPT I-cache on CPU0 [0.00] CPU features: detected: GIC system register CPU interface [0.00] CPU features: detected: EL2 vector hardening [0.00] Speculative Store Bypass Disable mitigation not required [0.00] CPU features: detected: ARM erratum 1319367 [0.00] Built 1 zonelists, mobility grouping on. Total pages: 1531656 [0.00] Kernel command line: console=ttyAMA0,115200 root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0 iommu.passthrough=0 [0.00] Dentry cache hash table entries: 1048576 (order: 11, 8388608 bytes, linear) [0.00] Inode-cache hash table entries: 524288 (order: 10, 4194304 bytes, linear) [0.00] mem auto-init: stack:off, heap alloc:off, heap free:off [0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB) [0.00] Memory: 5991500K/6223872K available (10172K kernel code, 1376K rwdata, 3888K rodata, 960K init, 4326K bss, 199604K reserved, 32768K cma-reserved) [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16, Nodes=1 [0.00] rcu: Preemptible hierarchical RCU implementation. [0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=16. [0.00] Tasks RCU enabled. [0.00] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies. [0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=16 [0.00] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0 [0.00] GICv3: GIC: Using split EOI/Deactivate mode [0.00] GICv3: 256 SPIs implemented [0.00] GICv3: 0 Extended SPIs implemented [0.00] GICv3: Distributor has no Range Selector support [0.00] GICv3: 16 PPIs implemented [0.00] GICv3: no VLPI support, no direct LPI support [0.00] GICv3: CPU0: found redistributor 0 region 0:0x0620 [0.00] ITS [mem 0x0602-0x0603] [0.00] ITS@0x0602: allocated 65536 Devices @2178d0 (flat, esz 8, psz 64K, shr 0) [0.00]
Re: [PATCH] iommu: silence iommu group prints
On Thu, Feb 27, 2020 at 06:19:10PM +, Robin Murphy wrote: > On 27/02/2020 1:48 pm, Russell King - ARM Linux admin wrote: > > On Thu, Feb 27, 2020 at 01:44:56PM +, Robin Murphy wrote: > > > On 27/02/2020 11:57 am, Russell King wrote: > > > > On the LX2160A, there are lots (about 160) of IOMMU messages produced > > > > during boot; this is excessive. Reduce the severity of these messages > > > > to debug level. > > > > > > That's... a lot. Does the system really have that many devices, or is some > > > driver being stupid and repeatedly populating and destroying an entire bus > > > in a probe-deferral dance? > > > > It's all the devices created by for the mc-bus for the DPAA2 > > networking support. I don't know the technicalities, just that > > the boot is spammed with these messages. > > Well, the "technicalities" are really just whether the thing before the > colon on each message is unique or not. If you're seeing multiple add and > remove calls pertaining to the same device (or frankly any remove calls at > all during boot) then it smacks of something somewhere wasting time and > resources with unnecessary busywork, which is indicative of either poor > design or an actual bug, either of which would deserve fixing. It's not the same device. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: silence iommu group prints
On Thu, Feb 27, 2020 at 01:44:56PM +, Robin Murphy wrote: > On 27/02/2020 11:57 am, Russell King wrote: > > On the LX2160A, there are lots (about 160) of IOMMU messages produced > > during boot; this is excessive. Reduce the severity of these messages > > to debug level. > > That's... a lot. Does the system really have that many devices, or is some > driver being stupid and repeatedly populating and destroying an entire bus > in a probe-deferral dance? It's all the devices created by for the mc-bus for the DPAA2 networking support. I don't know the technicalities, just that the boot is spammed with these messages. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: fix the module name for disable_bypass parameter
On Tue, Feb 11, 2020 at 05:36:55PM -0600, Li Yang wrote:
> Since commit cd221bd24ff5 ("iommu/arm-smmu: Allow building as a module"),
> there is a side effect that the module name is changed from arm-smmu to
> arm-smmu-mod. So the kernel parameter for disable_bypass need to be
> changed too. Fix the Kconfig help and error message to the correct
> parameter name.
Hmm, this seems to be a user-visible change - so those of us who have
been booting with "arm-smmu.disable_bypass=0" now need to change that
depending on which kernel is being booted - which is not nice, and
makes the support side on platforms that need this kernel parameter
harder.
>
> Signed-off-by: Li Yang
> ---
> drivers/iommu/Kconfig| 2 +-
> drivers/iommu/arm-smmu.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index d2fade984999..fb54be903c60 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -415,7 +415,7 @@ config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT
> hardcode the bypass disable in the code.
>
> NOTE: the kernel command line parameter
> - 'arm-smmu.disable_bypass' will continue to override this
> + 'arm-smmu-mod.disable_bypass' will continue to override this
> config.
>
> config ARM_SMMU_V3
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 16c4b87af42b..2ffe8ff04393 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -512,7 +512,7 @@ static irqreturn_t arm_smmu_global_fault(int irq, void
> *dev)
> if (IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT) &&
> (gfsr & ARM_SMMU_sGFSR_USF))
> dev_err(smmu->dev,
> - "Blocked unknown Stream ID 0x%hx; boot with
> \"arm-smmu.disable_bypass=0\" to allow, but this may have security
> implications\n",
> + "Blocked unknown Stream ID 0x%hx; boot with
> \"arm-smmu-mod.disable_bypass=0\" to allow, but this may have security
> implications\n",
> (u16)gfsynr1);
> else
> dev_err(smmu->dev,
> --
> 2.17.1
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: Report USF more clearly
On Fri, Sep 13, 2019 at 12:48:37PM +0100, Robin Murphy wrote: > Although CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT is a welcome tool > for smoking out inadequate firmware, the failure mode is non-obvious > and can be confusing for end users. Add some special-case reporting of > Unidentified Stream Faults to help clarify this particular symptom. Having encountered this on a board that turned up this week, it may be better to use the hex representation of the stream ID, especially as it seems normal for the stream ID to be made up of implementation defined bitfields. If we want to stick with decimal, maybe masking the stream ID with the number of allowable bits would be a good idea, so that the decimal value remains meaningful should other bits be non-zero? > CC: Douglas Anderson > Signed-off-by: Robin Murphy > --- > drivers/iommu/arm-smmu.c | 5 + > drivers/iommu/arm-smmu.h | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index b7cf24402a94..76ac8c180695 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -499,6 +499,11 @@ static irqreturn_t arm_smmu_global_fault(int irq, void > *dev) > dev_err_ratelimited(smmu->dev, > "\tGFSR 0x%08x, GFSYNR0 0x%08x, GFSYNR1 0x%08x, GFSYNR2 > 0x%08x\n", > gfsr, gfsynr0, gfsynr1, gfsynr2); > + if (IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT) && > + (gfsr & sGFSR_USF)) > + dev_err_ratelimited(smmu->dev, > + "Stream ID %hu may not be described by firmware, try > booting with \"arm-smmu.disable_bypass=0\"\n", > + (u16)gfsynr1); > > arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, gfsr); > return IRQ_HANDLED; > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h > index c9c13b5785f2..46f7e161e83e 100644 > --- a/drivers/iommu/arm-smmu.h > +++ b/drivers/iommu/arm-smmu.h > @@ -79,6 +79,8 @@ > #define ID7_MINORGENMASK(3, 0) > > #define ARM_SMMU_GR0_sGFSR 0x48 > +#define sGFSR_USFBIT(2) I do wonder if this is another instance where writing "(1 << 1)" would have resulted in less chance of a mistake being made... wrapping stuff up into macros is not always better! 9.6.15SMMU_sGFSR, Global Fault Status Register The SMMU_sGFSR bit assignments are: USF, bit[1] Unidentified stream fault. The possible values of this bit are: 0 No Unidentified stream fault. 1 Unidentified stream fault. So this wants to be: #define sGFSR_USF BIT(1) -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] vmalloc: lift the arm flag for coherent mappings to common code
On Fri, Aug 30, 2019 at 08:29:21AM +0200, Christoph Hellwig wrote: > The arm architecture had a VM_ARM_DMA_CONSISTENT flag to mark DMA > coherent remapping for a while. Lift this flag to common code so > that we can use it generically. We also check it in the only place > VM_USERMAP is directly check so that we can entirely replace that > flag as well (although I'm not even sure why we'd want to allow > remapping DMA appings, but I'd rather not change behavior). Good, because if you did change that behaviour, you'd break almost every ARM framebuffer and cripple ARM audio drivers. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
On Tue, Aug 06, 2019 at 05:45:03PM +0100, Russell King - ARM Linux admin wrote: > On Tue, Aug 06, 2019 at 05:08:54PM +0100, Will Deacon wrote: > > On Sat, Aug 03, 2019 at 08:48:12AM +0200, Christoph Hellwig wrote: > > > On Fri, Aug 02, 2019 at 11:38:03AM +0100, Will Deacon wrote: > > > > > > > > So this boils down to a terminology mismatch. The Arm architecture > > > > doesn't have > > > > anything called "write combine", so in Linux we instead provide what > > > > the Arm > > > > architecture calls "Normal non-cacheable" memory for > > > > pgprot_writecombine(). > > > > Amongst other things, this memory type permits speculation, unaligned > > > > accesses > > > > and merging of writes. I found something in the architecture spec about > > > > non-cachable memory, but it's written in Armglish[1]. > > > > > > > > pgprot_noncached(), on the other hand, provides what the architecture > > > > calls > > > > Strongly Ordered or Device-nGnRnE memory. This is intended for mapping > > > > MMIO > > > > (i.e. PCI config space) and therefore forbids speculation, preserves > > > > access > > > > size, requires strict alignment and also forces write responses to come > > > > from > > > > the endpoint. > > > > > > > > I think the naming mismatch is historical, but on arm64 we wanted to > > > > use the > > > > same names as arm32 so that any drivers using these things directly > > > > would get > > > > the same behaviour. > > > > > > That all makes sense, but it totally needs a comment. I'll try to draft > > > one based on this. I've also looked at the arm32 code a bit more, and > > > it seems arm always (?) supported Normal non-cacheable attribute, but > > > Linux only optionally uses it for arm v6+ because of fears of drivers > > > missing barriers. > > > > I think it was also to do with aliasing, but I don't recall all of the > > details. > > ARMv6+ is where the architecture significantly changed to introduce > the idea of [Normal, Device, Strongly Ordered] where Normal has the > cache attributes. > > Before that, we had just "uncached/unbuffered, uncached/buffered, > cached/unbuffered, cached/buffered" modes. > > The write buffer (enabled by buffered modes) has no architected > guarantees about how long writes will sit in it, and there is only > the "drain write buffer" instruction to push writes out. > > Up to and including ARMv5, we took the easy approach of just using > the "uncached/unbuffered" mode since that is (a) the safest, and (b) > avoids write buffers that alias when there are multiple different > mappings. > > We could have used a different approach, making all IO writes contain > a "drain write buffer" instruction, and map DMA memory as "buffered", > but as there were no Linux barriers defined to order memory accesses > to DMA memory (so, for example, ring buffers can be updated in the > correct order) back in those days, using the uncached/unbuffered mode > was the sanest and most reliable solution. > > > > > > The other really weird things is that in arm32 > > > pgprot_dmacoherent incudes the L_PTE_XN bit, which from my understanding > > > is the no-execture bit, but pgprot_writecombine does not. This seems to > > > not very unintentional. So minus that the whole DMA_ATTR_WRITE_COMBІNE > > > seems to be about flagging old arm specific drivers as having the proper > > > barriers in places and otherwise is a no-op. > > > > I think it only matters for Armv7 CPUs, but yes, we should probably be > > setting L_PTE_XN for both of these memory types. > > Conventionally, pgprot_writecombine() has only been used to change > the memory type and not the permissions. Since writecombine memory > is still capable of being executed, I don't see any reason to set XN > for it. > > If the user wishes to mmap() using PROT_READ|PROT_EXEC, then is there > really a reason for writecombine to set XN overriding the user? > > That said, pgprot_writecombine() is mostly used for framebuffers, which > arguably shouldn't be executable anyway - but who'd want to mmap() the > framebuffer with PROT_EXEC? > > > > > > Here is my tentative plan: > > > > > > - respin this patch with a small fix to handle the > > >DMA_ATTR_NON_CONSISTENT (as in ignore it unless actually supported), > > &g
Re: [PATCH] dma-mapping: fix page attributes for dma_mmap_*
On Tue, Aug 06, 2019 at 05:08:54PM +0100, Will Deacon wrote: > On Sat, Aug 03, 2019 at 08:48:12AM +0200, Christoph Hellwig wrote: > > On Fri, Aug 02, 2019 at 11:38:03AM +0100, Will Deacon wrote: > > > > > > So this boils down to a terminology mismatch. The Arm architecture > > > doesn't have > > > anything called "write combine", so in Linux we instead provide what the > > > Arm > > > architecture calls "Normal non-cacheable" memory for > > > pgprot_writecombine(). > > > Amongst other things, this memory type permits speculation, unaligned > > > accesses > > > and merging of writes. I found something in the architecture spec about > > > non-cachable memory, but it's written in Armglish[1]. > > > > > > pgprot_noncached(), on the other hand, provides what the architecture > > > calls > > > Strongly Ordered or Device-nGnRnE memory. This is intended for mapping > > > MMIO > > > (i.e. PCI config space) and therefore forbids speculation, preserves > > > access > > > size, requires strict alignment and also forces write responses to come > > > from > > > the endpoint. > > > > > > I think the naming mismatch is historical, but on arm64 we wanted to use > > > the > > > same names as arm32 so that any drivers using these things directly would > > > get > > > the same behaviour. > > > > That all makes sense, but it totally needs a comment. I'll try to draft > > one based on this. I've also looked at the arm32 code a bit more, and > > it seems arm always (?) supported Normal non-cacheable attribute, but > > Linux only optionally uses it for arm v6+ because of fears of drivers > > missing barriers. > > I think it was also to do with aliasing, but I don't recall all of the > details. ARMv6+ is where the architecture significantly changed to introduce the idea of [Normal, Device, Strongly Ordered] where Normal has the cache attributes. Before that, we had just "uncached/unbuffered, uncached/buffered, cached/unbuffered, cached/buffered" modes. The write buffer (enabled by buffered modes) has no architected guarantees about how long writes will sit in it, and there is only the "drain write buffer" instruction to push writes out. Up to and including ARMv5, we took the easy approach of just using the "uncached/unbuffered" mode since that is (a) the safest, and (b) avoids write buffers that alias when there are multiple different mappings. We could have used a different approach, making all IO writes contain a "drain write buffer" instruction, and map DMA memory as "buffered", but as there were no Linux barriers defined to order memory accesses to DMA memory (so, for example, ring buffers can be updated in the correct order) back in those days, using the uncached/unbuffered mode was the sanest and most reliable solution. > > > The other really weird things is that in arm32 > > pgprot_dmacoherent incudes the L_PTE_XN bit, which from my understanding > > is the no-execture bit, but pgprot_writecombine does not. This seems to > > not very unintentional. So minus that the whole DMA_ATTR_WRITE_COMBІNE > > seems to be about flagging old arm specific drivers as having the proper > > barriers in places and otherwise is a no-op. > > I think it only matters for Armv7 CPUs, but yes, we should probably be > setting L_PTE_XN for both of these memory types. Conventionally, pgprot_writecombine() has only been used to change the memory type and not the permissions. Since writecombine memory is still capable of being executed, I don't see any reason to set XN for it. If the user wishes to mmap() using PROT_READ|PROT_EXEC, then is there really a reason for writecombine to set XN overriding the user? That said, pgprot_writecombine() is mostly used for framebuffers, which arguably shouldn't be executable anyway - but who'd want to mmap() the framebuffer with PROT_EXEC? > > > Here is my tentative plan: > > > > - respin this patch with a small fix to handle the > >DMA_ATTR_NON_CONSISTENT (as in ignore it unless actually supported), > >but keep the name as-is to avoid churn. This should allow 5.3 > >inclusion and backports > > - remove DMA_ATTR_WRITE_COMBINE support from mips, probably also 5.3 > >material. > > - move all architectures but arm over to just define > >pgprot_dmacoherent, including a comment with the above explanation > >for arm64. > > That would be great, thanks. > > > - make DMA_ATTR_WRITE_COMBINE a no-op and schedule it for removal, > >thus removing the last instances of arch_dma_mmap_pgprot > > All sounds good to me, although I suppose 32-bit Arm platforms without > CONFIG_ARM_DMA_MEM_BUFFERABLE may run into issues if DMA_ATTR_WRITE_COMBINE > disappears. Only one way to find out... Looking at the results of grep, I think only OMAP2+ and Exynos may be affected. However, removing writecombine support from the DMA API is going to have a huge impact for framebuffers on earlier ARMs - that's where we do expect framebuffers to be mapped "uncached/buffered" for
Re: SATA broken with LPAE
On Thu, Jun 27, 2019 at 11:15:30AM +0200, Christoph Hellwig wrote: > On Thu, Jun 27, 2019 at 10:07:53AM +0100, Russell King - ARM Linux admin > wrote: > > dmabounce has only ever been used with specific devices that have weird > > setups. Otherwise, we've never expected what you describe on ARM. I > > also don't recognise your assertion about the way the DMA API should > > behave as ever having been documented as a requirement for architectures > > to implement. > > That requirement has basically always been there since at least the > 2.6.x days. The history here is that when 64-bit architectures showed > up they all had iommus, so this wasn't an issue. Next was x86 with > highmem, which added special bounce buffering for block I/O and networking > only. Then ia64 showed up that didn't always have an iommu and swiotlb > was added as a "software IOMMU". At this point we had to bounce buffering > schemes for block and networking, while everything else potentially > DMAing to higher memory relied on swiotlb, which was picked up by > basically every architecture that could have memory not covered by a > 32-bit mask and didn't have an iommu. If it wasn't documented... > Except it seems arm never did > that and has been lucky as people didn't try anything that is not > block or networking on their extended physical address space. Because no one knew that the requirement had been introduced, and we've been reliant on all the code you've been stripping out. You're now causing regressions on 32-bit ARM, so you get to fix it. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: SATA broken with LPAE
On Wed, Jun 26, 2019 at 02:53:25PM +0200, Christoph Hellwig wrote: > Hi Roger, > > it seems the arm dma direct mapping code isn't doing the right thing > here. On other platforms that have > 4G memory we always use swiotlb > for bounce buffering in case a device that can't DMA to all the memory. > > Arm is the odd one out and uses its own dmabounce framework instead, > but it seems like it doesn't get used in this case. We need to make > sure dmabounce (or swiotlb for that matter) is set up if > 32-bit > addressing is supported. I'm not really an arm platform expert, > but some of those on the Cc list are and might chime in on how to > do that. dmabounce has only ever been used with specific devices that have weird setups. Otherwise, we've never expected what you describe on ARM. I also don't recognise your assertion about the way the DMA API should behave as ever having been documented as a requirement for architectures to implement. dmabounce comes with it some serious issues that have been known to cause OOMs with old kernels (which have never been solved), so that really isn't the answer. This kind of breakage that is now being reported is exactly the problem that I thought would happen with your patches. 32-bit ARM is in maintenance only now, there is no longer any appetite nor funding for development work on core code. -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported
On Tue, May 21, 2019 at 02:47:29PM +0200, Christoph Hellwig wrote:
> Since Linux 5.1 we allow drivers to just set the largest DMA mask they
> support instead of falling back to smaller ones.
This doesn't make sense. "they" is confusing - why would a driver set
a DMA mask larger than the driver supports? Or is "they" not
referring to the drivers (in which case, what is it referring to?)
> When fixing up all the dma ops instances to allow for this behavior
> the arm direct mapping code was missed. Fix it up by removing the
> sanity check, as all the actual mapping code handles this case just
> fine.
>
> Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections")
> Signed-off-by: Christoph Hellwig
> ---
> arch/arm/mm/dma-mapping.c | 20 +---
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 0a75058c11f3..bdf0d236aaee 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -219,25 +219,7 @@ EXPORT_SYMBOL(arm_coherent_dma_ops);
>
> static int __dma_supported(struct device *dev, u64 mask, bool warn)
> {
> - unsigned long max_dma_pfn;
> -
> - /*
> - * If the mask allows for more memory than we can address,
> - * and we actually have that much memory, then we must
> - * indicate that DMA to this device is not supported.
> - */
> - if (sizeof(mask) != sizeof(dma_addr_t) &&
> - mask > (dma_addr_t)~0 &&
> - dma_to_pfn(dev, ~0) < max_pfn - 1) {
> - if (warn) {
> - dev_warn(dev, "Coherent DMA mask %#llx is larger than
> dma_addr_t allows\n",
> - mask);
> - dev_warn(dev, "Driver did not use or check the return
> value from dma_set_coherent_mask()?\n");
> - }
> - return 0;
> - }
The point of this check is to trap the case where we have, for example,
8GB of memory, but dma_addr_t is 32-bit. We can allocate in the high
4GB, but we can't represent the address in a dma_addr_t.
> -
> - max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
> + unsigned long max_dma_pfn = min(max_pfn, arm_dma_pfn_limit);
>
> /*
>* Translate the device's DMA mask to a PFN limit. This
> --
> 2.20.1
>
>
--
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold
On Tue, May 21, 2019 at 02:47:28PM +0200, Christoph Hellwig wrote:
> The dma masks in struct device are always 64-bits wide. But for builds
> using a 32-bit dma_addr_t we need to ensure we don't store an
> unsupportable value. Before Linux 5.0 this was handled at least by
> the ARM dma mapping code by never allowing to set a larger dma_mask,
> but these days we allow the driver to just set the largest supported
> value and never fall back to a smaller one. Ensure this always works
> by truncating the value.
So how does the driver negotiation for >32bit addresses work if we don't
fail for large masks?
I'm thinking about all those PCI drivers that need DAC cycles for >32bit
addresses, such as e1000, which negotiate via (eg):
/* there is a workaround being applied below that limits
* 64-bit DMA addresses to 64-bit hardware. There are some
* 32-bit adapters that Tx hang when given 64-bit DMA addresses
*/
pci_using_dac = 0;
if ((hw->bus_type == e1000_bus_type_pcix) &&
!dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(64))) {
pci_using_dac = 1;
} else {
err = dma_set_mask_and_coherent(>dev, DMA_BIT_MASK(32));
if (err) {
pr_err("No usable DMA config, aborting\n");
goto err_dma;
}
}
and similar. If we blindly trunate the 64-bit to 32-bit, aren't we
going to end up with PCI cards using DAC cycles to a host bridge that
do not support DAC cycles?
>
> Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections")
> Signed-off-by: Christoph Hellwig
> ---
> kernel/dma/mapping.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index f7afdadb6770..1f628e7ac709 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -317,6 +317,12 @@ void arch_dma_set_mask(struct device *dev, u64 mask);
>
> int dma_set_mask(struct device *dev, u64 mask)
> {
> + /*
> + * Truncate the mask to the actually supported dma_addr_t width to
> + * avoid generating unsupportable addresses.
> + */
> + mask = (dma_addr_t)mask;
> +
> if (!dev->dma_mask || !dma_supported(dev, mask))
> return -EIO;
>
> @@ -330,6 +336,12 @@ EXPORT_SYMBOL(dma_set_mask);
> #ifndef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK
> int dma_set_coherent_mask(struct device *dev, u64 mask)
> {
> + /*
> + * Truncate the mask to the actually supported dma_addr_t width to
> + * avoid generating unsupportable addresses.
> + */
> + mask = (dma_addr_t)mask;
> +
> if (!dma_supported(dev, mask))
> return -EIO;
>
> --
> 2.20.1
>
>
--
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: remove NULL struct device support in the DMA API
On Thu, Mar 21, 2019 at 03:52:28PM -0700, Christoph Hellwig wrote: > We still have a few drivers which pass a NULL struct device pointer > to DMA API functions, which generally is a bad idea as the API > implementations rely on the device not only for ops selection, but > also the dma mask and various other attributes, and many implementations > have been broken for NULL device support for a while. I think I must be missing something, but... My understanding is that ISA DMA is normally limited to 24 bits of address - indeed, the x86 version only programs 24 bits of DMA address. Looking through this series, it appears that the conversions mean that the DMA mask for ISA becomes the full all-ones DMA mask, which would of course lead to memory corruption if only 24 bits of the address end up being programmed into the hardware. Maybe you could say why you think this series is safe in regard to ISA DMA? > > This series removes the few remaning users that weren't picked up in > the last merge window and then removes core support for this "feature". > > A git tree is also available at: > > git://git.infradead.org/users/hch/misc.git dma-remove-NULL-dev-support > > Gitweb: > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-remove-NULL-dev-support > -- 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 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
