Re: [PATCHv7 3/3] iommu: arm-smmu-impl: Add sdm845 implementation hook
On Fri 20 Sep 01:04 PDT 2019, Sai Prakash Ranjan wrote:
> From: Vivek Gautam
>
> Add reset hook for sdm845 based platforms to turn off
> the wait-for-safe sequence.
>
> Understanding how wait-for-safe logic affects USB and UFS performance
> on MTP845 and DB845 boards:
>
> Qcom's implementation of arm,mmu-500 adds a WAIT-FOR-SAFE logic
> to address under-performance issues in real-time clients, such as
> Display, and Camera.
> On receiving an invalidation requests, the SMMU forwards SAFE request
> to these clients and waits for SAFE ack signal from real-time clients.
> The SAFE signal from such clients is used to qualify the start of
> invalidation.
> This logic is controlled by chicken bits, one for each - MDP (display),
> IFE0, and IFE1 (camera), that can be accessed only from secure software
> on sdm845.
>
> This configuration, however, degrades the performance of non-real time
> clients, such as USB, and UFS etc. This happens because, with wait-for-safe
> logic enabled the hardware tries to throttle non-real time clients while
> waiting for SAFE ack signals from real-time clients.
>
> On mtp845 and db845 devices, with wait-for-safe logic enabled by the
> bootloaders we see degraded performance of USB and UFS when kernel
> enables the smmu stage-1 translations for these clients.
> Turn off this wait-for-safe logic from the kernel gets us back the perf
> of USB and UFS devices until we re-visit this when we start seeing perf
> issues on display/camera on upstream supported SDM845 platforms.
> The bootloaders on these boards implement secure monitor callbacks to
> handle a specific command - QCOM_SCM_SVC_SMMU_PROGRAM with which the
> logic can be toggled.
>
> There are other boards such as cheza whose bootloaders don't enable this
> logic. Such boards don't implement callbacks to handle the specific SCM
> call so disabling this logic for such boards will be a no-op.
>
> This change is inspired by the downstream change from Patrick Daly
> to address performance issues with display and camera by handling
> this wait-for-safe within separte io-pagetable ops to do TLB
> maintenance. So a big thanks to him for the change and for all the
> offline discussions.
>
> Without this change the UFS reads are pretty slow:
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=10 conv=sync
> 10+0 records in
> 10+0 records out
> 10485760 bytes (10.0MB) copied, 22.394903 seconds, 457.2KB/s
> real0m 22.39s
> user0m 0.00s
> sys 0m 0.01s
>
> With this change they are back to rock!
> $ time dd if=/dev/sda of=/dev/zero bs=1048576 count=300 conv=sync
> 300+0 records in
> 300+0 records out
> 314572800 bytes (300.0MB) copied, 1.030541 seconds, 291.1MB/s
> real0m 1.03s
> user0m 0.00s
> sys 0m 0.54s
>
> Signed-off-by: Vivek Gautam
> Reviewed-by: Robin Murphy
> Signed-off-by: Sai Prakash Ranjan
Reviewed-by: Bjorn Andersson
> ---
> drivers/iommu/Makefile| 2 +-
> drivers/iommu/arm-smmu-impl.c | 5 +++-
> drivers/iommu/arm-smmu-qcom.c | 51 +++
> drivers/iommu/arm-smmu.h | 3 +++
> 4 files changed, 59 insertions(+), 2 deletions(-)
> create mode 100644 drivers/iommu/arm-smmu-qcom.c
>
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 4f405f926e73..86dadd13b2e6 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -13,7 +13,7 @@ obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
> obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
> obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
> obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
> -obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
> +obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o arm-smmu-qcom.o
> obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
> obj-$(CONFIG_DMAR_TABLE) += dmar.o
> obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index 5c87a38620c4..b2fe72a8f019 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -109,7 +109,7 @@ static struct arm_smmu_device
> *cavium_smmu_impl_init(struct arm_smmu_device *smm
> #define ARM_MMU500_ACR_S2CRB_TLBEN (1 << 10)
> #define ARM_MMU500_ACR_SMTNMB_TLBEN (1 << 8)
>
> -static int arm_mmu500_reset(struct arm_smmu_device *smmu)
> +int arm_mmu500_reset(struct arm_smmu_device *smmu)
> {
> u32 reg, major;
> int i;
> @@ -170,5 +170,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
> arm_smmu_device *smmu)
> "calxeda,smmu-secure-config-access"))
> smmu->impl = _impl;
>
> + if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
> + return qcom_smmu_impl_init(smmu);
> +
> return smmu;
> }
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> new file mode 100644
> index ..24c071c1d8b0
> --- /dev/null
> +++
Re: [PATCH v4 0/7] Add support for QCOM IOMMU v2 and 500
On Tue 01 Oct 15:01 PDT 2019, khol...@gmail.com wrote: > From: AngeloGioacchino Del Regno > > Some Qualcomm Family-B SoCs have got a different version of the QCOM > IOMMU, specifically v2 and 500, which perfectly adhere to the current > qcom_iommu driver, but need some variations due to slightly different > hypervisor behavior. > Do you think it's out of the question to get the arm-smmu driver to play nice with this platform? If not, would it be possible to change the DT binding so that we specify the SID and then read the SMR and S2CR registers to figure out the associated context bank? Regards, Bjorn > The personal aim is to upstream MSM8956 as much as possible. > > This code has been tested on two Sony phones featuring the Qualcomm > MSM8956 SoC. > > Changes in v2: > - Fixed optional properties placement in documentation > > Changes in v3: > - Rebased onto linux-next 01/10/2019 > - Added missing SCM commit (required by the AArch64 PT switch support) > > Changes in v4: > - Removed rej files from the SCM patch (I'm truly sorry for the noise...) > > Angelo G. Del Regno (1): > firmware: qcom: scm: Add function to set IOMMU pagetable addressing > > AngeloGioacchino Del Regno (6): > iommu/qcom: Use the asid read from device-tree if specified > iommu/qcom: Write TCR before TTBRs to fix ASID access behavior > iommu/qcom: Properly reset the IOMMU context > iommu/qcom: Add support for AArch64 IOMMU pagetables > iommu/qcom: Index contexts by asid number to allow asid 0 > iommu/qcom: Add support for QCIOMMUv2 and QCIOMMU-500 secured contexts > > .../devicetree/bindings/iommu/qcom,iommu.txt | 5 + > drivers/firmware/qcom_scm-32.c| 6 + > drivers/firmware/qcom_scm-64.c| 15 ++ > drivers/firmware/qcom_scm.c | 7 + > drivers/firmware/qcom_scm.h | 4 + > drivers/iommu/qcom_iommu.c| 134 ++ > include/linux/qcom_scm.h | 2 + > 7 files changed, 145 insertions(+), 28 deletions(-) > > -- > 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 2019-10-04 9:37 pm, Tim Harvey wrote: On Fri, Oct 4, 2019 at 11:34 AM Robin Murphy wrote: On 04/10/2019 18:13, Tim Harvey wrote: [...] No difference... still need 'arm-smmu.disable_bypass=n' to boot. Are all four iommu-map props above supposed to be the same? Seems to me they all point to the same thing which looks wrong. Hmm... :/ Those mappings just set Stream ID == PCI RID (strictly each one should only need to cover the bus range assigned to that bridge, but it's not crucial) which is the same thing the driver assumes for the mmu-masters property, so either that's wrong and never could have worked anyway - have you tried VFIO on this platform? - or there are other devices also mastering through the SMMU that aren't described at all. Are you able to capture a boot log? The SMMU faults do encode information about the offending ID, and you can typically correlate their appearance reasonably well with endpoint drivers probing. Robin, VFIO is enabled in the kernel but I don't know anything about how to test/use it: $ grep VFIO .config CONFIG_KVM_VFIO=y CONFIG_VFIO_IOMMU_TYPE1=y CONFIG_VFIO_VIRQFD=y CONFIG_VFIO=y # CONFIG_VFIO_NOIOMMU is not set CONFIG_VFIO_PCI=y CONFIG_VFIO_PCI_MMAP=y CONFIG_VFIO_PCI_INTX=y # CONFIG_VFIO_PLATFORM is not set # CONFIG_VFIO_MDEV is not set No worries - since it's a networking-focused SoC I figured there was a chance you might be using DPDK or similar userspace drivers with the NIC VFs, but I was just casting around for a quick and easy baseline of whether the SMMU works at all (another way would be using Qemu to run a VM with one or more PCI devices assigned). I do have a boot console yet I'm not seeing any smmu faults at all. Perhaps I've mis-diagnosed the issue completely. To be clear when I boot with arm-smmu.disable_bypass=y the serial console appears to not accept input in userspace and with arm-smmu.disable_bypass=n I'm fine. I'm using a buildroot initramfs rootfs for simplicity. The system isn't hung as I originally expected as the LED heartbeat trigger continues blinking... I just can't get console to accept input. Curiouser and curiouser... I'm inclined to suspect that the interrupt configuration might also be messed up, such that the SMMU is blocking traffic and jammed up due to pending faults, but you're not getting the IRQ delivered to find out. Does this patch help reveal anything? http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=29ac3648b580920692c9b417b2fc606995826517 (untested, but it's a direct port of the one I've used for SMMUv3 to diagnose something similar) This shows: Yay (ish)! [ and the tangential challenge would be to find out what the real global fault interrupt is, 'cause apparently it's not SPI 68... ] arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0140, GFSYNR2 0x If that stream ID were a PCI RID, it would be 01:08.0 arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x And this guy would be 00:02.0 So it seems that either the stream ID mapping is non-trivial (and "mmu-masters" couldn't have worked), or there are secret magic endpoints writing to memory during boot. Either way it probably needs some input from Cavium/Marvell to get straight. In the meantime, unless just disabling and ignoring the SMMU altogether is a viable option, I guess we have to resign to this being one of those "non-good" reasons for needing global bypass :( Robin. (note to self: it would probably be useful if we dump GFAR in these logs too. These are all writes, so it's possible they could be MSI attempts targeting the ITS rather than DMA as such) arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0:
Re: [PATCH v2] dma-mapping: Move vmap address checks into dma_map_single()
On 10/4/19 2:28 PM, Kees Cook wrote:
> As we've seen from USB and other areas, we need to always do runtime
> checks for DMA operating on memory regions that might be remapped. This
> moves the existing checks from USB into dma_map_single(), but leaves
> the slightly heavier checks as they are.
>
> Suggested-by: Laura Abbott
> Signed-off-by: Kees Cook
Nearly the same might be necessary for
drivers/usb/gadget/udc/core.c::usb_gadget_map_request_by_dev(). You
might also want to check other subsystems, SPI tries to be fairly smart
about vmalloc'd and kmap'd buffer, some I2C drivers have explicit checks
(they should probably rely on the core), and finally MTD is an area
where this has also been historically dealt with.
> ---
> v2: Only add is_vmalloc_addr()
> v1: https://lore.kernel.org/lkml/201910021341.7819A660@keescook
> ---
> drivers/usb/core/hcd.c | 8 +---
> include/linux/dma-mapping.h | 7 +++
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index f225eaa98ff8..281568d464f9 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1410,10 +1410,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd,
> struct urb *urb,
> if (hcd->self.uses_pio_for_control)
> return ret;
> if (hcd_uses_dma(hcd)) {
> - if (is_vmalloc_addr(urb->setup_packet)) {
> - WARN_ONCE(1, "setup packet is not dma
> capable\n");
> - return -EAGAIN;
> - } else if (object_is_on_stack(urb->setup_packet)) {
> + if (object_is_on_stack(urb->setup_packet)) {
> WARN_ONCE(1, "setup packet is on stack\n");
> return -EAGAIN;
> }
> @@ -1479,9 +1476,6 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
> urb *urb,
> ret = -EAGAIN;
> else
> urb->transfer_flags |= URB_DMA_MAP_PAGE;
> - } else if (is_vmalloc_addr(urb->transfer_buffer)) {
> - WARN_ONCE(1, "transfer buffer not dma
> capable\n");
> - ret = -EAGAIN;
> } else if (object_is_on_stack(urb->transfer_buffer)) {
> WARN_ONCE(1, "transfer buffer is on stack\n");
> ret = -EAGAIN;
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4a1c4fca475a..12dbd07f74f2 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -583,6 +583,13 @@ static inline unsigned long
> dma_get_merge_boundary(struct device *dev)
> static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
> size_t size, enum dma_data_direction dir, unsigned long attrs)
> {
> + /* DMA must never operate on areas that might be remapped. */
> + if (WARN_ONCE(is_vmalloc_addr(ptr),
> + "%s %s: driver maps %lu bytes from vmalloc area\n",
> + dev ? dev_driver_string(dev) : "unknown driver",
> + dev ? dev_name(dev) : "unknown device", size))
> + return DMA_MAPPING_ERROR;
> +
> debug_dma_map_single(dev, ptr, size);
> return dma_map_page_attrs(dev, virt_to_page(ptr), offset_in_page(ptr),
> size, dir, attrs);
>
--
Florian
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] dma-mapping: Move vmap address checks into dma_map_single()
As we've seen from USB and other areas, we need to always do runtime
checks for DMA operating on memory regions that might be remapped. This
moves the existing checks from USB into dma_map_single(), but leaves
the slightly heavier checks as they are.
Suggested-by: Laura Abbott
Signed-off-by: Kees Cook
---
v2: Only add is_vmalloc_addr()
v1: https://lore.kernel.org/lkml/201910021341.7819A660@keescook
---
drivers/usb/core/hcd.c | 8 +---
include/linux/dma-mapping.h | 7 +++
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index f225eaa98ff8..281568d464f9 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1410,10 +1410,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
if (hcd->self.uses_pio_for_control)
return ret;
if (hcd_uses_dma(hcd)) {
- if (is_vmalloc_addr(urb->setup_packet)) {
- WARN_ONCE(1, "setup packet is not dma
capable\n");
- return -EAGAIN;
- } else if (object_is_on_stack(urb->setup_packet)) {
+ if (object_is_on_stack(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is on stack\n");
return -EAGAIN;
}
@@ -1479,9 +1476,6 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct
urb *urb,
ret = -EAGAIN;
else
urb->transfer_flags |= URB_DMA_MAP_PAGE;
- } else if (is_vmalloc_addr(urb->transfer_buffer)) {
- WARN_ONCE(1, "transfer buffer not dma
capable\n");
- ret = -EAGAIN;
} else if (object_is_on_stack(urb->transfer_buffer)) {
WARN_ONCE(1, "transfer buffer is on stack\n");
ret = -EAGAIN;
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fca475a..12dbd07f74f2 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -583,6 +583,13 @@ static inline unsigned long dma_get_merge_boundary(struct
device *dev)
static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
size_t size, enum dma_data_direction dir, unsigned long attrs)
{
+ /* DMA must never operate on areas that might be remapped. */
+ if (WARN_ONCE(is_vmalloc_addr(ptr),
+ "%s %s: driver maps %lu bytes from vmalloc area\n",
+ dev ? dev_driver_string(dev) : "unknown driver",
+ dev ? dev_name(dev) : "unknown device", size))
+ return DMA_MAPPING_ERROR;
+
debug_dma_map_single(dev, ptr, size);
return dma_map_page_attrs(dev, virt_to_page(ptr), offset_in_page(ptr),
size, dir, attrs);
--
2.17.1
--
Kees Cook
[PATCH] iommu/intel: Use fallback generic_device_group() for ACPI devices
[2.073922] DMAR: ACPI device "INT33C2:00" under DMAR at fed91000 as 00:15.1
[2.073983] DMAR: ACPI device "INT33C3:00" under DMAR at fed91000 as 00:15.2
[2.074027] DMAR: ACPI device "INT33C0:00" under DMAR at fed91000 as 00:15.3
[2.074072] DMAR: ACPI device "INT33C1:00" under DMAR at fed91000 as 00:15.4
[2.074114] DMAR: Failed to find handle for ACPI object \_SB.PCI0.UA01
[2.074156] DMAR: Failed to find handle for ACPI object \_SB.PCI0.SDHC
[2.074208] DMAR: No ATSR found
[2.074572] DMAR: dmar0: Using Queued invalidation
[2.074629] DMAR: dmar1: Using Queued invalidation
[2.110029] pci :00:00.0: Adding to iommu group 0
[2.115703] pci :00:02.0: Adding to iommu group 1
[2.116221] pci :00:03.0: Adding to iommu group 2
[2.116759] pci :00:14.0: Adding to iommu group 3
[2.117276] pci :00:16.0: Adding to iommu group 4
[2.117762] pci :00:1b.0: Adding to iommu group 5
[2.118264] pci :00:1c.0: Adding to iommu group 6
[2.118733] pci :00:1c.2: Adding to iommu group 7
[2.119289] pci :00:1d.0: Adding to iommu group 8
[2.119846] pci :00:1f.0: Adding to iommu group 9
[2.119960] pci :00:1f.2: Adding to iommu group 9
[2.120073] pci :00:1f.3: Adding to iommu group 9
[2.120549] pci :06:00.0: Adding to iommu group 10
[2.120631] [ cut here ]
[2.120681] WARNING: CPU: 2 PID: 1 at drivers/iommu/iommu.c:1275
pci_device_group+0x109/0x120
[2.120723] Modules linked in:
[2.120744] CPU: 2 PID: 1 Comm: swapper/0 Not tainted
5.4.0-rc1-CI-CI_DRM_7000+ #1
[2.120782] Hardware name: Dell Inc. XPS 12-9Q33/XPS 12-9Q33, BIOS A04
12/03/2013
[2.120821] RIP: 0010:pci_device_group+0x109/0x120
[2.120848] Code: e9 ff ff 48 85 c0 48 89 c5 75 bd 48 8d 74 24 10 4c 89 e7
e8 49 ea ff ff 48 85 c0 48 89 c5 75 a8 e8 fc ee ff ff 48 89 c5 eb 9e <0f> 0b 48
c7 c5 ea ff ff ff eb 93 e8 37 5f a7 ff 0f 1f 80 00 00 00
[2.120933] RSP: :c9037cd0 EFLAGS: 00010202
[2.120961] RAX: 81639810 RBX: ffea RCX:
[2.120996] RDX: RSI: 403efd19 RDI: 88811c08
[2.121031] RBP: 88811c08 R08: 88811a5188f8 R09: fffe
[2.121066] R10: ca7d066a R11: 2161dc90 R12: 888118320a58
[2.121100] R13: 888119fc1e50 R14: 0001 R15: 888119fc2300
[2.121136] FS: () GS:88811b90()
knlGS:
[2.121176] CS: 0010 DS: ES: CR0: 80050033
[2.121205] CR2: CR3: 05210001 CR4: 001606e0
[2.121240] Call Trace:
[2.121264] iommu_group_get_for_dev+0x77/0x210
[2.121295] intel_iommu_add_device+0x54/0x1c0
[2.121323] iommu_probe_device+0x43/0xc0
[2.121350] intel_iommu_init+0x11fb/0x12c9
[2.121383] ? set_debug_rodata+0xc/0xc
[2.121410] ? set_debug_rodata+0xc/0xc
[2.121434] ? e820__memblock_setup+0x5b/0x5b
[2.121458] ? pci_iommu_init+0x11/0x3a
[2.121471] ? rcu_read_lock_sched_held+0x4d/0x80
[2.121471] pci_iommu_init+0x11/0x3a
[2.121471] do_one_initcall+0x58/0x2ff
[2.121471] ? set_debug_rodata+0xc/0xc
[2.121471] ? rcu_read_lock_sched_held+0x4d/0x80
[2.121471] kernel_init_freeable+0x137/0x1c7
[2.121471] ? rest_init+0x250/0x250
[2.121471] kernel_init+0x5/0x100
[2.121471] ret_from_fork+0x3a/0x50
[2.121471] irq event stamp: 1252438
[2.121471] hardirqs last enabled at (1252437): []
__slab_alloc.isra.84.constprop.89+0x4d/0x70
[2.121471] hardirqs last disabled at (1252438): []
trace_hardirqs_off_thunk+0x1a/0x20
[2.121471] softirqs last enabled at (1252382): []
__do_softirq+0x385/0x47f
[2.121471] softirqs last disabled at (1252375): []
irq_exit+0xba/0xc0
[2.121471] ---[ end trace 610717c918cf08f3 ]---
[2.121974] DMAR: ACPI name space devices didn't probe correctly
[2.122069] DMAR: Intel(R) Virtualization Technology for Directed I/O
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111906
Signed-off-by: Chris Wilson
Cc: Joerg Roedel
---
Take the patch with a pinch of salt; it seems to be the pattern used by
other iommu backends, but I don't know if it is even suitable for iommu
and what appear to be ACPI devices rather than the expected PCI.
---
drivers/iommu/intel-iommu.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ed321b808176..e231be0d0534 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5939,6 +5939,14 @@ static bool intel_iommu_is_attach_deferred(struct
iommu_domain *domain,
return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO;
}
+static struct iommu_group *intel_iommu_device_group(struct device *dev)
+{
+ if (dev_is_pci(dev))
+ return pci_device_group(dev);
+ else
+
Re: [PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
On Fri, Oct 4, 2019 at 11:34 AM Robin Murphy wrote: > > On 04/10/2019 18:13, Tim Harvey wrote: > [...] > >>> No difference... still need 'arm-smmu.disable_bypass=n' to boot. Are > >>> all four iommu-map props above supposed to be the same? Seems to me > >>> they all point to the same thing which looks wrong. > >> > >> Hmm... :/ > >> > >> Those mappings just set Stream ID == PCI RID (strictly each one should > >> only need to cover the bus range assigned to that bridge, but it's not > >> crucial) which is the same thing the driver assumes for the mmu-masters > >> property, so either that's wrong and never could have worked anyway - > >> have you tried VFIO on this platform? - or there are other devices also > >> mastering through the SMMU that aren't described at all. Are you able to > >> capture a boot log? The SMMU faults do encode information about the > >> offending ID, and you can typically correlate their appearance > >> reasonably well with endpoint drivers probing. > >> > > > > Robin, > > > > VFIO is enabled in the kernel but I don't know anything about how to > > test/use it: > > $ grep VFIO .config > > CONFIG_KVM_VFIO=y > > CONFIG_VFIO_IOMMU_TYPE1=y > > CONFIG_VFIO_VIRQFD=y > > CONFIG_VFIO=y > > # CONFIG_VFIO_NOIOMMU is not set > > CONFIG_VFIO_PCI=y > > CONFIG_VFIO_PCI_MMAP=y > > CONFIG_VFIO_PCI_INTX=y > > # CONFIG_VFIO_PLATFORM is not set > > # CONFIG_VFIO_MDEV is not set > > No worries - since it's a networking-focused SoC I figured there was a > chance you might be using DPDK or similar userspace drivers with the NIC > VFs, but I was just casting around for a quick and easy baseline of > whether the SMMU works at all (another way would be using Qemu to run a > VM with one or more PCI devices assigned). > > > I do have a boot console yet I'm not seeing any smmu faults at all. > > Perhaps I've mis-diagnosed the issue completely. To be clear when I > > boot with arm-smmu.disable_bypass=y the serial console appears to not > > accept input in userspace and with arm-smmu.disable_bypass=n I'm fine. > > I'm using a buildroot initramfs rootfs for simplicity. The system > > isn't hung as I originally expected as the LED heartbeat trigger > > continues blinking... I just can't get console to accept input. > > Curiouser and curiouser... I'm inclined to suspect that the interrupt > configuration might also be messed up, such that the SMMU is blocking > traffic and jammed up due to pending faults, but you're not getting the > IRQ delivered to find out. Does this patch help reveal anything? > > http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=29ac3648b580920692c9b417b2fc606995826517 > > (untested, but it's a direct port of the one I've used for SMMUv3 to > diagnose something similar) This shows: arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0140, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x ... arm-smmu 8300.smmu0: Unexpected global fault, this could be serious arm-smmu 8300.smmu0: GFSR 0x8002, GFSYNR0 0x0002, GFSYNR1 0x0010, GFSYNR2 0x ^^^ these two repeat over and over > > That said, it's also puzzling that no other drivers are reporting DMA > errors or timeouts either - is there any chance that some device is set > running by the
Re: [PATCH] dma-mapping: Lift address space checks out of debug code
On Fri, Oct 04, 2019 at 07:50:54PM +0100, Robin Murphy wrote: > On 03/10/2019 22:38, Kees Cook wrote: > > What do you think about the object_is_on_stack() check? That does a > > dereference through "current" to find the stack bounds... > > I guess it depends what the aim is - is it just to bail out of operations > which have near-zero chance of working correctly and every chance of going > catastrophically wrong, or to lay down strict argument checking for the API > in general? (for cache-coherent devices, or if the caller is careful to > ensure the appropriate alignment, DMA from a non-virtually-mapped stack can > be *technically* fine, it's just banned in general because those necessary > assumptions can be tricky to meet and aren't at all portable). Okay, then since the vmap check is both the cheapest and the most important to catch in the face of breaking everything, I'll move that in and we can keep USB's other checks separately. -- Kees Cook ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dma-mapping: Lift address space checks out of debug code
On 03/10/2019 22:38, Kees Cook wrote:
On Thu, Oct 03, 2019 at 10:42:45AM +0100, Robin Murphy wrote:
On 03/10/2019 00:58, Kees Cook wrote:
On Wed, Oct 02, 2019 at 10:15:43PM +0100, Robin Murphy wrote:
Hi Kees,
On 2019-10-02 9:46 pm, Kees Cook wrote:
As we've seen from USB and other areas, we need to always do runtime
checks for DMA operating on memory regions that might be remapped. This
consolidates the (existing!) checks and makes them on by default. A
warning will be triggered for any drivers still using DMA on the stack
(as has been seen in a few recent reports).
Suggested-by: Laura Abbott
Signed-off-by: Kees Cook
---
include/linux/dma-debug.h | 8
include/linux/dma-mapping.h | 8 +++-
kernel/dma/debug.c | 16
3 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/include/linux/dma-debug.h b/include/linux/dma-debug.h
index 4208f94d93f7..2af9765d9af7 100644
--- a/include/linux/dma-debug.h
+++ b/include/linux/dma-debug.h
@@ -18,9 +18,6 @@ struct bus_type;
extern void dma_debug_add_bus(struct bus_type *bus);
-extern void debug_dma_map_single(struct device *dev, const void *addr,
-unsigned long len);
-
extern void debug_dma_map_page(struct device *dev, struct page *page,
size_t offset, size_t size,
int direction, dma_addr_t dma_addr);
@@ -75,11 +72,6 @@ static inline void dma_debug_add_bus(struct bus_type *bus)
{
}
-static inline void debug_dma_map_single(struct device *dev, const void *addr,
- unsigned long len)
-{
-}
-
static inline void debug_dma_map_page(struct device *dev, struct page *page,
size_t offset, size_t size,
int direction, dma_addr_t dma_addr)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4a1c4fca475a..2d6b8382eab1 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -583,7 +583,13 @@ static inline unsigned long dma_get_merge_boundary(struct
device *dev)
static inline dma_addr_t dma_map_single_attrs(struct device *dev, void *ptr,
size_t size, enum dma_data_direction dir, unsigned long attrs)
{
- debug_dma_map_single(dev, ptr, size);
+ /* DMA must never operate on stack or other remappable places. */
+ WARN_ONCE(is_vmalloc_addr(ptr) || !virt_addr_valid(ptr),
This stands to absolutely cripple I/O performance on arm64, because every
valid call will end up going off and scanning the memblock list, which is
not something we want on a fastpath in non-debug configurations. We'd need a
much better solution to the "pfn_valid() vs. EFI no-map" problem before this
might be viable.
Ah! Interesting. I didn't realize this was fast-path (I don't know the
DMA code at all). I thought it was more of a "one time setup" before
actual DMA activity started.
That's strictly true, it's just that many workloads can involve tens of
thousands of "one time"s per second ;)
Overhead on the dma_map_* paths has shown to have a direct impact on
throughput in such situations, hence various optimisation effort in IOVA
allocation for IOMMU-based DMA ops, and the recent work to remove indirect
calls entirely for the common dma-direct/SWIOTLB cases.
Regardless, is_vmalloc_addr() is extremely light (a bounds check), and is the
most important part of this as far as catching stack-based DMA attempts.
I thought virt_addr_valid() was cheap too, but I see it's much heavier on
arm64.
I just went to compare what the existing USB check does, and it happens
immediately before its call to dma_map_single(). Both checks are simple
bounds checks, so it shouldn't be an issue:
if (is_vmalloc_addr(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is not dma
capable\n");
return -EAGAIN;
} else if (object_is_on_stack(urb->setup_packet)) {
WARN_ONCE(1, "setup packet is on stack\n");
return -EAGAIN;
}
urb->setup_dma = dma_map_single(
hcd->self.sysdev,
urb->setup_packet,
sizeof(struct usb_ctrlrequest),
In the USB case, it'll actually refuse to do the operation. Should
dma_map_single() similarly fail? I could push these checks down into
dma_map_single(), which would be a no-change on behavior for USB and
gain the checks on all other callers...
I think it would be reasonable to pull the is_vmalloc_addr() check inline,
as that probably covers 90+% of badness (especially given vmapped stacks),
and as you say should be reliably cheap everywhere. Callers are certainly
expected to use dma_mapping_error() and
Re: [PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
On 04/10/2019 18:13, Tim Harvey wrote: [...] No difference... still need 'arm-smmu.disable_bypass=n' to boot. Are all four iommu-map props above supposed to be the same? Seems to me they all point to the same thing which looks wrong. Hmm... :/ Those mappings just set Stream ID == PCI RID (strictly each one should only need to cover the bus range assigned to that bridge, but it's not crucial) which is the same thing the driver assumes for the mmu-masters property, so either that's wrong and never could have worked anyway - have you tried VFIO on this platform? - or there are other devices also mastering through the SMMU that aren't described at all. Are you able to capture a boot log? The SMMU faults do encode information about the offending ID, and you can typically correlate their appearance reasonably well with endpoint drivers probing. Robin, VFIO is enabled in the kernel but I don't know anything about how to test/use it: $ grep VFIO .config CONFIG_KVM_VFIO=y CONFIG_VFIO_IOMMU_TYPE1=y CONFIG_VFIO_VIRQFD=y CONFIG_VFIO=y # CONFIG_VFIO_NOIOMMU is not set CONFIG_VFIO_PCI=y CONFIG_VFIO_PCI_MMAP=y CONFIG_VFIO_PCI_INTX=y # CONFIG_VFIO_PLATFORM is not set # CONFIG_VFIO_MDEV is not set No worries - since it's a networking-focused SoC I figured there was a chance you might be using DPDK or similar userspace drivers with the NIC VFs, but I was just casting around for a quick and easy baseline of whether the SMMU works at all (another way would be using Qemu to run a VM with one or more PCI devices assigned). I do have a boot console yet I'm not seeing any smmu faults at all. Perhaps I've mis-diagnosed the issue completely. To be clear when I boot with arm-smmu.disable_bypass=y the serial console appears to not accept input in userspace and with arm-smmu.disable_bypass=n I'm fine. I'm using a buildroot initramfs rootfs for simplicity. The system isn't hung as I originally expected as the LED heartbeat trigger continues blinking... I just can't get console to accept input. Curiouser and curiouser... I'm inclined to suspect that the interrupt configuration might also be messed up, such that the SMMU is blocking traffic and jammed up due to pending faults, but you're not getting the IRQ delivered to find out. Does this patch help reveal anything? http://linux-arm.org/git?p=linux-rm.git;a=commitdiff;h=29ac3648b580920692c9b417b2fc606995826517 (untested, but it's a direct port of the one I've used for SMMUv3 to diagnose something similar) That said, it's also puzzling that no other drivers are reporting DMA errors or timeouts either - is there any chance that some device is set running by the firmware/bootloader and not taken over by a kernel driver? Robin.
Re: [PATCH v4 3/4] iommu/ioasid: Add custom allocators
On Wed, Oct 02, 2019 at 12:42:42PM -0700, Jacob Pan wrote: > IOASID allocation may rely on platform specific methods. One use case is > that when running in the guest, in order to obtain system wide global > IOASIDs, emulated allocation interface is needed to communicate with the > host. Here we call these platform specific allocators custom allocators. > > Custom IOASID allocators can be registered at runtime and take precedence > over the default XArray allocator. They have these attributes: > > - provides platform specific alloc()/free() functions with private data. > - allocation results lookup are not provided by the allocator, lookup > request must be done by the IOASID framework by its own XArray. > - allocators can be unregistered at runtime, either fallback to the next > custom allocator or to the default allocator. > - custom allocators can share the same set of alloc()/free() helpers, in > this case they also share the same IOASID space, thus the same XArray. > - switching between allocators requires all outstanding IOASIDs to be > freed unless the two allocators share the same alloc()/free() helpers. > > Signed-off-by: Jean-Philippe Brucker > Signed-off-by: Jacob Pan > Link: https://lkml.org/lkml/2019/4/26/462 Reviewed-by: Jean-Philippe Brucker ___ 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 Fri, Oct 4, 2019 at 9:36 AM Robin Murphy wrote:
>
> On 04/10/2019 16:23, Tim Harvey wrote:
> > On Thu, Oct 3, 2019 at 3:24 PM Robin Murphy wrote:
> >>
> >> On 2019-10-03 9:51 pm, Tim Harvey wrote:
> >>> On Thu, Oct 3, 2019 at 1:42 PM Robin Murphy wrote:
>
> Hi Tim,
>
> On 2019-10-03 7:27 pm, Tim Harvey wrote:
> > On Fri, Mar 1, 2019 at 11:21 AM 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
> >> ---
> >
> > Hi Doug / Robin,
> >
> > I ran into this breaking things on OcteonTx boards based on CN80XX
> > CPU. The IOMMU configuration is a bit beyond me and I'm hoping you can
> > offer some advice. The IOMMU here is cavium,smmu-v2 as defined in
> > https://github.com/Gateworks/dts-newport/blob/master/cn81xx-linux.dtsi
> >
> > Booting with 'arm-smmu.disable_bypass=n' does indeed work around the
> > breakage as the commit suggests.
> >
> > Any suggestions for a proper fix?
>
> Ah, you're using the old "mmu-masters" binding (and in a way which isn't
> well-defined - it's never been specified what the stream ID argument(s)
> would mean for a PCI host bridge, and Linux just ignores them). The
> ideal thing would be to update the DT to generic "iommu-map" properties
> - it's been a long time since I last played with a ThunderX, but I
> believe the SMMU stream IDs should just be the same as the ITS device
> IDs (which is how the "mmu-masters" mapping would have played out
> anyway).
>
> The arm-smmu driver support for the old binding has always relied on
> implicit bypass - there are technical reasons why we can't realistically
> support the full functionality offered to the generic bindings, but it
> would be possible to add some degree of workaround to prevent it
> interacting quite so poorly with disable_bypass, if necessary. Do you
> have deployed systems with DTs that can't be updated, but still might
> need to run new kernels?
>
> >>>
> >>> Robin,
> >>>
> >>> Thanks for the response. I don't care too much about supporting new
> >>> kernels with the current DT - I'm good with fixing this with a DT
> >>> change. Would you be able to give me an example? I would love to see
> >>> Cavium mainline an cn81xx dts/dtsi in arch/arm64/boot/dts to be used
> >>> as a base as the only thing we have to go off of currently is the
> >>> Cavium SDK which has fairly old kernel support.
> >>
> >> No promises (it's a late-night hack from my sofa), but try giving this a
> >> go...
> >>
> >> Robin.
> >>
> >> ->8-
> >> diff --git a/cn81xx-linux.dtsi b/cn81xx-linux.dtsi
> >> index 3b759d9575fe..dabc9047c674 100644
> >> --- a/cn81xx-linux.dtsi
> >> +++ b/cn81xx-linux.dtsi
> >> @@ -234,7 +234,7 @@
> >> clocks = <>;
> >> };
> >>
> >> - smmu0@8300 {
> >> + smmu: smmu0@8300 {
> >> compatible = "cavium,smmu-v2";
> >> reg = <0x8300 0x0 0x0 0x200>;
> >> #global-interrupts = <1>;
> >> @@ -249,23 +249,18 @@
> >> <0 69 4>, <0 69 4>, <0 69 4>, <0 69
> >> 4>, <0 69 4>, <0 69 4>,
> >> <0 69 4>, <0 69 4>, <0 69 4>, <0 69
> >> 4>, <0 69 4>, <0 69 4>,
> >> <0 69 4>, <0 69 4>, <0
Re: [PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
On 04/10/2019 16:23, Tim Harvey wrote:
On Thu, Oct 3, 2019 at 3:24 PM Robin Murphy wrote:
On 2019-10-03 9:51 pm, Tim Harvey wrote:
On Thu, Oct 3, 2019 at 1:42 PM Robin Murphy wrote:
Hi Tim,
On 2019-10-03 7:27 pm, Tim Harvey wrote:
On Fri, Mar 1, 2019 at 11:21 AM 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
---
Hi Doug / Robin,
I ran into this breaking things on OcteonTx boards based on CN80XX
CPU. The IOMMU configuration is a bit beyond me and I'm hoping you can
offer some advice. The IOMMU here is cavium,smmu-v2 as defined in
https://github.com/Gateworks/dts-newport/blob/master/cn81xx-linux.dtsi
Booting with 'arm-smmu.disable_bypass=n' does indeed work around the
breakage as the commit suggests.
Any suggestions for a proper fix?
Ah, you're using the old "mmu-masters" binding (and in a way which isn't
well-defined - it's never been specified what the stream ID argument(s)
would mean for a PCI host bridge, and Linux just ignores them). The
ideal thing would be to update the DT to generic "iommu-map" properties
- it's been a long time since I last played with a ThunderX, but I
believe the SMMU stream IDs should just be the same as the ITS device
IDs (which is how the "mmu-masters" mapping would have played out anyway).
The arm-smmu driver support for the old binding has always relied on
implicit bypass - there are technical reasons why we can't realistically
support the full functionality offered to the generic bindings, but it
would be possible to add some degree of workaround to prevent it
interacting quite so poorly with disable_bypass, if necessary. Do you
have deployed systems with DTs that can't be updated, but still might
need to run new kernels?
Robin,
Thanks for the response. I don't care too much about supporting new
kernels with the current DT - I'm good with fixing this with a DT
change. Would you be able to give me an example? I would love to see
Cavium mainline an cn81xx dts/dtsi in arch/arm64/boot/dts to be used
as a base as the only thing we have to go off of currently is the
Cavium SDK which has fairly old kernel support.
No promises (it's a late-night hack from my sofa), but try giving this a
go...
Robin.
->8-
diff --git a/cn81xx-linux.dtsi b/cn81xx-linux.dtsi
index 3b759d9575fe..dabc9047c674 100644
--- a/cn81xx-linux.dtsi
+++ b/cn81xx-linux.dtsi
@@ -234,7 +234,7 @@
clocks = <>;
};
- smmu0@8300 {
+ smmu: smmu0@8300 {
compatible = "cavium,smmu-v2";
reg = <0x8300 0x0 0x0 0x200>;
#global-interrupts = <1>;
@@ -249,23 +249,18 @@
<0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>, <0 69
4>, <0 69 4>,
<0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>, <0 69
4>, <0 69 4>,
<0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>, <0
69 4>;
-
- mmu-masters = < 0x100>,
- < 0x200>,
- < 0x300>,
- < 0x400>;
-
+ #iommu-cells = <1>;
+ dma-coherent;
};
ecam0: pci@8480 {
compatible = "pci-host-ecam-generic";
device_type = "pci";
- msi-parent = <>;
msi-map = <0 0 0x1>;
+ iommu-map = <0 0 0x1>;
bus-range = <0 31>;
#size-cells = <2>;
#address-cells = <3>;
-
Re: [PATCH v2] iommu/arm-smmu: Break insecure users by disabling bypass by default
On Thu, Oct 3, 2019 at 3:24 PM Robin Murphy wrote:
>
> On 2019-10-03 9:51 pm, Tim Harvey wrote:
> > On Thu, Oct 3, 2019 at 1:42 PM Robin Murphy wrote:
> >>
> >> Hi Tim,
> >>
> >> On 2019-10-03 7:27 pm, Tim Harvey wrote:
> >>> On Fri, Mar 1, 2019 at 11:21 AM 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
> ---
> >>>
> >>> Hi Doug / Robin,
> >>>
> >>> I ran into this breaking things on OcteonTx boards based on CN80XX
> >>> CPU. The IOMMU configuration is a bit beyond me and I'm hoping you can
> >>> offer some advice. The IOMMU here is cavium,smmu-v2 as defined in
> >>> https://github.com/Gateworks/dts-newport/blob/master/cn81xx-linux.dtsi
> >>>
> >>> Booting with 'arm-smmu.disable_bypass=n' does indeed work around the
> >>> breakage as the commit suggests.
> >>>
> >>> Any suggestions for a proper fix?
> >>
> >> Ah, you're using the old "mmu-masters" binding (and in a way which isn't
> >> well-defined - it's never been specified what the stream ID argument(s)
> >> would mean for a PCI host bridge, and Linux just ignores them). The
> >> ideal thing would be to update the DT to generic "iommu-map" properties
> >> - it's been a long time since I last played with a ThunderX, but I
> >> believe the SMMU stream IDs should just be the same as the ITS device
> >> IDs (which is how the "mmu-masters" mapping would have played out anyway).
> >>
> >> The arm-smmu driver support for the old binding has always relied on
> >> implicit bypass - there are technical reasons why we can't realistically
> >> support the full functionality offered to the generic bindings, but it
> >> would be possible to add some degree of workaround to prevent it
> >> interacting quite so poorly with disable_bypass, if necessary. Do you
> >> have deployed systems with DTs that can't be updated, but still might
> >> need to run new kernels?
> >>
> >
> > Robin,
> >
> > Thanks for the response. I don't care too much about supporting new
> > kernels with the current DT - I'm good with fixing this with a DT
> > change. Would you be able to give me an example? I would love to see
> > Cavium mainline an cn81xx dts/dtsi in arch/arm64/boot/dts to be used
> > as a base as the only thing we have to go off of currently is the
> > Cavium SDK which has fairly old kernel support.
>
> No promises (it's a late-night hack from my sofa), but try giving this a
> go...
>
> Robin.
>
> ->8-
> diff --git a/cn81xx-linux.dtsi b/cn81xx-linux.dtsi
> index 3b759d9575fe..dabc9047c674 100644
> --- a/cn81xx-linux.dtsi
> +++ b/cn81xx-linux.dtsi
> @@ -234,7 +234,7 @@
> clocks = <>;
> };
>
> - smmu0@8300 {
> + smmu: smmu0@8300 {
> compatible = "cavium,smmu-v2";
> reg = <0x8300 0x0 0x0 0x200>;
> #global-interrupts = <1>;
> @@ -249,23 +249,18 @@
> <0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>,
> <0 69 4>, <0 69 4>,
> <0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>,
> <0 69 4>, <0 69 4>,
> <0 69 4>, <0 69 4>, <0 69 4>, <0 69 4>,
> <0 69 4>;
> -
> - mmu-masters = < 0x100>,
> - < 0x200>,
> - < 0x300>,
> - < 0x400>;
> -
> + #iommu-cells = <1>;
> + dma-coherent;
> };
>
> ecam0:
[RESEND TRIVIAL 2/3] treewide: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- certs/Kconfig | 14 ++--- init/Kconfig | 28 +- kernel/trace/Kconfig | 8 lib/Kconfig| 2 +- lib/Kconfig.debug | 36 +- lib/Kconfig.kgdb | 8 mm/Kconfig | 28 +- samples/Kconfig| 2 +- security/apparmor/Kconfig | 2 +- security/integrity/Kconfig | 24 +++ security/integrity/ima/Kconfig | 12 ++-- security/safesetid/Kconfig | 24 +++ 12 files changed, 94 insertions(+), 94 deletions(-) diff --git a/certs/Kconfig b/certs/Kconfig index c94e93d8bccf..0358c66d3d7c 100644 --- a/certs/Kconfig +++ b/certs/Kconfig @@ -6,14 +6,14 @@ config MODULE_SIG_KEY default "certs/signing_key.pem" depends on MODULE_SIG help - Provide the file name of a private key/certificate in PEM format, - or a PKCS#11 URI according to RFC7512. The file should contain, or - the URI should identify, both the certificate and its corresponding - private key. +Provide the file name of a private key/certificate in PEM format, +or a PKCS#11 URI according to RFC7512. The file should contain, or +the URI should identify, both the certificate and its corresponding +private key. - If this option is unchanged from its default "certs/signing_key.pem", - then the kernel will automatically generate the private key and - certificate as described in Documentation/admin-guide/module-signing.rst +If this option is unchanged from its default "certs/signing_key.pem", +then the kernel will automatically generate the private key and +certificate as described in Documentation/admin-guide/module-signing.rst config SYSTEM_TRUSTED_KEYRING bool "Provide system-wide ring of trusted keys" diff --git a/init/Kconfig b/init/Kconfig index b4daad2bac23..e1a6f31da281 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -169,10 +169,10 @@ config BUILD_SALT string "Build ID Salt" default "" help - The build ID is used to link binaries and their debug info. Setting - this option will use the value in the calculation of the build id. - This is mostly useful for distributions which want to ensure the - build is unique between builds. It's safe to leave the default. + The build ID is used to link binaries and their debug info. Setting + this option will use the value in the calculation of the build id. + This is mostly useful for distributions which want to ensure the + build is unique between builds. It's safe to leave the default. config HAVE_KERNEL_GZIP bool @@ -1327,9 +1327,9 @@ menuconfig EXPERT select DEBUG_KERNEL help This option allows certain base kernel options and settings - to be disabled or tweaked. This is for specialized - environments which can tolerate a "non-standard" kernel. - Only use this if you really know what you are doing. + to be disabled or tweaked. This is for specialized + environments which can tolerate a "non-standard" kernel. + Only use this if you really know what you are doing. config UID16 bool "Enable 16-bit UID system calls" if EXPERT @@ -1439,11 +1439,11 @@ config BUG bool "BUG() support" if EXPERT default y help - Disabling this option eliminates support for BUG and WARN, reducing - the size of your kernel image and potentially quietly ignoring - numerous fatal conditions. You should only consider disabling this - option for embedded systems with no facilities for reporting errors. - Just say Y. + Disabling this option eliminates support for BUG and WARN, reducing + the size of your kernel image and potentially quietly ignoring + numerous fatal conditions. You should only consider disabling this + option for embedded systems with no facilities for reporting errors. + Just say Y. config ELF_CORE depends on COREDUMP @@ -1459,8 +1459,8 @@ config PCSPKR_PLATFORM select I8253_LOCK default y help - This option allows to disable the internal PC-Speaker - support, saving some memory. + This option allows to disable the internal PC-Speaker + support, saving some memory. config BASE_FULL default y diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index e08527f50d2a..0393003f102f 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -76,7 +76,7
[RESEND TRIVIAL 3/3] treewide: arch: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- arch/Kconfig | 4 ++-- arch/alpha/Kconfig | 2 +- arch/arm/Kconfig.debug | 4 ++-- arch/arm/mach-ep93xx/Kconfig | 8 arch/arm/mach-hisi/Kconfig | 2 +- arch/arm/mach-ixp4xx/Kconfig | 16 arch/arm/mach-mmp/Kconfig | 2 +- arch/arm/mach-omap1/Kconfig| 14 +++--- arch/arm/mach-prima2/Kconfig | 6 +++--- arch/arm/mach-s3c24xx/Kconfig | 4 ++-- arch/arm/mach-s3c64xx/Kconfig | 6 +++--- arch/arm/plat-samsung/Kconfig | 2 +- arch/arm64/Kconfig | 6 +++--- arch/arm64/Kconfig.debug | 2 +- arch/h8300/Kconfig | 4 ++-- arch/h8300/Kconfig.cpu | 4 ++-- arch/m68k/Kconfig.bus | 2 +- arch/m68k/Kconfig.debug| 16 arch/m68k/Kconfig.machine | 8 arch/nds32/Kconfig.cpu | 18 +- arch/openrisc/Kconfig | 26 +- arch/powerpc/Kconfig.debug | 18 +- arch/powerpc/platforms/Kconfig.cputype | 2 +- arch/riscv/Kconfig.socs| 2 +- arch/sh/boards/Kconfig | 2 +- arch/sh/mm/Kconfig | 2 +- arch/um/Kconfig| 2 +- arch/x86/Kconfig | 18 +- 28 files changed, 101 insertions(+), 101 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 5f8a5d84dbbe..8d4f77bbed29 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -76,7 +76,7 @@ config JUMP_LABEL depends on HAVE_ARCH_JUMP_LABEL depends on CC_HAS_ASM_GOTO help - This option enables a transparent branch optimization that +This option enables a transparent branch optimization that makes certain almost-always-true or almost-always-false branch conditions even cheaper to execute within the kernel. @@ -84,7 +84,7 @@ config JUMP_LABEL scheduler functionality, networking code and KVM have such branches and include support for this optimization technique. - If it is detected that the compiler has support for "asm goto", +If it is detected that the compiler has support for "asm goto", the kernel will compile such branches with just a nop instruction. When the condition flag is toggled to true, the nop will be converted to a jump instruction to execute the diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index ef179033a7c2..30a6291355cb 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -545,7 +545,7 @@ config NR_CPUS default "4" if !ALPHA_GENERIC && !ALPHA_MARVEL help MARVEL support can handle a maximum of 32 CPUs, all the others - with working support have a maximum of 4 CPUs. + with working support have a maximum of 4 CPUs. config ARCH_DISCONTIGMEM_ENABLE bool "Discontiguous Memory Support" diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug index 8bcbd0cd739b..0e5d52fbddbd 100644 --- a/arch/arm/Kconfig.debug +++ b/arch/arm/Kconfig.debug @@ -274,7 +274,7 @@ choice select DEBUG_UART_8250 help Say Y here if you want the debug print routines to direct - their output to the CNS3xxx UART0. + their output to the CNS3xxx UART0. config DEBUG_DAVINCI_DA8XX_UART1 bool "Kernel low-level debugging on DaVinci DA8XX using UART1" @@ -828,7 +828,7 @@ choice select DEBUG_UART_8250 help Say Y here if you want kernel low-level debugging support - on Rockchip RV1108 based platforms. + on Rockchip RV1108 based platforms. config DEBUG_RV1108_UART1 bool "Kernel low-level debugging messages via Rockchip RV1108 UART1" diff --git a/arch/arm/mach-ep93xx/Kconfig b/arch/arm/mach-ep93xx/Kconfig index f2db5fd38145..bf81dfab7f1b 100644 --- a/arch/arm/mach-ep93xx/Kconfig +++ b/arch/arm/mach-ep93xx/Kconfig @@ -126,10 +126,10 @@ config MACH_MICRO9S Contec Micro9-Slim board. config MACH_SIM_ONE -bool "Support Simplemachines Sim.One board" -help - Say 'Y' here if you want your kernel to support the - Simplemachines Sim.One board. + bool "Support Simplemachines Sim.One board" + help + Say 'Y' here if you want your kernel to support the + Simplemachines Sim.One board. config MACH_SNAPPER_CL15 bool "Support Bluewater Systems Snapper CL15 Module" diff --git a/arch/arm/mach-hisi/Kconfig
[RESEND TRIVIAL 1/3] treewide: drivers: Fix Kconfig indentation
Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^/\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski --- drivers/acpi/Kconfig | 8 +- drivers/ata/Kconfig | 12 +-- drivers/auxdisplay/Kconfig| 14 +-- drivers/base/firmware_loader/Kconfig | 2 +- drivers/block/Kconfig | 28 +++--- drivers/block/mtip32xx/Kconfig| 2 +- drivers/char/Kconfig | 6 +- drivers/char/agp/Kconfig | 2 +- drivers/char/hw_random/Kconfig| 10 +- drivers/char/ipmi/Kconfig | 20 ++-- drivers/clk/Kconfig | 2 +- drivers/clk/mediatek/Kconfig | 10 +- drivers/clk/versatile/Kconfig | 2 +- drivers/clocksource/Kconfig | 20 ++-- drivers/cpufreq/Kconfig.x86 | 6 +- drivers/cpuidle/Kconfig | 8 +- drivers/cpuidle/Kconfig.arm | 16 ++-- drivers/crypto/Kconfig| 4 +- drivers/crypto/caam/Kconfig | 14 +-- drivers/crypto/chelsio/Kconfig| 30 +++--- drivers/crypto/stm32/Kconfig | 6 +- drivers/crypto/ux500/Kconfig | 16 ++-- drivers/devfreq/Kconfig | 6 +- drivers/dma/Kconfig | 46 - drivers/edac/Kconfig | 2 +- drivers/firmware/Kconfig | 4 +- drivers/firmware/efi/Kconfig | 2 +- drivers/hid/Kconfig | 2 +- drivers/hwmon/Kconfig | 14 +-- drivers/i2c/busses/Kconfig| 16 ++-- drivers/i2c/muxes/Kconfig | 18 ++-- drivers/iio/gyro/Kconfig | 8 +- drivers/infiniband/hw/bnxt_re/Kconfig | 12 +-- drivers/input/keyboard/Kconfig| 8 +- drivers/input/mouse/Kconfig | 6 +- drivers/input/tablet/Kconfig | 20 ++-- drivers/input/touchscreen/Kconfig | 2 +- drivers/iommu/Kconfig | 2 +- drivers/irqchip/Kconfig | 10 +- drivers/isdn/hardware/mISDN/Kconfig | 2 +- drivers/macintosh/Kconfig | 6 +- drivers/md/Kconfig| 54 +-- drivers/media/Kconfig | 6 +- drivers/media/radio/si470x/Kconfig| 4 +- drivers/memstick/core/Kconfig | 18 ++-- drivers/memstick/host/Kconfig | 4 +- drivers/misc/Kconfig | 16 ++-- drivers/mtd/nand/onenand/Kconfig | 12 +-- drivers/nfc/nfcmrvl/Kconfig | 2 +- drivers/pci/Kconfig | 24 ++--- drivers/pci/controller/dwc/Kconfig| 6 +- drivers/pci/hotplug/Kconfig | 2 +- drivers/perf/Kconfig | 14 +-- drivers/phy/hisilicon/Kconfig | 6 +- drivers/pinctrl/Kconfig | 18 ++-- drivers/pinctrl/freescale/Kconfig | 12 +-- drivers/pinctrl/qcom/Kconfig | 34 +++ drivers/platform/chrome/Kconfig | 6 +- drivers/platform/mellanox/Kconfig | 4 +- drivers/platform/x86/Kconfig | 48 +- drivers/power/avs/Kconfig | 12 +-- drivers/power/supply/Kconfig | 30 +++--- drivers/regulator/Kconfig | 8 +- drivers/rpmsg/Kconfig | 2 +- drivers/rtc/Kconfig | 6 +- drivers/scsi/Kconfig | 22 ++--- drivers/scsi/aic7xxx/Kconfig.aic7xxx | 14 +-- drivers/scsi/pcmcia/Kconfig | 2 +- drivers/scsi/qedf/Kconfig | 4 +- drivers/scsi/smartpqi/Kconfig | 8 +- drivers/soc/fsl/Kconfig | 8 +- drivers/soc/qcom/Kconfig | 22 ++--- drivers/soc/rockchip/Kconfig | 18 ++-- drivers/spi/Kconfig | 18 ++-- drivers/staging/fbtft/Kconfig | 12 +-- drivers/staging/fwserial/Kconfig | 6 +- drivers/staging/most/Kconfig | 8 +- drivers/staging/nvec/Kconfig | 10 +- drivers/staging/pi433/Kconfig | 24 ++--- drivers/staging/uwb/Kconfig | 42 .../vc04_services/bcm2835-audio/Kconfig | 12 +-- drivers/staging/wusbcore/Kconfig | 2 +- drivers/tty/Kconfig | 26 ++--- drivers/tty/hvc/Kconfig | 4 +- drivers/tty/serial/8250/Kconfig | 2 +- drivers/tty/serial/Kconfig
