Patch Set 9:

Patch Set 9:

Patch Set 8:

There has been lots of discussion here, but nobody asked if ramstage is really the right place to do that: Disabling bus master for TBT is to prevent wild (potentially hostile) DMA accesses early on in the boot cycle, right? Ideally we'd do that in the bootblock, or in romstage before RAM is available or used for anything, at any rate.

If we're deferring this to ramstage, an attacker could just overwrite the ramstage before its executed: blast the entry point with "jmp -1" for 5 seconds, then write whatever code they really want to see executed somewhere and overwrite the jmp again. Since we're typically decompressing the ramstage, and decompression works from low to high addresses, while the entry point is at the beginning, it shouldn't even be hard to win that race.

Is bus mastering enabled early, though? It would be nice to document
the reset state and if FSP does anything about it.

Beside that, I doubt that this is TBT specific. We shouldn't enable
BM for anything that we don't use. 99% of the BM enable settings in
coreboot seem more like a bug than a feature. I do understand that
this might not be the right time to clean it up. But I'm with Patrick
here, we should prevent that BM gets enabled at all (or disable it as
early as possible if it's enabled due to some bug that can't be fixed),
not disable it at a random point later just to comply with a document.

Based on Coreboot's TBT setting, FSP configures TBT without enabling TBT's BME at romstage. There is no coreboot code with iommu at pre-boot to block DMA access to critical memory region in the early boot phase, nor DMA remapping in later boot phase to enable BME.
It seems no coreboot/fsp manipulating TBT BME at early boot. This patch tries to disable TBT BME at ramstage. Please advise.

FWIW, the EDS shows that the BME bit is disabled at reset for all PCIe devices, D29:F0-F7, D28:F0-F7, and D27: F0-F7. Given that FSP updates may happen and its policies may change, I agree it's probably safest to disable BME before handing off control to the payload, as well as in bootblock.

View Change

To view, visit change 40968. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9650b9dd4df1f9bee53ae3737b7bf60b2ef8017b
Gerrit-Change-Number: 40968
Gerrit-PatchSet: 9
Gerrit-Owner: John Zhao <john.zhao@intel.com>
Gerrit-Reviewer: Alex Levin <levinale@google.com>
Gerrit-Reviewer: Caveh Jalali <caveh@chromium.org>
Gerrit-Reviewer: Divya S Sasidharan <divya.s.sasidharan@intel.com>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Prashant Malani <pmalani@google.com>
Gerrit-Reviewer: Shamile Khan <shamile.khan@intel.com>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Aaron Durbin <adurbin@chromium.org>
Gerrit-CC: Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.corp-partner.google.com>
Gerrit-CC: Divya Sasidharan <divya.s.sasidharan@intel.corp-partner.google.com>
Gerrit-CC: Lalithambika Krishnakumar <lalithambika.krishnakumar@intel.corp-partner.google.com>
Gerrit-CC: Nico Huber <nico.h@gmx.de>
Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-CC: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jun 2020 21:25:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment