mbox series

[v2,00/42,SRU,U] Fix can't find root device on VMD when secureboot enabled

Message ID 20200602074843.1743493-1-vicamo.yang@canonical.com
Headers show
Series Fix can't find root device on VMD when secureboot enabled | expand

Message

You-Sheng Yang June 2, 2020, 7:48 a.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1876707

[Impact]

On platforms with NVMe attached to VMD controller, enable SecureBoot
would also force enable iommu:

  DMAR: Intel-IOMMU force enabled due to platform opt in

While devices behind the VMD controller, also a PCI bridge, maybe forced
to use a DMA domain by current Intel-IOMMU driver, this may break some
relationships between sub devices behind VMD controller and between the
VMD controller and its children devices, and finally caused undefined
system behavior.

On devices at hand, this results in kernel NULL dereference at
__intel_map_single called from nvme_reset_work and fails root device
lookup at boot.

  kernel: BUG: kernel NULL pointer dereference, address: 0000000000000018
  kernel: #PF: supervisor read access in kernel mode
  kernel: #PF: error_code(0x0000) - not-present page
  kernel: PGD 0 P4D 0
  kernel: Oops: 0000 [#2] SMP NOPTI
  kernel: CPU: 1 PID: 254 Comm: kworker/u8:4 Tainted: G D W
  5.7.0-050700rc3-generic #202004262131
  kernel: Hardware name: Dell Inc. Vostro 5402/, BIOS 0.1.2 04/13/2020
  kernel: Workqueue: nvme-reset-wq nvme_reset_work [nvme]
  kernel: RIP: 0010:__intel_map_single+0xa3/0x1a0
  ...

[Fix]

Patchset[1] currently landed in linux-next beginning with commit
327d5b2fee91 ("iommu/vt-d: Allow 32bit devices to uses DMA domain")
gives the solution to this problem. However, it's based on a massive
subsystem rewrite in patchset[2] beginning with commit 441ff2ff8327
("Move default domain allocation to separate function").

On v5.6, it also depends on yet a few more patch series landed in
v5.7-rc1 beginning with commit 098accf2da94 ("iommu: Use C99 flexible
array in fwspec") that rewrote private data access, changed struct
names, etc.

Yet a few additional patches included as fixes to above changes.

[1]: https://lore.kernel.org/linux-iommu/7928dd48-93da-62f0-b455-6e6b248d0fae@linux.intel.com/
[2]: https://lore.kernel.org/linux-iommu/20200429133712.31431-1-joro@8bytes.org/

[Test Case]

Test on platforms with VMD/NVMe and enable SecureBoot. System should
boot normally rather than into initramfs emergency shell.

[Regression Potential]
For unstable, all the patches are from linux-next and we will probably catch up
soon, so should be safe to place a LOW here.

For oem-5.6, the fixing patchset is depending on iommu group setup
refactoring that touched almost every architecture/platform uses iommu
although we would only care amd64 among them. Even with follow-up fixes
included, this is still a 60-patches change and deserves some more
attention. Medium.

[Other Info]

v2: update patch cherry-pick origin info, fix a conflict due to commit
    549bdfcb8046 ("s390/pci: adaptation of iommu to multifunction") that was for
    bug 1874056.

Arnd Bergmann (1):
  iommu/renesas: Fix unused-function warning

Joerg Roedel (34):
  iommu: Move default domain allocation to separate function
  iommu/amd: Implement iommu_ops->def_domain_type call-back
  iommu/vt-d: Wire up iommu_ops->def_domain_type
  iommu/amd: Remove dma_mask check from check_device()
  iommu/amd: Return -ENODEV in add_device when device is not handled by
    IOMMU
  iommu: Add probe_device() and release_device() call-backs
  iommu: Move default domain allocation to iommu_probe_device()
  iommu: Keep a list of allocated groups in __iommu_probe_device()
  iommu: Move new probe_device path to separate function
  iommu: Split off default domain allocation from group assignment
  iommu: Move iommu_group_create_direct_mappings() out of
    iommu_group_add_device()
  iommu: Export bus_iommu_probe() and make is safe for re-probing
  iommu/amd: Remove dev_data->passthrough
  iommu/amd: Convert to probe/release_device() call-backs
  iommu/vt-d: Convert to probe/release_device() call-backs
  iommu/arm-smmu: Convert to probe/release_device() call-backs
  iommu/pamu: Convert to probe/release_device() call-backs
  iommu/s390: Convert to probe/release_device() call-backs
  iommu/virtio: Convert to probe/release_device() call-backs
  iommu/msm: Convert to probe/release_device() call-backs
  iommu/mediatek: Convert to probe/release_device() call-backs
  iommu/mediatek-v1 Convert to probe/release_device() call-backs
  iommu/qcom: Convert to probe/release_device() call-backs
  iommu/rockchip: Convert to probe/release_device() call-backs
  iommu/tegra: Convert to probe/release_device() call-backs
  iommu/renesas: Convert to probe/release_device() call-backs
  iommu/omap: Remove orphan_dev tracking
  iommu/omap: Convert to probe/release_device() call-backs
  iommu/exynos: Use first SYSMMU in controllers list for IOMMU core
  iommu/exynos: Convert to probe/release_device() call-backs
  iommu: Remove add_device()/remove_device() code-paths
  iommu: Move more initialization to __iommu_probe_device()
  iommu: Unexport iommu_group_get_for_dev()
  iommu: Don't call .probe_finalize() under group->mutex

Lu Baolu (3):
  iommu/vt-d: Allow 32bit devices to uses DMA domain
  iommu/vt-d: Allow PCI sub-hierarchy to use DMA domain
  iommu/vt-d: Apply per-device dma_ops

Qian Cai (1):
  iommu/amd: Fix variable "iommu" set but not used

Sai Praneeth Prakhya (2):
  iommu: Add def_domain_type() callback in iommu_ops
  iommu: Remove functions that support private domain

Tero Kristo via iommu (1):
  iommu/omap: Add check for iommu group when no IOMMU in use

 drivers/iommu/amd_iommu.c       | 100 +++----
 drivers/iommu/amd_iommu_types.h |   1 -
 drivers/iommu/arm-smmu-v3.c     |  38 +--
 drivers/iommu/arm-smmu.c        |  39 ++-
 drivers/iommu/exynos-iommu.c    |  24 +-
 drivers/iommu/fsl_pamu_domain.c |  22 +-
 drivers/iommu/intel-iommu.c     | 460 +++-----------------------------
 drivers/iommu/iommu.c           | 458 +++++++++++++++++++++----------
 drivers/iommu/ipmmu-vmsa.c      |  59 ++--
 drivers/iommu/msm_iommu.c       |  34 +--
 drivers/iommu/mtk_iommu.c       |  24 +-
 drivers/iommu/mtk_iommu_v1.c    |  50 ++--
 drivers/iommu/omap-iommu.c      | 102 ++-----
 drivers/iommu/qcom_iommu.c      |  24 +-
 drivers/iommu/rockchip-iommu.c  |  26 +-
 drivers/iommu/s390-iommu.c      |  22 +-
 drivers/iommu/tegra-gart.c      |  24 +-
 drivers/iommu/tegra-smmu.c      |  31 +--
 drivers/iommu/virtio-iommu.c    |  41 +--
 include/linux/iommu.h           |  33 ++-
 20 files changed, 563 insertions(+), 1049 deletions(-)

Comments

Seth Forshee June 22, 2020, 7:38 p.m. UTC | #1 
On Tue, Jun 02, 2020 at 03:48:01PM +0800, You-Sheng Yang wrote:
> BugLink: https://bugs.launchpad.net/bugs/1876707
> 
> [Impact]
> 
> On platforms with NVMe attached to VMD controller, enable SecureBoot
> would also force enable iommu:
> 
>   DMAR: Intel-IOMMU force enabled due to platform opt in
> 
> While devices behind the VMD controller, also a PCI bridge, maybe forced
> to use a DMA domain by current Intel-IOMMU driver, this may break some
> relationships between sub devices behind VMD controller and between the
> VMD controller and its children devices, and finally caused undefined
> system behavior.
> 
> On devices at hand, this results in kernel NULL dereference at
> __intel_map_single called from nvme_reset_work and fails root device
> lookup at boot.
> 
>   kernel: BUG: kernel NULL pointer dereference, address: 0000000000000018
>   kernel: #PF: supervisor read access in kernel mode
>   kernel: #PF: error_code(0x0000) - not-present page
>   kernel: PGD 0 P4D 0
>   kernel: Oops: 0000 [#2] SMP NOPTI
>   kernel: CPU: 1 PID: 254 Comm: kworker/u8:4 Tainted: G D W
>   5.7.0-050700rc3-generic #202004262131
>   kernel: Hardware name: Dell Inc. Vostro 5402/, BIOS 0.1.2 04/13/2020
>   kernel: Workqueue: nvme-reset-wq nvme_reset_work [nvme]
>   kernel: RIP: 0010:__intel_map_single+0xa3/0x1a0
>   ...
> 
> [Fix]
> 
> Patchset[1] currently landed in linux-next beginning with commit
> 327d5b2fee91 ("iommu/vt-d: Allow 32bit devices to uses DMA domain")
> gives the solution to this problem. However, it's based on a massive
> subsystem rewrite in patchset[2] beginning with commit 441ff2ff8327
> ("Move default domain allocation to separate function").
> 
> On v5.6, it also depends on yet a few more patch series landed in
> v5.7-rc1 beginning with commit 098accf2da94 ("iommu: Use C99 flexible
> array in fwspec") that rewrote private data access, changed struct
> names, etc.
> 
> Yet a few additional patches included as fixes to above changes.
> 
> [1]: https://lore.kernel.org/linux-iommu/7928dd48-93da-62f0-b455-6e6b248d0fae@linux.intel.com/
> [2]: https://lore.kernel.org/linux-iommu/20200429133712.31431-1-joro@8bytes.org/
> 
> [Test Case]
> 
> Test on platforms with VMD/NVMe and enable SecureBoot. System should
> boot normally rather than into initramfs emergency shell.
> 
> [Regression Potential]
> For unstable, all the patches are from linux-next and we will probably catch up
> soon, so should be safe to place a LOW here.
> 
> For oem-5.6, the fixing patchset is depending on iommu group setup
> refactoring that touched almost every architecture/platform uses iommu
> although we would only care amd64 among them. Even with follow-up fixes
> included, this is still a 60-patches change and deserves some more
> attention. Medium.
> 
> [Other Info]
> 
> v2: update patch cherry-pick origin info, fix a conflict due to commit
>     549bdfcb8046 ("s390/pci: adaptation of iommu to multifunction") that was for
>     bug 1874056.

Applied to unstable/master, thanks!