mbox series

[v2,00/60,SRU,OEM-5.6] Fix can't find root device on VMD when secureboot enabled

Message ID 20200602074421.1742802-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:43 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.

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

Joerg Roedel (49):
  iommu: Define dev_iommu_fwspec_get() for !CONFIG_IOMMU_API
  ACPI/IORT: Remove direct access of dev->iommu_fwspec
  drm/msm/mdp5: Remove direct access of dev->iommu_fwspec
  iommu/tegra-gart: Remove direct access of dev->iommu_fwspec
  iommu: Rename struct iommu_param to dev_iommu
  iommu: Move iommu_fwspec to struct dev_iommu
  iommu/arm-smmu: Fix uninitilized variable warning
  iommu: Introduce accessors for iommu private data
  iommu/arm-smmu-v3: Use accessor functions for iommu private data
  iommu/arm-smmu: Use accessor functions for iommu private data
  iommu/renesas: Use accessor functions for iommu private data
  iommu/mediatek: Use accessor functions for iommu private data
  iommu/qcom: Use accessor functions for iommu private data
  iommu/virtio: Use accessor functions for iommu private data
  iommu: Move fwspec->iommu_priv to struct dev_iommu
  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

Kevin Hao (1):
  iommu: Fix the memory leak in dev_iommu_free()

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

Robin Murphy (2):
  iommu: Use C99 flexible array in fwspec
  iommu/arm-smmu: Refactor master_cfg/fwspec usage

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/acpi/arm64/iort.c                |   6 +-
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |   2 +-
 drivers/iommu/amd_iommu.c                | 100 ++---
 drivers/iommu/amd_iommu_types.h          |   1 -
 drivers/iommu/arm-smmu-v3.c              |  46 +--
 drivers/iommu/arm-smmu.c                 |  92 ++---
 drivers/iommu/exynos-iommu.c             |  24 +-
 drivers/iommu/fsl_pamu_domain.c          |  22 +-
 drivers/iommu/intel-iommu.c              | 460 ++-------------------
 drivers/iommu/iommu.c                    | 501 ++++++++++++++++-------
 drivers/iommu/ipmmu-vmsa.c               |  66 +--
 drivers/iommu/msm_iommu.c                |  34 +-
 drivers/iommu/mtk_iommu.c                |  35 +-
 drivers/iommu/mtk_iommu_v1.c             |  62 ++-
 drivers/iommu/omap-iommu.c               | 102 +----
 drivers/iommu/qcom_iommu.c               |  85 ++--
 drivers/iommu/rockchip-iommu.c           |  26 +-
 drivers/iommu/s390-iommu.c               |  22 +-
 drivers/iommu/tegra-gart.c               |  26 +-
 drivers/iommu/tegra-smmu.c               |  31 +-
 drivers/iommu/virtio-iommu.c             |  52 +--
 include/linux/device.h                   |   9 +-
 include/linux/iommu.h                    |  68 +--
 23 files changed, 710 insertions(+), 1162 deletions(-)

Comments

Timo Aaltonen June 12, 2020, 10:21 a.m. UTC | #1 
On 2.6.2020 10.43, 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.
> 
> Arnd Bergmann (1):
>   iommu/renesas: Fix unused-function warning
> 
> Joerg Roedel (49):
>   iommu: Define dev_iommu_fwspec_get() for !CONFIG_IOMMU_API
>   ACPI/IORT: Remove direct access of dev->iommu_fwspec
>   drm/msm/mdp5: Remove direct access of dev->iommu_fwspec
>   iommu/tegra-gart: Remove direct access of dev->iommu_fwspec
>   iommu: Rename struct iommu_param to dev_iommu
>   iommu: Move iommu_fwspec to struct dev_iommu
>   iommu/arm-smmu: Fix uninitilized variable warning
>   iommu: Introduce accessors for iommu private data
>   iommu/arm-smmu-v3: Use accessor functions for iommu private data
>   iommu/arm-smmu: Use accessor functions for iommu private data
>   iommu/renesas: Use accessor functions for iommu private data
>   iommu/mediatek: Use accessor functions for iommu private data
>   iommu/qcom: Use accessor functions for iommu private data
>   iommu/virtio: Use accessor functions for iommu private data
>   iommu: Move fwspec->iommu_priv to struct dev_iommu
>   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
> 
> Kevin Hao (1):
>   iommu: Fix the memory leak in dev_iommu_free()
> 
> 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
> 
> Robin Murphy (2):
>   iommu: Use C99 flexible array in fwspec
>   iommu/arm-smmu: Refactor master_cfg/fwspec usage
> 
> 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/acpi/arm64/iort.c                |   6 +-
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |   2 +-
>  drivers/iommu/amd_iommu.c                | 100 ++---
>  drivers/iommu/amd_iommu_types.h          |   1 -
>  drivers/iommu/arm-smmu-v3.c              |  46 +--
>  drivers/iommu/arm-smmu.c                 |  92 ++---
>  drivers/iommu/exynos-iommu.c             |  24 +-
>  drivers/iommu/fsl_pamu_domain.c          |  22 +-
>  drivers/iommu/intel-iommu.c              | 460 ++-------------------
>  drivers/iommu/iommu.c                    | 501 ++++++++++++++++-------
>  drivers/iommu/ipmmu-vmsa.c               |  66 +--
>  drivers/iommu/msm_iommu.c                |  34 +-
>  drivers/iommu/mtk_iommu.c                |  35 +-
>  drivers/iommu/mtk_iommu_v1.c             |  62 ++-
>  drivers/iommu/omap-iommu.c               | 102 +----
>  drivers/iommu/qcom_iommu.c               |  85 ++--
>  drivers/iommu/rockchip-iommu.c           |  26 +-
>  drivers/iommu/s390-iommu.c               |  22 +-
>  drivers/iommu/tegra-gart.c               |  26 +-
>  drivers/iommu/tegra-smmu.c               |  31 +-
>  drivers/iommu/virtio-iommu.c             |  52 +--
>  include/linux/device.h                   |   9 +-
>  include/linux/iommu.h                    |  68 +--
>  23 files changed, 710 insertions(+), 1162 deletions(-)
> 

oh well, it's all from 5.8~ and I guess we'll spot regressions pretty
quickly, so applied to oem-5.6-next..