Re: [PATCH v1 0/3] IOMMU: Tegra SMMU fixes
07.03.2019 1:50, Dmitry Osipenko пишет: > Hello, > > This small series primarily fixes a bug that affects Terga30 and > Terga114 platforms, it also carries two patches that improve SMMU > functionality and clean up code a tad. > > Dmitry Osipenko (3): > iommu/tegra-smmu: Fix invalid ASID bits on Tegra30/114 > iommu/tegra-smmu: Properly release domain resources > iommu/tegra-smmu: Respect IOMMU API read-write protections > > drivers/iommu/tegra-smmu.c | 41 -- > 1 file changed, 31 insertions(+), 10 deletions(-) > Joerg, could you please apply this series? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] drm/panfrost: Add initial panfrost driver
On Mon, Apr 1, 2019 at 2:12 PM Robin Murphy wrote:
>
> On 01/04/2019 08:47, Rob Herring wrote:
> > This adds the initial driver for panfrost which supports Arm Mali
> > Midgard and Bifrost family of GPUs. Currently, only the T860 and
> > T760 Midgard GPUs have been tested.
>
> FWIW, on an antique T624 (Juno) it seems to work no worse than the kbase
> driver plus panfrost-nondrm, which is to say it gets far enough to prove
> that the userspace definitely doesn't support T624 (kmscube manages to
> show a grey background, but the GPU is constantly falling over with page
> faults trying to dereference address 0 - for obvious reasons I'm not
> going to get any further involved in debugging that).
>
> A couple of discoveries and general observations below.
>
> > v2:
> > - Add GPU reset on job hangs (Tomeu)
> > - Add RuntimePM and devfreq support (Tomeu)
> > - Fix T760 support (Tomeu)
> > - Add a TODO file (Rob, Tomeu)
> > - Support multiple in fences (Tomeu)
> > - Drop support for shared fences (Tomeu)
> > - Fill in MMU de-init (Rob)
> > - Move register definitions back to single header (Rob)
> > - Clean-up hardcoded job submit todos (Rob)
> > - Implement feature setup based on features/issues (Rob)
> > - Add remaining Midgard DT compatible strings (Rob)
> >
> > Cc: Maarten Lankhorst
> > Cc: Maxime Ripard
> > Cc: Sean Paul
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Cc: Alyssa Rosenzweig
> > Cc: Lyude Paul
> > Cc: Eric Anholt
> > Signed-off-by: Marty E. Plummer
> > Signed-off-by: Tomeu Vizoso
> > Signed-off-by: Rob Herring
> > ---
> [...]
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
> > b/drivers/gpu/drm/panfrost/panfrost_device.c
> > new file mode 100644
> > index ..227ba5202a6f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -0,0 +1,227 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright 2018 Marty E. Plummer */
> > +/* Copyright 2019 Linaro, Ltd, Rob Herring */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "panfrost_device.h"
> > +#include "panfrost_devfreq.h"
> > +#include "panfrost_features.h"
> > +#include "panfrost_gpu.h"
> > +#include "panfrost_job.h"
> > +#include "panfrost_mmu.h"
> > +
> > +static int panfrost_clk_init(struct panfrost_device *pfdev)
> > +{
> > + int err;
> > + unsigned long rate;
> > +
> > + pfdev->clock = devm_clk_get(pfdev->dev, NULL);
> > + if (IS_ERR(pfdev->clock)) {
>
> The DT binding says clocks are optional, but this doesn't treat them as
> such.
Hum, I would think effectively clocks are always there and necessary
for thermal reasons.
> > + spin_lock_init(&pfdev->mm_lock);
> > +
> > + /* 4G enough for now. can be 48-bit */
> > + drm_mm_init(&pfdev->mm, SZ_32M >> PAGE_SHIFT, SZ_4G);
>
> You probably want a dma_set_mask_and_coherent() call for your 'real'
> output address size somewhere - the default 32-bit mask works out OK for
> RK3399, but on systems with RAM above 4GB io-pgtable will get very
> unhappy about DMA bounce-buffering.
Yes, I have a todo for figuring out the # of physaddr bits in the mmu
setup (as this call is just relevant to the input address side).
Though maybe just calling dma_set_mask_and_coherent() is enough and I
don't need to know the exact number of output bits for the io-pgtable
setup?
> > + return err;
> > +}
> > +
> > +static int panfrost_remove(struct platform_device *pdev)
> > +{
> > + struct panfrost_device *pfdev = platform_get_drvdata(pdev);
> > + struct drm_device *ddev = pfdev->ddev;
> > +
> > + drm_dev_unregister(ddev);
> > + pm_runtime_get_sync(pfdev->dev);
> > + pm_runtime_put_sync_autosuspend(pfdev->dev);
> > + pm_runtime_disable(pfdev->dev);
> > + panfrost_device_fini(pfdev);
> > + drm_dev_put(ddev);
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id dt_match[] = {
> > + { .compatible = "arm,mali-t604" },
> > + { .compatible = "arm,mali-t624" },
> > + { .compatible = "arm,mali-t628" },
> > + { .compatible = "arm,mali-t720" },
> > + { .compatible = "arm,mali-t760" },
> > + { .compatible = "arm,mali-t820" },
> > + { .compatible = "arm,mali-t830" },
> > + { .compatible = "arm,mali-t860" },
> > + { .compatible = "arm,mali-t880" },
>
> Any chance of resurrecting the generic "arm,mali-midgard" compatible? :P
I wouldn't mind, but we still need all these and I don't think we'd be
adding more. For bifrost, we're adding 'arm,mali-bifrost' from the
start.
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c
> > b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> > new file mode 100644
> > index ..867e2ba3a761
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
> [...]
> > + /* Limit read & write ID width for AXI */
> > + if (panfrost_has_hw_feature(pfdev,
> > HW_FEATURE_3BIT_EXT_RW_L2_MMU_CONFIG))
> > + quirks &= ~(L2_MMU_CONFIG_3BIT_LIMIT_EXTERNAL_READS |
> >
Re: 5.1-rc1: mpt init crash in scsi_map_dma, dma_4v_map_sg on sparc64
On 4/2/19 2:30 PM, Meelis Roos wrote:
[ 17.566584] scsi host0: ioc0: LSISAS1064 A3, FwRev=010ah,
Ports=1, MaxQ=511, IRQ=27
[ 17.595897] mptsas: ioc0: attaching ssp device: fw_channel 0,
fw_id 0, phy 0, sas_addr 0x5000c5001799a45d
[ 17.598465] Unable to handle kernel NULL pointer dereference
[ 17.598623] tsk->{mm,active_mm}->context =
[ 17.598723] tsk->{mm,active_mm}->pgd = 88802000
[ 17.598774] \|/ \|/
[ 17.598774] "@'/ .. \`@"
[ 17.598774] /_| \__/ |_\
[ 17.598774] \__U_/
[ 17.598894] swapper/0(1): Oops [#1]
[ 17.598937] CPU: 12 PID: 1 Comm: swapper/0 Not tainted 5.1.0-rc1
#118
[ 17.598994] TSTATE: 80e01601 TPC: 004483a8 TNPC:
004483ac Y: Not tainted
[ 17.599086] TPC:
You may use gdb to figure out what the NULL pointer points to:
gdb vmlinux
l *(dma_4v_map_sg+0xe8)
gdb did not parse the file but objdump --disassemble worked and +0xe8
seems to be 4483a8
Of course that was right there in the panic message, as TPC is the
address of the instruction that faulted:
ldx [ %i4 ], %g1
For anyone wishing to dig into this further, here is my off the cuff
analysis:
I believe the fault is happening on this line:
base_shift = tbl->table_map_base >> IO_PAGE_SHIFT;
The tbl variable is assigned to one of two values in the statement
above, but since the register dump shows the value in %i4 was 0x10, that
strongly suggests that it executed this:
tbl = &atu->tbl;
Because the offset of the tbl field in struct atu is 0x10, and that was
computed here:
448384: b8 07 60 10 add %i5, 0x10, %i4
(The offset of tbl in struct iommu is 0, so we would have seen that 0 in
%i4 if it had taken the iommu path.)
From the register dump, the value in %i5 was 0. And that came from this
instruction:
4482f4: fa 58 e2 58 ldx [ %g3 + 0x258 ], %i5
Likewise, %g3 came from here:
4482d4: c6 5e 22 18 ldx [ %i0 + 0x218 ], %g3
And %i0 is arg0, struct device *dev. So the code is loading some field
in struct device at offset 0x218, which is consistent with the source:
iommu = dev->archdata.iommu;
So %g3 points to struct iommu, and the code is trying to load the value
at offset 0x258 in that structure, probably this:
atu = iommu->atu;
And atu is the NULL pointer.
Now whether this is the problem, I don't know. It may be that mask
(*dev->dma_mask) was wrong, causing the code to take the &atu->tbl path
instead of the &iommu->tbl path. We can see from the code that mask is
in %g7, and the register dump shows the value of %g7 is fff,
while DMA_BIT_MASK(32) is in %g1 and is , so this might
be the result of some confusion over 32 bit vs 64 bit stuff.
I hope these bits of information help somebody debug further.
Rob
004482c0 :
4482c0: 9d e3 be b0 save %sp, -336, %sp
4482c4: 80 a6 e0 03 cmp %i3, 3
4482c8: 02 40 00 c1 be,pn %icc, 4485cc
4482cc: 92 10 21 e2 mov 0x1e2, %o1
4482d0: 80 a0 00 1a cmp %g0, %i2
4482d4: c6 5e 22 18 ldx [ %i0 + 0x218 ], %g3
4482d8: 82 10 20 00 clr %g1
4482dc: 84 60 3f ff subc %g0, -1, %g2
4482e0: 83 78 e4 01 movre %g3, 1, %g1
4482e4: 80 90 80 01 orcc %g2, %g1, %g0
4482e8: 12 40 00 bd bne,pn %icc, 4485dc
4482ec: 80 a6 e0 01 cmp %i3, 1
4482f0: 84 10 20 03 mov 3, %g2
4482f4: fa 58 e2 58 ldx [ %g3 + 0x258 ], %i5
4482f8: 85 64 60 01 move %icc, 1, %g2
4482fc: b8 0f 20 02 and %i4, 2, %i4
448300: c0 77 a7 f7 clrx [ %fp + 0x7f7 ]
448304: 82 10 a0 04 or %g2, 4, %g1
448308: c0 26 60 18 clr [ %i1 + 0x18 ]
44830c: 85 7f 14 01 movrne %i4, %g1, %g2
448310: 8f 52 00 00 rdpr %pil, %g7
448314: 82 11 e0 0e or %g7, 0xe, %g1
448318: 91 90 60 00 wrpr %g1, 0, %pil
44831c: ce 77 a7 bf stx %g7, [ %fp + 0x7bf ]
448320: 0f 00 02 00 sethi %hi(0x8), %g7
448324: 27 00 00 40 sethi %hi(0x1), %l3
448328: ce 77 a7 df stx %g7, [ %fp + 0x7df ]
44832c: 0f 00 28 21 sethi %hi(0xa08400), %g7
448330: 8e 11 e2 b0 or %g7, 0x2b0, %g7 ! a086b0
448334: f0 71 c0 05 stx %i0, [ %g7 + %g5 ]
448338: 82 01 c0 05 add %g7, %g5, %g1
44833c: c4 70 60 08 stx %g2, [ %g1 + 8 ]
448340: 84 10 3f ff mov -1, %g2
448344: c0 70 60 20 clrx [ %g1 + 0x20 ]
448348: c4 70 60 10 stx %g2, [ %g1 + 0x10 ]
44834c: c2 5e 22 00 ldx [ %i0 + 0x200 ], %g1
448350: 22 c0 40 0d brz,a,pn %g1, 448384
448354: c2 5e 21 e0 ldx [ %i0 + 0x1e0 ], %g1
448358: e6 00 40 00 ld [ %g1 ], %l3
44835c: 05 00 00 40
Re: 5.1-rc1: mpt init crash in scsi_map_dma, dma_4v_map_sg on sparc64
[ 17.566584] scsi host0: ioc0: LSISAS1064 A3, FwRev=010ah, Ports=1,
MaxQ=511, IRQ=27
[ 17.595897] mptsas: ioc0: attaching ssp device: fw_channel 0, fw_id 0, phy
0, sas_addr 0x5000c5001799a45d
[ 17.598465] Unable to handle kernel NULL pointer dereference
[ 17.598623] tsk->{mm,active_mm}->context =
[ 17.598723] tsk->{mm,active_mm}->pgd = 88802000
[ 17.598774] \|/ \|/
[ 17.598774] "@'/ .. \`@"
[ 17.598774] /_| \__/ |_\
[ 17.598774] \__U_/
[ 17.598894] swapper/0(1): Oops [#1]
[ 17.598937] CPU: 12 PID: 1 Comm: swapper/0 Not tainted 5.1.0-rc1 #118
[ 17.598994] TSTATE: 80e01601 TPC: 004483a8 TNPC:
004483ac Y: Not tainted
[ 17.599086] TPC:
You may use gdb to figure out what the NULL pointer points to:
gdb vmlinux
l *(dma_4v_map_sg+0xe8)
gdb did not parse the file but objdump --disassemble worked and +0xe8 seems to
be 4483a8
004482c0 :
4482c0: 9d e3 be b0 save %sp, -336, %sp
4482c4: 80 a6 e0 03 cmp %i3, 3
4482c8: 02 40 00 c1 be,pn %icc, 4485cc
4482cc: 92 10 21 e2 mov 0x1e2, %o1
4482d0: 80 a0 00 1a cmp %g0, %i2
4482d4: c6 5e 22 18 ldx [ %i0 + 0x218 ], %g3
4482d8: 82 10 20 00 clr %g1
4482dc: 84 60 3f ff subc %g0, -1, %g2
4482e0: 83 78 e4 01 movre %g3, 1, %g1
4482e4: 80 90 80 01 orcc %g2, %g1, %g0
4482e8: 12 40 00 bd bne,pn %icc, 4485dc
4482ec: 80 a6 e0 01 cmp %i3, 1
4482f0: 84 10 20 03 mov 3, %g2
4482f4: fa 58 e2 58 ldx [ %g3 + 0x258 ], %i5
4482f8: 85 64 60 01 move %icc, 1, %g2
4482fc: b8 0f 20 02 and %i4, 2, %i4
448300: c0 77 a7 f7 clrx [ %fp + 0x7f7 ]
448304: 82 10 a0 04 or %g2, 4, %g1
448308: c0 26 60 18 clr [ %i1 + 0x18 ]
44830c: 85 7f 14 01 movrne %i4, %g1, %g2
448310: 8f 52 00 00 rdpr %pil, %g7
448314: 82 11 e0 0e or %g7, 0xe, %g1
448318: 91 90 60 00 wrpr %g1, 0, %pil
44831c: ce 77 a7 bf stx %g7, [ %fp + 0x7bf ]
448320: 0f 00 02 00 sethi %hi(0x8), %g7
448324: 27 00 00 40 sethi %hi(0x1), %l3
448328: ce 77 a7 df stx %g7, [ %fp + 0x7df ]
44832c: 0f 00 28 21 sethi %hi(0xa08400), %g7
448330: 8e 11 e2 b0 or %g7, 0x2b0, %g7 ! a086b0
448334: f0 71 c0 05 stx %i0, [ %g7 + %g5 ]
448338: 82 01 c0 05 add %g7, %g5, %g1
44833c: c4 70 60 08 stx %g2, [ %g1 + 8 ]
448340: 84 10 3f ff mov -1, %g2
448344: c0 70 60 20 clrx [ %g1 + 0x20 ]
448348: c4 70 60 10 stx %g2, [ %g1 + 0x10 ]
44834c: c2 5e 22 00 ldx [ %i0 + 0x200 ], %g1
448350: 22 c0 40 0d brz,a,pn %g1, 448384
448354: c2 5e 21 e0 ldx [ %i0 + 0x1e0 ], %g1
448358: e6 00 40 00 ld [ %g1 ], %l3
44835c: 05 00 00 40 sethi %hi(0x1), %g2
448360: c2 58 60 08 ldx [ %g1 + 8 ], %g1
448364: 80 a4 e0 00 cmp %l3, 0
448368: 02 c8 40 06 brz %g1, 448380
44836c: a7 64 40 02 move %icc, %g2, %l3
448370: 25 00 00 08 sethi %hi(0x2000), %l2
448374: a4 00 40 12 add %g1, %l2, %l2
448378: a5 34 b0 0d srlx %l2, 0xd, %l2
44837c: e4 77 a7 df stx %l2, [ %fp + 0x7df ]
448380: c2 5e 21 e0 ldx [ %i0 + 0x1e0 ], %g1
448384: b8 07 60 10 add %i5, 0x10, %i4
448388: c2 58 40 00 ldx [ %g1 ], %g1
44838c: c2 77 a7 d7 stx %g1, [ %fp + 0x7d7 ]
448390: 82 10 3f ff mov -1, %g1
448394: ce 5f a7 d7 ldx [ %fp + 0x7d7 ], %g7
448398: 83 30 70 20 srlx %g1, 0x20, %g1
44839c: 80 a1 c0 01 cmp %g7, %g1
4483a0: b9 65 10 03 movleu %xcc, %g3, %i4
4483a4: 80 a6 a0 00 cmp %i2, 0
4483a8: c2 5f 00 00 ldx [ %i4 ], %g1
4483ac: 83 30 70 0d srlx %g1, 0xd, %g1
4483b0: 04 40 01 26 ble,pn %icc, 448848
4483b4: c2 77 a7 9f stx %g1, [ %fp + 0x79f ]
4483b8: c2 5f a7 df ldx [ %fp + 0x7df ], %g1
4483bc: 84 10 3f ff mov -1, %g2
4483c0: 23 00 28 21 sethi %hi(0xa08400), %l1
4483c4: ce 5f a7 df ldx [ %fp + 0x7df ], %g7
4483c8: a2 14 62 b0 or %l1, 0x2b0, %l1
4483cc: 86 10 20 01 mov 1, %g3
4483d0: 82 00 7f ff add %g1, -1, %g1
4483d4: e6 27 a7 af st %l3, [ %fp + 0x7af ]
4483d8: ab 30 b0 33 srlx %g2, 0x33, %l5
4483dc: 8e 08 40 07 and %g1, %g7, %g7
4483e0: c2 77 a7 cf stx %g1, [ %fp + 0x7cf ]
4483e4: a0 10 00 19 mov %i1, %l0
4483e8: f2 77 a7 a7 stx %i1, [ %fp
Re: [PATCH v2] iommu/amd: Reserve exclusion range in iova-domain
On Fri Mar 29 19, Joerg Roedel wrote:
From: Joerg Roedel
If a device has an exclusion range specified in the IVRS
table, this region needs to be reserved in the iova-domain
of that device. This hasn't happened until now and can cause
data corruption on data transfered with these devices.
Treat exclusion ranges as reserved regions in the iommu-core
to fix the problem.
Fixes: be2a022c0dd0 ('x86, AMD IOMMU: add functions to parse IOMMU memory
mapping requirements for devices')
Signed-off-by: Joerg Roedel
I have a version of this that applies to 4.4 and 4,9 using the older
dm_region code if that would be useful for stable.
--8<--
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 0ad8b7c78a43..f388458624cf 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3165,11 +3165,14 @@ static void amd_iommu_get_dm_regions(struct device *dev,
}
region->start = entry->address_start;
+ region->type = IOMMU_RESV_DIRECT;
region->length = entry->address_end - entry->address_start;
if (entry->prot & IOMMU_PROT_IR)
region->prot |= IOMMU_READ;
if (entry->prot & IOMMU_PROT_IW)
region->prot |= IOMMU_WRITE;
+ if (entry->prot & IOMMU_UNITY_MAP_FLAG_EXCL_RANGE)
+ region->type = IOMMU_RESV_RESERVED;
list_add_tail(®ion->list, head);
}
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 94f1bf772ec9..d84041bc77ac 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1495,6 +1495,9 @@ static int __init init_unity_map_range(struct ivmd_header
*m)
if (e == NULL)
return -ENOMEM;
+ if (m->flags & IVMD_FLAG_EXCL_RANGE)
+ init_exclusion_range(m);
+
switch (m->type) {
default:
kfree(e);
@@ -1541,9 +1544,7 @@ static int __init init_memory_definitions(struct
acpi_table_header *table)
while (p < end) {
m = (struct ivmd_header *)p;
- if (m->flags & IVMD_FLAG_EXCL_RANGE)
- init_exclusion_range(m);
- else if (m->flags & IVMD_FLAG_UNITY_MAP)
+ if (m->flags & (IVMD_FLAG_UNITY_MAP | IVMD_FLAG_EXCL_RANGE))
init_unity_map_range(m);
p += m->length;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index b08cf57bf455..31d27eb70565 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -324,6 +324,8 @@
#define IOMMU_PROT_IR 0x01
#define IOMMU_PROT_IW 0x02
+#define IOMMU_UNITY_MAP_FLAG_EXCL_RANGE(1 << 2)
+
/* IOMMU capabilities */
#define IOMMU_CAP_IOTLB 24
#define IOMMU_CAP_NPCACHE 26
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a070fa39521a..ef4aa2879952 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -351,6 +351,9 @@ static int iommu_group_create_direct_mappings(struct
iommu_group *group,
start = ALIGN(entry->start, pg_size);
end = ALIGN(entry->start + entry->length, pg_size);
+ if (entry->type != IOMMU_RESV_DIRECT)
+ continue;
+
for (addr = start; addr < end; addr += pg_size) {
phys_addr_t phys_addr;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f28dff313b07..15b7378f67f3 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -115,18 +115,23 @@ enum iommu_attr {
DOMAIN_ATTR_MAX,
};
+#define IOMMU_RESV_DIRECT (1 << 0)
+#define IOMMU_RESV_RESERVED(1 << 1)
+
/**
* struct iommu_dm_region - descriptor for a direct mapped memory region
* @list: Linked list pointers
* @start: System physical start address of the region
* @length: Length of the region in bytes
* @prot: IOMMU Protection flags (READ/WRITE/...)
+ * @type: Type of region (DIRECT, RESERVED)
*/
struct iommu_dm_region {
struct list_headlist;
phys_addr_t start;
size_t length;
int prot;
+ int type;
};
#ifdef CONFIG_IOMMU_API
--
2.21.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
On 01/03/2019 20:20, Douglas Anderson wrote: > If you're bisecting why your peripherals stopped working, it's > probably this CL. Specifically if you see this in your dmesg: > Unexpected global fault, this could be serious > ...then it's almost certainly this CL. > > Running your IOMMU-enabled peripherals with the IOMMU in bypass mode > is insecure and effectively disables the protection they provide. > There are few reasons to allow unmatched stream bypass, and even fewer > good ones. > > This patch starts the transition over to make it much harder to run > your system insecurely. Expected steps: > > 1. By default disable bypass (so anyone insecure will notice) but make >it easy for someone to re-enable bypass with just a KConfig change. >That's this patch. > > 2. After people have had a little time to come to grips with the fact >that they need to set their IOMMUs properly and have had time to >dig into how to do this, the KConfig will be eliminated and bypass >will simply be disabled. Folks who are truly upset and still >haven't fixed their system can either figure out how to add >'arm-smmu.disable_bypass=n' to their command line or revert the >patch in their own private kernel. Of course these folks will be >less secure. > > Suggested-by: Robin Murphy > Signed-off-by: Douglas Anderson > --- > > Changes in v2: > - Flipped default to 'yes' and changed comments a lot. > > drivers/iommu/Kconfig| 25 + > drivers/iommu/arm-smmu.c | 3 ++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 1ca1fa107b21..a4210672804a 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -359,6 +359,31 @@ config ARM_SMMU > Say Y here if your SoC includes an IOMMU device implementing > the ARM SMMU architecture. > > +config ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT > + bool "Default to disabling bypass on ARM SMMU v1 and v2" > + depends on ARM_SMMU > + default y > + help > + Say Y here to (by default) disable bypass streams such that > + incoming transactions from devices that are not attached to > + an iommu domain will report an abort back to the device and > + will not be allowed to pass through the SMMU. > + > + Any old kernels that existed before this KConfig was > + introduced would default to _allowing_ bypass (AKA the > + equivalent of NO for this config). However the default for > + this option is YES because the old behavior is insecure. > + > + There are few reasons to allow unmatched stream bypass, and > + even fewer good ones. If saying YES here breaks your board > + you should work on fixing your board. This KConfig option > + is expected to be removed in the future and we'll simply > + hardcode the bypass disable in the code. > + > + NOTE: the kernel command line parameter > + 'arm-smmu.disable_bypass' will continue to override this > + config. > + > config ARM_SMMU_V3 > bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support" > depends on ARM64 > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 045d93884164..930c07635956 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -110,7 +110,8 @@ static int force_stage; > module_param(force_stage, int, S_IRUGO); > MODULE_PARM_DESC(force_stage, > "Force SMMU mappings to be installed at a particular stage of > translation. A value of '1' or '2' forces the corresponding stage. All other > values are ignored (i.e. no stage is forced). Note that selecting a specific > stage will disable support for nested translation."); > -static bool disable_bypass; > +static bool disable_bypass = > + IS_ENABLED(CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT); > module_param(disable_bypass, bool, S_IRUGO); > MODULE_PARM_DESC(disable_bypass, > "Disable bypass streams such that incoming transactions from devices > that are not attached to an iommu domain will report an abort back to the > device and will not be allowed to pass through the SMMU."); > OK, so my defconfig contains neither IOMMU_DEFAULT_PASSTHROUGH nor ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT. Thus my .config contains: # CONFIG_IOMMU_DEFAULT_PASSTHROUGH is not set CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y I have checked that disable_bypass is indeed set to true. And that we can override the default value on the command-line with arm-smmu.disable_bypass=0 And that PCIe and UFS still work on my board (they are "behind" the ANOC1 SMMU). Reviewed-by: Marc Gonzalez Tested-by: Marc Gonzalez Regards. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 2/3] arm64: dts: qcom: msm8998: Add PCIe SMMU node
On 30/03/2019 14:18, Vivek Gautam wrote: You should probably have some "bus" and "iface" clocks too, per the requirement of "qcom,smmu-v2". Maybe Vivek might know what's relevant for MSM8998? As Jeffrey rightly mentioned, these clocks are not under the control of Linux. So, we won't need to add clocks to this SMMU. OK, in that case the "clock-names" part of binding doc probably wants refining to reflect which implementations do actually require clocks. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] drm/panfrost: Add initial panfrost driver
On 02/04/2019 01:33, Alyssa Rosenzweig wrote:
the userspace definitely doesn't support T624
This is true, yes. Shouldn't be too hard to backport; if there's still
interest in Midgard 1st/2nd gen, I suppose I can grab hardware and sort
it out...
I'm quite likely the only person trying this on an Arm Juno board, and
even then it's really only for giggles because I can :) I guess there
might be a fair few Odroid-XU3/XU4 (T628) users interested, though.
You probably want a dma_set_mask_and_coherent() call for your 'real' output
address size somewhere - the default 32-bit mask works out OK for RK3399,
but on systems with RAM above 4GB io-pgtable will get very unhappy about DMA
bounce-buffering.
Out of curiosity, are there Mali systems with >4GB RAM? That sounds
awesome :)
Now that the "early-access Armv8 silicon" angle has well and truly
expired, Juno is essentially a prototyping platform where the SoC just
serves to (slowly) drive interesting things in FPGA cards, so although
it may have 8GB of RAM, it's not all that exciting. There is one
somewhat more realistic board I'm aware of, namely HiKey 970 with a G72
and 6GB.
Any chance of resurrecting the generic "arm,mali-midgard" compatible? :P
...Would that require editing everybody's DT file?
If they already have one of the strings from the current upstream
binding, no - I only mean to suggest adding it as an additional
last-level fallback. That would aid compatibility with downstream DTs,
for example RK3288 which currently has zero overlap:
upstream: "rockchip,rk3288-mali", "arm,mali-t760";
downstream: "arm,malit764", "arm,malit76x", "arm,malit7xx",
"arm,mali-midgard";
Similarly, it might be reasonable for panfrost_{gpu,mmu,job}_init() to
retry platform_get_irq_byname() with uppercase interrupt names if the
expected ones aren't found - obviously the upstream binding comes first
and foremost, but I don't see any harm in quietly supporting bits of the
downstream binding if it makes users' lives easier when switching
between mainline and vendor kernels.
Cheers,
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/3] PCI: qcom: Setup PCIE20_PARF_BDF_TRANSLATE_N
On 02/04/2019 10:08, Stanimir Varbanov wrote: > On 3/28/19 7:01 PM, Marc Gonzalez wrote: > >> Initialize PCIE20_PARF_BDF_TRANSLATE_N for ops_2_3_2. >> >> Signed-off-by: Marc Gonzalez >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c >> b/drivers/pci/controller/dwc/pcie-qcom.c >> index 0ed235d560e3..5e5522a427b8 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -54,6 +54,7 @@ >> #define PCIE20_PARF_LTSSM 0x1B0 >> #define PCIE20_PARF_SID_OFFSET 0x234 >> #define PCIE20_PARF_BDF_TRANSLATE_CFG 0x24C >> +#define PCIE20_PARF_BDF_TRANSLATE_N 0x250 >> >> #define PCIE20_ELBI_SYS_CTRL0x04 >> #define PCIE20_ELBI_SYS_CTRL_LT_ENABLE BIT(0) >> @@ -602,6 +603,9 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie) >> val |= BIT(31); >> writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2); >> >> +val = PCI_DEVID(1, 0); >> +writel(val, pcie->parf + PCIE20_PARF_BDF_TRANSLATE_N); > > I wonder, shouldn't this register manipulation happen in common code > e.g. pcie-designware-host.c I don't think so. PARF (PCIe Auxiliary Register File) appears to be a vendor-specific area. Perhaps someone with access to the Designware documentation could check. (Though it is possible that several vendors have implemented exactly the same PARF layout.) > and of course the manipulation should be conditional because > not all platforms might have iommu. My patch tweaks PCIE20_PARF_BDF_TRANSLATE_N in qcom_pcie_init_2_3_2() which means 8996 and 8998 only (and possibly sdm660) $ git grep smmu-exist vendor -- arch/arm/boot/dts/qcom vendor:arch/arm/boot/dts/qcom/msm8996.dtsi: qcom,smmu-exist; vendor:arch/arm/boot/dts/qcom/msm8996.dtsi: qcom,smmu-exist; vendor:arch/arm/boot/dts/qcom/msm8996.dtsi: qcom,smmu-exist; vendor:arch/arm/boot/dts/qcom/msm8998-interposer-sdm660.dtsi: qcom,smmu-exist; vendor:arch/arm/boot/dts/qcom/msm8998.dtsi: qcom,smmu-exist; https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/drivers/pci/host/pci-msm.c?h=LE.UM.1.3.r3.25#n3880 > Also, could we have more generic iommu-map description like the one for > sdm845 at [1] ? I assume you meant https://patchwork.kernel.org/patch/10831293/ I don't think that such a map is quite right for msm8998 -- and even for sdm845 :-p There is only a single PCIe master, other than the root complex: # lspci 00:00.0 PCI bridge: Qualcomm Device 0105 01:00.0 Ethernet controller: Qualcomm Atheros AR8151 v2.0 Gigabit Ethernet (rev c0) Hence iommu-map = <0x100 &anoc1_smmu 0x1480 1>; i.e. map stream-id 0x1480 to device-id 01:00.0 We could pick a different stream-id for 0x100 but I don't see any advantage (??) in doing so... If a SoC designer took the msm8998 and added a PCIe bridge to support multiple PCIe EPs, then they would override the iommu-map in their board-specific DTS. I do have a nagging feeling that this patch is not perfect for 3 reasons: 1) Did msm8996 PCIe work without tweaking PCIE20_PARF_BDF_TRANSLATE_N? If so, why? What is the difference with msm8998 PCIe? "MSM8998 does not have static SID configuration for PCIe." What about MSM8996? 2) OF: /soc/pci@1c0: no iommu-map translation for rid 0x0 on (null) The kernel warns that the root complex has no asssigned stream-id. I'm not sure that's actually an issue... 3) I just blindly write PCI_DEVID(1, 0) to PCIE20_PARF_BDF_TRANSLATE_N To be really generic, I would have to walk the iommu-map and do for_each_iommu_map_entry offset = PCIE20_PARF_BDF_TRANSLATE_N + (out_base - 0x1480) * 4; writel(rid_base, pcie->parf + offset); Perhaps do this in of_pci_iommu_init() ? Maybe have to tweak of_map_rid() or duplicate some code... On first approximation, maybe I can just do the blind write, but only for 8998. That way, I don't change anything for the other platforms. Comments? Regards. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: 5.1-rc1: mpt init crash in scsi_map_dma, dma_4v_map_sg on sparc64
On Tue, Mar 19, 2019 at 7:20 PM Meelis Roos wrote:
>
> Tried 5.1-rc1 on a bunch of sparcs, this hits all my sparcs with sun4v and
> mpt scsi.
>
> [2.733263] Fusion MPT base driver 3.04.20
> [2.742995] Copyright (c) 1999-2008 LSI Corporation
> [2.743052] Fusion MPT SAS Host driver 3.04.20
> [2.743881] mptbase: ioc0: Initiating bringup
> [3.737822] ioc0: LSISAS1064 A3: Capabilities={Initiator}
> [ 17.566584] scsi host0: ioc0: LSISAS1064 A3, FwRev=010ah, Ports=1,
> MaxQ=511, IRQ=27
> [ 17.595897] mptsas: ioc0: attaching ssp device: fw_channel 0, fw_id 0, phy
> 0, sas_addr 0x5000c5001799a45d
> [ 17.598465] Unable to handle kernel NULL pointer dereference
> [ 17.598623] tsk->{mm,active_mm}->context =
> [ 17.598723] tsk->{mm,active_mm}->pgd = 88802000
> [ 17.598774] \|/ \|/
> [ 17.598774] "@'/ .. \`@"
> [ 17.598774] /_| \__/ |_\
> [ 17.598774] \__U_/
> [ 17.598894] swapper/0(1): Oops [#1]
> [ 17.598937] CPU: 12 PID: 1 Comm: swapper/0 Not tainted 5.1.0-rc1 #118
> [ 17.598994] TSTATE: 80e01601 TPC: 004483a8 TNPC:
> 004483ac Y: Not tainted
> [ 17.599086] TPC:
You may use gdb to figure out what the NULL pointer points to:
gdb vmlinux
> l *(dma_4v_map_sg+0xe8)
Thanks,
Ming Lei
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/3] PCI: qcom: Setup PCIE20_PARF_BDF_TRANSLATE_N
Hi Marc, Thank you for the work on that! On 3/28/19 7:01 PM, Marc Gonzalez wrote: > Initialize PCIE20_PARF_BDF_TRANSLATE_N for ops_2_3_2. > > Signed-off-by: Marc Gonzalez > --- > drivers/pci/controller/dwc/pcie-qcom.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c > b/drivers/pci/controller/dwc/pcie-qcom.c > index 0ed235d560e3..5e5522a427b8 100644 > --- a/drivers/pci/controller/dwc/pcie-qcom.c > +++ b/drivers/pci/controller/dwc/pcie-qcom.c > @@ -54,6 +54,7 @@ > #define PCIE20_PARF_LTSSM0x1B0 > #define PCIE20_PARF_SID_OFFSET 0x234 > #define PCIE20_PARF_BDF_TRANSLATE_CFG0x24C > +#define PCIE20_PARF_BDF_TRANSLATE_N 0x250 > > #define PCIE20_ELBI_SYS_CTRL 0x04 > #define PCIE20_ELBI_SYS_CTRL_LT_ENABLE BIT(0) > @@ -602,6 +603,9 @@ static int qcom_pcie_init_2_3_2(struct qcom_pcie *pcie) > val |= BIT(31); > writel(val, pcie->parf + PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2); > > + val = PCI_DEVID(1, 0); > + writel(val, pcie->parf + PCIE20_PARF_BDF_TRANSLATE_N); I wonder, shouldn't this register manipulation happen in common code e.g. pcie-designware-host.c and of course the manipulation should be conditional because not all platforms might have iommu. Also, could we have more generic iommu-map description like the one for sdm845 at [1] ? -- regards, Stan ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/2] arm64: dts: qcom: msm8998: Add ANOC1 SMMU node
On 01/04/2019 17:40, Marc Gonzalez wrote:
> The MSM8998 ANOC1(*) SMMU services BLSP2, PCIe, UFS, and USB.
> (*) Aggregate Network-on-Chip #1
>
> Based on the following DTS downstream:
> https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm-arm-smmu-8998.dtsi?h=LE.UM.1.3.r3.25#n18
>
> Signed-off-by: Marc Gonzalez
> ---
> Changes from v1:
> Split off from "PCIe and AR8151 on APQ8098/MSM8998" series
> Change compatible string to use qcom,msm8998-smmu-v2
> ---
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> index ef71e8f1d102..f807ea3e2c6e 100644
> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> @@ -606,6 +606,21 @@
> #thermal-sensor-cells = <1>;
> };
>
> + anoc1_smmu: arm,smmu@168 {
Doh! This should probably be anoc1_smmu: iommu@168
Bjorn, can you fix it up when you pick up the patch, or do you need me
to respin?
Regards.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
